Re: [PATCH v3 1/3] powerpc/powernv: Add OPAL API interface to get secureboot state

Hi Nayna,

quoted

quoted

Since OPAL can support different types of backend which can vary in the
variable interpretation, a new OPAL API call named OPAL_SECVAR_BACKEND, is
added to retrieve the supported backend version. This helps the consumer
to know how to interpret the variable.

(Firstly, apologies that I haven't got around to asking about this yet!)

Are pluggable/versioned backend a good idea?

There are a few things that worry me about the idea:

  - It adds complexity in crypto (or crypto-adjacent) code, and that
    increases the likelihood that we'll accidentally add a bug with bad
    consequences.

Sorry, I think I am not clear on what exactly you mean here.Can you 
please elaborate or give specifics ?

Cryptosystems with greater flexibility can have new kinds of
vulnerabilities arise from the greater complexity. The first sort of
thing that comes to mind is a downgrade attack like from TLS. I think
you're protected from this because the mode cannot be negotiatied at run
time, but in general it's security sensitive code so I'd like it to be
as simple as possible.

quoted

  - If we are worried about a long-term-future change to how secure-boot
    works, would it be better to just add more get/set calls to opal at
    the point at which we actually implement the new system?

The intention is to avoid to re-implement the key/value interface for 
each scheme. Do you mean to deprecate the old APIs and add new APIs with 
every scheme ?

Yes, because I expect the scheme would change very, very rarely.

quoted

  - Under what circumstances would would we change the kernel-visible
    behaviour of skiboot? Are we expecting to change the behaviour,
    content or names of the variables in future? Otherwise the only
    relevant change I can think of is a change to hardware platforms, and
    I'm not sure how a change in hardware would lead to change in
    behaviour in the kernel. Wouldn't Skiboot hide h/w differences?

Backends are intended to be an agreement for firmware, kernel and 
userspace on what the format of variables are, what variables should be 
expected, how they should be signed, etc. Though we don't expect it to 
happen very often, we want to anticipate possible changes in the 
firmware which may affect the kernel such as new features, support of 
new authentication mechanisms, addition of new variables. Corresponding 
skiboot patches are on - 
https://lists.ozlabs.org/pipermail/skiboot/2019-June/014641.html

I still feel like this is holding onto ongoing complexity for very
little gain, but perhaps this is because I can't picture a specific
change that would actually require a wholesale change to the scheme.

You mention new features, support for new authentication mechanisms, and
addition of new variables.

 - New features is a bit too generic to answer specifically. In general
   I accept that there exists some new feature that would be
   sufficiently backwards-incompatible as to require a new version. I
   just can't think of one off the top of my head and so I'm not
   convinced it's worth the complexity. Did you have something in mind?

 - By support for new authentication mechanisms, I assume you mean new
   mechanisms for authenticating variable updates? This is communicated
   in edk2 via the attributes field. Looking at patch 5 from the skiboot
   series:

+ * When the attribute EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS is set,
+ * then the Data buffer shall begin with an instance of a complete (and
+ * serialized) EFI_VARIABLE_AUTHENTICATION_2 descriptor.

   Could a new authentication scheme be communicated by setting a
   different attribute value? Or are we not carrying attributes in the
   metadata blob?

 - For addition of new variables, I'm confused as to why this would
   require a new API - wouldn't it just be exposed in the normal way via
   opal_secvar_get(_next)?

I guess I also somewhat object to calling it a 'backend' if we're using
it as a version scheme. I think the skiboot storage backends are true
backends - they provide different implementations of the same
functionality with the same API, but this seems like you're using it to
indicate different functionality. It seems like we're using it as if it
were called OPAL_SECVAR_VERSION.

quoted

  - What is the correct fallback behaviour if a kernel receives a result
    that it does not expect? If a kernel expecting BackendV1 is instead
    informed that it is running on BackendV2, then the cannot access the
    secure variable at all, so it cannot load keys that are potentially
    required to successfully boot (e.g. to validate the module for
    network card or graphics!)

The backend is declaredby the firmware, and is set at compile-time. The 
kernel queriesfirmware on whichbackend is in use, and the backend will 
not change at runtime.If the backend in use by the firmware is not 
supported by the kernel (e.g. kernel is too old), the kernel does not 
attempt to read any secure variables, as it won't understand what the 
format is. This is a secure boot failure condition, as we cannot verify 
the next kernel. With addition of new backends in the skiboot, the 
support will be added to the kernel. Note: skiboot and skiroot should 
always be in sync with backend support.

Seems reasonable. I'm thinking specifically about the kernel loaded
after skiroot; and yes, on reflection just failing to boot is the only
sensible thing you can do.

Regards,
Daniel