hyperledger / aries-askar

Secure storage designed for Hyperledger Aries agents.
Apache License 2.0
58 stars 42 forks source link

P256 keys using Secure Enclave and Android StrongBox #245

Closed berendsliedrecht closed 2 months ago

berendsliedrecht commented 2 months ago

This PR contains the functionality to add support for hardware-backed P256 keys on iOS and Android Using iOS Secure Enclave and Android StrongBox. This is done by using the secure-env library. iOS is accessed via Security.framework and Android via JNI. Using these features you must build the library with mobile_secure_element or askar-crypto directly with p256_hardware.

During the implementation I ran into some issues due to askar-crypto being no_std. because of this I made the rng: impl KeyMaterial optional as well as the id: &str. My initial intention was to create a structure or enum for KeyGenOps, but due to not always having access to an allocator I could not leverage Box and dynamic dispatch. If there are any solutions for this, I would be happy to change the current implementation.

askar-crypto now also has a method to get the key by id. Which ties askar-crypto directly to storage, but I could not find any other way of doing this due to the nature of Secure Elements.

Over FFI, when supplying an id to askar_key_generate, the key will be created on hardware. This seems a bit too implicit for me. My next approach would be generate a random id, uuid or something and simply pass in a boolean for hardware_backed.

Implemented that we generate an id now for secure element keys and the user just has to provide hardware_backed argument over FFI as an i8.

Still in draft for now, as I have to do some testing.

Will also include an updated python wrapper (only software as key_backend allowed) and an updated node.js wrapper. React Native will be done in #247 as that will also include the React Native Example app and move to pnpm.

TimoGlastra commented 2 months ago

In credo we sign based on the public key, will that also be possible with the hardware keys? Or do we need to pass the key id specifically?

berendsliedrecht commented 2 months ago

In credo we sign based on the public key, will that also be possible with the hardware keys? Or do we need to pass the key id specifically?

Do you have an example of this code? If I can check which path it takes I can see whether it'll work or not. I don't think we can go from a public key to storage and get the HSM key. Software keys are stored as a JWK, but for secure element keys I chose to store just the id. If this is a very strong requirement I could do something like:

{
  "id": "CUSTOM_USER_DEFINED_ID",
  "public_key_base58": "bla..."
}

instead of the id alone.

TimoGlastra commented 2 months ago

Could we make the id the public key? Or is it generated by the secure enclave?

this is the code in Credo, we fetch the key based on the publicKeyBase58: https://github.com/openwallet-foundation/credo-ts/blob/8f49d84d52143bd9136cebd502bd9d10b5881463/packages/askar/src/wallet/AskarBaseWallet.ts#L214

If we can't do that, it will require A LOT of effort in Credo, as we would have to introduce a new concept of keyIds (which is a good thing probably), but it will be a big breaking chagne as we pass a Key instance around on MANY levels and layers

genaris commented 2 months ago

Could we make the id the public key? Or is it generated by the secure enclave?

this is the code in Credo, we fetch the key based on the publicKeyBase58: https://github.com/openwallet-foundation/credo-ts/blob/8f49d84d52143bd9136cebd502bd9d10b5881463/packages/askar/src/wallet/AskarBaseWallet.ts#L214

If we can't do that, it will require A LOT of effort in Credo, as we would have to introduce a new concept of keyIds (which is a good thing probably), but it will be a big breaking chagne as we pass a Key instance around on MANY levels and layers

In Credo we store the key using publicKeyBase58 as its name (id). Does this new approach prevent us from specifying the id we want when creating a key in hardware?

berendsliedrecht commented 2 months ago

Could we make the id the public key? Or is it generated by the secure enclave?

Yes! The name is not equal to the underlying Secure Element id, they are separate identifiers.

If we can't do that, it will require A LOT of effort in Credo, as we would have to introduce a new concept of keyIds (which is a good thing probably), but it will be a big breaking chagne as we pass a Key instance around on MANY levels and layers.

We can defer it for now, and possibly introduce it later, but it is not required.

In Credo we store the key using publicKeyBase58 as its name (id). Does this new approach prevent us from specifying the id we want when creating a key in hardware?

No it does not. It will work exactly the same as before. askar_key_generate now takes a key_backend argument which can be Software (default) or Secure Element. And based on that backend we generate a uuid for the Secure Element and only use that internally. After creation you can fetch the public key and use that as the name of the row.

TimoGlastra commented 2 months ago

Okay perfect, sounds good :)

andrewwhitehead commented 2 months ago

Looks great so far, maybe there would be a need to inspect what backends are supported by Askar for the current platform?

berendsliedrecht commented 2 months ago

Looks great so far, maybe there would be a need to inspect what backends are supported by Askar for the current platform?

Yes that should be quite easy based on the feature flags. I will add it to askar-crypto and expose it over ffi as askar_get_supported_key_backends(out: *mut FfiStringListHandle) -> ErrorCode.

berendsliedrecht commented 2 months ago

CI is finally finished so it is ready.