Re: [PATCH 16/20] PKCS#7: Add an optional authenticated attribute to hold firmware name [ver #5]

[David Howells <dhowells@redhat.com>]

Andy Lutomirski <luto@kernel.org> wrote:

> This is insecure because PKCS#7 authenticated attributes are broken (see
> RFC2315 section 9.4 note 4).  You need to either require that everything have
> authenticated attributes or require that nothing have authenticated
> attributes.   Maybe this insecurity doesn't matter in practice, but I don't
> wouldn't want to bet on it.

You can also fudge the signature (or a hash) by adding extra data to or
modifying the data blob and by switching signature values between signature
blobs.

PKCS#7 authenticated attributes aren't as broken as you make out.  They are
added to the signature hash - so an attacker *would* have to fudge things to
make it work.  Further, we can easily make it so that auth attrs are
*required*.

> On top of that, this is a ton of code to support something trivial.

I don't think it's as bad as you're making it out to be.

> And it requires an OID to be registered (ick).

That shouldn't be too hard to achieve - at least if we don't mind having RH
space OIDs.

> Earlier you suggested just appending the signature purpose to the thing being
> signed.  What's wrong with that?

You can't tell the difference between a corrupted key/signature and a firmware
blob being loaded for the wrong request.  Firstly, I want to be able to detect
the difference and secondly, it makes it easier to debug it if something does
go wrong.

> P.S.  Or you could stop using PKCS#7 if possible.

We've discussed this before.  We have to have a PKCS#7 parser in the kernel
anyway if we're going to support signed PE files for kexec.

David