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

proposal: crypto/ed25519: change GenerateKey to return private key only #37431

Closed
yoseplee opened this issue Feb 25, 2020 · 1 comment
Closed

Comments

@yoseplee
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.13.3 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

What did you do?

  • Consistency needed: Change GenerateKey() to return only private key with error because it seems awkward that it returns both public key and private key while NewKeyFromSeed() returns only private key. In addition, it is so confused for someone who are not familiar with crypto/ed25519 package
  • I know there might be a huge side effect when standard package is changed, but my opinion is that it worth modifying it right now, considering the current increase of gophers. As more gophers mean that more go code will be, this is perfect time to change it
  1. As-is
// GenerateKey generates a public/private key pair using entropy from rand.
// If rand is nil, crypto/rand.Reader will be used.
func GenerateKey(rand io.Reader) (PublicKey, PrivateKey, error) {
	if rand == nil {
		rand = cryptorand.Reader
	}

	seed := make([]byte, SeedSize)
	if _, err := io.ReadFull(rand, seed); err != nil {
		return nil, nil, err
	}

	privateKey := NewKeyFromSeed(seed)
	publicKey := make([]byte, PublicKeySize)
	copy(publicKey, privateKey[32:])

	return publicKey, privateKey, nil
}
  1. To-be
// GenerateKey generates a private pair using entropy from rand.
// If rand is nil, crypto/rand.Reader will be used.
func GenerateKey(rand io.Reader) (PrivateKey, error) {
	if rand == nil {
		rand = cryptorand.Reader
	}

	seed := make([]byte, SeedSize)
	if _, err := io.ReadFull(rand, seed); err != nil {
		return nil, nil, err
	}

	privateKey := NewKeyFromSeed(seed)
	return privateKey, nil
}

What did you expect to see?

  1. Better coding experiment. No more confusion in package document(https://pkg.go.dev/crypto/ed25519?tab=doc)
  2. Improve code consistency for not only package itself but also codes using it

What did you see instead?

@gopherbot gopherbot added this to the Proposal milestone Feb 25, 2020
@FiloSottile
Copy link
Contributor

I don't necessarily disagree that it would be cleaner to return just the PrivateKey, but unfortunately the API is now locked in by the Go 1 Compatibility Promise.

https://golang.org/doc/go1compat

@golang golang locked and limited conversation to collaborators Feb 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants