Open ycscaly opened 1 year ago
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?
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?
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).
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?)
Something like this should work
let mut bytes = [0 as u8; 32]; csprng.fill_bytes(&mut bytes); bincode::deserialize(&bytes).unwrap()
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.
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]>
?
You can't impl Into<[u8]>
, because [u8]
isn't Sized
.
Into<FieldBytes<C>>
is probably the closest thing.
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 functionsgenerate_keypair()
andderive_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 likelet 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,
S: Signer,