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.