Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

signature: add support for generating & deriving keys (i.e. Signer & Verifier) #1244

Open
ycscaly opened this issue Feb 6, 2023 · 8 comments
Labels
signature Digital signature crate

Comments

@ycscaly
Copy link
Contributor

ycscaly commented Feb 6, 2023

I understand that not all implementors necessarily can expose this for various reasons. However, a lot of types can in fact implement this, and for those, it will be very useful for users of this library to be able to constrain generic parameters to be only those that can be generated/derived.

What I suggest is adding a new trait, e.g. Derivable or whatever name may be appropriate, which will expose two functions generate_keypair() and derive_keypair()

Then, we can implement that for the popular implementors (k256, p256, ed25519-dalek) easily. And, for example, I as a user of this library can expect K: Keypair + Derivable and generate new instances of this key in a generic fashion like let keypair = K::generate_keypair().

This is much better than the current situation, where my code needs to look like this:
fn generates_keypair< V: Verifier<Sig>, S: Signer<Sig>, >( generate_signing_keypair: fn() -> (V, S), ) -> (V,S) { generate_signing_keypair() }
I think that receiving this function as a parameter makes no sense. Currently, I only need to generate keys for tests -- but this still is very ugly, and unnecessarily so.

@tarcieri
Copy link
Member

tarcieri commented Feb 6, 2023

I'm not sure what your proposed generate_keypair() and derive_keypair() are supposed to do, or why they're "Derivable".

First, I'm a bit confused at your function signature. You mention the Keypair trait in the context of your proposed Derivable trait, but don't use it for your generates_keypair example? Why not?

Neither of these functions are parameterized which means they can only work by side effects, which means they can't e.g. take a deterministic RNG.

The generate_keypair trait looks a bit like rand::Distribution?

We can have a trait for random generation of keypairs, gated on the rand_core trait, but I think it should take a parameterized RNG at least by default (with a potential wrapper to use OsRng when getrandom is available).

That doesn't help you persist keypairs though, so I'm a bit confused what you're doing with generic code that can only generate a keypair but not store it?

@ycscaly
Copy link
Contributor Author

ycscaly commented Feb 6, 2023

Sorry I wasn't explicit enough.

Yes, I meant for parameters to be passed (I just thought they are implied from similarly named functions (e.g. in rust-hpke or ed25519-dalek), but let me be explicit):
trait DerivableSigner<S: Signer<Sig> + Keypair, Sig> { fn generate_signer<R: CryptoRngCore + ?Sized>(csprng: &mut R) -> S; fn derive_signer(ikm: &[u8]) -> S; }
There's definitely things to discuss here, like what should the trait be named and whether we should operate on Keypair or on Signer (changed to Signer + Keypair because it seemed more appropriate, we can also think on whether to return signer or (Signer, Verifier)) and finally whether we should enforce the key material in derive_signer() to be of the right key length etc.

But I wanted first to get the general idea across. What are your thoughts?

@tarcieri
Copy link
Member

tarcieri commented Feb 6, 2023

we can also think on whether to return signer or (Signer, Verifier)

The Keypair trait means you don't need to compute Verifier. Instead you can call Keypair::verifying_key to obtain Keypair::VerifyingKey (which also means you don't need to explicitly notate the type for it, since it's known via the associated type)

I still don't know what you mean or want regarding derive_signer.

I think a trait to randomly generate keys is OK, but it should be parameterized by default with &mut impl CryptoRngCore (see #1148).

@ycscaly
Copy link
Contributor Author

ycscaly commented Feb 6, 2023

I still don't know what you mean or want regarding derive_signer.

Ok, I now think I understand your point and agree with it: if what I want is to generate a keypair from the array of byte representing the private key scalar, I can just use Verifier::deserialize() for that purpose. That's assuming I know the internal structure (which should be a simple 32-byte array for curve scalars.)

Wouldn't this, in fact, also invalidate the need to have a trait for randomly generating keys? For I can generate a random 32-byte array, then deserialize (we could wrap this logic by implementing it for Verifier<Sig> + Deserialize but I don't think that we need implementors to implement this?)

@ycscaly
Copy link
Contributor Author

ycscaly commented Feb 6, 2023

Something like this should work

let mut bytes = [0 as u8; 32]; csprng.fill_bytes(&mut bytes); bincode::deserialize(&bytes).unwrap()

@tarcieri
Copy link
Member

tarcieri commented Feb 6, 2023

if what I want is to generate a keypair from the array of byte representing the private key scalar, I can just use Verifier::deserialize() for that purpose

There's also TryFrom<&[u8]>, which avoids the need for serde.

Wouldn't this, in fact, also invalidate the need to have a trait for randomly generating keys? For I can generate a random 32-byte array, then deserialize

Not all 32-byte strings are valid private keys, and that's even assuming you're using an elliptic curve with a ~256-bit base field modulus.

Elliptic curve private keys are elements of the scalar field, modulo the curve's order. If the 32-byte string (when interpreted as a 256-bit integer) overflows the curve's order it will be rejected.

We provide a random(rng: &mut impl CryptoRngCore) -> Self inherent method for most keys for this purpose, which for ECC uses rejection sampling of private scalars. This could potentially be moved to a trait.

@ycscaly
Copy link
Contributor Author

ycscaly commented Feb 6, 2023

Thanks this reply cleared it up for me. Yes, I'd love to have such a trait.

Regarding TryFrom<&[u8]>, it's also missing for schnorr on k256 so maybe it'll be good to add it on the same PR.

Is there something similar for serializing to bytes without serde? I see a to_bytes() method but if its not in a trait, how can I constrain my parameter to implement it? am I missing something again here? perhaps we should implement Into<[u8]>?

@tarcieri
Copy link
Member

tarcieri commented Feb 6, 2023

You can't impl Into<[u8]>, because [u8] isn't Sized.

Into<FieldBytes<C>> is probably the closest thing.

@newpavlov newpavlov added the signature Digital signature crate label Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
signature Digital signature crate
Projects
None yet
Development

No branches or pull requests

4 participants
@tarcieri @newpavlov @ycscaly and others