golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.03k stars 17.67k forks source link

x/crypto/acme/autocert: support ECDSA on TLS1.3-only connections #50522

Open mjl- opened 2 years ago

mjl- commented 2 years ago

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.

dr2chase commented 2 years ago

@FiloSottile can you give this a look?

cespare commented 2 years ago

/cc @golang/security

mjl- commented 2 years ago

I looked into this some more. I tried verifying that I would indeed get a error connecting with TLS1.3 with ECDSA only. I wanted to configure a tls.Config with only ECDSA as signature scheme. It looks like tls.Config does not allow setting signature schemes. I don't know if this is due to TLS1.3 the specification, or the Go implementation. I did notice this comment for CipherSuites: "Note that TLS 1.3 ciphersuites are not configurable.". I know modern crypto aims for fewer options/negotations. Perhaps RSA support is always required for TLS1.3. If so, the claim the title of this issue is incorrect (I extrapolated it from what I saw with autocert). At least it is clear that my knowledge about TLS1.3 is lacking.

But there is more. I wrote this program to connect to my autocert server. To see what the tls.ClientHelloInfo looks like, and if connection succeeds:

package main

import (
        "crypto/tls"
        "log"
        "os"
)

func main() {
        log.SetFlags(0)
        addr := os.Args[1]

        config := &tls.Config{
                MinVersion: tls.VersionTLS13,
        }
        if _, err := tls.Dial("tcp", addr, config); err != nil {
                log.Fatalf("dial: %s", err)
        }
        log.Printf("connected")
}

It resulted in this tls.ClientHelloInfo:

&tls.ClientHelloInfo{
    CipherSuites:[]uint16{0xc02b, 0xc02f, 0xc02c, 0xc030, 0xcca9, 0xcca8, 0xc009, 0xc013, 0xc00a, 0xc014, 0x9c, 0x9d, 0x2f, 0x35, 0xc012, 0xa, 0x1301, 0x1302, 0x1303},
    ServerName:"host.example",
    SupportedCurves:[]tls.CurveID{0x1d, 0x17, 0x18, 0x19},
    SupportedPoints:[]uint8{0x0},
    SignatureSchemes:[]tls.SignatureScheme{0x804, 0x403, 0x807, 0x805, 0x806, 0x401, 0x501, 0x601, 0x503, 0x603, 0x201, 0x203},
    SupportedProtos:[]string(nil),
    SupportedVersions:[]uint16{0x304},
...

I am surprised that cipher suites for TLS <=1.2 appear in the list for a TLS1.3-only connection. I tested with go1.17.6 and go1.18beta1, same result. I thought may be the crypto/tls server is inserting them.

To check that, I tried with a different ecosystem: curl --tlsv1.3 https://host.example. That resulted in:

&tls.ClientHelloInfo{
    CipherSuites:[]uint16{0x1302, 0x1303, 0x1301, 0xff},
    ServerName:"host.example",
    SupportedCurves:[]tls.CurveID{0x1d, 0x17, 0x1e, 0x19, 0x18},
    SupportedPoints:[]uint8{0x0, 0x1, 0x2},
    SignatureSchemes:[]tls.SignatureScheme{0x403, 0x503, 0x603, 0x807, 0x808, 0x809, 0x80a, 0x80b, 0x804, 0x805, 0x806, 0x401, 0x501, 0x601},
    SupportedProtos:[]string{"h2", "http/1.1"},
    SupportedVersions:[]uint16{0x304},
...

No TLS<=1.2 cipher suites this time. So it looks like the crypto/tls client inserts the TLS<=1.2 cipher suites in a TLS1.3-only connection. And that causes the supportsECDSA check in autocert to succeed. The request made by curl results in supportECDSA returning false, so the (successful) connection uses RSA, not ECDSA.

In the ClientHelloInfo's above, the signature schemes are different. So I'm concluding that it signature schemes are configurable with TLS1.3. But RSA support may always be required, and "ECDSA-only TLS1.3 connections" may not be a thing.

For reference, here is the acmecert server I used for testing with https://github.com/letsencrypt/pebble:

package main

import (
    "context"
    "crypto/ecdsa"
    "crypto/elliptic"
    "crypto/rand"
    "crypto/tls"
    "log"
    "net/http"
    "os"
    "strings"
    "time"

    "golang.org/x/crypto/acme"
    "golang.org/x/crypto/acme/autocert"
)

func main() {
    log.SetFlags(0)

    hostname := os.Args[1]

    key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
    if err != nil {
        log.Fatal(err)
    }
    client := &acme.Client{
        DirectoryURL: "https://localhost:14000/dir",
        Key:          key,
    }

    m := &autocert.Manager{
        Cache:      dirCache("cert"),
        Prompt:     autocert.AcceptTOS,
        HostPolicy: autocert.HostWhitelist(hostname),
        Email:      "acme@host.example",
        Client:     client,
    }

    loggingGetCertificate := func(hello *tls.ClientHelloInfo) (*tls.Certificate, error) {
        log.Printf("hello:\n%#v", hello)
        cert, err := m.GetCertificate(hello)
        if err != nil {
            log.Printf("getting certificate for %q: %s", hello.ServerName, err)
        }
        return cert, err
    }

    tryGetCertificate := func(name string) {
        hello := &tls.ClientHelloInfo{
            ServerName: name,

            // Make us fetch an ECDSA P256 cert.
            // CipherSuites: []uint16{tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, tls.TLS_AES_128_GCM_SHA256},
            CipherSuites:      []uint16{tls.TLS_AES_128_GCM_SHA256},
            SupportedCurves:   []tls.CurveID{tls.CurveP256},
            SignatureSchemes:  []tls.SignatureScheme{tls.ECDSAWithP256AndSHA256},
            SupportedVersions: []uint16{tls.VersionTLS13},
        }
        loggingGetCertificate(hello)
    }

    go func() {
        time.Sleep(100 * time.Millisecond)
        tryGetCertificate(hostname)
    }()

    httpsTLSConfig := *m.TLSConfig()
    httpsTLSConfig.GetCertificate = loggingGetCertificate
    s := &http.Server{
        Addr:      hostname + ":443",
        TLSConfig: &httpsTLSConfig,
    }
    log.Printf("listening for https")
    log.Fatal(s.ListenAndServeTLS("", ""))
}

type dirCache autocert.DirCache

func (d dirCache) Delete(ctx context.Context, name string) error {
    err := autocert.DirCache(d).Delete(ctx, name)
    if err != nil {
        log.Printf("deleting %q from dir cache: %s", name, err)
    } else if !strings.HasSuffix(name, "+token") {
        log.Printf("cert delete %q", name)
    }
    return err
}

func (d dirCache) Get(ctx context.Context, name string) ([]byte, error) {
    buf, err := autocert.DirCache(d).Get(ctx, name)
    if err != nil {
        log.Printf("getting %q from dir cache: %s", name, err)
    } else if !strings.HasSuffix(name, "+token") {
        log.Printf("cert get %q", name)
    }
    return buf, err
}

func (d dirCache) Put(ctx context.Context, name string, data []byte) error {
    err := autocert.DirCache(d).Put(ctx, name, data)
    if err != nil {
        log.Printf("storing %q in dir cache: %s", name, err)
    } else if !strings.HasSuffix(name, "+token") {
        log.Printf("cert store %q", name)
    }
    return err
}

And go.mod:

module acmecert

go 1.17

require golang.org/x/crypto v0.0.0-20211215153901-e495a2d5b3d3

require (
        golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2 // indirect
        golang.org/x/text v0.3.6 // indirect
)
mjl- commented 2 years ago

An ECDSA TLS1.3 connection indeed fails:

$ openssl s_client -tls1_3 -connect host.example:443 -sigalgs "ECDSA+SHA256"
CONNECTED(00000004)
139741787190656:error:14094410:SSL routines:ssl3_read_bytes:sslv3 alert handshake failure:../ssl/record/rec_layer_s3.c:1543:SSL alert number 40
---
no peer certificate available
---
No client certificate CA names sent
---
SSL handshake has read 7 bytes and written 215 bytes
Verification: OK
---
New, (NONE), Cipher is (NONE)
Secure Renegotiation IS NOT supported
Compression: NONE
Expansion: NONE
No ALPN negotiated
Early data was not sent
Verify return code: 0 (ok)
---

Logging from acmecert as posted above (with an additional log line added to "supportsECDSA", according to current detection of support):

hello:
&tls.ClientHelloInfo{
        CipherSuites:[]uint16{0x1302, 0x1303, 0x1301, 0xff},
        ServerName:"host.example",
        SupportedCurves:[]tls.CurveID{0x1d, 0x17, 0x1e, 0x19, 0x18},
        SupportedPoints:[]uint8{0x0, 0x1, 0x2},
        SignatureSchemes:[]tls.SignatureScheme{0x403},
        SupportedProtos:[]string(nil),
        SupportedVersions:[]uint16{0x304},
...
}
client supports ECDSA: false
http: TLS handshake error from 0.0.0.0:44898: tls: peer doesn't support any of the certificate's signature algorithms
FiloSottile commented 2 years ago

supportsECDSA is indeed an extremely limited and partial check. The assumption is that RSA is always the safe fallback, too, which is not necessarily true. I added ClientHelloInfo.SupportsCertificate in Go 1.14 to address this. It's been in the standard library long enough that we should just switch autocert to it and delete supportsECDSA.