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

From: David Howells
Date: Fri May 29 2015 - 08:40:50 EST


Andy Lutomirski <luto@xxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/