From patchwork Wed Jun 8 21:21:11 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonas Witschel X-Patchwork-Id: 2066 Return-Path: Delivered-To: patchwork@archlinux.org Received: from mail.archlinux.org [2a01:4f9:c010:3052::1] by patchwork.archlinux.org with IMAP (fetchmail-6.4.30) for (single-drop); Wed, 08 Jun 2022 21:21:34 +0000 (UTC) Received: from mail.archlinux.org by mail.archlinux.org with LMTP id wCK4Ht0SoWIlWQEAK+/4rw (envelope-from ) for ; Wed, 08 Jun 2022 21:21:33 +0000 Received: from lists.archlinux.org (lists.archlinux.org [95.217.236.249]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mail.archlinux.org (Postfix) with ESMTPS id B0F4C10B6605; Wed, 8 Jun 2022 21:21:32 +0000 (UTC) Received: from lists.archlinux.org (localhost [IPv6:::1]) by lists.archlinux.org (Postfix) with ESMTP id 88344109ACEF; Wed, 8 Jun 2022 21:21:32 +0000 (UTC) X-Original-To: pacman-dev@lists.archlinux.org Delivered-To: pacman-dev@lists.archlinux.org Received: from mail.archlinux.org (mail.archlinux.org [95.216.189.61]) by lists.archlinux.org (Postfix) with ESMTPS id 33A86109ACDE for ; Wed, 8 Jun 2022 21:21:31 +0000 (UTC) From: Jonas Witschel DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=archlinux.org; s=dkim-rsa; t=1654723290; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=AVb31oYQITfRKyTYX60+X1fUX2pjQ8CrlbsdX1etsTU=; b=xJNH8+XNtcR0Z+sgaE8Cex8+LA7hhMdVCZKYq1eqIVv4ssPmv5qrhIjDoL09tpYA/HHC4M sNk0VZQex+HIicQN3tFyZZ9OgUgjbV9srffaoJzHsSnB6U1hey4hSIh4lItsbPs1yaha2z 0dIhGgklFDaPQ9L8LLxzsZTKN1e9OERll7cbtn1gVWh2P3lXoDm1N+5DU/L/HOOBKQ49a1 Ilzx3uo3FMVUndHnnTI0muAB3U/31qeUr3nwgDszUbEKn+JBCQKeLCOGxq7pyY0gLnGK/Q TdNCXi0BqYNH/JO32sqT3kCd231pG5dq7fewkZxjTXagTxookLtp/r+WwCAprdYIxGEkpU O2+/iptg0TizWAukr8RKMaDyntgLNdDKl0VqBF7hgdPWygP5pSin9VnuWv1/sqSLE0HNdw g33OLC31BEoLuTjdkHA4FJr6j6k5uX82h41h9oi+Pf1MHchyRtt3Q0KmC13izYHqW0E6Jo xX9Lf6eOJ+xDeKHkq+XkAGVJ2XlJj4eLUNAu4YOWfhz4PdivM0hiZFIFMOsdhKLgCTRRS7 wBiukhFAvYDJX21DJ5H7waCyZq9BsPFjRx0m40VRFRyEP99+2o1ntLPyeUET4Z4/ItJ/+p +PmI1za8fC6tUDvtMwCgv+Bh9Px0j85iBpcX8ZuqA/NAtOl2ZacrY= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=archlinux.org; s=dkim-ed25519; t=1654723291; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=AVb31oYQITfRKyTYX60+X1fUX2pjQ8CrlbsdX1etsTU=; b=XoRLJLI7nwU5p7mXhTgnrXnYVyqk5pPtTkcvhZWIn9KflvXYQqSALYshr7lNE1qKOEzvaF T7f29A9T1QyEGJDw== To: pacman-dev@lists.archlinux.org Cc: Levente Polyak , Jonas Witschel Subject: [PATCH v2] libmakepkg/integrity: handle PGP signature files containing multiple signatures Date: Wed, 8 Jun 2022 23:21:11 +0200 Message-Id: <20220608212111.328157-1-diabonas@archlinux.org> X-Mailer: git-send-email 2.36.1 In-Reply-To: <20220608211655.iqwohkiehelvmuzx@archlinux.org> References: <20220608211655.iqwohkiehelvmuzx@archlinux.org> MIME-Version: 1.0 X-BeenThere: pacman-dev@lists.archlinux.org X-Mailman-Version: 2.1.39 Precedence: list List-Id: Discussion list for pacman development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: pacman-dev-bounces@lists.archlinux.org Sender: "pacman-dev" Authentication-Results: mail.archlinux.org; dkim=pass header.d=archlinux.org header.s=dkim-rsa header.b=xJNH8+XN; dkim=pass header.d=archlinux.org header.s=dkim-ed25519 header.b=XoRLJLI7; spf=pass (mail.archlinux.org: domain of pacman-dev-bounces@lists.archlinux.org designates 95.217.236.249 as permitted sender) smtp.mailfrom=pacman-dev-bounces@lists.archlinux.org; dmarc=pass (policy=none) header.from=archlinux.org X-Rspamd-Server: mail.archlinux.org X-Spamd-Result: default: False [-6.61 / 15.00]; REPLY(-4.00)[]; DWL_DNSWL_MED(-2.00)[archlinux.org:dkim]; MID_CONTAINS_FROM(1.00)[]; DMARC_POLICY_ALLOW(-0.50)[archlinux.org,none]; R_MISSING_CHARSET(0.50)[]; RCVD_DKIM_ARC_DNSWL_MED(-0.50)[]; RCVD_IN_DNSWL_MED(-0.40)[95.216.189.61:received,95.217.236.249:from]; R_DKIM_ALLOW(-0.20)[archlinux.org:s=dkim-rsa,archlinux.org:s=dkim-ed25519]; R_SPF_ALLOW(-0.20)[+ip4:95.217.236.249:c]; MAILLIST(-0.20)[mailman]; MIME_GOOD(-0.10)[text/plain]; HAS_LIST_UNSUB(-0.01)[]; FROM_HAS_DN(0.00)[]; PREVIOUSLY_DELIVERED(0.00)[pacman-dev@lists.archlinux.org]; RCPT_COUNT_THREE(0.00)[3]; TO_DN_SOME(0.00)[]; ASN(0.00)[asn:24940, ipnet:95.217.0.0/16, country:DE]; FROM_NEQ_ENVFROM(0.00)[diabonas@archlinux.org,pacman-dev-bounces@lists.archlinux.org]; DKIM_TRACE(0.00)[archlinux.org:+]; RCVD_COUNT_THREE(0.00)[3]; ARC_NA(0.00)[]; MIME_TRACE(0.00)[0:+]; NEURAL_HAM(-0.00)[-1.000]; RCVD_TLS_LAST(0.00)[]; FORGED_SENDER_MAILLIST(0.00)[] X-Rspamd-Queue-Id: B0F4C10B6605 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 --- .../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"