[pacman-dev] [PATCH 2/5] pacman-key: import everything then revoke on --populate

Allan McRae

23 Aug 2011 23 Aug '11

6:17 a.m.

The optimization of only importing keys that were not to be later revoked was a not smart enough. For example, if a key was in both a repos keyring and its revoke list, alternate runs of pacman-key --populate would add then remove the key from the pacman keyring. This problem is made worse when considering the possibility of multiple keyrings being imported. Instead, import all keys followed by the revoking of all keys. This may result in a key being added then revoked, but that is not much of an issue given that is a very fast operation. Signed-off-by: Allan McRae --- scripts/pacman-key.sh.in | 23 ++++++++++------------- 1 files changed, 10 insertions(+), 13 deletions(-) diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index 8580100..3e31abb 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -199,7 +199,16 @@ populate_keyring() { local key local key_id - # Read the key ids to an array. The conversion from whatever is inside the file + # Add keys from requested keyrings + for keyring in ${KEYRINGIDS[@]}; do + msg "$(gettext "Appending keys from %s.gpg...")" "$keyring" + local add_keys="$("${GPG_NOKEYRING[@]}" --keyring "${KEYRING_IMPORT_DIR}/${keyring}.gpg" --with-colons --list-keys | grep ^pub | cut -d: -f5)" + for key_id in ${add_keys}; do + "${GPG_NOKEYRING[@]}" --keyring "${KEYRING_IMPORT_DIR}/${keyring}.gpg" --export "${key_id}" | "${GPG_PACMAN[@]}" --import + done + done + + # Read the revoked key IDs to an array. The conversion from whatever is inside the file # to key ids is important, because key ids are the only guarantee of identification # for the keys. local -A removed_ids @@ -236,18 +245,6 @@ populate_keyring() { done fi - # Add keys from requested keyrings - for keyring in ${KEYRINGIDS[@]}; do - msg "$(gettext "Appending keys from %s.gpg...")" "$keyring" - local add_keys="$("${GPG_NOKEYRING[@]}" --keyring "${KEYRING_IMPORT_DIR}/${keyring}.gpg" --with-colons --list-keys | grep ^pub | cut -d: -f5)" - for key_id in ${add_keys}; do - # There is no point in adding a key that will be deleted right after - if [[ -z "${removed_ids[$key_id]}" ]]; then - "${GPG_NOKEYRING[@]}" --keyring "${KEYRING_IMPORT_DIR}/${keyring}.gpg" --export "${key_id}" | "${GPG_PACMAN[@]}" --import - fi - done - done - # Remove the keys not marked to keep if (( ${#removed_ids[@]} > 0 )); then msg "$(gettext "Removing revoked keys from keyring...")" -- 1.7.6

Show replies by date

Allan McRae

23 Aug 23 Aug

6:17 a.m.

New subject: [pacman-dev] [PATCH 3/5] pacman-key: update trust database for relevant operations

After most operations that touch the keyring, it is a good idea to always run a check on the trustdb as this prevents gpg complaining on later operations. Signed-off-by: Allan McRae --- scripts/pacman-key.sh.in | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-) diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index 3e31abb..6d07482 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -253,10 +253,6 @@ populate_keyring() { "${GPG_PACMAN[@]}" --quiet --batch --yes --delete-key "${key_id}" done fi - - # Update trustdb, just to be sure - msg "$(gettext "Updating trust database...")" - "${GPG_PACMAN[@]}" --batch --check-trustdb } receive_keys() { @@ -417,4 +413,9 @@ esac (( UPDATEDB )) && "${GPG_PACMAN[@]}" --batch --check-trustdb (( VERIFY )) && "${GPG_PACMAN[@]}" --verify $SIGNATURE +if (( ADD || DELETE || EDITKEY || IMPORT || IMPORT_TRUSTDB || POPULATE || RECEIVE )); then + msg "$(gettext "Updating trust database...")" + "${GPG_PACMAN[@]}" --batch --check-trustdb +fi + # vim: set ts=2 sw=2 noet: -- 1.7.6

Dave Reisner

10:21 a.m.

New subject: [pacman-dev] [PATCH 3/5] pacman-key: update trust database for relevant operations

On Tue, Aug 23, 2011 at 04:17:44PM +1000, Allan McRae wrote:

...

After most operations that touch the keyring, it is a good idea to always run a check on the trustdb as this prevents gpg complaining on later operations.

Signed-off-by: Allan McRae --- scripts/pacman-key.sh.in | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index 3e31abb..6d07482 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -253,10 +253,6 @@ populate_keyring() { "${GPG_PACMAN[@]}" --quiet --batch --yes --delete-key "${key_id}" done fi - - # Update trustdb, just to be sure - msg "$(gettext "Updating trust database...")" - "${GPG_PACMAN[@]}" --batch --check-trustdb }

receive_keys() { @@ -417,4 +413,9 @@ esac (( UPDATEDB )) && "${GPG_PACMAN[@]}" --batch --check-trustdb (( VERIFY )) && "${GPG_PACMAN[@]}" --verify $SIGNATURE

+if (( ADD || DELETE || EDITKEY || IMPORT || IMPORT_TRUSTDB || POPULATE || RECEIVE )); then + msg "$(gettext "Updating trust database...")" + "${GPG_PACMAN[@]}" --batch --check-trustdb +fi + # vim: set ts=2 sw=2 noet: -- 1.7.6

Just a different approach... instead of hardcoding another list, tag them in options parsing and check for UPDATEDB last. Patch below is against master, but probably gives you a better idea of what I mean... d diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index 74ecfcf..2ebe22d 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -336,20 +336,20 @@ fi while true; do case "$1" in - -a|--add) ADD=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYFILES=($1) ;; + -a|--add) UPDATEDB=1 ADD=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYFILES=($1) ;; --config) shift; CONFIG=$1 ;; - -d|--delete) DELETE=1; shift; KEYIDS=($1) ;; - --edit-key) EDITKEY=1; shift; KEYIDS=($1) ;; + -d|--delete) UPDATEDB=1 DELETE=1; shift; KEYIDS=($1) ;; + --edit-key) UPDATEDB=1 EDITKEY=1; shift; KEYIDS=($1) ;; -e|--export) EXPORT=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYIDS=($1) ;; -f|--finger) FINGER=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYIDS=($1) ;; --gpgdir) shift; PACMAN_KEYRING_DIR=$1 ;; - --import) IMPORT=1; shift; IMPORT_DIRS=($1) ;; - --import-trustdb) IMPORT_TRUSTDB=1; shift; IMPORT_DIRS=($1) ;; + --import) UPDATEDB=1 IMPORT=1; shift; IMPORT_DIRS=($1) ;; + --import-trustdb) UPDATEDB=1 IMPORT_TRUSTDB=1; shift; IMPORT_DIRS=($1) ;; --init) INIT=1 ;; -l|--list-keys) LISTKEYS=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYIDS=($1) ;; --list-sigs) LISTSIGS=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYIDS=($1) ;; - -r|--receive) RECEIVE=1; shift; TMP=($1); KEYSERVER=${TMP[0]}; KEYIDS=(${TMP[@]:1}); unset TMP;; - --reload) RELOAD=1 ;; + -r|--receive) UPDATEDB=1 RECEIVE=1; shift; TMP=($1); KEYSERVER=${TMP[0]}; KEYIDS=(${TMP[@]:1}); unset TMP;; + --reload) UPDATEDB=1 RELOAD=1 ;; -u|--updatedb) UPDATEDB=1 ;; -v|--verify) VERIFY=1; shift; SIGNATURE=$1 ;; @@ -387,7 +387,7 @@ GPG_PACMAN=(gpg --homedir ${PACMAN_KEYRING_DIR} --no-permission-warning) # check only a single operation has been given numopt=$(( ADD + DELETE + EDITKEY + EXPORT + FINGER + IMPORT + IMPORT_TRUSTDB + - INIT + LISTKEYS + LISTSIGS + RECEIVE + RELOAD + UPDATEDB + VERIFY )) + INIT + LISTKEYS + LISTSIGS + RECEIVE + RELOAD + VERIFY )) case $numopt in 0) @@ -415,7 +415,8 @@ esac (( LISTSIGS )) && "${GPG_PACMAN[@]}" --batch --list-sigs "${KEYIDS[@]}" (( RECEIVE )) && receive_keys (( RELOAD )) && reload_keyring -(( UPDATEDB )) && "${GPG_PACMAN[@]}" --batch --check-trustdb (( VERIFY )) && "${GPG_PACMAN[@]}" --verify $SIGNATURE +(( UPDATEDB )) && "${GPG_PACMAN[@]}" --batch --check-trustdb + # vim: set ts=2 sw=2 noet:

Allan McRae

6:17 a.m.

New subject: [pacman-dev] [PATCH 4/5] pacman-key: Improve documentation for --populate

Signed-off-by: Allan McRae --- doc/pacman-key.8.txt | 19 +++++++++++++++---- 1 files changed, 15 insertions(+), 4 deletions(-) diff --git a/doc/pacman-key.8.txt b/doc/pacman-key.8.txt index f61c2ec..ff8d38d 100644 --- a/doc/pacman-key.8.txt +++ b/doc/pacman-key.8.txt @@ -82,10 +82,8 @@ Options *\--populate* []:: Reload the default keys from the (optionally provided) keyrings in - +{pkgdatadir}/keyrings+. Each keyring is provided in a file foo.gpg that - contains the keys for the foo keyring. Optionally the file foo-revoked - contains a list of revoked key IDs for that keyring. These files are - required to be signed (detached) by a trusted PGP key. + +{pkgdatadir}/keyrings+. For more information, see + <> below. *-u, \--updatedb*:: Equivalent to \--check-trustdb in GnuPG. @@ -97,6 +95,19 @@ Options Displays the program version. +Providing a Keyring for Import +------------------------------ +A distribution or other repository provided may want to provide a set of valid +PGP keys used in the signing of its packages and repository databases that can +be readily imported into the pacman keyring. This is achieved by providing a +PGP keyring file `foo.gpg` that contains the keys for the foo keyring in the +directory +{pkgdatadir}/keyrings+. Optionally the file `foo-revoked` can be +provided containing a list of revoked key IDs for that keyring. These files are +required to be signed (detached) by a trusted PGP key that the user must +manually import to the pacman keyring. This prevents a potentially malicious +repository adding keys to the pacman keyring without the users knowledge. + + See Also -------- linkman:pacman[8], linkman:pacman.conf[5] -- 1.7.6

Allan McRae

6:17 a.m.

New subject: [pacman-dev] [PATCH 5/5] pacman-key: rework and document holding keys in keyring

The HoldKey option was undocumented and was not suited for pacman.conf. Instead use the file "/etc/pacman.d/gnupg/heldkeys" to contain a list of keys not to be removed from the pacman keyring with the --populate option. Signed-off-by: Allan McRae --- doc/pacman-key.8.txt | 8 ++++++-- scripts/pacman-key.sh.in | 12 +++++------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/doc/pacman-key.8.txt b/doc/pacman-key.8.txt index ff8d38d..077b3ba 100644 --- a/doc/pacman-key.8.txt +++ b/doc/pacman-key.8.txt @@ -101,12 +101,16 @@ A distribution or other repository provided may want to provide a set of valid PGP keys used in the signing of its packages and repository databases that can be readily imported into the pacman keyring. This is achieved by providing a PGP keyring file `foo.gpg` that contains the keys for the foo keyring in the -directory +{pkgdatadir}/keyrings+. Optionally the file `foo-revoked` can be +directory +{pkgdatadir}/keyrings+. Optionally the file `foo-revoked` can be provided containing a list of revoked key IDs for that keyring. These files are required to be signed (detached) by a trusted PGP key that the user must -manually import to the pacman keyring. This prevents a potentially malicious +manually import to the pacman keyring. This prevents a potentially malicious repository adding keys to the pacman keyring without the users knowledge. +A key being marked as revoked always takes priority over the key being added to +the pacman keyring, regardless of the keyring it is provided in. To prevent a +key from being revoked when using --populate, its ID can be listed in ++{sysconfdir}/pacman.d/gnupg/heldkeys+. See Also -------- diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index 6d07482..da12a1e 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -232,17 +232,15 @@ populate_keyring() { fi done - # List of keys that must be kept installed, even if in the list of keys to be removed - local HOLD_KEYS="$(get_from "$CONFIG" "HoldKeys")" - - # Remove the keys that must be kept from the set of keys that should be removed - if [[ -n ${HOLD_KEYS} ]]; then - for key in ${HOLD_KEYS}; do + # Read list of keys that must be kept installed and remove them from the list + # of keys to be removed + if [[ -f "${PACMAN_KEYRING_DIR}/heldkeys" ]]; then + while read key; do key_id="$("${GPG_PACMAN[@]}" --quiet --with-colons --list-key "${key}" | grep ^pub | cut -d: -f5)" if [[ -n "${removed_ids[$key_id]}" ]]; then unset removed_ids[$key_id] fi - done + done < "${PACMAN_KEYRING_DIR}/heldkeys" fi # Remove the keys not marked to keep -- 1.7.6

Dan McGee

6:30 a.m.

New subject: [pacman-dev] [PATCH 5/5] pacman-key: rework and document holding keys in keyring

On Tue, Aug 23, 2011 at 1:17 AM, Allan McRae wrote:

...

The HoldKey option was undocumented and was not suited for pacman.conf. Instead use the file "/etc/pacman.d/gnupg/heldkeys" to contain a list of keys not to be removed from the pacman keyring with the --populate option.

Signed-off-by: Allan McRae ---

We have 'heldkeys' and 'HOLD_KEYS' here- perhaps we should pick one tense and word-splitting combo? I'd prefer holdkeys/HOLDKEYS, only because the tense of 'heldkeys' seems different than anything else we do to me. Otherwise this looks pretty good. Is it worth providing an example in pacman-key documentation of a valid format for the held and revoked lists?

...

doc/pacman-key.8.txt | 8 ++++++-- scripts/pacman-key.sh.in | 12 +++++------- 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/doc/pacman-key.8.txt b/doc/pacman-key.8.txt index ff8d38d..077b3ba 100644 --- a/doc/pacman-key.8.txt +++ b/doc/pacman-key.8.txt @@ -101,12 +101,16 @@ A distribution or other repository provided may want to provide a set of valid PGP keys used in the signing of its packages and repository databases that can be readily imported into the pacman keyring. This is achieved by providing a PGP keyring file `foo.gpg` that contains the keys for the foo keyring in the -directory +{pkgdatadir}/keyrings+. Optionally the file `foo-revoked` can be +directory +{pkgdatadir}/keyrings+. Optionally the file `foo-revoked` can be provided containing a list of revoked key IDs for that keyring. These files are required to be signed (detached) by a trusted PGP key that the user must -manually import to the pacman keyring. This prevents a potentially malicious +manually import to the pacman keyring. This prevents a potentially malicious repository adding keys to the pacman keyring without the users knowledge.

+A key being marked as revoked always takes priority over the key being added to +the pacman keyring, regardless of the keyring it is provided in. To prevent a +key from being revoked when using --populate, its ID can be listed in ++{sysconfdir}/pacman.d/gnupg/heldkeys+.

See Also -------- diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index 6d07482..da12a1e 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -232,17 +232,15 @@ populate_keyring() { fi done

- # List of keys that must be kept installed, even if in the list of keys to be removed - local HOLD_KEYS="$(get_from "$CONFIG" "HoldKeys")" - - # Remove the keys that must be kept from the set of keys that should be removed - if [[ -n ${HOLD_KEYS} ]]; then - for key in ${HOLD_KEYS}; do + # Read list of keys that must be kept installed and remove them from the list + # of keys to be removed + if [[ -f "${PACMAN_KEYRING_DIR}/heldkeys" ]]; then + while read key; do key_id="$("${GPG_PACMAN[@]}" --quiet --with-colons --list-key "${key}" | grep ^pub | cut -d: -f5)" if [[ -n "${removed_ids[$key_id]}" ]]; then unset removed_ids[$key_id] fi - done + done < "${PACMAN_KEYRING_DIR}/heldkeys" fi

# Remove the keys not marked to keep -- 1.7.6

Allan McRae

26 Aug 26 Aug

10:31 a.m.

New subject: [pacman-dev] [PATCH 5/5] pacman-key: rework and document holding keys in keyring

On 23/08/11 16:30, Dan McGee wrote:

...

On Tue, Aug 23, 2011 at 1:17 AM, Allan McRae wrote:

...
The HoldKey option was undocumented and was not suited for pacman.conf. Instead use the file "/etc/pacman.d/gnupg/heldkeys" to contain a list of keys not to be removed from the pacman keyring with the --populate option.

Signed-off-by: Allan McRae ---

We have 'heldkeys' and 'HOLD_KEYS' here- perhaps we should pick one tense and word-splitting combo? I'd prefer holdkeys/HOLDKEYS, only because the tense of 'heldkeys' seems different than anything else we do to me.

Otherwise this looks pretty good. Is it worth providing an example in pacman-key documentation of a valid format for the held and revoked lists?

I'm not sure if it is worth providing an example as it is just a list of key IDs. We can always add one later if this is something that needs clarified. Allan

4637

Age (days ago)

4640

Last active (days ago)

List overview

Download

6 comments

3 participants

participants (3)

Allan McRae
Dan McGee
Dave Reisner