acme: return err from deprecated TLS-SNI-[01|02] functions

The TLSSNI01ChallengeCert and TLSSNI02ChallengeCert functions have been
marked deprecated since 2022. The package documentation indicates
pre-RFC 8555 functionality is retained for compilation success, but will
return errors. This commit makes these two deprecated functions match
that description.

No meaningful support for these draft standard challenge types exists
in the ACME ecosystem, and they are insecure for use in shared hosting
environments.

Change-Id: I1c17980a0630092c70eb971b3453a0f115834be0
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/676835
Auto-Submit: Daniel McCarney <daniel@binaryparadox.net>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>

2 files changed, 44 insertions, 144 deletions

diff --git a/acme/acme.go b/acme/acme.go
index 3902b94..6be00d1 100644
--- a/acme/acme.go
+++ b/acme/acme.go
@@ -31,7 +31,6 @@ import (
 	"crypto/x509/pkix"
 	"encoding/asn1"
 	"encoding/base64"
-	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -470,7 +469,7 @@ func (c *Client) WaitAuthorization(ctx context.Context, url string) (*Authorizat
 		// while waiting for a final authorization status.
 		d := retryAfter(res.Header.Get("Retry-After"))
 		if d == 0 {
-			// Given that the fastest challenges TLS-SNI and HTTP-01
+			// Given that the fastest challenges TLS-ALPN and HTTP-01
 			// require a CA to make at least 1 network round trip
 			// and most likely persist a challenge state,
 			// this default delay seems reasonable.
@@ -571,44 +570,21 @@ func (c *Client) HTTP01ChallengePath(token string) string {
 }
 
 // TLSSNI01ChallengeCert creates a certificate for TLS-SNI-01 challenge response.
+// Always returns an error.
 //
-// Deprecated: This challenge type is unused in both draft-02 and RFC versions of the ACME spec.
-func (c *Client) TLSSNI01ChallengeCert(token string, opt ...CertOption) (cert tls.Certificate, name string, err error) {
-	ka, err := keyAuth(c.Key.Public(), token)
-	if err != nil {
-		return tls.Certificate{}, "", err
-	}
-	b := sha256.Sum256([]byte(ka))
-	h := hex.EncodeToString(b[:])
-	name = fmt.Sprintf("%s.%s.acme.invalid", h[:32], h[32:])
-	cert, err = tlsChallengeCert([]string{name}, opt)
-	if err != nil {
-		return tls.Certificate{}, "", err
-	}
-	return cert, name, nil
+// Deprecated: This challenge type was only present in pre-standardized ACME
+// protocol drafts and is insecure for use in shared hosting environments.
+func (c *Client) TLSSNI01ChallengeCert(token string, opt ...CertOption) (tls.Certificate, string, error) {
+	return tls.Certificate{}, "", errPreRFC
 }
 
 // TLSSNI02ChallengeCert creates a certificate for TLS-SNI-02 challenge response.
+// Always returns an error.
 //
-// Deprecated: This challenge type is unused in both draft-02 and RFC versions of the ACME spec.
-func (c *Client) TLSSNI02ChallengeCert(token string, opt ...CertOption) (cert tls.Certificate, name string, err error) {
-	b := sha256.Sum256([]byte(token))
-	h := hex.EncodeToString(b[:])
-	sanA := fmt.Sprintf("%s.%s.token.acme.invalid", h[:32], h[32:])
-
-	ka, err := keyAuth(c.Key.Public(), token)
-	if err != nil {
-		return tls.Certificate{}, "", err
-	}
-	b = sha256.Sum256([]byte(ka))
-	h = hex.EncodeToString(b[:])
-	sanB := fmt.Sprintf("%s.%s.ka.acme.invalid", h[:32], h[32:])
-
-	cert, err = tlsChallengeCert([]string{sanA, sanB}, opt)
-	if err != nil {
-		return tls.Certificate{}, "", err
-	}
-	return cert, sanA, nil
+// Deprecated: This challenge type was only present in pre-standardized ACME
+// protocol drafts and is insecure for use in shared hosting environments.
+func (c *Client) TLSSNI02ChallengeCert(token string, opt ...CertOption) (tls.Certificate, string, error) {
+	return tls.Certificate{}, "", errPreRFC
 }
 
 // TLSALPN01ChallengeCert creates a certificate for TLS-ALPN-01 challenge response.
@@ -772,7 +748,7 @@ func defaultTLSChallengeCertTemplate() *x509.Certificate {
 	}
 }
 
-// tlsChallengeCert creates a temporary certificate for TLS-SNI challenges
+// tlsChallengeCert creates a temporary certificate for TLS-ALPN challenges
 // with the given SANs and auto-generated public/private key pair.
 // The Subject Common Name is set to the first SAN to aid debugging.
 // To create a cert with a custom key pair, specify WithKey option.
diff --git a/acme/acme_test.go b/acme/acme_test.go
index d286888..a3118f4 100644
--- a/acme/acme_test.go
+++ b/acme/acme_test.go
@@ -9,7 +9,6 @@ import (
 	"context"
 	"crypto/rand"
 	"crypto/rsa"
-	"crypto/tls"
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/base64"
@@ -22,8 +21,6 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"reflect"
-	"sort"
-	"strings"
 	"testing"
 	"time"
 )
@@ -692,71 +689,6 @@ func TestLinkHeader(t *testing.T) {
 	}
 }
 
-func TestTLSSNI01ChallengeCert(t *testing.T) {
-	const (
-		token = "evaGxfADs6pSRb2LAv9IZf17Dt3juxGJ-PCt92wr-oA"
-		// echo -n <token.testKeyECThumbprint> | shasum -a 256
-		san = "dbbd5eefe7b4d06eb9d1d9f5acb4c7cd.a27d320e4b30332f0b6cb441734ad7b0.acme.invalid"
-	)
-
-	tlscert, name, err := newTestClient().TLSSNI01ChallengeCert(token)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if n := len(tlscert.Certificate); n != 1 {
-		t.Fatalf("len(tlscert.Certificate) = %d; want 1", n)
-	}
-	cert, err := x509.ParseCertificate(tlscert.Certificate[0])
-	if err != nil {
-		t.Fatal(err)
-	}
-	if len(cert.DNSNames) != 1 || cert.DNSNames[0] != san {
-		t.Fatalf("cert.DNSNames = %v; want %q", cert.DNSNames, san)
-	}
-	if cert.DNSNames[0] != name {
-		t.Errorf("cert.DNSNames[0] != name: %q vs %q", cert.DNSNames[0], name)
-	}
-	if cn := cert.Subject.CommonName; cn != san {
-		t.Errorf("cert.Subject.CommonName = %q; want %q", cn, san)
-	}
-}
-
-func TestTLSSNI02ChallengeCert(t *testing.T) {
-	const (
-		token = "evaGxfADs6pSRb2LAv9IZf17Dt3juxGJ-PCt92wr-oA"
-		// echo -n evaGxfADs6pSRb2LAv9IZf17Dt3juxGJ-PCt92wr-oA | shasum -a 256
-		sanA = "7ea0aaa69214e71e02cebb18bb867736.09b730209baabf60e43d4999979ff139.token.acme.invalid"
-		// echo -n <token.testKeyECThumbprint> | shasum -a 256
-		sanB = "dbbd5eefe7b4d06eb9d1d9f5acb4c7cd.a27d320e4b30332f0b6cb441734ad7b0.ka.acme.invalid"
-	)
-
-	tlscert, name, err := newTestClient().TLSSNI02ChallengeCert(token)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if n := len(tlscert.Certificate); n != 1 {
-		t.Fatalf("len(tlscert.Certificate) = %d; want 1", n)
-	}
-	cert, err := x509.ParseCertificate(tlscert.Certificate[0])
-	if err != nil {
-		t.Fatal(err)
-	}
-	names := []string{sanA, sanB}
-	if !reflect.DeepEqual(cert.DNSNames, names) {
-		t.Fatalf("cert.DNSNames = %v;\nwant %v", cert.DNSNames, names)
-	}
-	sort.Strings(cert.DNSNames)
-	i := sort.SearchStrings(cert.DNSNames, name)
-	if i >= len(cert.DNSNames) || cert.DNSNames[i] != name {
-		t.Errorf("%v doesn't have %q", cert.DNSNames, name)
-	}
-	if cn := cert.Subject.CommonName; cn != sanA {
-		t.Errorf("CommonName = %q; want %q", cn, sanA)
-	}
-}
-
 func TestTLSALPN01ChallengeCert(t *testing.T) {
 	const (
 		token   = "evaGxfADs6pSRb2LAv9IZf17Dt3juxGJ-PCt92wr-oA"
@@ -813,6 +745,7 @@ func TestTLSChallengeCertOpt(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	domain := "example.com"
 	tmpl := &x509.Certificate{
 		SerialNumber: big.NewInt(2),
 		Subject:      pkix.Name{Organization: []string{"Test"}},
@@ -821,52 +754,43 @@ func TestTLSChallengeCertOpt(t *testing.T) {
 	opts := []CertOption{WithKey(key), WithTemplate(tmpl)}
 
 	client := newTestClient()
-	cert1, _, err := client.TLSSNI01ChallengeCert("token", opts...)
+	cert, err := client.TLSALPN01ChallengeCert("token", domain, opts...)
 	if err != nil {
 		t.Fatal(err)
 	}
-	cert2, _, err := client.TLSSNI02ChallengeCert("token", opts...)
+
+	// verify generated cert private key
+	tlskey, ok := cert.PrivateKey.(*rsa.PrivateKey)
+	if !ok {
+		t.Fatalf("tlscert.PrivateKey is %T; want *rsa.PrivateKey", cert.PrivateKey)
+	}
+	if tlskey.D.Cmp(key.D) != 0 {
+		t.Errorf("tlskey.D = %v; want %v", tlskey.D, key.D)
+	}
+	// verify generated cert public key
+	x509Cert, err := x509.ParseCertificate(cert.Certificate[0])
 	if err != nil {
 		t.Fatal(err)
 	}
-
-	for i, tlscert := range []tls.Certificate{cert1, cert2} {
-		// verify generated cert private key
-		tlskey, ok := tlscert.PrivateKey.(*rsa.PrivateKey)
-		if !ok {
-			t.Errorf("%d: tlscert.PrivateKey is %T; want *rsa.PrivateKey", i, tlscert.PrivateKey)
-			continue
-		}
-		if tlskey.D.Cmp(key.D) != 0 {
-			t.Errorf("%d: tlskey.D = %v; want %v", i, tlskey.D, key.D)
-		}
-		// verify generated cert public key
-		x509Cert, err := x509.ParseCertificate(tlscert.Certificate[0])
-		if err != nil {
-			t.Errorf("%d: %v", i, err)
-			continue
-		}
-		tlspub, ok := x509Cert.PublicKey.(*rsa.PublicKey)
-		if !ok {
-			t.Errorf("%d: x509Cert.PublicKey is %T; want *rsa.PublicKey", i, x509Cert.PublicKey)
-			continue
-		}
-		if tlspub.N.Cmp(key.N) != 0 {
-			t.Errorf("%d: tlspub.N = %v; want %v", i, tlspub.N, key.N)
-		}
-		// verify template option
-		sn := big.NewInt(2)
-		if x509Cert.SerialNumber.Cmp(sn) != 0 {
-			t.Errorf("%d: SerialNumber = %v; want %v", i, x509Cert.SerialNumber, sn)
-		}
-		org := []string{"Test"}
-		if !reflect.DeepEqual(x509Cert.Subject.Organization, org) {
-			t.Errorf("%d: Subject.Organization = %+v; want %+v", i, x509Cert.Subject.Organization, org)
-		}
-		for _, v := range x509Cert.DNSNames {
-			if !strings.HasSuffix(v, ".acme.invalid") {
-				t.Errorf("%d: invalid DNSNames element: %q", i, v)
-			}
+	tlspub, ok := x509Cert.PublicKey.(*rsa.PublicKey)
+	if !ok {
+		t.Fatalf("x509Cert.PublicKey is %T; want *rsa.PublicKey", x509Cert.PublicKey)
+	}
+	if tlspub.N.Cmp(key.N) != 0 {
+		t.Errorf("tlspub.N = %v; want %v", tlspub.N, key.N)
+	}
+	// verify template option
+	sn := big.NewInt(2)
+	if x509Cert.SerialNumber.Cmp(sn) != 0 {
+		t.Errorf("SerialNumber = %v; want %v", x509Cert.SerialNumber, sn)
+	}
+	org := []string{"Test"}
+	if !reflect.DeepEqual(x509Cert.Subject.Organization, org) {
+		t.Errorf("Subject.Organization = %+v; want %+v", x509Cert.Subject.Organization, org)
+	}
+	for _, v := range x509Cert.DNSNames {
+		if v != domain {
+			t.Errorf("invalid DNSNames element: %q", v)
 		}
 	}
 }