On 6/25/22 15:40, Jonas Witschel wrote:
On 2022-06-25 22:09, Allan McRae wrote:
Somewhat offtopic - I'd argue that it is poor packaging to have more validpgpkeys in a PKGBUILD than the key used to verify the source for that particular package version. Lazily just adding more keys to a PKGBUILD and not removing unneeded ones in case they are needed in the future is not ideal. We should indeed be more careful in actively removing old keys from validpgpkeys. However some upstreams have multiple active maintainers that "arbitrarily" sign new releases, in which case keeping all of them in the array can be helpful to convey an existing relationship of trust (without having to go through the revision control system to figure out whether the key of a new release has been already used in the past, or whether some other means of establishing a chain of trust is necessary).
While I agree with old keys should be thrown out and revisited regularly whether upstream deprecated any, I disagree with only declaring the set needed for a single particular package version. If we require packagers to fiddle with the validpgpkeys array for virtually every release in a multi-maintainer project, this will weaken effective security in the long run. Requiring to re-check history and the established chain of trust for the current set of keys on each iteration will lead to a bigger window for errors and ultimately lead to fatigue related to carefully handling any changes to the validpgpkeys array. When such a security property becomes a burden to maintain, humans by nature will find a way to make it less annoying. Touching the validpgpkeys array would become the quick easygoing norm instead of a careful exception. This has also happened to pinned git sources where packagers over time simply switched back to refnames -- but this will hopefully get resolved by the immutable git sources patch.
How about... all signatures from trusted keys (either through PGP web of trust, or via being in validpgpkeys) need to validate?
That does not solve the rogue maintainer issue (it is up to a packager to ask why the number of signatures dropped...), but it does address not needing to validate a legacy key. From a security standpoint I am fine with this approach (CC Levente for his opinion).
From an implementation standpoint I will note that this complicates things a bit due to the structure of the GnuPG status file: for a valid signature, we are guaranteed to get a VALIDSIG status code containing the full key fingerprint [1], matching the information that we record in validpgpkeys. For an invalid signature, we are only guaranteed to get a BADSIG/ERRSIG containing the (long) key ID of the key [2] (because the key might not even be available locally and the signature packet alone only contains the key ID instead of the full fingerprint). So in order to check whether a bad signature was produced by a key in validpgpkeys, we need to collect the corresponding key ID and check whether there is a *partial* match with one of the fingerprints in the validpgpkeys array.
I will note that it is easy to find colliding key IDs [3], so normally one should not rely on the key ID alone (even if it is the "long" 64 bit instead of the "short" 32 bit ID). In this case I think using the ID should be fine though, since we only use it for "negative" matching as an additional security check rather than relying on the key ID for "positive" matching of trusted keys: if someone manages to upload a signature file containing a bad signature with a spoofed key ID, the validation would still fail (rightfully so), while a good signature from a spoofed key ID would not be considered as valid because the fingerprint in VALIDSIG does not match.
If this seems like a reasonable approach to everybody, I would be happy to implement the necessary partial comparison of the key IDs of invalid signatures in a v3 of the patch. However I would like to hear Levente's opinion first since it was he who initially brought up the concerns regarding the handling of files containing multiple signatures.
This is quite exactly what went through my mind while thinking about the proposal, hence I second what Jonas summarized. As Jonas' previous mail (before the one I'm replying to) described: A rouge maintainer would need to swap both, the tarball as well as the signature. As a simple alternative, a new version containing just a single signature could be released. Hence the signature would not need to contain any BADSIG/ERRSIG from other maintainers in both scenarios. The only real advantage that would result from the proposed changes would be if a trusted attacker is able to replace the sources but only append signatures at storage or in transit. -- which kind of sounds unlikely. I currently do not see any major disadvantages with the proposed changes besides complexity. But considering that the net benefit of this approach would be uncertain, I feel kind of mixed if that additional check would bring any effective security. Cheers, Levente