[pacman-dev] [PATCH v2] pacman-key: ignores keys already lsigned/deleted

[Morgan Adamiec]

On Mon, 4 Nov 2019 at 19:36, Matthew Sexton <wsdmatty at gmail.com> wrote:
>
> Added two new functions, lsigned_already() and revoked_already()
> that check whether a key has been locally signed or revoked
> respectively during --populate. If the key is already signed
> or revoked, it is quietly ignored.
>
> Suggested-by: Eli Schwartz <eschwartz at archlinux.org>
> Signed-off-by: Matthew Sexton <wsdmatty at gmail.com>
> ---
> v2. (ACTUAL v2.) Previous email was erroneous, I attached the wrong
> patch. Corrected some inconsistencies. Gave proper attribution to Eli,
>
>  scripts/pacman-key.sh.in | 43 +++++++++++++++++++++++++++++++++++++---
>  1 file changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in
> index 3627a805..25bcb3de 100644
> --- a/scripts/pacman-key.sh.in
> +++ b/scripts/pacman-key.sh.in
> @@ -247,7 +247,7 @@ check_keyring() {
>                 fi
>         fi
>
> -       if (( LSIGNKEY )); then
> +       if (( LSIGNKEY || POPULATE )); then
>                 if [[ $(secret_keys_available) -lt 1 ]]; then
>                         error "$(gettext "There is no secret key available to sign with.")"
>                         msg "$(gettext "Use '%s' to generate a default secret key.")" "pacman-key --init"
> @@ -337,13 +337,18 @@ populate_keyring() {
>                 local key_count=0
>                 msg "$(gettext "Disabling revoked keys in keyring...")"
>                 for key_id in "${!revoked_ids[@]}"; do
> +            if revoked_already "$key_id" ; then
> +                continue
> +            fi
>                         if (( VERBOSE )); then
>                                 msg2 "$(gettext "Disabling key %s...")" "${key_id}"
>                         fi
>                         printf 'disable\nquit\n' | LANG=C "${GPG_PACMAN[@]}" --command-fd 0 --quiet --batch --edit-key "${key_id}" 2>/dev/null
>                         key_count=$((key_count+1))
>                 done
> -               msg2 "$(gettext "Disabled %s keys.")" "${key_count}"
> +               if (( key_count )); then
> +                       msg2 "$(gettext "Disabled %s keys.")" "${key_count}"
> +               fi
>         fi
>  }
>
> @@ -447,6 +452,22 @@ list_sigs() {
>                 exit 1
>         fi
>  }
> +lsigned_already() {
> +       # Determines whether a key has already been signed locally by getting the
> +       # local pacman secret key and comparing it against signatures on the key
> +       # returns 0 if key is signed, 1 if it is unsigned
> +       secret_key=$("${GPG_PACMAN[@]}" --with-colons --list-secret-key | head -n1 | awk -F : '{print $5}')
> +    while IFS=: read -r _ valid _ _ signkey _; do
> +            if [[ "$valid" != "!" ]]; then
> +                continue
> +            fi
> +            if [[ "$signkey" = "$secret_key" ]]; then
> +                return 0
> +            fi
> +       done < <("${GPG_PACMAN[@]}" --with-colons --check-signatures "$1")
> +       return 1
> +
> +}
>
>  lsign_keys() {
>         check_keyids_exist
> @@ -454,6 +475,7 @@ lsign_keys() {
>         local ret=0
>         local key_count=0
>         for key_id in "$@"; do
> +               if lsigned_already "$key_id" ; then     continue; fi
>                 if (( VERBOSE )); then
>                         msg2 "$(gettext "Locally signing key %s...")" "${key_id}"
>                 fi
> @@ -469,7 +491,9 @@ lsign_keys() {
>         if (( ret )); then
>                 exit 1
>         fi
> -       msg2 "$(gettext "Locally signed %s keys.")" "${key_count}"
> +       if (( key_count )); then
> +               msg2 "$(gettext "Locally signed %s keys.")" "${key_count}"
> +       fi
>  }
>
>  receive_keys() {
> @@ -511,6 +535,19 @@ refresh_keys() {
>         fi
>  }
>
> +revoked_already() {
> +
> +    while IFS=: read -r type _ _ _ _ _ _ _ _ _ _ flags _; do
> +            if [[ "$type" != "pub" ]]; then
> +                continue
> +            fi
> +            if [[ "$flags" = *"D"* ]]; then
> +                return 0
> +            fi
> +       done < <("${GPG_PACMAN[@]}" --with-colons --list-key "$1")
> +       return 1
> +}
> +
>  verify_sig() {
>         local ret=0 sig=$1 file=$2
>         if [[  -z $file && -f ${sig%.*} ]]; then
> --
> 2.23.0

This may just be my personal preference. But "pacman-key: ignores
keys already lsigned/deleted" kind of sounds past tense, almost like
pacman already does this and you're reporting a bug. I much prefer
just saying "pacman-key: ignore already signed/deleted keys".