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/ecdh: allow external curve and key implementations #69863

Open
MagicalTux opened this issue Oct 12, 2024 · 2 comments
Open

proposal: crypto/ecdh: allow external curve and key implementations #69863

MagicalTux opened this issue Oct 12, 2024 · 2 comments
Labels
Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone

Comments

@MagicalTux
Copy link

Proposal Details

At this point this is more a number of thoughts and description of issues I've encountered trying to work with crypto/ecdh rather than a proposal to change things in a specific way. crypto/ecdh is used by a lot of people, so breaking changes are out of the picture unless maybe if a /v2 version is created, but I am not sure this is warranted or even qualifies.

My goal here is first to start a discussion.

Golang's crypto package is nice because objects like crypto.PrivateKey are interfaces that allows both the use of local secrets managed via the standard libraries, or hardware encryption where secrets aren't stored on the machine at all.

It seems that this was a goal, as crypto/ecdh.Curve is an interface that could be implemented by a third party package, for the exception that it has private functions and it is impossible to implement NewPrivateKey or NewPublicKey because both are opaque objects.

Ideally both ecdh.PublicKey and ecdh.PrivateKey would be interfaces, however now that the package is widely used it is not possible to change that without causing a lot of breakage.

An alternative would be to have a method to create private/public key objects by specifying the data (bytes) and curve interface, and making the private elements of the curve interface public. Actually it would be better if the private key data could be an interface for the private key, as in the case of a tpm it would be a Handle, and in some cases libraries may prefer keeping track of specific values for performance.

Point is, the ecdh interface as it exists is very nice. Creating an ephemeral private key to encode a message to a recipient is as easy as recipient.Curve().GenerateKey(rand.Reader), it's a shame however that it is not possible to implement other curves.

Current library has a nice API, the part about PrivateKey can actually be worked around fairly easily as long as the curve is one supported by crypto/ecdh. Working with other curves however is proving to be extremely difficult, dirty alternatives include things like implementing GenerateKey directly on the public key object that returns not a *ecdh.PrivateKey but a custom object that simply implements the minimum required.

Some suggestions here:

type PrivateKeyInterface interface {
    Curve() Curve
    ECDH(remote *PublicKey) ([]byte, error)
    Equal(x crypto.PrivateKey) bool
    Public() crypto.PublicKey
}

type GenericCurve interface {
    // Using a different name so that to not break existing Curve
    GeneratePrivateKey(rand io.Reader) (PrivateKeyInterface, error)

    // The current ecdh package does not expose the following methods, making these public would allow custom implementations so I'm curious for the more specific reason than "breaking backward compatibility" for an interface that cannot be implemented externally anyway
    ECDH(local *PrivateKey, remote *PublicKey) ([]byte, error)
    PrivateKeyToPublicKey(PrivateKeyInterface) *PublicKey
}

// NewPublicKey would simply return a public key object
func NewPublicKey(c Curve, data []byte) *PublicKey

// This could be nice to allow custom curve objects to be marshalable, and maybe parsable if there is some method to register a type
type X509Marshalable interface {
    GetOID() pkix.AlgorithmIdentifier
    Bytes() []byte // current name in ecdh
}

More curves are being requested, and being able to extend things without the need for users of crypto/ecdh to think too much about makes sense I believe.

@gopherbot gopherbot added this to the Proposal milestone Oct 12, 2024
@seankhliao seankhliao added the Proposal-Crypto Proposal related to crypto packages or other security issues label Oct 12, 2024
@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals Oct 13, 2024
@ianlancetaylor
Copy link
Member

CC @golang/security

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
Status: Incoming
Development

No branches or pull requests

5 participants