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

From: Nayna
Date: Fri Jun 14 2019 - 18:27:29 EST

On 06/12/2019 07:04 PM, Daniel Axtens wrote:
Hi Nayna,

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

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

So, the design is not making the assumption that a particular scheme will change often. It is just allowing the flexibility for addition of new schemes or enhancements if needed.

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

That is the exact reason for having pluggable backend, because we cannot determine now if there will be a need of new scheme in future or not.

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?

That is the idea to keep the design flexible to be able to handle future additions with maximum reuse. Example, supporting new algorithms or a different handling of secure variable updates by different vendors.

- 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

+ * 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

Sorry, probably it wasn't clear. By addition of new variables, we meant that over time we might have to add new "volatile" variables that "fine tunes" secure boot state. This might impact the kernel if it needs to understand new variables to define its policies. However, this will not result in change of API, it will result in change of the version.

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

We are changing how we are exposing the version to the kernel. The version will be exposed as device-tree entry rather than a OPAL runtime service. We are not tied to the name "backend", we can switch to calling it as "scheme" unless there is a better name.

Thanks & Regards,
ÂÂÂÂÂ - Nayna