circl icon indicating copy to clipboard operation
circl copied to clipboard

Make ed25519 and ed448 usable for Golang interfaces

Open claucece opened this issue 5 years ago • 9 comments

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 avatar Apr 30 '20 07:04 claucece

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

claucece avatar Apr 30 '20 10:04 claucece

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

bwesterb avatar Apr 30 '20 10:04 bwesterb

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 ;)

claucece avatar Apr 30 '20 13:04 claucece

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

armfazh avatar May 04 '20 20:05 armfazh

@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..

claucece avatar May 04 '20 21:05 claucece

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)

armfazh avatar May 04 '20 22:05 armfazh

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...

claucece avatar May 07 '20 04:05 claucece

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

armfazh avatar May 07 '20 19:05 armfazh

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

claucece avatar May 13 '20 05:05 claucece