cloudflare / circl

CIRCL: Cloudflare Interoperable Reusable Cryptographic Library
http://blog.cloudflare.com/introducing-circl
Other
1.22k stars 136 forks source link

Make ed25519 and ed448 usable for Golang interfaces #109

Open claucece opened 4 years ago

claucece commented 4 years ago

Hi!

Thanks so much for all the work on this!

So, the implementation for ed25519 and ed448 expose a certain API that is quite similar to the way golang handles it in its libraries (ecdsa, rsa, and ed25519). However, when I tried to use it for the crypto signer interface, it panics due to:

panic: interface conversion: ed25519.PrivateKey is not crypto.Signer: missing method Public [recovered]
    panic: interface conversion: ed25519.PrivateKey is not crypto.Signer: missing method Public

I was wondering if we could create a method func (priv *PrivateKey) Public() crypto.PublicKey (in similar fashion as https://golang.org/pkg/crypto/ecdsa/#PrivateKey.Public and https://golang.org/pkg/crypto/ed25519/#PrivateKey.Public) so the APIs can be used for other interfaces as the crypto signer...

claucece commented 4 years ago

@armfazh I can implement this.. but I want to know if you think it is a good idea ;)

bwesterb commented 4 years ago

The *KeyPair type seems to implement crypto.Signer already, but if that wasn't obvious then we might want to change the API anyways.

claucece commented 4 years ago

Yeah, the KeyPair type does indeed (and it will work with it). But, as the other APIs (in Golang libraries) do it over the PrivateKey, it seems a little odd. But it can still work ;)

armfazh commented 4 years ago

@claucece do you want to proceed with the changes needed?

claucece commented 4 years ago

@armfazh I can make a PR changing the API to work with the PrivateKey type... let me know if it is ok, as it can still work with the KeyPair type..

armfazh commented 4 years ago

please go forward with that PR. However, I see some trade-offs regarding compatibility. Let me know your feedback.

On the one hand, I would like that circl be a drop-in replacement of stdlib. This implies that the functions NewKeyFromSeed, GenerateKey, Sign, Verify and PrivateKey implementing crypto.signer must be preserved (but internally using faster circl functions).

On the other hand, having a keypair type allows us to add different flavors for signing e.g. SignPure, SignPh, and SignCtx (as in RFC8032). Currently, only SignPure is supported, and SignPh can be added soon (https://github.com/golang/go/issues/ 31804)

claucece commented 4 years ago

So, mmm.. I did a little bit of research around this. On most libraries I checked, there is two interfaces exposed:

  1. signPure
  2. signPh

Often times, SignCtx is ignored.

What we can do is either:

  1. Implement two exposed functions for SignPure, SignPh, and SignCtx, that are compatible (or will be compatible) with the stdlib. Following the golang proposal, we can still use Sign for SignPure and SignPh, and perhaps create a SignCtx that works for SignPure and SignPh as well. Per the golang proposal, there will still be two distinct functions for verifying (which is not great as it seems they can be unified into one): VerifyPh, VerifyPure, and we will need to add VerifyPhCtx, VerifyPureCtx. It will be great if we can unify the verifying functions into Verify (for VerifyPure and VerifyPh with no context), and VerifyCtx (for VerifyPure and VerifyPh with context).
  2. Unify them all into a single Sign and Verify, while having an extra argument Options that can work as described on the golang issue. Of course, this will make it incompatible with the stdlib.

For compatibility, I'll say that going with option 1 is the best, until the golang issue is actually resolved (after that, something like option 2 will work). It seems like they are still debating around the best way to approach the issue...

armfazh commented 4 years ago

let's focus then on SignPure and SignPh following your first recommendation.

claucece commented 4 years ago

Perfect! I'll prepare a PR for both curves ;)