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: concrete interfaces for crypto.PublicKey and crypto.PrivateKey #30140

Open
solderjs opened this issue Feb 8, 2019 · 8 comments

Comments

@solderjs
Copy link

commented Feb 8, 2019

Currently crypto.PublicKey and crypto.PrivateKey are empty interfaces.

This has bitten me a number of times because errors that could easily be caught by tooling or at compile time become runtime errors (sometimes very confusing ones).

It appears that all Private Keys in the crypto package include PublicKey or Public() crypto.PublicKey. I assume that, by their very nature, any future invention of asymmetric keys will include the Public Key as part of the definition of the Private Key.

There's also a well-known and intuitive standard way to compute a key Thumbprint that would work on any existing asymmetric keys and any future asymmetric keys. I believe there are also one or more well-known ways to compute a Fingerprint (SSH, among others), but perhaps Thumbprint is more distinct in that there is only one such specification (as far as I know).

I propose that crypto.PrivateKey be changed as follows:

type PrivateKey {
    Public() crypto.PublicKey
}

And that crypto.PublicKey likewise change to something like this:

type PublicKey {
    ThumbprintSHA256() []byte
}

@gopherbot gopherbot added this to the Proposal milestone Feb 8, 2019

@gopherbot gopherbot added the Proposal label Feb 8, 2019

@FiloSottile FiloSottile added the Go2 label Feb 8, 2019

@FiloSottile

This comment has been minimized.

Copy link
Member

commented Feb 8, 2019

Interfaces can't be changed in Go 1 due to the Go 1 compatibility promise.

@solderjs

This comment has been minimized.

Copy link
Author

commented Feb 9, 2019

Thanks for the quick reply.

Before you tag this as Go2 only:

First Point

Adding ThumbprintSHA256() []byte to all of the crypto.PublicKey types and and Public() crypto.PublicKey to "crypto/dsa" would not break compile time compatibility at all and would make it possible for someone like myself to easily type out the interface is my own code without having to wrap the object.

It makes them interface-able. No breakage.

Second Point

Although you could technically say that changing the interface breaks compatibility, it's actually just moving run time errors to compile time errors.

However, I'm willing to concede that some weirdo, somewhere, has used crypto.PublicKey as passed in a string and somehow manages to make his code work anyway. Or, perhaps more likely, someone has some dead code somewhere which doesn't actually execute (otherwise it would panic) but that is leftover in the code.

That being said, the bigger issue is the first point. It's trivial to add an interface.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2019

Although you could technically say that changing the interface breaks compatibility, it's actually just moving run time errors to compile time errors.

Technicalities matter. We can't make this change in Go 1.

@solderjs

This comment has been minimized.

Copy link
Author

commented Feb 19, 2019

Forget the second point entirely. How about the 1st point?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2019

@FiloSottile What do you think of the idea of adding a standard method to every PublicKey and PrivateKey implementation? Does anybody actually define their own PublicKey or PrivateKey implementation?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Apr 16, 2019

@FiloSottile

This comment has been minimized.

Copy link
Member

commented Apr 17, 2019

I'm ok with making Public() crypto.PublicKey a convention. I don't really get why anyone would be using DSA in 2019, and I wish I could freeze and deprecate crypto/dsa, but since this came up a couple times now (#27889, /cc @Merovius) I would accept a CL making dsa.PrivateKey a crypto.Signer at this point, sure.

I had never heard of a thumbprint, and I don't want to touch JWT specs if at all possible. I'm not opposed to adding a common method to all public keys, but I can't think of a good one.

@FiloSottile FiloSottile added Proposal-Crypto and removed Go2 labels Apr 17, 2019

@solderjs

This comment has been minimized.

Copy link
Author

commented Apr 17, 2019

How about Fingerprint then? That's also well standardized, a la ssh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.