Persist keypair used by example opcert signer #6821
Conversation
| { | ||
| // Storage doesn't have an existing keypair. Let's create one and add it to the storage. | ||
| ReturnErrorOnFailure(mIssuer.Initialize()); | ||
| ReturnErrorOnFailure(mIssuer.Serialize(serializedKey)); |
There was a problem hiding this comment.
This is storing a private key outside of secure storage, where it may leak and be exfiltrated.
Key storage should use different APIs than regular storage, so that crypto-accelerated or trustzone target can rely on their secure storage of the private part of the key pair.
There was a problem hiding this comment.
These are only used for test tools. The intent is not use it for any product release. That's why it's named ExampleOperationalCredentialsIssuer.cpp
There was a problem hiding this comment.
Yes, I understand it's example code. I just see other usages elsewhere of how P-256 keys are used that make assumptions about serialization that are incompatible with iOS/Mac Secure Enclave, Android Keystore, ARM TrustZone in Cortex-M33, etc
I think it's OK in the short term, but it absolutely violates some of the Threat Model security requirements which are explicit in the spec, so I'm pointing it out early to make sure it doesn't "spread" too much.
There was a problem hiding this comment.
Agree with @tcarmelveilleux , I'm curious if we can either put this in a place that it's clear it's an example (directory), or make it only optionally compile in somehow?
There was a problem hiding this comment.
I added a warning and a file note to alert the users of the API about the storage of encryption keys.
a2a87ef to
1fed564
Compare
Problem
The tools that are using example operational certificate signer should persist their signing key, so that on reruns same credentials are used for CASE session establishment.
Summary of Changes
Updated
ExampleOperationalCredentialsIssuerclass to take a reference to storage delegate. OnInit, it'll lookup the storage for an existing keypair. If not found, it'll create one and persist it for the next run.Updated users of
ExampleOperationalCredentialsIssuerto provide their storage delegates.