Repository: go
Issue: x/crypto/acme/autocert: support ECDSA on TLS1.3-only connections
Labels: NeedsInvestigation

Latest version of x/crypto/acme/autocert at the time of writing:

https://pkg.go.dev/golang.org/x/crypto@v0.0.0-20211215153901-e495a2d5b3d3/acme/autocert

### What did you do?

Call Manager.GetCertificate to force an immediate retrieval of missing certificates (during application startup).

### What did you expect to see?

I expected autocert to fetch an ECDSA certificate.

### What did you see instead?

An RSA certificate was fetched instead.
Then, the first time I actually connect to my service (using standard tools like curl and openssl s_client), an ECDSA certificate was fetched. Because ECDSA is typically preferred over RSA by such tools.

### Analysis

My initial tls.ClientHelloInfo had only ServerName set, no ciphersuites, versions, etc. This (correctly) fails the supportsECDSA check at: https://cs.opensource.google/go/x/crypto/+/e495a2d5:acme/autocert/autocert.go;drc=e495a2d5b3d3be43468d0ebb413f46eeaedf7eb3;l=322

That's fair, so I tried with an `hello` that supports only ECDSA:

```
tryGetCertificate := func(name string) {
	hello := &tls.ClientHelloInfo{
		ServerName: name,

		CipherSuites: []uint16{tls.TLS_AES_128_GCM_SHA256},
		SupportedCurves: []tls.CurveID{tls.CurveP256},
		SignatureSchemes: []tls.SignatureScheme{tls.ECDSAWithP256AndSHA256},
		SupportedVersions: []uint16{tls.VersionTLS13},
	}
	m.GetCertificate(hello)
}
```

However, that still results in an RSA certificate being fetched: supportsECDSA does not take TLS1.3 configs into account, requiring an ECDSA-supporting ciphersuite for TLS <= 1.2. See: https://cs.opensource.google/go/x/crypto/+/e495a2d5:acme/autocert/autocert.go;l=353;drc=e495a2d5b3d3be43468d0ebb413f46eeaedf7eb3
Background info: With TLS1.3,  the signature schemes have been taken/separated out of CipherSuite definitions.

I believe supportsECDSA should be updated to recognize TLS1.3, with a check for ECDSA in SignatureSchemes.
Furthermore, I believe supportsECDSA should be modified to also check for RSA support in the ClientHelloInfo: There is no point in fetching an RSA certificate if the client cannot use it. I understand RSA used to be the default signature scheme. But it may not be in the future, and clients are already free to select the signature schemes they wish to use.

I can work around my immediate issue (forcing a fetch of an ECDSA certificate) by simply including tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 in the CipherSuites slice. Then it passes the supportsECDSA check. But actual TLS1.3-only connections will still fetch an RSA certificate, not an ECDSA certificate, even if RSA isn't supported by the client.

I can write a patch if this sounds reasonable.

Another idea: We could change Manager.GetCertificate to be satisfied with an existing RSA certificate in case of a non-existing ECDSA certificate. We would have to be careful that the supported RSA-using-cipher-suites aren't otherwise weaker than the ECDSA cipher suites. That's why I'm not sure it is worth it. But if we were to expand the signature-scheme-compatibility to check for RSA too, perhaps it is worth it.