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

Make ed25519 and ed448 usable for Golang interfaces #109

Open
claucece opened this issue Apr 30, 2020 · 9 comments
Open

Make ed25519 and ed448 usable for Golang interfaces #109

claucece opened this issue Apr 30, 2020 · 9 comments

Comments

@claucece
Copy link
Contributor

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
Copy link
Contributor Author

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

@bwesterb
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Contributor

armfazh commented May 4, 2020

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

@claucece
Copy link
Contributor Author

claucece commented May 4, 2020

@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
Copy link
Contributor

armfazh commented May 4, 2020

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
Copy link
Contributor Author

claucece commented May 7, 2020

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
Copy link
Contributor

armfazh commented May 7, 2020

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

@claucece
Copy link
Contributor Author

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

armfazh pushed a commit that referenced this issue Jul 7, 2020
* Implement signing capabilities with prehashed messages for 25519
* Implement verification capabilities with prehashed messages for 25519
* Add tests for prehashed signing and verifying for ed25519
* Make linter happy
* Add rfc8032 test
* Make linter happy again
* Remove newline
* Separate signPure and signPh functions
* PoC: implement ed25519ctx. Needs refactoring
* Finish verification for ed25519
* Fix lint
* Fix comments and check correctly for context
* Fix spelling
* Remove functions
* Solve issues from review
* Fix tests
* Remove verify function
* Add note
* Hash message on the inside
* Add context to the main Sign function
* Add missing example
* Implement multipurpose verify function
* Fix lint
* Extract dom define function
* Solve comments from review
* Change ed25519 API
* Fix lint
* Fix final comment from review
* Fix comments from review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants