Hi Konstantin, A few comments on the proposal, inline below: On Wed, Feb 26, 2020 at 12:25:02PM -0500, Konstantin Ryabitsev wrote: > 4. gpg --locate-keys kees@xxxxxxxxxx I suppose you have in mind WKD instead of the now-DoS-broken key servers? > $ egrep '^(Author|Email|Subject)' i | sha256sum > 2a02abe02216f626105622aee2f26ab10c155b6442e23441d90fc5fe4071b86e - What if there are multiple such lines? What if there's Subject-justkidding? That grep doesn't use a colon in it. Are there other malleability issues to account for here? > To calculate the "p" hash, we first need to remove any surrounding junk > that isn't just the patch itself. This has to be done extremely carefully, and it's probably worth reading the source of git apply and gnu patch and such to find any edge cases. Again, potential malleability issues which could be exploitable if not done carefully. > We then take the first 8 characters of the i-m-p hash to create the > attestation-ID: 2a02abe0-215cf3f1-2acb5798. It's now easy to forge collisions and complicate the lookup process by forcing clients to do lots of trial hashing and lore fetching. Remember: anybody can submit to the signatures list. If you want an attestation ID that uses each of the three hashes, hash the hashes together instead of truncating and concatenating them. Shouldn't this step: > - the developer executes "git-send-email" be after these steps: > - the developer runs "attest-patches -a *.patches" > - the developer sends attestation.eml to signatures@xxxxxxxxxx > (or the tool auto-POSTs it to the submission URL, as mentioned) ? Otherwise there's a small race window. > - if attestation passes, get-lore-mbox adds two trailers to each patch: > "Attestation-by:" and "Attestation-verified:". In our example case > those are: > Attestation-by: Kees Cook <kees@xxxxxxxxxx> (pgp:8972F4DFDC6DC026) > Attestation-verified: Konstantin Ryabitsev <konstantin@xxxxxxxxxxxxxxxxxxx> What do these lines actually add that is useful? The Signed-off-by is already a non-cryptographic trail of which maintainers patches flow through. These two add another, also non-cryptographic, summary. If they're forgible, then what's the point? Seems like clutter. If anything, these signatures help maintainers verify that the sender actually sent it before they merge it into their trees. But after that, those commit messages could contain anything, including bogus "Attestation-by" lines. It seems like it's basically just tinfoil virtue signaling at that point? Ostensibly the maintainers should also be reading the patches for content anyway... > Okay, what do you all think? I believe this scheme has the following > merits: > Downsides: > > - we aren't solving the problem of delegated trust, which will continue > to be the hardest part behind any distributed development effort > Another odd quirk worth considering: vanilla patches aren't tied to a specific commit base, so there could be a "replay attack" where an attacker resends an old patch that still applies without issue, but means a different thing in the present state of the tree. For example, Alice sends patch P in November 2019. Bob discovers it causes a remotely exploitable vulnerability in December 2019 and submits a revert patch. The seasons change a few times, and it's now March 2025, maintainers have changed a bit, but the code is still mostly the same. Eve resubmits P which has Alice's name on it. Signature verifies. Doom ensues. Real git pgp signing involves a signature over the whole object, which contains the hash of the parent, which avoids this issue. General reflection: this is solving a odd "problem". As mentioned above, maintainers should be reading commits as they come in on the mailing list. There might be a practical argument to be made, though, that sometimes they trust things on face value, even emailed commits, and so this gives some extra assurance. I also wonder if this adds even more overhead to what hipsters are already decrying as an overly clunky contribution process. Even more generally, is the lack of signatures on email patches a real problem we're facing? Regards, Jason
