PGP signature files can contain multiple signatures from different keys, see e.g. the source tarballs of recent GnuPG releases like version 2.2.35. parse_gpg_statusfile() is currently not equipped to handle the gpg output for such signature files: it assumes that the file contains only one signature and therefore overwrites the verification results of previous signatures, effectively only considering the last signature in the file.
It is not clearly documented how a signature file containing multiple signatures should be handled. The gpgv verification tools opts to check *all* signatures and to fail if any of them are not valid or untrusted. At first glance this appears to make sense, but there are severe disadvantages: in order to verify a signature file with multiple signatures, all its keys have to be added to validpgpkeys. This means old or otherwise untrusted keys might need to be added just for the verification to succeed. On the other hand, there is no way to set a threshold of keys to be used for the verification in the PKGBUILD. This means that a signature file only containing a signature by one of these additional keys would now be accepted as valid as well. In conclusion, it is better to leave it to packagers to choose which of the keys in a signature file they want to trust and to consider a signature file as valid if *any* of the signatures it contains are valid. To implement this, the logic in parse_gpg_statusfile() needs to be refined so that $success and $trusted are true if any valid/trusted signature is found. We also need to store the verification results per key (in order to present more specific error messages) and all the validated fingerprints (in order to check whether there is an intersection with validpgpkeys). Furthermore, the error handling of missing/revoked/bad keys in check_pgpsigs() needs to be moved so that it can be non-fatal as long as there is a single valid signature. Signed-off-by: Jonas Witschel <[email protected]> --- .../integrity/verify_signature.sh.in | 103 ++++++++++-------- scripts/libmakepkg/util/util.sh.in | 12 ++ 2 files changed, 68 insertions(+), 47 deletions(-) diff --git a/scripts/libmakepkg/integrity/verify_signature.sh.in b/scripts/libmakepkg/integrity/verify_signature.sh.in index ad5bb66d..f77d1189 100644 --- a/scripts/libmakepkg/integrity/verify_signature.sh.in +++ b/scripts/libmakepkg/integrity/verify_signature.sh.in @@ -32,7 +32,7 @@ check_pgpsigs() { msg "$(gettext "Verifying source file signatures with %s...")" "gpg" - local netfile proto pubkey success status fingerprint trusted + local netfile proto pubkey success status fingerprints trusted local warnings=0 local errors=0 local statusfile=$(mktemp) @@ -57,49 +57,59 @@ check_pgpsigs() { # these variables are assigned values in parse_gpg_statusfile success=0 - status= - pubkey= - fingerprint= - trusted= + declare -A status + fingerprints=() + trusted=0 parse_gpg_statusfile "$statusfile" if (( ! $success )); then printf '%s' "$(gettext "FAILED")" >&2 - case "$status" in - "missingkey") - printf ' (%s)' "$(gettext "unknown public key") $pubkey" >&2 - ;; - "revokedkey") - printf " ($(gettext "public key %s has been revoked"))" "$pubkey" >&2 - ;; - "bad") - printf ' (%s)' "$(gettext "bad signature from public key") $pubkey" >&2 - ;; - "error") - printf ' (%s)' "$(gettext "error during signature verification")" >&2 - ;; - esac errors=1 else if (( ${#validpgpkeys[@]} == 0 && !trusted )); then - printf "%s ($(gettext "the public key %s is not trusted"))" $(gettext "FAILED") "$fingerprint" >&2 + printf '%s' "$(gettext "FAILED")" >&2 + for pubkey in "${fingerprints[@]}"; do + printf " ($(gettext "the public key %s is not trusted"))" "$pubkey" >&2 + done errors=1 - elif (( ${#validpgpkeys[@]} > 0 )) && ! in_array "$fingerprint" "${validpgpkeys[@]}"; then - printf "%s (%s %s)" "$(gettext "FAILED")" "$(gettext "invalid public key")" "$fingerprint" >&2 + elif (( ${#validpgpkeys[@]} > 0 )) && ! arrays_intersect fingerprints validpgpkeys; then + printf '%s' "$(gettext "FAILED")" >&2 + for pubkey in "${fingerprints[@]}"; do + printf " (%s %s)" "$(gettext "invalid public key")" "$pubkey" >&2 + done errors=1 else printf '%s' "$(gettext "Passed")" >&2 - case "$status" in + fi + fi + + if (( warnings )); then + for pubkey in "${!status[@]}"; do + case "${status["$pubkey"]}" in + "good") + printf ' (%s)' "$(gettext "good signature from public key") $pubkey" >&2 + ;; + "missingkey") + printf ' (%s)' "$(gettext "unknown public key") $pubkey" >&2 + ;; + "revokedkey") + printf " ($(gettext "public key %s has been revoked"))" "$pubkey" >&2 + ;; + "bad") + printf ' (%s)' "$(gettext "bad signature from public key") $pubkey" >&2 + ;; + "error") + printf ' (%s)' "$(gettext "error during signature verification")" >&2 + ;; "expired") - printf ' (%s)' "$(gettext "WARNING:") $(gettext "the signature has expired.")" >&2 - warnings=1 + printf " (%s $(gettext "the signature by key %s has expired."))" "$(gettext "WARNING:")" "$pubkey" >&2 ;; "expiredkey") - printf ' (%s)' "$(gettext "WARNING:") $(gettext "the key has expired.")" >&2 - warnings=1 + printf " (%s $(gettext "the key %s has expired."))" "$(gettext "WARNING:")" "$pubkey" >&2 ;; esac - fi + done fi + printf '\n' >&2 done @@ -204,50 +214,49 @@ parse_gpg_statusfile() { while read -r _ type arg1 _ _ _ _ arg6 _ _ _ arg10 _; do case "$type" in GOODSIG) - pubkey=$arg1 success=1 - status="good" + status["$arg1"]="good" ;; EXPSIG) - pubkey=$arg1 success=1 - status="expired" + warnings=1 + status["$arg1"]="expired" ;; EXPKEYSIG) - pubkey=$arg1 success=1 - status="expiredkey" + warnings=1 + status["$arg1"]="expiredkey" ;; REVKEYSIG) - pubkey=$arg1 - success=0 - status="revokedkey" + warnings=1 + status["$arg1"]="revokedkey" ;; BADSIG) - pubkey=$arg1 - success=0 - status="bad" + warnings=1 + status["$arg1"]="bad" ;; ERRSIG) - pubkey=$arg1 - success=0 + warnings=1 if [[ $arg6 == 9 ]]; then - status="missingkey" + status["$arg1"]="missingkey" else - status="error" + status["$arg1"]="error" fi ;; VALIDSIG) if [[ $arg10 ]]; then # If the file was signed with a subkey, arg10 contains # the fingerprint of the primary key - fingerprint=$arg10 + fingerprints+=("$arg10") else - fingerprint=$arg1 + fingerprints+=("$arg1") fi ;; TRUST_UNDEFINED|TRUST_NEVER) - trusted=0 + # If the signature file contains multiple signatures, we + # consider the file to be valid if any of the signatures is + # made by a trusted key, so having other untrusted signatures + # is not a fatal error ;; TRUST_MARGINAL|TRUST_FULLY|TRUST_ULTIMATE) trusted=1 diff --git a/scripts/libmakepkg/util/util.sh.in b/scripts/libmakepkg/util/util.sh.in index 6c0e589a..15499227 100644 --- a/scripts/libmakepkg/util/util.sh.in +++ b/scripts/libmakepkg/util/util.sh.in @@ -53,6 +53,18 @@ is_array() { return $ret } +# Returns 0 if the two arrays passed by name have intersecting elements, +# 1 otherwise +arrays_intersect() { + local -n _arrays_intersect_first=$1 + local -n _arrays_intersect_second=$2 + local element + for element in "${_arrays_intersect_first[@]}"; do + in_array "$element" "${_arrays_intersect_second[@]}" && return 0 + done + return 1 +} + # Canonicalize a directory path if it exists canonicalize_path() { local path="$1" -- 2.36.1
