Re: Patch attestation RFC + proof of concept

["Jason A. Donenfeld" <Jason@zx2c4.com>]

On Thu, Feb 27, 2020 at 10:29 PM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> 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://lore.kernel.org/git/20200221171312.xyzsrvebuwiw6pgj@chatter.i7.local/
> 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.