Re: [Ksummit-discuss] Topics for the Maintainer's Summit

[Konstantin Ryabitsev <konstantin@linuxfoundation.org>]

On Mon, Sep 09, 2019 at 12:59:26PM +0200, Geert Uytterhoeven wrote:
> So the weakest link seems to be the step between public review and
> import into git by the maintainer.  E.g.
>   - The review chain on multiple submissions: Vn+1 may contain an evil
>      change that was not in Vn.  As this happens in public, it may be
>      noticed by reviewers.

Theoretically, a tool that is able to show interdiffs between
patches/series would be able to help identify those.

>   - The path between patch submission and git-am:  if a patchwork
>     instance is compromised, evil changes may sneak in.

I'm not sure we can fix this problem in any way that would be
meaningful. I did have some thoughts about this -- for example, we could
run an API service on lore.kernel.org that would allow issuing queries
like:
- do you have a record of a patch
  - with patch-id c052719edde5b0c648b2e4629a7fbd72fd948652
  - sent by shengjiu.wang@nxp.com
  - sent to list with id linux-kernel.vger.kernel.org
  - sent on 2019-09-09

If the query returns "True" then you know that at least lore.kernel.org
has received the same patch, but I'm not sure if we'd be solving
anything in the grander scheme of things. We still have to trust
kernel.org, plus an attacker could have sent a malicious patch to LKML
with a forged "From" to make this check return true. Nobody really reads
LKML, so sneaking something unnoticed would be fairly easy.

Alternatively, a developer could require that all submitted patches
must include a cryptographic signature. It could even be made relatively
painless, especially if we taught git tools to do it.

For git-format-patch we'd need something like this:

- generate a patch-id (using `git patch-id --stable`)
- generate a signify-compatible signature for the patch-id
- add 'Patch-id-sig' and 'Patch-id-key' into the basement (under "---")

E.g., for the same patch as the example I used above:

------
Subject: [PATCH 1/3] ASoC: fsl_asrc: Use in(out)put_format instead of in(out)put_word_width

snd_pcm_format_t is more formal than enum asrc_word_width, which has
two property, width and physical width, which is more accurate than
enum asrc_word_width. So it is better to use in(out)put_format
instead of in(out)put_word_width.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/fsl/fsl_asrc.c | 56 +++++++++++++++++++++++++++-------------
 sound/soc/fsl/fsl_asrc.h |  4 +--
 2 files changed, 40 insertions(+), 20 deletions(-)

 Patch-id-sig: RWT9fcUvSnHPLrCB+miMdp13r39W9az07CWW+b4OLz5zdtPUaj0N9qnfdNB+cbs8T1jYzHPIWfaf+B6z/ZvSVG9rfE1/Xx6+EgU=
 Patch-id-key: RWT9fcUvSnHPLqqyfLbkGBMEscBWciFFp2iBj2XnZPzW69OVIoYwZ25q

diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
...etc...
------

Then we teach git-am to keep a trust-on-first-use (TOFU) database of
public keys, so that:

- if there are no entries matching the email in From:, the key is
  added and automatically considered trusted
- if there are entries matching From:, the public key is compared
  with the TOFU database
  - if match, the key is used to validate that the patch-id hasn't
    changed
  - if no match, generate a warning and let the developer decide if they
    trust the new key or not
- if there are entries matching From: in the local TOFU db but the patch
  does not have a "Patch-id-sig", generate a warning

This would be more workable than using PGP signatures, but this would
still be a significant pain point for everyone, so I'm not sure how many
would consider using this. What seems reasonable from the security
high-horse I'm sitting on is not always reasonable for others.
:)

-K