| Message ID | 20210826200720.48020-1-daan.j.demeyer@gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [pacman-dev] pacman-key: Reorder key_is_lsigned() and key_is_revoked() checks to reduce gpg trustdb checks | expand |
Ping On Thu, 26 Aug 2021 at 21:07, Daan De Meyer <daan.j.demeyer@gmail.com> 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(-) > > 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 > -- > 2.33.0 >
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 >
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