[pacman-dev] [RFC] makepkg: Introduce acceptkeys array
If acceptkeys 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. Failure to verify the signature due to a missing public key is also treated as an error instead of a warning. --- scripts/makepkg.sh.in | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index e230c15..40c5b48 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1250,7 +1250,7 @@ check_pgpsigs() { msg "$(gettext "Verifying source file signatures with %s...")" "gpg" - local file pubkey ext decompress found + local file pubkey ext decompress found success local warning=0 local errors=0 local statusfile=$(mktemp) @@ -1296,7 +1296,11 @@ 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 + if (( ${#acceptkeys[@]} > 0 )); then + errors=1 + else + warnings=1 + fi else errors=1 fi @@ -1306,13 +1310,25 @@ check_pgpsigs() { printf '%s (%s)' "$(gettext "FAILED")" "$(gettext "the key has been revoked.")" >&2 errors=1 else - printf '%s' "$(gettext "Passed")" >&2 - if grep -q "EXPSIG" "$statusfile"; then - printf ' (%s)' "$(gettext "WARNING:") $(gettext "the signature has expired.")" >&2 - warnings=1 - elif grep -q "EXPKEYSIG" "$statusfile"; then - printf ' (%s)' "$(gettext "WARNING:") $(gettext "the key has expired.")" >&2 - warnings=1 + success=1 + if (( ${#acceptkeys[@]} > 0 )); then + pubkey=$(grep VALIDSIG "$statusfile" | sed -nr 's/.* VALIDSIG ([A-Z0-9]*) .*/\1/p;' | awk '{print tolower($0)}') + if ! in_array $pubkey ${acceptkeys[@]}; then + printf '%s' "$(gettext "FAILED")" >&2 + printf " ($(gettext 'the fingerprint %s is not accepted.'))" "$pubkey" >&2 + success=0 + errors=1 + fi + fi + if (( $success )); then + printf '%s' "$(gettext "Passed")" >&2 + if grep -q "EXPSIG" "$statusfile"; then + printf ' (%s)' "$(gettext "WARNING:") $(gettext "the signature has expired.")" >&2 + warnings=1 + elif grep -q "EXPKEYSIG" "$statusfile"; then + printf ' (%s)' "$(gettext "WARNING:") $(gettext "the key has expired.")" >&2 + warnings=1 + fi fi fi printf '\n' >&2 @@ -2809,7 +2825,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 acceptkeys BUILDFILE=${BUILDFILE:-$BUILDSCRIPT} if [[ ! -f $BUILDFILE ]]; then -- 1.9.0
On 07/03/14 05:05, Thomas Bächler wrote:
If acceptkeys 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. Failure to verify the signature due to a missing public key is also treated as an error instead of a warning. --- scripts/makepkg.sh.in | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-)
Fine. Small comments below. Also needs documentation. Just a small bikeshed... acceptkeys does not sound right. How about sourcepgpkeys?
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index e230c15..40c5b48 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1250,7 +1250,7 @@ check_pgpsigs() {
msg "$(gettext "Verifying source file signatures with %s...")" "gpg"
- local file pubkey ext decompress found + local file pubkey ext decompress found success local warning=0 local errors=0 local statusfile=$(mktemp) @@ -1296,7 +1296,11 @@ 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 + if (( ${#acceptkeys[@]} > 0 )); then + errors=1 + else + warnings=1 + fi
This seems strange from a user interface perspective - a missing key in a keyring can be a warning or error depending on other aspects of the PKGBUILD.
else errors=1 fi @@ -1306,13 +1310,25 @@ check_pgpsigs() { printf '%s (%s)' "$(gettext "FAILED")" "$(gettext "the key has been revoked.")" >&2 errors=1 else - printf '%s' "$(gettext "Passed")" >&2 - if grep -q "EXPSIG" "$statusfile"; then - printf ' (%s)' "$(gettext "WARNING:") $(gettext "the signature has expired.")" >&2 - warnings=1 - elif grep -q "EXPKEYSIG" "$statusfile"; then - printf ' (%s)' "$(gettext "WARNING:") $(gettext "the key has expired.")" >&2 - warnings=1 + success=1 + if (( ${#acceptkeys[@]} > 0 )); then + pubkey=$(grep VALIDSIG "$statusfile" | sed -nr 's/.* VALIDSIG ([A-Z0-9]*) .*/\1/p;' | awk '{print tolower($0)}')
tolower? I'd expect PGP keys to be given with capital letters. At least have acceptkeys run through tolower too.
+ if ! in_array $pubkey ${acceptkeys[@]}; then
So the PKGBUILD needs to specify the full fingerprint? This allows acceptkeys to specify shorter values: grep -f <(printf '%s$\n' "${acceptkeys[@]}") <(printf '%s\n' "$pubkey")
+ printf '%s' "$(gettext "FAILED")" >&2 + printf " ($(gettext 'the fingerprint %s is not accepted.'))" "$pubkey" >&2
Maybe: sources are not allowed to be signed by the PGP key %s
+ success=0 + errors=1 + fi + fi + if (( $success )); then + printf '%s' "$(gettext "Passed")" >&2 + if grep -q "EXPSIG" "$statusfile"; then + printf ' (%s)' "$(gettext "WARNING:") $(gettext "the signature has expired.")" >&2 + warnings=1 + elif grep -q "EXPKEYSIG" "$statusfile"; then + printf ' (%s)' "$(gettext "WARNING:") $(gettext "the key has expired.")" >&2 + warnings=1 + fi fi fi printf '\n' >&2 @@ -2809,7 +2825,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 acceptkeys
BUILDFILE=${BUILDFILE:-$BUILDSCRIPT} if [[ ! -f $BUILDFILE ]]; then
Am 08.03.2014 07:34, schrieb Allan McRae:
Fine. Small comments below. Also needs documentation.
I'll do documentation when it's final.
Just a small bikeshed... acceptkeys does not sound right. How about sourcepgpkeys?
Also strange. Their not source packages. Maybe rather 'sourcekeys', or 'acceptsourcekeys'?
if ! pubkey=$(awk '/NO_PUBKEY/ { print $3; exit 1; }' "$statusfile"); then printf ' (%s)' "$(gettext "unknown public key") $pubkey" >&2 - warnings=1 + if (( ${#acceptkeys[@]} > 0 )); then + errors=1 + else + warnings=1 + fi
This seems strange from a user interface perspective - a missing key in a keyring can be a warning or error depending on other aspects of the PKGBUILD.
With this option, I want to make signature checking strict and authoritative: Accept a file only when we are sure it has been properly verified. I don't want to have to second-guess makepkg's decision by looking through the output. When I build a new kernel, I want to be confident that makepkg will abort if it cannot verify the source. The old interface is extremely weird to me: Missing public keys or untrusted keys are not fatal - we just accept everything. If someone replaces the file and just puts a signature with a random in place, we accept the file (wtf?). Now, trust is not the correct solution in this case: If I trust a key for signing package A, that has nothing to do with package B. Another possibility that makes the whole interface clearer: Maybe make this option mandatory.
+ pubkey=$(grep VALIDSIG "$statusfile" | sed -nr 's/.* VALIDSIG ([A-Z0-9]*) .*/\1/p;' | awk '{print tolower($0)}')
tolower? I'd expect PGP keys to be given with capital letters. At least have acceptkeys run through tolower too.
It just looks nicer. As you mention it now, I noticed they are uppercase everywhere, so we should probably drop that. Should we just do nothing and expect uppercase fingerprints from the user?
+ if ! in_array $pubkey ${acceptkeys[@]}; then
So the PKGBUILD needs to specify the full fingerprint?
Yes. It's the only thing that is unique.
This allows acceptkeys to specify shorter values: grep -f <(printf '%s$\n' "${acceptkeys[@]}") <(printf '%s\n' "$pubkey")
I don't like it. A PKGBUILD isn't supposed to be "fuzzy". Using the fingerprint takes one second longer once, but gives more security for the lifetime of the package (for the Linux package, there are two fingerprints that need to be added, and they never change).
+ printf '%s' "$(gettext "FAILED")" >&2 + printf " ($(gettext 'the fingerprint %s is not accepted.'))" "$pubkey" >&2
Maybe: sources are not allowed to be signed by the PGP key %s
The other messages are so short in comparison. But yes, I'd be fine with that message.
participants (2)
-
Allan McRae
-
Thomas Bächler