[pacman-dev] [PATCH 1/3] makepkg: abort if signature verification fails due to a missing public key
--- scripts/makepkg.sh.in | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index e230c15..4c235cf 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1296,10 +1296,8 @@ check_pgpsigs() { printf '%s' "$(gettext "FAILED")" >&2 if ! pubkey=$(awk '/NO_PUBKEY/ { print $3; exit 1; }' "$statusfile"); then printf ' (%s)' "$(gettext "unknown public key") $pubkey" >&2 - warnings=1 - else - errors=1 fi + errors=1 printf '\n' >&2 else if grep -q "REVKEYSIG" "$statusfile"; then -- 1.9.0
--- scripts/makepkg.sh.in | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 4c235cf..015bdd7 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1303,6 +1303,9 @@ check_pgpsigs() { if grep -q "REVKEYSIG" "$statusfile"; then printf '%s (%s)' "$(gettext "FAILED")" "$(gettext "the key has been revoked.")" >&2 errors=1 + elif grep -q -e "TRUST_UNDEFINED" -e "TRUST_NEVER" "$statusfile"; then + printf '%s (%s)' "$(gettext "FAILED")" "$(gettext "the key is not trusted")" >&2 + errors=1 else printf '%s' "$(gettext "Passed")" >&2 if grep -q "EXPSIG" "$statusfile"; then -- 1.9.0
If validpgpkeys is set in the PKGBUILD, signature checking fails if the fingerprint of the key used to create the signature is not listed in the array. The key's trust value is ignored. --- doc/PKGBUILD.5.txt | 7 +++++++ scripts/makepkg.sh.in | 16 ++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/doc/PKGBUILD.5.txt b/doc/PKGBUILD.5.txt index 50d8347..7a1e924 100644 --- a/doc/PKGBUILD.5.txt +++ b/doc/PKGBUILD.5.txt @@ -128,6 +128,13 @@ Files in the source array with extensions `.sig`, `.sign` or, `.asc` are recognized by makepkg as PGP signatures and will be automatically used to verify the integrity of the corresponding source file. +*validpgpkeys (array)*:: + An array of PGP fingerprints. If this array is non-empty, makepkg will + only accept signatures from the keys listed here and will ignore the + trust values from the keyring. ++ +Fingerprints must be uppercase and must not contain whitespace characters. + *noextract (array)*:: An array of file names corresponding to those from the source array. Files listed here will not be extracted with the rest of the source files. This diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 015bdd7..6eb6d11 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1244,6 +1244,15 @@ check_checksums() { fi } +is_valid_pgpkey() { + local pubkey + + pubkey=$(grep VALIDSIG "$statusfile" | sed -nr 's/.* VALIDSIG ([A-Z0-9]*) .*/\1/p;') + echo "$pubkey" + in_array "$pubkey" ${validpgpkeys[@]} + return $? +} + check_pgpsigs() { (( SKIPPGPCHECK )) && return 0 ! source_has_signatures && return 0 @@ -1303,9 +1312,12 @@ check_pgpsigs() { if grep -q "REVKEYSIG" "$statusfile"; then printf '%s (%s)' "$(gettext "FAILED")" "$(gettext "the key has been revoked.")" >&2 errors=1 - elif grep -q -e "TRUST_UNDEFINED" -e "TRUST_NEVER" "$statusfile"; then + elif (( ${#validpgpkeys[@]} == 0 )) && grep -q -e "TRUST_UNDEFINED" -e "TRUST_NEVER" "$statusfile"; then printf '%s (%s)' "$(gettext "FAILED")" "$(gettext "the key is not trusted")" >&2 errors=1 + elif (( ${#validpgpkeys[@]} > 0 )) && ! pubkey=$(is_valid_pgpkey "$statusfile"); then + printf "%s (%s $pubkey)" "$(gettext "FAILED")" "$(gettext "invalid key")" + errors=1 else printf '%s' "$(gettext "Passed")" >&2 if grep -q "EXPSIG" "$statusfile"; then @@ -2810,7 +2822,7 @@ fi unset pkgname pkgbase pkgver pkgrel epoch pkgdesc url license groups provides unset md5sums replaces depends conflicts backup source install changelog build -unset makedepends optdepends options noextract +unset makedepends optdepends options noextract validpgpkeys BUILDFILE=${BUILDFILE:-$BUILDSCRIPT} if [[ ! -f $BUILDFILE ]]; then -- 1.9.0
On Sat, Mar 08, 2014 at 05:40:17PM +0100, Thomas Bächler wrote:
If validpgpkeys is set in the PKGBUILD, signature checking fails if the fingerprint of the key used to create the signature is not listed in the array.
The key's trust value is ignored. --- doc/PKGBUILD.5.txt | 7 +++++++ scripts/makepkg.sh.in | 16 ++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/doc/PKGBUILD.5.txt b/doc/PKGBUILD.5.txt index 50d8347..7a1e924 100644 --- a/doc/PKGBUILD.5.txt +++ b/doc/PKGBUILD.5.txt @@ -128,6 +128,13 @@ Files in the source array with extensions `.sig`, `.sign` or, `.asc` are recognized by makepkg as PGP signatures and will be automatically used to verify the integrity of the corresponding source file.
+*validpgpkeys (array)*:: + An array of PGP fingerprints. If this array is non-empty, makepkg will + only accept signatures from the keys listed here and will ignore the + trust values from the keyring. ++ +Fingerprints must be uppercase and must not contain whitespace characters. + *noextract (array)*:: An array of file names corresponding to those from the source array. Files listed here will not be extracted with the rest of the source files. This diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 015bdd7..6eb6d11 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1244,6 +1244,15 @@ check_checksums() { fi }
+is_valid_pgpkey() { + local pubkey + + pubkey=$(grep VALIDSIG "$statusfile" | sed -nr 's/.* VALIDSIG ([A-Z0-9]*) .*/\1/p;')
I think you just want: pubkey=$(sed -n '/VALIDSIG/ s/.* VALIDSIG \([[:alnum:]]*\) .*/\1/p' "$statusfile") sed's -r flag isn't portable.
+ echo "$pubkey"
Don't you only want to echo this if the check that follows succeeds?
+ in_array "$pubkey" ${validpgpkeys[@]}
The array needs quoting.
+ return $?
Wholly redundant for this function in its current form.
+} + check_pgpsigs() { (( SKIPPGPCHECK )) && return 0 ! source_has_signatures && return 0 @@ -1303,9 +1312,12 @@ check_pgpsigs() { if grep -q "REVKEYSIG" "$statusfile"; then printf '%s (%s)' "$(gettext "FAILED")" "$(gettext "the key has been revoked.")" >&2 errors=1 - elif grep -q -e "TRUST_UNDEFINED" -e "TRUST_NEVER" "$statusfile"; then + elif (( ${#validpgpkeys[@]} == 0 )) && grep -q -e "TRUST_UNDEFINED" -e "TRUST_NEVER" "$statusfile"; then printf '%s (%s)' "$(gettext "FAILED")" "$(gettext "the key is not trusted")" >&2 errors=1 + elif (( ${#validpgpkeys[@]} > 0 )) && ! pubkey=$(is_valid_pgpkey "$statusfile"); then + printf "%s (%s $pubkey)" "$(gettext "FAILED")" "$(gettext "invalid key")" + errors=1
Is there a decent way to extract the real status from the file once and then do string comparisons in bash, rather than forking to grep all the time?
else printf '%s' "$(gettext "Passed")" >&2 if grep -q "EXPSIG" "$statusfile"; then @@ -2810,7 +2822,7 @@ fi
unset pkgname pkgbase pkgver pkgrel epoch pkgdesc url license groups provides unset md5sums replaces depends conflicts backup source install changelog build -unset makedepends optdepends options noextract +unset makedepends optdepends options noextract validpgpkeys
BUILDFILE=${BUILDFILE:-$BUILDSCRIPT} if [[ ! -f $BUILDFILE ]]; then -- 1.9.0
Am 08.03.2014 18:20, schrieb Dave Reisner:
+ pubkey=$(grep VALIDSIG "$statusfile" | sed -nr 's/.* VALIDSIG ([A-Z0-9]*) .*/\1/p;')
I think you just want:
pubkey=$(sed -n '/VALIDSIG/ s/.* VALIDSIG \([[:alnum:]]*\) .*/\1/p' "$statusfile")
sed's -r flag isn't portable.
I took that from another place in makepkg: scripts/makepkg.sh.in:1740: for sofile in $(LC_ALL=C readelf -d "$filename" 2>/dev/null | sed -nr 's/.*Shared library: \[(.*)\].*/\1/p') If it's not portable, it should be fixed there, too.
+ echo "$pubkey"
Don't you only want to echo this if the check that follows succeeds?
Actually, I only need it in the failure case.
+ in_array "$pubkey" ${validpgpkeys[@]}
The array needs quoting.
Not according to the documentation, but sure, I'll add it.
+ return $?
Wholly redundant for this function in its current form.
You are right, although I hate that implicit return value stuff in bash.
+} + check_pgpsigs() { (( SKIPPGPCHECK )) && return 0 ! source_has_signatures && return 0 @@ -1303,9 +1312,12 @@ check_pgpsigs() { if grep -q "REVKEYSIG" "$statusfile"; then printf '%s (%s)' "$(gettext "FAILED")" "$(gettext "the key has been revoked.")" >&2 errors=1 - elif grep -q -e "TRUST_UNDEFINED" -e "TRUST_NEVER" "$statusfile"; then + elif (( ${#validpgpkeys[@]} == 0 )) && grep -q -e "TRUST_UNDEFINED" -e "TRUST_NEVER" "$statusfile"; then printf '%s (%s)' "$(gettext "FAILED")" "$(gettext "the key is not trusted")" >&2 errors=1 + elif (( ${#validpgpkeys[@]} > 0 )) && ! pubkey=$(is_valid_pgpkey "$statusfile"); then + printf "%s (%s $pubkey)" "$(gettext "FAILED")" "$(gettext "invalid key")" + errors=1
Is there a decent way to extract the real status from the file once and then do string comparisons in bash, rather than forking to grep all the time?
You just used the word 'decent' in a sentence that talks about gnupg. We could use read to parse the file, set some variables and test those. Is that desirable?
On Sat, Mar 08, 2014 at 06:34:53PM +0100, Thomas Bächler wrote:
Am 08.03.2014 18:20, schrieb Dave Reisner:
+ pubkey=$(grep VALIDSIG "$statusfile" | sed -nr 's/.* VALIDSIG ([A-Z0-9]*) .*/\1/p;')
I think you just want:
pubkey=$(sed -n '/VALIDSIG/ s/.* VALIDSIG \([[:alnum:]]*\) .*/\1/p' "$statusfile")
sed's -r flag isn't portable.
I took that from another place in makepkg:
scripts/makepkg.sh.in:1740: for sofile in $(LC_ALL=C readelf -d "$filename" 2>/dev/null | sed -nr 's/.*Shared library: \[(.*)\].*/\1/p')
If it's not portable, it should be fixed there, too.
Hrmm. It seems that, at least, more recent freebsd now suppots this: -r Same as -E for compatibility with GNU sed. I'd prefer that we avoid it, though. We can absolutely get by without it in both cases.
+ echo "$pubkey"
Don't you only want to echo this if the check that follows succeeds?
Actually, I only need it in the failure case.
+ in_array "$pubkey" ${validpgpkeys[@]}
The array needs quoting.
Not according to the documentation, but sure, I'll add it.
+ return $?
Wholly redundant for this function in its current form.
You are right, although I hate that implicit return value stuff in bash.
+} + check_pgpsigs() { (( SKIPPGPCHECK )) && return 0 ! source_has_signatures && return 0 @@ -1303,9 +1312,12 @@ check_pgpsigs() { if grep -q "REVKEYSIG" "$statusfile"; then printf '%s (%s)' "$(gettext "FAILED")" "$(gettext "the key has been revoked.")" >&2 errors=1 - elif grep -q -e "TRUST_UNDEFINED" -e "TRUST_NEVER" "$statusfile"; then + elif (( ${#validpgpkeys[@]} == 0 )) && grep -q -e "TRUST_UNDEFINED" -e "TRUST_NEVER" "$statusfile"; then printf '%s (%s)' "$(gettext "FAILED")" "$(gettext "the key is not trusted")" >&2 errors=1 + elif (( ${#validpgpkeys[@]} > 0 )) && ! pubkey=$(is_valid_pgpkey "$statusfile"); then + printf "%s (%s $pubkey)" "$(gettext "FAILED")" "$(gettext "invalid key")" + errors=1
Is there a decent way to extract the real status from the file once and then do string comparisons in bash, rather than forking to grep all the time?
You just used the word 'decent' in a sentence that talks about gnupg.
You're right, I must apologize for that faux pas.
We could use read to parse the file, set some variables and test those. Is that desirable?
I forget what the format is of the status files, exactly. I don't mind calling external tools to parse once, but calling them repeatedly to operate on the same section of a file seems wasteful.
Am 08.03.2014 18:47, schrieb Dave Reisner:
We could use read to parse the file, set some variables and test those. Is that desirable?
I forget what the format is of the status files, exactly. I don't mind calling external tools to parse once, but calling them repeatedly to operate on the same section of a file seems wasteful.
Lines of space-separated keywords. We can parse these easily with 'read', if we want to.
participants (2)
-
Dave Reisner
-
Thomas Bächler