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

Jump to bottom

crypto: document error behavior for Signer.PublicKey & Decrypter.PublicKey #30510

Closed
optnfast opened this issue Mar 1, 2019 · 3 comments
Closed

crypto: document error behavior for Signer.PublicKey & Decrypter.PublicKey #30510

optnfast opened this issue Mar 1, 2019 · 3 comments
Labels
Documentation FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.14

Comments

@optnfast
Copy link

optnfast commented Mar 1, 2019

crypto.Decrypter and the similar crypto.Signer both define PublicKey() methods which are supposed to return the public key corresponding to the private key used for decryption/signing.

An implementation of PublicKey() that supports nShield HSMs may fail for a couple of reasons:

  • if all you have is a private key handle, it is not (usually) possible to recover the public key from it.
  • an error may occur while exporting a key.

With this in mind my requests are:

  1. Please could the API documentation define what PublicKey() should do when an error occurs. Returning nil or panicing with an informative message seem like the likely options.
  2. If at some time backward compatibility can be broken, please add an additional error return value to PublicKey().
@bradfitz bradfitz changed the title Please document error behavior for crypto.Signer.PublicKey & crypto.Decrypter.PublicKey crypto: document error behavior for Signer.PublicKey & Decrypter.PublicKey Mar 1, 2019
@bradfitz bradfitz added this to the Go1.13 milestone Mar 1, 2019
@bradfitz
Copy link
Contributor

bradfitz commented Mar 1, 2019

/cc @FiloSottile

@bradfitz bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 1, 2019
@FiloSottile
Copy link
Contributor

It should probably panic, letting a nil float around is almost always regrettable. Also, most if not all crypto.Signer users are going to fail because they use the public key type to learn the key type.

@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@FiloSottile
Copy link
Contributor

I tried coming up with some wording for this, but in fact I don't think an implementation of Signer or Decrypter that can't provide the public key will work anywhere. That might have been a mistake, but it's too late to change.

When I had situations like this I wrote wrappers that take the public key from configuration.

@golang golang locked and limited conversation to collaborators Sep 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Documentation FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.14
Development

No branches or pull requests
5 participants
@bradfitz @andybons @FiloSottile @optnfast @gopherbot