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

[Andy Lutomirski]

On Fri, May 29, 2015 at 5:40 AM, David Howells <dhowells@xxxxxxxxxx> wrote:
> 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.

So there's another design error in PKCS#7?  Great!

>
> 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.
>

This seems only barely useful.  In any event, you could still do it by
appending the purpose as long as the purpose was literally appended to
the payload as opposed to being implicit and only covered by the
signature.

>> 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.

If you neuter the PKCS#7 parser to requre authenticated attributes,
will this use case still work?

Honestly, it still might be less code in the end if you used PKCS#7
solely for kexec.

--Andy
--
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/