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/ecdsa: add UnmarshalPublicKey #63963
Comments
You can use the func ecdhToECDSAPublicKey(key *ecdh.PublicKey) *ecdsa.PublicKey {
rawKey := key.Bytes()
switch key.Curve() {
case ecdh.P256():
return &ecdsa.PublicKey{
Curve: elliptic.P256(),
X: big.NewInt(0).SetBytes(rawKey[1:33]),
Y: big.NewInt(0).SetBytes(rawKey[33:]),
}
case ecdh.P384():
return &ecdsa.PublicKey{
Curve: elliptic.P384(),
X: big.NewInt(0).SetBytes(rawKey[1:49]),
Y: big.NewInt(0).SetBytes(rawKey[49:]),
}
case ecdh.P521():
return &ecdsa.PublicKey{
Curve: elliptic.P521(),
X: big.NewInt(0).SetBytes(rawKey[1:67]),
Y: big.NewInt(0).SetBytes(rawKey[67:]),
}
default:
panic("cannot convert non-NIST *ecdh.PublicKey to *ecdsa.PublicKey")
}
} But I agree that it is not convenient to write this. |
Ah, thanks for the info, that at least looks more efficient, if a little obscure. I'll use that instead of x509 for the time being 👍 What does it look like for PrivateKey? |
Not tested, but i think something like this should work: key := &ecdsa.PrivateKey{
PublicKey: *ecdhToECDSAPublicKey(priv.PublicKey()),
D: big.NewInt(0).SetBytes(priv.Bytes()),
} |
Gave that a test, seems to work, thanks! |
I think this proposal looks like this now: package ecdsa // crypto/ecdsa
// NewPrivateKeyFromECDH converts [*ecdh.PrivateKey] into [*PrivateKey].
// It panics when the [*ecdh.PrivateKey.Curve] is not one of [ecdh.P256],
// [ecdh.P384], [ecdh.P521].
func NewPrivateKeyFromECDH(priv *ecdh.PrivateKey) *ecdsa.PrivateKey
// NewPublicKeyFromECDH converts [*ecdh.PublicKey] into [*PublicKey].
// It panics when the [*ecdh.PublicKey.Curve] is not one of [ecdh.P256],
// [ecdh.P384], [ecdh.P521].
func NewPublicKeyFromECDH(priv *ecdh.PublicKey) *ecdsa.PublicKey As an alternative we might provide functions that parse PublicKey/PrivateKey from raw bytes, similar to package ecdsa // crypto/ecdsa
func NewPrivateKey(curve elliptic.Curve, raw []byte) (*ecdsa.PrivateKey, error)
func NewPublicKey(curve elliptic.Curve, raw []byte) (*ecdsa.PublicKey, error) |
cc @golang/security |
Hey @FiloSottile could you offer guidance on this? I also have an uncompressed point public key for ECDSA, and would reach for |
Looking at this again, an @ThadThompson sounds like you also need |
@mrwonko Yeah, right, but because there is only one error condition it might also be a bool instead. Either way is fine. func NewPrivateKeyFromECDH(priv *ecdh.PrivateKey) (priv *ecdsa.PrivateKey, ok bool) |
Keys should generally not be reused for both ECDH and ECDSA. It can be safe, but it's not something we want to encourage blindly. crypto/ecdsa to crypto/ecdh exists because crypto/x509 parsing functions will unavoidably return a crypto/ecdsa key, where in fact you might want a crypto/ecdh one. (The encoding of the two are the same.) However, here as far as I can tell what folks need is not a conversion, but a way to go from uncompressed point encoding (the input to If that's right, I think we should add new parsing functions to crypto/ecdsa, rather than involve crypto/ecdh. @mrwonko, I see the details on the public key format, but what format are you parsing the private key from? |
@FiloSottile yes, exactly. I'm writing a solution involving both signatures and key exchange. While using different keys for ECDH and ECDSA, the key exchange is being used in HPKE which specifies uncompressed point serialization for the P curves (https://datatracker.ietf.org/doc/html/rfc9180#section-7.1.1). It seemed reasonable to just use the same format for the signature public key as well. It would be intuitive if there were corresponding functions to load and validate the keys for their respective use cases - even if the exact same logic is running under the hood. |
@FiloSottile I generate the key using the Terraform If there was a function to directly parse it into an |
@ThadThompson yeah I think there's a good argument for uncompressed point to
@mrwonko, I am confused, in #63963 (comment) you were looking for crypto/ecdh -> crypto/ecdsa. Is what you are doing ECDH or ECDSA? |
The proposed parsing API is
Should we add MarshalPublicKey too, a PublicKey.Marshal method, or let clients use PublicKey.ECDH.Bytes? I'm not a fan of PublicKey.Marshal because it would encourage using that over x509.MarshalPKIXPublicKey, and this is not a common encoding. MarshalPublicKey is a little better because it's less immediately discoverable. /cc @golang/proposal-review |
As I've seen it described in various places as one of 'SECG', 'X963' or 'raw' format, having that comment is significantly helpful to anyone coming from a different system. I don't have a strong feeling on it, but if I came along and saw |
Sorry for the confusion, I was looking at the wrong use case. This is for webpush-go, which needs to turn incoming bytes into an |
elliptic.Marshal was deprecated, we can replace it with the ECDH methods even though we aren't using ECDH here. See: - golang/go@f03fb14 We still using elliptic.Unmarshal because this issue needs to be resolved: - golang/go#63963
elliptic.Marshal was deprecated, we can replace it with the ECDH methods even though we aren't using ECDH here. See: - golang/go@f03fb14 We still using elliptic.Unmarshal because this issue needs to be resolved: - golang/go#63963
The proposed |
I looked into Web Push a bit further, to figure out if the private key raw encoding is necessary, and I don't think it is. RFC 8292, Section 3.2 does indeed use the raw encoding for ECDSA public keys, but there is no specified format for the private key, so I think implementations should just use the standard PKCS#8. https://datatracker.ietf.org/doc/html/rfc8292#section-3.2 (Also note that all the other operations in Web Push aside from the VAPID signature can be done with crypto/ecdh. The spec is clear that "An application server MUST select a different private key for the key exchange [RFC8291] and signing the authentication token." so there is no need for conversions.) Looks like Nebula exposed both raw private and public key encoding for ECDSA, though. Overall, one RFC and one proprietary protocol are not indicative of widespread adoption of the raw key format for ECDSA. Personally, I might actually prefer it over PKIX / PKCS#8, but since the latter is predominant, I think deprecated functions might actually be sufficient as support: they give a clear signal that this is not the common path. I'm leaning towards not implementing this, but happy to keep it open to collect more feedback. |
elliptic.Marshal was deprecated, we can replace it with the ECDH methods even though we aren't using ECDH here. See: - golang/go@f03fb14 We still using elliptic.Unmarshal because this issue needs to be resolved: - golang/go#63963
I'm working with the JS Web Push API to send push messages, which uses an ECDH Key in base64-encoded X9.62 uncompressed form, like
BJ932huv68tUDxifpf6qlzuRa_JBF-2E9J47alSQRuxpmt3QFtiCnhqXlPgZuGWKZzcp5jAzQYeHpMUB990JiHM
. I can easily parse that usingecdh.P256().NewPublicKey(base64DecodedBytes)
, but that yields an*ecdh.PublicKey
, while all the cryptography functions need an*ecdsa.PublicKey
.There appears to be no straightforward conversion from
*ecdh.PublicKey
to*ecdsa.PublicKey
, the best thing I've found is an x509 PKIX roundtrip:That feels clunky, inefficient, and I lose type safety / need a type assertion. For the inverse conversion, there's already an
ecdsa.PublicKey.ECDH()
function. I propose adding something similar for ecdh to ecdsa conversion, both for public and private keys. I imagine it will have to be added to the ecdsa package to avoid cyclic imports.The text was updated successfully, but these errors were encountered: