On Thu, Feb 27, 2020 at 10:29 PM Konstantin Ryabitsev <konstantin@xxxxxxxxxxxxxxxxxxx> wrote: > This is why I've been keeping pgpkeys.git repository around. > https://git.kernel.org/pub/scm/docs/kernel/pgpkeys.git Ah, right, that seems good. There's also signify, by the way, if you ever feel the urge to jump ship from pgp. We can add cgit support for it too. > I leave all this to be handled by git-mailinfo. We don't process the > message directly, and whatever is returned by git-mailinfo is what git > decides to use when you run "git am". > Yes, fully agreed. I am hoping to convince git folks that we need this > functionality natively present in git-mailinfo: > https://firstname.lastname@example.org/ > To reiterate, I want to use whatever git uses and make as few design > decisions as possible. Probably wise. But it doesn't really "solve" the problem, since somebody still needs to write the code to have the set of properties you care about for signatures. No matter who you foist the responsibility on to, you'll probably need to audit it pretty carefully. > This doesn't really concern me as much. Anyone can submit attestation > signatures, true, but we throw out anything that doesn't validate and > also any PGP keys where none of the UIDs match the From: header. So, if > a malicious person sends a bunch of attestations, then they will just be > noise and fail at the "gpg --verify" stage because you won't have that > key on your keyring. That From header is forgeable too, by the way. (Some patches even have two From headers -- one in the body of the email for git and one that actually sent the email.) > Why does it matter? You send the patch to lkml. Then your Internet cuts out while the firemen deal with an unidentified creature stuck in a nearby tree meowing loudly. Finally it's back on and you submit the attestation. However, while the cat was ruining your workflow, tglx went to apply your patch but wasn't able to find the signature email, since you hadn't posted it yet. Hence, a better flow is to just post the signature email first. > I agree that Attestation-verified is redundant with Signed-off-by. I > think Attestation-by is helpful for the purposes of seeing the > attestation chain. If Developer A sent patches to Submaintainer B, and > Submaintainer B sent it to Maintainer C, these headers will help us > figure out which steps were attested. If you only see: > > Attestation-by: Submaintainer B > > You'll know that there probably was no attestation submitted/checked for > when the patches travelled between Developer A and Submaintainer B. > > However, I don't feel strongly about this. If the community decides that > these trailers are not useful, we can drop them. Most of this can be > otherwise traced via Link: trailers. I think this too is redundant and mostly theater. Those trailers aren't authenticated, so they communicate basically nothing useful except, "I'm one of the cool maintainers who use this thing!" For everyone else, it's just clutter. > The proof-of-concept tool doesn't do anything with this info, but the > real tool can implement a "window of validity" check that would reject > attestation if signatures are too old. Or at least warn in bright red or something. But do note: this is mostly a compromise-hack to work around a fundamental design issue here, and it doesn't actually solve all cases; i.e. it depends on humans having some short term memory during the validity period about what's going on too. > 2. A maintainer reviews patches that arrived in their mailbox and finds > them perfectly good. Then they run "get-lore-mbox" or "git pw" to get > these patches in a format that's easy to apply to their git repo, but > they suddenly get a *slightly different* set of patches, because now > we explicitly trust that whoever is in charge of > lore.kernel.org/patchwork.kernel.org isn't being malicious. It doesn't help with the reverse scenario, where what's in the developer's inbox or displayed on the lore webserver running a backdoored nginx doesn't match the patches that they eventually apply. That might seem like an unrealistic attack scenario, but then think about it combined with the get-lore-mbox feature to grab the latest patchset version and other similar shenanigans. Or, maybe Eve reposts v1 as v20, and this mailing list thread convinces you to ignore the [PATCH vX] from the metadata hash, or maintainers don't care about that hash verifying anyway since it tends to get mangled. I like that this all uses email and simple python command line tools and whatnot. But the whole scheme just seems kind of brittle and clunky.