x/crypto/acme/autocert: ToS prompt check for ToS-less ACME servers

benburkert commented 8 months ago

Go version

go version go1.21.4 darwin/arm64

What operating system and processor architecture are you using (`go env`)?

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/benburkert/Library/Caches/go-build'
GOENV='/Users/benburkert/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/benburkert/.asdf/installs/golang/1.21.4/packages/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/benburkert/.asdf/installs/golang/1.21.4/packages'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/benburkert/.asdf/installs/golang/1.21.4/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/benburkert/.asdf/installs/golang/1.21.4/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.21.4'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/benburkert/src/golang.org/x/crypto/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/yw/d_dr0rn16ks575_14_fppcfw0000gn/T/go-build1407559308=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

Use (x/crypto/acme/autocert).Manager without initializing the Prompt field to request a certificate from an ACME provider (https://zerossl.com/ for example) that doesn't require the optional termsOfServiceAgreed portion of newAccount requests:

&autocert.Manager{
    HostPolicy: autocert.HostWhitelist("example-domain.test"),

    Client: &acme.Client{
        DirectoryURL: "https://acme.zerossl.com/v2/DV90",
    },

    ExternalAccountBinding: &acme.ExternalAccountBinding{
        KID: eabKID,
        Key: eabKey,
    },
}

What did you expect to see?

A valid certificate via the autocert.Manager. The Manager.Client.DirectoryURL is explicitly being set so the check for nil Prompt field does not seem necessary.

What did you see instead?

An error message log: http: TLS handshake error from <redacted>: acme/autocert: Manager.Prompt not set

seankhliao commented 8 months ago

Note the field comment:

To always accept the terms, the callers can use AcceptTOS.

cc @golang/security

benburkert commented 8 months ago

Yes, and even setting Prompt to func(string) { return false } works for this ACME provider, since they do account provisioning beforehand. It seems quirky to require the Prompt field when the part of the RFC that it maps to is marked as optional. Changing this m.Prompt == nil to a m.Prompt == nil && (m.Client == nil || m.Client.DirectoryURL == "") would keep the check for the default provider (Let's Encrypt requires it) and drop it for a custom provider.

golang / go