On 27/8/21 6:07 am, Daan De Meyer wrote:
> Every time we modify gpg's state by signing or revoking a key, gpg
> marks the trustdb as stale and rechecks it the next time key_is_lsigned()
> or key_is_revoked() is called.
> 
> Currently, we alternate calls signing of keys and calling key_is_lsigned()
> (idem for revoking) which means that for each key we sign (or revoke), gpg
> will check the trustdb once. When populating the keyring, this means the
> trustdb is checked roughly 50 times. Each time the trustdb is checked,
> we get the following output once:
> 
> ```
> gpg: checking the trustdb
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 1EB2638FF56C0C53: no user ID for key signature packet of class 10
> gpg: key 1EB2638FF56C0C53: no user ID for key signature packet of class 10
> gpg: marginals needed: 3  completes needed: 1  trust model: pgp
> gpg: depth: 0  valid:   1  signed:   6  trust: 0-, 0q, 0n, 0m, 0f, 1u
> gpg: depth: 1  valid:   6  signed:  83  trust: 0-, 0q, 0n, 6m, 0f, 0u
> gpg: depth: 2  valid:  78  signed:  25  trust: 78-, 0q, 0n, 0m, 0f, 0u
> gpg: next trustdb check due at 2021-12-01
> ```
> 
> This repeated 50x leads to incredibly verbose output from pacman-key.
> 
> To avoid checking the trustb so many times, we can simply do all the
> key_is_lsigned() and key_is_revoked() checks upfront. Inbetween read
> operations the trustdb is not marked stale and inbetween write operations
> the trustdb is also not marked stale. This reduces the amount of trustdb
> checks from 50 to 1.
> 
> The output of pacman-key --populate now looks as follows:
> 
> ```
> gpg: /var/tmp/mkosi-pqii6f64/root/etc/pacman.d/gnupg/trustdb.gpg: trustdb 
> created
> gpg: no ultimately trusted keys found
> gpg: starting migration from earlier GnuPG versions
> gpg: porting secret keys from 
> '/var/tmp/mkosi-pqii6f64/root/etc/pacman.d/gnupg/secring.gpg' to gpg-agent
> gpg: migration succeeded
> ==> Generating pacman master key. This may take some time.
> gpg: Generating pacman keyring master key...
> gpg: key D429469316A97B49 marked as ultimately trusted
> gpg: directory 
> '/var/tmp/mkosi-pqii6f64/root/etc/pacman.d/gnupg/openpgp-revocs.d' created
> gpg: revocation certificate stored as 
> '/var/tmp/mkosi-pqii6f64/root/etc/pacman.d/gnupg/openpgp-revocs.d/059BC618296EDA8B4614FAD4D429469316A97B49.rev'
> gpg: Done
> ==> Updating trust database...
> gpg: marginals needed: 3  completes needed: 1  trust model: pgp
> gpg: depth: 0  valid:   1  signed:   0  trust: 0-, 0q, 0n, 0m, 0f, 1u
> ==> Appending keys from archlinux.gpg...
> ==> Locally signing trusted keys in keyring...
>   -> Locally signed 6 keys.
> ==> Importing owner trust values...
> gpg: setting ownertrust to 4
> gpg: setting ownertrust to 4
> gpg: setting ownertrust to 4
> gpg: setting ownertrust to 4
> gpg: inserting ownertrust of 4
> gpg: setting ownertrust to 4
> ==> Disabling revoked keys in keyring...
>   -> Disabled 44 keys.
> ==> Updating trust database...
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 786C63F330D7CB92: no user ID for key signature packet of class 10
> gpg: key 1EB2638FF56C0C53: no user ID for key signature packet of class 10
> gpg: key 1EB2638FF56C0C53: no user ID for key signature packet of class 10
> gpg: marginals needed: 3  completes needed: 1  trust model: pgp
> gpg: depth: 0  valid:   1  signed:   6  trust: 0-, 0q, 0n, 0m, 0f, 1u
> gpg: depth: 1  valid:   6  signed:  83  trust: 0-, 0q, 0n, 6m, 0f, 0u
> gpg: depth: 2  valid:  78  signed:  25  trust: 78-, 0q, 0n, 0m, 0f, 0u
> gpg: next trustdb check due at 2021-12-01
> ```
> ---
>  scripts/pacman-key.sh.in | 32 +++++++++++++++++---------------
>  1 file changed, 17 insertions(+), 15 deletions(-)
> 


I'm accepting both patches.   From my queries the "no user ID ..." blah
blah "error" is generally not relevant for the pacman keyring usage, so
can be silenced with --quiet.  Issues that need addressed should still
be shown.

The ordering of checks to reduce trust db checks also seems a good idea
on top of --quiet.

I have adjusted the commit message on this patch to reflect --quiet was
applied first, and also remove a lot of useless output inclusion.

A

> diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in
> index 50342649..721079b7 100644
> --- a/scripts/pacman-key.sh.in
> +++ b/scripts/pacman-key.sh.in
> @@ -333,12 +333,29 @@ populate_keyring() {
>                               # skip blank lines, comments; these are valid 
> in this file
>                               [[ -z $key_id || ${key_id:0:1} = \# ]] && 
> continue
>  
> +                             if key_is_lsigned "$key_id" ; then
> +                                     continue
> +                             fi
> +
>                               # Mark this key to be lsigned
>                               trusted_ids[$key_id]=$keyring
>                       done < "${KEYRING_IMPORT_DIR}/${keyring}-trusted"
>               fi
>       done
>  
> +     local -A revoked_ids
> +     for keyring in "${KEYRINGIDS[@]}"; do
> +             if [[ -s $KEYRING_IMPORT_DIR/$keyring-revoked ]]; then
> +                     while read -r key_id; do
> +                             if key_is_revoked "$key_id" ; then
> +                                     continue
> +                             fi
> +
> +                             revoked_ids["$key_id"]=1
> +                     done <"$KEYRING_IMPORT_DIR/$keyring-revoked"
> +             fi
> +     done
> +
>       if (( ${#trusted_ids[@]} > 0 )); then
>               msg "$(gettext "Locally signing trusted keys in keyring...")"
>               lsign_keys "${!trusted_ids[@]}"
> @@ -350,22 +367,10 @@ populate_keyring() {
>               done
>       fi
>  
> -     local -A revoked_ids
> -     for keyring in "${KEYRINGIDS[@]}"; do
> -             if [[ -s $KEYRING_IMPORT_DIR/$keyring-revoked ]]; then
> -                     while read -r key_id; do
> -                             revoked_ids["$key_id"]=1
> -                     done <"$KEYRING_IMPORT_DIR/$keyring-revoked"
> -             fi
> -     done
> -
>       if (( ${#revoked_ids[@]} > 0 )); then
>               local key_count=0
>               msg "$(gettext "Disabling revoked keys in keyring...")"
>               for key_id in "${!revoked_ids[@]}"; do
> -                     if key_is_revoked "$key_id" ; then
> -                             continue
> -                     fi
>                       if (( VERBOSE )); then
>                               msg2 "$(gettext "Disabling key %s...")" 
> "${key_id}"
>                       fi
> @@ -485,9 +490,6 @@ lsign_keys() {
>       local ret=0
>       local key_count=0
>       for key_id in "$@"; do
> -             if key_is_lsigned "$key_id" ; then
> -                     continue
> -             fi
>               if (( VERBOSE )); then
>                       msg2 "$(gettext "Locally signing key %s...")" 
> "${key_id}"
>               fi
> 

Reply via email to