TBD54566975 / web5-rs

Apache License 2.0
6 stars 4 forks source link

Add serialization of private key material (`KeyExporter`) #211

Open KendallWeihe opened 2 months ago

KendallWeihe commented 2 months ago

Requirement

Enable re-instantiation of BearerDid instances from serialized private key material, which implies we must support serializing private key material.

Use Cases

Test Suites

We want to rely on deterministic test vectors (composed of static data, some of which is encrypted or signed by private key material) which are freely interoperable across languages.

Workaround for Secure Enclave & HSM Limitations

(@mistermoe help me out here, fill gaps and correct me where I'm wrong)

There are no guarantees that all key management solutions will support our set of cryptographic requirements. For example, Ed25519 isn't supported in AWS KMS. In such a case, we must workaround this by serializing the private key material, storing it in a secure environment (ex. AWS Secrets Manager), deserializing the private key material at runtime, and relying on memory safety for security requirements. In other words, we can't assume the usage of a secure enclave or HSM solution by the application.

Others???

If there are other use cases for the serialization of private key material, then please comment such use cases below!

Solution

We can agree for certain to add a KeyExporter trait to the keys crate (inspiration can be taken from KeyImporter #204).

The contested matter is whether or not to first-class conceptualize a PortableDid type struct. The inclusion of the PortableDid type struct gives us an explicit data structure which represents a fully self-contained serialized instance of a BearerDid. The alternative would be to rely on the calling code (the app developer) to maintain the state associating the serialized private key(s) with the given DID.

It's not obvious to me that it's beneficial for us, at this time, to introduce a PortableDid at the rust core layer. It's not obvious it'll assist us with bindings. There's also a layer of complexity here with regards to #142 and if we decide to full-commit to JWK as the sole key representation or not.

For the sake of this ticket, I would recommend we do not introduce the PortableDid concept. With recognition we may if it becomes abundantly clear to do so. I'm open to challenging this though. By adding the KeyExporter, at a minimum, we enable the concept of a PortableDid even though, without the implementation, we don't provide it.

This ticket is relevant to https://github.com/TBD54566975/web5-spec/issues/112#issuecomment-1924691567


All of the below web5 SDKs have PortableDid as a dedicated type struct.

KendallWeihe commented 2 months ago

Actually, this triggered a concern with portable DID in our other SDKs. What happens in the following case:

  1. Create a did:dht
  2. Export it as a portable DID (which includes the DID Document)
  3. Update the did:dht (as in, update the DID Document in the network)
  4. At this point, what's in the portable DID's Document is different than the Document persisted in the network
  5. Instantiate a bearer DID from the portable DID
  6. At this point the developer has an out-of-date DID Document in their bearer DID instance

I think we should consider removing the DID Document from the portable DID type and instead resolve upon each instantiation (upon each "import from a portable DID"). As I see it, if we want to create a dedicated type for portable DID's then that type should merely consist of:

{
    "uri": "did:dht...",
    "privateKeyJwks": [...]
}
mistermoe commented 2 months ago

aye we definitely chatted about this during the design phase of portable did. @frankhinek do you remember our rationale for choosing to include the did document as part of portable did?

ah i think i remember. w.r.t. did:dht specifically, it may be the case that a did was created but not published. if so, without including the did document in the portable did itself, fidelity is lost (e.g. on creation, n services were provided to create)

This definitely doesn't negate the issue you've raised @KendallWeihe. The scenario you describe is entirely possible. At the time we decided that it was unlikely and felt that the furthest we should probably go (but didn't in any of the sdks) for the time being would be, on import, attempt to resolve. if resolution is successful, diff the portable did doc and the published did doc. if there's a difference, throw exception

KendallWeihe commented 2 months ago

Excellent, let's move the discussion in the above two comments to https://github.com/TBD54566975/web5-spec/issues/156

KendallWeihe commented 2 months ago

Then again, now that I think about this use case:

Workaround for Secure Enclave & HSM Limitations

In such a use case, wouldn't the solution be to have the key manager implementation:

  1. Generate the private key
  2. Immediately store in the secure environment (ex: AWS Secrets Manager)
  3. Subsequently read from the secure environment when the key material is needed

In this use case, the private key material should still never leave the key manager, granted this would require a key manager implementation specific to the given secure environment (ex. AWS Secrets Manager) but I think that's appropriate and compatible with our approach of supporting "bring-your-own-key-manager" (#160).

And so really, the only use case we need to support is for generating test vectors. In which case, I think it's appropriate to build one-off code for that (or better yet, a CLI feature) rather than implementing it in the core SDK which would enable accidental security vulnerabilities.

Going to leave this ticket open for the sake of discussion but I don't see a concrete need for this feature.