proposal: crypto/x509: support extracting X25519 public keys from certificates

[EdSchouten]

Proposal Details

Even though X25519 key pairs (not to be confused with Ed25519 key pairs) can't be used to establish TLS connections, there are still cases in which you want to use them in combination with X.509 certificates. For example, if X25519 is used to perform public key authenticated encryption (e.g., NaCl's crypto_box), X.509 certificates may be used to authenticate the identity of the peer before encrypting/decrypting. RFC 8410 added the ability to embed X25519 public keys into X.509 certificates for such use cases.

Right now x509.ParseCertificate() completes successfully when presented with a certificate containing an X25519 public key. Unfortunately, it does end up discarding the public key, as Certificate.PublicKeyAlgorithm and Certificate.PublicKey will be set to UnknownPublicKeyAlgorithm and nil, respectively.

The proposal here is to actually let x509.ParseCertificate() extract the public key from the certificate and return it in the form of an *ecdh.PublicKey. The PublicKeyAlgorithm enumeration will be extended to include an additional element for this public key algorithm, which will simply be called X25519.

Code changes can be found here: https://go-review.googlesource.com/c/go/+/632875

[gabyhelp]

Related Issues

• proposal: crypto/tls, crypto/x509: EdDSA certificates support #25355 (closed)
• crypto/tls: create certs w/o KeyEncipherment KU for non-RSA keys in generate_cert.go #36500 (closed)

Related Code Changes

• crypto/x509: add support for PKCS8/PKIX X25519 key encodings

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[ianlancetaylor]

CC @golang/security

[rolandshoemaker]

Per the crypto/x509 package documentation "the primary goal of the package is to provide compatibility with the publicly trusted TLS certificate ecosystem and its policies and constraints".

Since X25519 is not supported by TLS, and X25519 keys are not allowed in certificates issued by publicly trusted CAs, per the CABF baseline requirements, I'm not sure there is a strong reason for us to add explicit support for these certificates.

As you say, ParseCertificate does parse these certificates. If you do need to use them for whatever purpose, you can still manually extract the key from the Certificate.RawSubjectPublicKeyInfo field and use it for your purposes. This is slightly more complex, but I think a worthwhile trade off to prevent people from accidentally misusing these certificates for purposes they are not intended for.

[FiloSottile]

We could probably include an Example for how to use cryptobyte to extract a public key from RawSubjectPublicKeyInfo.

[EdSchouten]

That sounds fine to me. That said, I am also interested in generating certificates. It looks like CreateCertificate() doesn’t respect RawSubjectPublicKeyInfo right now. Would it make sense to extend CreateCertificate() to do so?

[EdSchouten]

@rolandshoemaker @FiloSottile Friendly ping.

[rolandshoemaker]

I'm not sure I'd be comfortable blindly copying the contents of RawSubjectPublicKeyInfo into a certificate. Currently CreateCertificate will always result in a correctly encoded certificate, regardless of the way you populate the Certificate fields. With the proposed change we could end up producing a certificate that could not be parsed.

We could plausibly attempt to decode the RawSubjectPublicKeyInfo to check it's mostly valid, before inserting it into the certificate, but I'm not particularly convinced this is functionality we should be supporting regardless.

crypto/x509 is, like I said, very much focused on the basic use-case of the webpki, and we try to limit the amount of fiddling the user can do in order to reduce the complexity of the API surface and of our underlying implementation. I think in general, for experimentation, crypto/x509 is not the right library to be using.

[hslatman]

Was experimenting a bit with creating a certificate. Copying the RawSubjectPublicKeyInfo doesn't seem necessary when the public key is provided as the pub parameter. This small patch with just 4 additions resulted in a cert that seems to decode correctly:

diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go
index 1f06b4fbc5..610f65d64d 100644
--- a/src/crypto/x509/x509.go
+++ b/src/crypto/x509/x509.go
@@ -268,6 +268,7 @@ const (
        DSA // Only supported for parsing.
        ECDSA
        Ed25519
+       X25519
 )

 var publicKeyAlgoName = [...]string{
@@ -275,6 +276,7 @@ var publicKeyAlgoName = [...]string{
        DSA:     "DSA",
        ECDSA:   "ECDSA",
        Ed25519: "Ed25519",
+       X25519:  "X25519"
 }

 func (algo PublicKeyAlgorithm) String() string {
@@ -507,6 +509,8 @@ func getPublicKeyAlgorithmFromOID(oid asn1.ObjectIdentifier) PublicKeyAlgorithm
                return ECDSA
        case oid.Equal(oidPublicKeyEd25519):
                return Ed25519
+       case oid.Equal(oidPublicKeyX25519):
+               return X25519
        }
        return UnknownPublicKeyAlgorithm
 }

Addition of this case to the switch is just to prevent the x509: unsupported public key type: *ecdh.PublicKey error from being returned. It doesn't affect what's encoded in the cert, as that is already correctly encoded in marshalPublicKey.

The certificate (signed with EC signer):

-----BEGIN CERTIFICATE-----
MIIBRzCB7qADAgECAhBGAgu4Ttql2Ey+Z8si7oekMAoGCCqGSM49BAMCMCExHzAd
BgNVBAMTFk1pbmlDQSBJbnRlcm1lZGlhdGUgQ0EwHhcNMjUwOTIwMDAzNzI3WhcN
MjUwOTIxMDAzNzI3WjAWMRQwEgYDVQQDEwtURVNUIFgyNTUxOTAqMAUGAytlbgMh
AFIdZMPW2ezOCbC2qkaIsFVx+M0tQkGljanIOfYptddLo0IwQDAdBgNVHQ4EFgQU
m1sTfZtMcbIAjJlEFBmCem7sn64wHwYDVR0jBBgwFoAUrD9DUy3EpXItfkFp+u9c
tk60sh8wCgYIKoZIzj0EAwIDSAAwRQIhAPlWsCIQgsq+aZy1D81mR1yp4k7Q7qYa
IdzeGSSWEwqSAiBRNOtDDR2zqMmLAdMUYpQaq4JtQf7fUfa7+y1q4XY/OQ==
-----END CERTIFICATE-----

Output in an ASN1 decoder for the SPKI looks similar as for the example in https://datatracker.ietf.org/doc/html/rfc8410#section-10.2.

Edit: @EdSchouten only now noticing that those are part of your patch too.

[FiloSottile]

I am still not a fan of letting ParseCertificate accept X25519 certificates, because there are surely places that just accept anything that ParseCertificate accepts. It's letting *ecdh.PublicKey into a system it was not part of before. On the other hand, we already support X25519 in MarshalPKIXPublicKey, so maybe allowing it in CreateCertificate would be fine? A little weird that CreateCertificate and ParseCertificate would not round-trip, but CreateCertificate spits out bytes, not a Certificate.

The patch is even simpler, we just need to remove this check from CreateCertificate. CreateCertificateRequest doesn't have it, and I bet it just works with X25519 now. marshalPublicKey already rejects actually unknown key types.

	if getPublicKeyAlgorithmFromOID(publicKeyAlgorithm.Algorithm) == UnknownPublicKeyAlgorithm {
		return nil, fmt.Errorf("x509: unsupported public key type: %T", pub)
	}

Looks like I added it in CL 450815 to avoid unintentionally introducing support for X25519 certificates.

[hslatman]

The patch is even simpler, we just need to remove this check from CreateCertificate. CreateCertificateRequest doesn't have it, and I bet it just works with X25519 now. marshalPublicKey already rejects actually unknown key types.

x509.CreateCertificateRequest will fail early, because the X25519 key is not a crypto.Signer. Any authority wishing to certify a X25519 key will thus have to use a different format to obtain and validate a signing request for an X25519 key, but that's outside the scope of this proposal (and far outside the crypto/x509 scope).

	if getPublicKeyAlgorithmFromOID(publicKeyAlgorithm.Algorithm) == UnknownPublicKeyAlgorithm {
		return nil, fmt.Errorf("x509: unsupported public key type: %T", pub)
	}

👍

Looks like I added it in CL 450815 to avoid unintentionally introducing support for X25519 certificates.