[Skiboot] [PATCH v2 6/6] secvar/edk2: enforce a PK update enrolled in setup mode to be signed by itself

[Daniel Axtens]

> I'm okay with unifying this with [REDACTED] if it means we don't
> pull another project-that-must-not-be-named and have a thousand
> different quirks and different behaviors across implementations.
>
>> Does it take us closer to EDK2 compat or further away? I am guessing
>> EDK2 also requires that there is an AUTH structure but just throws it
>> away after maybe doing some basic syntactical sanity checks?
>> 
>
> Slightly further, though again, so far every example I'm finding does
> the same procedure. Apparently it's a bit of a mess, where certain
> firmware may allow bare ESLs or even certs.
>
> As a side soapbox moment: I'm not a fan of unauthenticated updates
> period. I don't see why we shouldn't be signature checking all
> updates, the convenience isn't really that much more when the updates
> still have to be signed in the AUTH format. Either that or we support
> bare ESLs when in setup mode, but that'd likely end up being more
> work to introduce more fun error cases when users enroll a PK first.
>
>> I'm not _really_ sold on the value of this but I'm open to being
>> convinced.
>> 
>
> Neither am I tbh, but this is bringing it in line with our
> documentation and examples. We don't have a ton of documentation
> to update if we skip this patch, but I tend to favor the
> documentation being what the code *should* be.

Yeah it's all a bit lineball to me too. Is there a doc link I can look
at? (internal is fine)

I pretty carefully chose the descriptor 'unauthenticated' as opposed to
'unsigned' when specifying what [redacted] would accept specifically
because an auth structure with a signature is still required. Having
said that, it's not too late to swap it to 'self-signed', and that might
actually be even less ambiguous. I'll ask about that on [internal
communications platform].

>> Beyond that it looks OK, I'll have a closer look once I get through the
>> refactorings.

It looks fine, I'd probably just nitpick the wording:

"PK not able to update itself, rejecting enrollment."

I think 'not able to verify itself' or even "Verification of
self-signature on PK failed" might be clearer?

Kind regards,
Daniel