Skip to content

Commit 227b76d

Browse files
x1ddosAlex Vaghin
authored and
Alex Vaghin
committed
acme/autocert: remove tls-sni-xx challenge support
These challenge types have been deprecated by popular ACME providers due to security issues in the ecosystem. Fixes golang/go#28370 Change-Id: I3270a6f5d3e5fbc53e4347a9a802df5f603c87de Reviewed-on: https://go-review.googlesource.com/c/crypto/+/194658 Run-TryBot: Alex Vaghin <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]>
1 parent 094676d commit 227b76d

File tree

3 files changed

+105
-106
lines changed

3 files changed

+105
-106
lines changed

acme/autocert/autocert.go

Lines changed: 10 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,9 @@ func defaultHostPolicy(context.Context, string) error {
8888
}
8989

9090
// Manager is a stateful certificate manager built on top of acme.Client.
91-
// It obtains and refreshes certificates automatically using "tls-alpn-01",
92-
// "tls-sni-01", "tls-sni-02" and "http-01" challenge types,
93-
// as well as providing them to a TLS server via tls.Config.
91+
// It obtains and refreshes certificates automatically using "tls-alpn-01"
92+
// or "http-01" challenge types, as well as providing them to a TLS server
93+
// via tls.Config.
9494
//
9595
// You must specify a cache implementation, such as DirCache,
9696
// to reuse obtained certificates across program restarts.
@@ -184,10 +184,8 @@ type Manager struct {
184184
// to be provisioned.
185185
// The entries are stored for the duration of the authorization flow.
186186
httpTokens map[string][]byte
187-
// certTokens contains temporary certificates for tls-sni and tls-alpn challenges
188-
// and is keyed by token domain name, which matches server name of ClientHello.
189-
// Keys always have ".acme.invalid" suffix for tls-sni. Otherwise, they are domain names
190-
// for tls-alpn.
187+
// certTokens contains temporary certificates for tls-alpn-01 challenges
188+
// and is keyed by the domain name which matches the ClientHello server name.
191189
// The entries are stored for the duration of the authorization flow.
192190
certTokens map[string]*tls.Certificate
193191
// nowFunc, if not nil, returns the current time. This may be set for
@@ -226,7 +224,7 @@ func (m *Manager) TLSConfig() *tls.Config {
226224

227225
// GetCertificate implements the tls.Config.GetCertificate hook.
228226
// It provides a TLS certificate for hello.ServerName host, including answering
229-
// tls-alpn-01 and *.acme.invalid (tls-sni-01 and tls-sni-02) challenges.
227+
// tls-alpn-01 challenges.
230228
// All other fields of hello are ignored.
231229
//
232230
// If m.HostPolicy is non-nil, GetCertificate calls the policy before requesting
@@ -235,9 +233,7 @@ func (m *Manager) TLSConfig() *tls.Config {
235233
// This does not affect cached certs. See HostPolicy field description for more details.
236234
//
237235
// If GetCertificate is used directly, instead of via Manager.TLSConfig, package users will
238-
// also have to add acme.ALPNProto to NextProtos for tls-alpn-01, or use HTTPHandler
239-
// for http-01. (The tls-sni-* challenges have been deprecated by popular ACME providers
240-
// due to security issues in the ecosystem.)
236+
// also have to add acme.ALPNProto to NextProtos for tls-alpn-01, or use HTTPHandler for http-01.
241237
func (m *Manager) GetCertificate(hello *tls.ClientHelloInfo) (*tls.Certificate, error) {
242238
if m.Prompt == nil {
243239
return nil, errors.New("acme/autocert: Manager.Prompt not set")
@@ -269,13 +265,10 @@ func (m *Manager) GetCertificate(hello *tls.ClientHelloInfo) (*tls.Certificate,
269265
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
270266
defer cancel()
271267

272-
// Check whether this is a token cert requested for TLS-SNI or TLS-ALPN challenge.
268+
// Check whether this is a token cert requested for TLS-ALPN challenge.
273269
if wantsTokenCert(hello) {
274270
m.tokensMu.RLock()
275271
defer m.tokensMu.RUnlock()
276-
// It's ok to use the same token cert key for both tls-sni and tls-alpn
277-
// because there's always at most 1 token cert per on-going domain authorization.
278-
// See m.verify for details.
279272
if cert := m.certTokens[name]; cert != nil {
280273
return cert, nil
281274
}
@@ -318,8 +311,7 @@ func wantsTokenCert(hello *tls.ClientHelloInfo) bool {
318311
if len(hello.SupportedProtos) == 1 && hello.SupportedProtos[0] == acme.ALPNProto {
319312
return true
320313
}
321-
// tls-sni-xx
322-
return strings.HasSuffix(hello.ServerName, ".acme.invalid")
314+
return false
323315
}
324316

325317
func supportsECDSA(hello *tls.ClientHelloInfo) bool {
@@ -688,7 +680,7 @@ func (m *Manager) revokePendingAuthz(ctx context.Context, uri []string) {
688680
func (m *Manager) verify(ctx context.Context, client *acme.Client, domain string) error {
689681
// The list of challenge types we'll try to fulfill
690682
// in this specific order.
691-
challengeTypes := []string{"tls-alpn-01", "tls-sni-02", "tls-sni-01"}
683+
challengeTypes := []string{"tls-alpn-01"}
692684
m.tokensMu.RLock()
693685
if m.tryHTTP01 {
694686
challengeTypes = append(challengeTypes, "http-01")
@@ -776,20 +768,6 @@ func (m *Manager) fulfill(ctx context.Context, client *acme.Client, chal *acme.C
776768
}
777769
m.putCertToken(ctx, domain, &cert)
778770
return func() { go m.deleteCertToken(domain) }, nil
779-
case "tls-sni-01":
780-
cert, name, err := client.TLSSNI01ChallengeCert(chal.Token)
781-
if err != nil {
782-
return nil, err
783-
}
784-
m.putCertToken(ctx, name, &cert)
785-
return func() { go m.deleteCertToken(name) }, nil
786-
case "tls-sni-02":
787-
cert, name, err := client.TLSSNI02ChallengeCert(chal.Token)
788-
if err != nil {
789-
return nil, err
790-
}
791-
m.putCertToken(ctx, name, &cert)
792-
return func() { go m.deleteCertToken(name) }, nil
793771
case "http-01":
794772
resp, err := client.HTTP01ChallengeResponse(chal.Token)
795773
if err != nil {

0 commit comments

Comments
 (0)