hyperledger / aries-askar

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

Refactoring: signing data using `LocalKey` #227

Closed Patrik-Stas closed 4 months ago

Patrik-Stas commented 6 months ago

While integrating askar to aries-vcx, following pattern has appeared in our sign function implementation:

        if let Some(key) = self
            .session()
            .await?
            .fetch_key(&did_from_key(key.to_owned()), false)
            .await?
        {
            let local_key = key.load_local_key()?;
            let key_alg = SigType::try_from_key_alg(local_key.algorithm())?;
            Ok(local_key.sign_message(msg, Some(key_alg.into()))?)
        }

What happens here is:

I find this un-intuitive that I need to use SigType::try_from_key_alg before I can use LocalKey's sign_message, even though the information about key type is in fact contained in local_key variable itself. Therefore I would rather welcome following API:

            let local_key = key.load_local_key()?;
            Ok(local_key.sign_message(msg?)

Such that sign_message would check the key type itself.

Additionally, the current API opens up doors to error where I load up local_key of certain cryptography type, but then pass wrong value of SigType argument to sign_message function.

I am open to make contribution, but before I proceed, is there's some perspective I am missing?

swcurran commented 6 months ago

@andrewwhitehead — could you please weigh in here?

andrewwhitehead commented 6 months ago

Hiya, I believe what you're doing is effectively the same as passing in None for the signature type. In that case the default signature type for that key is chosen.

Patrik-Stas commented 6 months ago

@andrewwhitehead excuse the lack of my knowledge - you indicate that there might be keys which can be used for variaty of signature types. Is that right?

andrewwhitehead commented 4 months ago

There are keys for which multiple signature types could be supported, for example EdDSA signatures can be deterministic or randomized: https://www.ietf.org/archive/id/draft-mattsson-cfrg-det-sigs-with-noise-04.html

At the moment none of the implemented key types support multiple signature types, though.