Re: RFE: use patchwork to submit a patch

[Daniel Axtens <dja@axtens.net>]

Toke Høiland-Jørgensen <toke@redhat.com> writes:

> Daniel Axtens <dja@axtens.net> writes:
>
>>> It does bring up that any new workflow has to have security protocol
>>> and threat model as part of its design.
>>
>> This is actually something that worries me about the patchwork
>> workflow. Maintainers seem to trust the patchwork version of a patch
>> without much (or any) verification that it matches what was sent to the
>> list.
>>
>> Say Alice emails a patch that says something like:
>>
>>  if ((permissions == allowed) && other_stuff) {
>>     do_things();
>>  }
>>  do_more_stuff(permissions);
>>
>> List members read that email, and many review it in the email client.
>>
>> However, say that the version in Patchwork actually reads like this:
>>
>>  if ((permission = allowed) && other_stuff) {
>>     do_things();
>>  }
>>  do_more_stuff(permissions);
>>
>> (You could get this with a malicious/compromised patchwork admin, or a
>> sufficiently advanced network adversary - patchwork takes the first mail
>> for a given message-id + project and later ones are discarded, so
>> there's a race you can win.)
>>
>> If the maintainer or someone else happens to apply the patch from
>> patchwork and then review it, or if tests happen to catch the relevant
>> case, we'll catch this. Otherwise...
>>
>> It's not the easiest or lowest risk way to get malicious code into the
>> kernel, but nonetheless, I worry about it.
>>
>> I can't think of a sensible way to fix this, unless we want to move to a
>> world where patch submissions are GPG signed, and teach patchwork to
>> verify sigs.
>
> It should be detectable, though, right?
>
> Say you have two independently administered patchwork instances (or even
> better, two different software packages entirely) that both subscribe to
> the mailing lists, and compare patch content with each other. They
> should at least be able to detect mismatches. Especially if you add a
> sanity check before discarding duplicate message-ids.
>
> This way you'd need to compromise multiple machines to achieve the kind
> of compromise you're worried about. And you can add more independent
> machines until you're satisfied that the risk is low enough :)

Yeah, it's detectable, even more simply if someone who subscribes to the
list checks patches they receive against a public patchwork. But my
concern isn't that it's undetectable, it's that it unlikely to be
detected in practice unless someone goes to the effort of writing code
to do it.

Regards,
Daniel

>
> -Toke