commit:     c5247ab0ea0b717f4037b6aa14108d7e41b5e2dd
Author:     Kerin Millar <kfm <AT> plushkava <DOT> net>
AuthorDate: Mon Jun 16 00:07:30 2025 +0000
Commit:     Sam James <sam <AT> gentoo <DOT> org>
CommitDate: Mon Jun 16 01:17:28 2025 +0000
URL:        https://gitweb.gentoo.org/proj/portage.git/commit/?id=c5247ab0

emerge-webrsync: fix insecure key verification in 
check_file_signature_gpg_unwrapped()

Presently, the check_file_signature_gpg_unwrapped() function exists so
as to be able to support the legacy "webrsync-gpg" option, which may be
defined by FEATURES and is (barely) documented by make.conf(5). It is
also called upon in cases where gemato would be used but cannot be
found, for whatever reason.

Unfortunately, the way in which it performs signature verification is
insecure. It captures the standard output of gpg(1) and tries to match
any of its lines against this pattern:

  "[GNUPG:] GOODSIG"*

Firstly, it is incorrect in that it does not require for "GOODSIG" to be
followed by a space. Secondly, and rather more seriously, it is simply
insufficient to match the GOODSIG token in isolation. Consider the
following gpg(1) output sample, which has been reduced so as to show
only the GOODSIG, VALIDSIG and TRUST_ULTIMATE tokens.

[GNUPG:] GOODSIG EC590EEAC9189250 Gentoo ebuild repository signing key (Autom...
[GNUPG:] VALIDSIG E1D6ABB63BFCFB4BA02FDF1CEC590EEAC9189250 2025-06-15 1749949...
[GNUPG:] TRUST_ULTIMATE 0 pgp

In order to be sure that the key is ultimately trusted, that its
signature is valid, and that nothing has expired, it is necessary to
match against all three of the aforementioned tokens.

Consider, also, the following output, for which the key was at a trust
level of 4 (fully trusted), rather than 5 (ultimately trusted).

[GNUPG:] GOODSIG EC590EEAC9189250 Gentoo ebuild repository signing key (Autom...
[GNUPG:] VALIDSIG E1D6ABB63BFCFB4BA02FDF1CEC590EEAC9189250 2025-06-15 1749949...
[GNUPG:] TRUST_UNDEFINED 0 pgp

It demonstrates that there is little sense in trying to match for keys
whose trust levels are lower than 5. Indeed, it is possible for there to
be a sequence comprised by all three of GOODSIG, VALIDSIG and
TRUST_NEVER. Such would indicate that the signature is invalid, yet the
current code would consider it to be valid.

Address this defect by introducing a helper function named gpg_verify(),
which is responsible for executing gpg --verify and capturing its
output, then efficiently matching its output against all three of these
tokens: GOODSIG, VALIDSIG, TRUST_ULTIMATE.

[sam: Note that while this isn't great, it's also not as important
as it would otherwise be, given gemato is in stage3s and we use it
whenever available since the fixes for bug #597800. The code here is
only a fallback option and we warn when using it.]

Bug: https://bugs.gentoo.org/597800
Fixes: ffd68477e5c1e1badf60c86ae221c90dad50390d
Link: 
https://www.gnupg.org/documentation/manuals/gnupg/Automated-signature-checking.html
Signed-off-by: Kerin Millar <kfm <AT> plushkava.net>
Signed-off-by: Sam James <sam <AT> gentoo.org>

 bin/emerge-webrsync | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/bin/emerge-webrsync b/bin/emerge-webrsync
index b27cb67ee2..895835fa36 100755
--- a/bin/emerge-webrsync
+++ b/bin/emerge-webrsync
@@ -291,7 +291,7 @@ check_file_signature_gemato() {
 
 check_file_signature_gpg_unwrapped() {
        local signature=$1 file=$2
-       local gnupg_status gpgdir line key
+       local gpgdir key
 
        if type -P gpg > /dev/null; then
                if [[ -n ${PORTAGE_GPG_KEY} ]] ; then
@@ -317,23 +317,28 @@ check_file_signature_gpg_unwrapped() {
                        die "gpgdir is not writable: ${gpgdir}"
                fi
 
-               if gnupg_status=$(gpg --no-default-keyring --homedir 
"${gpgdir}" --batch \
-                       --status-fd 1 --verify "${signature}" "${file}"); then
-                       while read -r line; do
-                               if [[ ${line} == "[GNUPG:] GOODSIG"* ]]; then
-                                       return
-                               fi
-                       done <<< "${gnupg_status}"
+               if ! gpg_verify "${gpgdir}" "${signature}" "${file}"; then
+                       # Exit early since it's typically inappropriate to try
+                       # other mirrors in this case (it may indicate a keyring
+                       # problem).
+                       die "signature verification failed"
                fi
-
-               # Exit early since it's typically inappropriate to try other
-               # mirrors in this case (it may indicate a keyring problem).
-               die "signature verification failed"
        else
                die "cannot check signature: gpg binary not found"
        fi
 }
 
+gpg_verify() {
+       local gpgdir=$1 signature=$2 file=$3
+       local output token
+
+       # 
https://www.gnupg.org/documentation/manuals/gnupg/Automated-signature-checking.html
+       output=$(gpg --no-default-keyring --homedir "${gpgdir}" --batch 
--status-fd 1 --verify "${signature}" "${file}") || return
+       for token in GOODSIG VALIDSIG TRUST_ULTIMATE; do
+               [[ $'\n'${output} == *$'\n[GNUPG:] '"${token} "* ]] || return
+       done
+}
+
 check_file_signature() {
        local signature=$1 file=$2
        local r=1

Reply via email to