All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Konstantin Ryabitsev <konstantin@linuxfoundation.org>,
	Russell King <rmk+kernel@armlinux.org.uk>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Peter Zijlstra <peterz@infradead.org>
Cc: tools@linux.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: injected body trailers
Date: Thu, 21 Oct 2021 14:21:04 -0700	[thread overview]
Message-ID: <202110211414.00C7DFE8@keescook> (raw)
In-Reply-To: <20211021204459.xd63t5xn24vktqf3@meerkat.local>

On Thu, Oct 21, 2021 at 04:44:59PM -0400, Konstantin Ryabitsev wrote:
> On Thu, Oct 21, 2021 at 01:22:31PM -0700, Kees Cook wrote:
> > Hi!
> > 
> > So, I just saw a DKIM failure, and it was entirely justified. :)
> > 
> > Grabbing thread from lore.kernel.org/all/20211021142516.1843042-1-ardb%40kernel.org/t.mbox.gz
> > Checking for newer revisions on https://lore.kernel.org/all/
> > Analyzing 1 messages in the thread
> > Checking attestation on all messages, may take a moment...
> > ---
> >   ✓ [PATCH] ARM: stackprotector: prefer compiler for TLS based per-task protector
> >     ✓ Signed: openpgp/ardb@kernel.org
> 
> You will notice that the openpgp signature passed. This is because we:
> 
> 1. record the length of the original message when we're creating the signature
>    (see l=2495 in X-Developer-Signature)
> 2. if the initial validation fails and the body is longer than l=2495, we trim
>    the body to that number of bytes
> 3. if the trimmed validation passes, we use that version for the patch body
>    content, since that's clearly what the developer intended

I suspected something like this was happening to make that one pass.
Nice.

> 
> >     ✗ BADSIG: DKIM/kernel.org      
> >     ✓ Signed: DKIM/lists.infradead.org (From: ardb@kernel.org)
> > ---
> > 
> > This is https://lore.kernel.org/all/20211021142516.1843042-1-ardb@kernel.org/
> > and for some reason, the linux-arm-kernel mailing list is injecting a
> > body trailer.
> 
> "For some reason" is really "that's the default for mailman-2". Mailman-2
> belongs to a wholly different era and *can* be configured to be DKIM
> compliant, but rarely is.
> 
> > I just downloaded this directly and removed the trailer, and the DKIM
> > passed. This experience has raise a few questions...
> > 
> > 1) Can (should) b4 grow logic to progressively strip lines off the end
> >    of a body until DKIM passes?
> 
> Ah, but then the lists.infradead.org DKIM will fail. Theoretically, we should
> always prioritize the signature that is closest aligned with the From: header,
> but that's not actually that straightforward, as DNS lookup and validation
> rules can get really complex.

Could each signature validation independently process the body, with
the smallest signed body being what is "produced"? i.e. GPG already
self-trims. DKIM could do the same, trying to find a matching body i.e. on
failure (slow path), trying trimming up to 10(?) lines progressively
looking for a match?

(Probably better is to just fix the mailing lists, but maybe this would
be useful for historical patch extraction? Dunno.)

> 
> > 2) Can the linux-arm-kernel mailing list please stop breaking DKIM?
> >    Who should authorize this change (rmk, Catalin)? And who can make
> >    the change (peterz)?
> 
> The relevant settings should be a) don't add any subject prefixes, b) don't
> add anything to the body trailers, c) don't rewrite any other headers (to, cc,
> reply-to, etc).

rmk, Catalin, Peter, can this get sorted out? Having mailing list
trailers is annoying beyond just DKIM breakage. :)

> 
> >    (I realize now that all the mail from linux-arm-kernel has been
> >    getting dropped into my Spam folder -- I normally don't notice since
> >    I'm usually CCed directly or via some other list on things I wanted
> >    to see.)
> > 
> > 3) Are there other lists for which lore is collecting emails where DKIM
> >    is persistently broken, and can we fix those lists too?
> 
> I would also note that lists.infradead.org should not really be adding its own
> DKIM signature to messages it sends out. It doesn't really serve any purpose
> unless the From: header is rewritten (but please don't do that either).

-Kees

-- 
Kees Cook

WARNING: multiple messages have this Message-ID (diff)
From: Kees Cook <keescook@chromium.org>
To: Konstantin Ryabitsev <konstantin@linuxfoundation.org>,
	Russell King <rmk+kernel@armlinux.org.uk>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Peter Zijlstra <peterz@infradead.org>
Cc: tools@linux.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: injected body trailers
Date: Thu, 21 Oct 2021 14:21:04 -0700	[thread overview]
Message-ID: <202110211414.00C7DFE8@keescook> (raw)
In-Reply-To: <20211021204459.xd63t5xn24vktqf3@meerkat.local>

On Thu, Oct 21, 2021 at 04:44:59PM -0400, Konstantin Ryabitsev wrote:
> On Thu, Oct 21, 2021 at 01:22:31PM -0700, Kees Cook wrote:
> > Hi!
> > 
> > So, I just saw a DKIM failure, and it was entirely justified. :)
> > 
> > Grabbing thread from lore.kernel.org/all/20211021142516.1843042-1-ardb%40kernel.org/t.mbox.gz
> > Checking for newer revisions on https://lore.kernel.org/all/
> > Analyzing 1 messages in the thread
> > Checking attestation on all messages, may take a moment...
> > ---
> >   ✓ [PATCH] ARM: stackprotector: prefer compiler for TLS based per-task protector
> >     ✓ Signed: openpgp/ardb@kernel.org
> 
> You will notice that the openpgp signature passed. This is because we:
> 
> 1. record the length of the original message when we're creating the signature
>    (see l=2495 in X-Developer-Signature)
> 2. if the initial validation fails and the body is longer than l=2495, we trim
>    the body to that number of bytes
> 3. if the trimmed validation passes, we use that version for the patch body
>    content, since that's clearly what the developer intended

I suspected something like this was happening to make that one pass.
Nice.

> 
> >     ✗ BADSIG: DKIM/kernel.org      
> >     ✓ Signed: DKIM/lists.infradead.org (From: ardb@kernel.org)
> > ---
> > 
> > This is https://lore.kernel.org/all/20211021142516.1843042-1-ardb@kernel.org/
> > and for some reason, the linux-arm-kernel mailing list is injecting a
> > body trailer.
> 
> "For some reason" is really "that's the default for mailman-2". Mailman-2
> belongs to a wholly different era and *can* be configured to be DKIM
> compliant, but rarely is.
> 
> > I just downloaded this directly and removed the trailer, and the DKIM
> > passed. This experience has raise a few questions...
> > 
> > 1) Can (should) b4 grow logic to progressively strip lines off the end
> >    of a body until DKIM passes?
> 
> Ah, but then the lists.infradead.org DKIM will fail. Theoretically, we should
> always prioritize the signature that is closest aligned with the From: header,
> but that's not actually that straightforward, as DNS lookup and validation
> rules can get really complex.

Could each signature validation independently process the body, with
the smallest signed body being what is "produced"? i.e. GPG already
self-trims. DKIM could do the same, trying to find a matching body i.e. on
failure (slow path), trying trimming up to 10(?) lines progressively
looking for a match?

(Probably better is to just fix the mailing lists, but maybe this would
be useful for historical patch extraction? Dunno.)

> 
> > 2) Can the linux-arm-kernel mailing list please stop breaking DKIM?
> >    Who should authorize this change (rmk, Catalin)? And who can make
> >    the change (peterz)?
> 
> The relevant settings should be a) don't add any subject prefixes, b) don't
> add anything to the body trailers, c) don't rewrite any other headers (to, cc,
> reply-to, etc).

rmk, Catalin, Peter, can this get sorted out? Having mailing list
trailers is annoying beyond just DKIM breakage. :)

> 
> >    (I realize now that all the mail from linux-arm-kernel has been
> >    getting dropped into my Spam folder -- I normally don't notice since
> >    I'm usually CCed directly or via some other list on things I wanted
> >    to see.)
> > 
> > 3) Are there other lists for which lore is collecting emails where DKIM
> >    is persistently broken, and can we fix those lists too?
> 
> I would also note that lists.infradead.org should not really be adding its own
> DKIM signature to messages it sends out. It doesn't really serve any purpose
> unless the From: header is rewritten (but please don't do that either).

-Kees

-- 
Kees Cook

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-10-21 21:21 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-21 20:22 injected body trailers Kees Cook
2021-10-21 20:22 ` Kees Cook
2021-10-21 20:44 ` Konstantin Ryabitsev
2021-10-21 20:44   ` Konstantin Ryabitsev
2021-10-21 21:21   ` Kees Cook [this message]
2021-10-21 21:21     ` Kees Cook
2021-10-21 21:29     ` Catalin Marinas
2021-10-21 21:29       ` Catalin Marinas
2021-10-21 21:42   ` David Woodhouse
2021-10-21 21:42     ` David Woodhouse
2021-10-21 22:57     ` Konstantin Ryabitsev
2021-10-21 22:57       ` Konstantin Ryabitsev
2021-10-21 23:00       ` David Woodhouse
2021-10-21 23:00         ` David Woodhouse
2021-10-21 23:33         ` Konstantin Ryabitsev
2021-10-21 23:33           ` Konstantin Ryabitsev
2021-10-21 23:41           ` David Woodhouse
2021-10-21 23:41             ` David Woodhouse
2021-10-22  3:53     ` Kees Cook
2021-10-22  3:53       ` Kees Cook
2021-10-22  7:32       ` Russell King (Oracle)
2021-10-22  7:32         ` Russell King (Oracle)
2021-10-22  8:05       ` Arnd Bergmann
2021-10-22  8:05         ` Arnd Bergmann
2021-10-22  9:41         ` Ard Biesheuvel
2021-10-22  9:41           ` Ard Biesheuvel
2021-10-22 11:43           ` Catalin Marinas
2021-10-22 11:43             ` Catalin Marinas
2021-10-26  8:30             ` Will Deacon
2021-10-26  8:30               ` Will Deacon
2021-10-21 22:23 ` Russell King (Oracle)
2021-10-21 22:23   ` Russell King (Oracle)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202110211414.00C7DFE8@keescook \
    --to=keescook@chromium.org \
    --cc=catalin.marinas@arm.com \
    --cc=konstantin@linuxfoundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=peterz@infradead.org \
    --cc=rmk+kernel@armlinux.org.uk \
    --cc=tools@linux.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.