Implement TLS-SNI-02 challenge validations. (letsencrypt#2585)

calavera · jsha · commit c71c3cff80b3 · 2017-03-22T10:17:59.000-07:00
I think these are all the necessary changes to implement TLS-SNI-02 validations, according to the section 7.3 of draft 05: https://tools.ietf.org/html/draft-ietf-acme-acme-05#section-7.3 I don't have much experience with this code, I'll really appreciate your feedback. Signed-off-by: David Calavera <david.calavera@gmail.com>
diff --git a/core/challenges.go b/core/challenges.go
@@ -13,12 +13,17 @@ func HTTPChallenge01() Challenge {
 	return newChallenge(ChallengeTypeHTTP01)
 }
 
-// TLSSNIChallenge01 constructs a random tls-sni-00 challenge
+// TLSSNIChallenge01 constructs a random tls-sni-01 challenge
 func TLSSNIChallenge01() Challenge {
 	return newChallenge(ChallengeTypeTLSSNI01)
 }
 
-// DNSChallenge01 constructs a random DNS challenge
+// TLSSNIChallenge02 constructs a random tls-sni-02 challenge
+func TLSSNIChallenge02() Challenge {
+	return newChallenge(ChallengeTypeTLSSNI02)
+}
+
+// DNSChallenge01 constructs a random dns-01 challenge
 func DNSChallenge01() Challenge {
 	return newChallenge(ChallengeTypeDNS01)
 }
diff --git a/core/core_test.go b/core/core_test.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"testing"
 
+	"github.com/letsencrypt/boulder/features"
 	"github.com/letsencrypt/boulder/test"
 	"gopkg.in/square/go-jose.v1"
 )
@@ -34,6 +35,11 @@ func TestChallenges(t *testing.T) {
 		t.Errorf("New tls-sni-01 challenge is not sane: %v", tlssni01)
 	}
 
+	tlssni02 := TLSSNIChallenge02()
+	if !tlssni02.IsSane(false) {
+		t.Errorf("New tls-sni-02 challenge is not sane: %v", tlssni02)
+	}
+
 	dns01 := DNSChallenge01()
 	if !dns01.IsSane(false) {
 		t.Errorf("New dns-01 challenge is not sane: %v", dns01)
@@ -43,6 +49,13 @@ func TestChallenges(t *testing.T) {
 	test.Assert(t, ValidChallenge(ChallengeTypeTLSSNI01), "Refused valid challenge")
 	test.Assert(t, ValidChallenge(ChallengeTypeDNS01), "Refused valid challenge")
 	test.Assert(t, !ValidChallenge("nonsense-71"), "Accepted invalid challenge")
+
+	test.Assert(t, !ValidChallenge(ChallengeTypeTLSSNI02), "Accepted invalid challenge")
+
+	_ = features.Set(map[string]bool{"AllowTLS02Challenges": true})
+	defer features.Reset()
+
+	test.Assert(t, ValidChallenge(ChallengeTypeTLSSNI02), "Refused valid challenge")
 }
 
 // objects.go
diff --git a/core/objects.go b/core/objects.go
@@ -12,6 +12,7 @@ import (
 
 	"gopkg.in/square/go-jose.v1"
 
+	"github.com/letsencrypt/boulder/features"
 	"github.com/letsencrypt/boulder/probs"
 	"github.com/letsencrypt/boulder/revocation"
 )
@@ -69,6 +70,7 @@ const (
 const (
 	ChallengeTypeHTTP01   = "http-01"
 	ChallengeTypeTLSSNI01 = "tls-sni-01"
+	ChallengeTypeTLSSNI02 = "tls-sni-02"
 	ChallengeTypeDNS01    = "dns-01"
 )
 
@@ -81,6 +83,8 @@ func ValidChallenge(name string) bool {
 		fallthrough
 	case ChallengeTypeDNS01:
 		return true
+	case ChallengeTypeTLSSNI02:
+		return features.Enabled(features.AllowTLS02Challenges)
 
 	default:
 		return false
@@ -261,6 +265,8 @@ func (ch Challenge) RecordsSane() bool {
 			}
 		}
 	case ChallengeTypeTLSSNI01:
+		fallthrough
+	case ChallengeTypeTLSSNI02:
 		if len(ch.ValidationRecord) > 1 {
 			return false
 		}
diff --git a/core/objects_test.go b/core/objects_test.go
@@ -57,7 +57,7 @@ func TestChallengeSanityCheck(t *testing.T) {
   }`), &accountKey)
 	test.AssertNotError(t, err, "Error unmarshaling JWK")
 
-	types := []string{ChallengeTypeHTTP01, ChallengeTypeTLSSNI01, ChallengeTypeDNS01}
+	types := []string{ChallengeTypeHTTP01, ChallengeTypeTLSSNI01, ChallengeTypeTLSSNI02, ChallengeTypeDNS01}
 	for _, challengeType := range types {
 		chall := Challenge{
 			Type:   challengeType,
diff --git a/features/featureflag_string.go b/features/featureflag_string.go
diff --git a/features/features.go b/features/features.go
@@ -18,6 +18,7 @@ const (
 	ResubmitMissingSCTsOnly
 	GoogleSafeBrowsingV4
 	UseAIAIssuerURL
+	AllowTLS02Challenges
 )
 
 // List of features and their default value, protected by fMu
@@ -29,6 +30,7 @@ var features = map[FeatureFlag]bool{
 	ResubmitMissingSCTsOnly:  false,
 	GoogleSafeBrowsingV4:     false,
 	UseAIAIssuerURL:          false,
+	AllowTLS02Challenges:     false,
 }
 
 var fMu = new(sync.RWMutex)
diff --git a/policy/pa.go b/policy/pa.go
@@ -286,6 +286,10 @@ func (pa *AuthorityImpl) ChallengesFor(identifier core.AcmeIdentifier) ([]core.C
 		challenges = append(challenges, core.TLSSNIChallenge01())
 	}
 
+	if features.Enabled(features.AllowTLS02Challenges) && pa.enabledChallenges[core.ChallengeTypeTLSSNI02] {
+		challenges = append(challenges, core.TLSSNIChallenge02())
+	}
+
 	if pa.enabledChallenges[core.ChallengeTypeDNS01] {
 		challenges = append(challenges, core.DNSChallenge01())
 	}
diff --git a/test/config-next/ca.json b/test/config-next/ca.json
@@ -134,7 +134,8 @@
       "serviceQueue": "CA.server"
     },
     "features": {
-        "IDNASupport": true
+        "IDNASupport": true,
+        "AllowTLS02Challenges": true
     }
   },
 
diff --git a/test/config-next/cert-checker.json b/test/config-next/cert-checker.json
@@ -3,7 +3,8 @@
     "dbConnectFile": "test/secrets/cert_checker_dburl",
     "maxDBConns": 10,
     "features": {
-      "IDNASupport": true
+      "IDNASupport": true,
+      "AllowTLS02Challenges": true
     },
     "hostnamePolicyFile": "test/hostname-policy.json"
   },
@@ -12,7 +13,8 @@
     "challenges": {
       "http-01": true,
       "tls-sni-01": true,
-      "dns-01": true
+      "dns-01": true,
+      "tls-sni-02": true
     }
   },
 
diff --git a/test/config-next/ra.json b/test/config-next/ra.json
@@ -46,7 +46,8 @@
     },
     "features": {
       "IDNASupport": true,
-      "AllowKeyRollover": true
+      "AllowKeyRollover": true,
+      "AllowTLS02Challenges": true
     }
   },
 
diff --git a/va/va.go b/va/va.go
@@ -303,7 +303,7 @@ func certNames(cert *x509.Certificate) []string {
 	return names
 }
 
-func (va *ValidationAuthorityImpl) validateTLSWithZName(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge, zName string) ([]core.ValidationRecord, *probs.ProblemDetails) {
+func (va *ValidationAuthorityImpl) validateTLSSNI01WithZName(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge, zName string) ([]core.ValidationRecord, *probs.ProblemDetails) {
 	addr, allAddrs, problem := va.getAddr(ctx, identifier.Value)
 	validationRecords := []core.ValidationRecord{
 		{
@@ -320,16 +320,96 @@ func (va *ValidationAuthorityImpl) validateTLSWithZName(ctx context.Context, ide
 	portString := strconv.Itoa(va.tlsPort)
 	hostPort := net.JoinHostPort(addr.String(), portString)
 	validationRecords[0].Port = portString
+
+	certs, problem := va.getTLSSNICerts(hostPort, identifier, challenge, zName)
+	if problem != nil {
+		return validationRecords, problem
+	}
+
+	leafCert := certs[0]
+	for _, name := range leafCert.DNSNames {
+		if subtle.ConstantTimeCompare([]byte(name), []byte(zName)) == 1 {
+			return validationRecords, nil
+		}
+	}
+
+	names := certNames(leafCert)
+	errText := fmt.Sprintf(
+		"Incorrect validation certificate for %s challenge. "+
+			"Requested %s from %s. Received %d certificate(s), "+
+			"first certificate had names %q",
+		challenge.Type, zName, hostPort, len(certs), strings.Join(names, ", "))
+	va.log.Info(fmt.Sprintf("Remote host failed to give %s challenge name. host: %s", challenge.Type, identifier))
+	return validationRecords, probs.Unauthorized(errText)
+}
+
+func (va *ValidationAuthorityImpl) validateTLSSNI02WithZNames(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge, sanAName, sanBName string) ([]core.ValidationRecord, *probs.ProblemDetails) {
+	addr, allAddrs, problem := va.getAddr(ctx, identifier.Value)
+	validationRecords := []core.ValidationRecord{
+		{
+			Hostname:          identifier.Value,
+			AddressesResolved: allAddrs,
+			AddressUsed:       addr,
+		},
+	}
+	if problem != nil {
+		return validationRecords, problem
+	}
+
+	// Make a connection with SNI = nonceName
+	portString := strconv.Itoa(va.tlsPort)
+	hostPort := net.JoinHostPort(addr.String(), portString)
+	validationRecords[0].Port = portString
+
+	certs, problem := va.getTLSSNICerts(hostPort, identifier, challenge, sanAName)
+	if problem != nil {
+		return validationRecords, problem
+	}
+
+	leafCert := certs[0]
+	if len(leafCert.DNSNames) != 2 {
+		names := strings.Join(certNames(leafCert), ", ")
+		msg := fmt.Sprintf("%s challenge certificate doesn't include exactly 2 DNSName entries. Received %d certificate(s), first certificate had names %q", challenge.Type, len(certs), names)
+		return validationRecords, probs.Malformed(msg)
+	}
+
+	var validSanAName, validSanBName bool
+	for _, name := range leafCert.DNSNames {
+		// Note: ConstantTimeCompare is not strictly necessary here, but can't hurt.
+		if subtle.ConstantTimeCompare([]byte(name), []byte(sanAName)) == 1 {
+			validSanAName = true
+		}
+
+		if subtle.ConstantTimeCompare([]byte(name), []byte(sanBName)) == 1 {
+			validSanBName = true
+		}
+	}
+
+	if validSanAName && validSanBName {
+		return validationRecords, nil
+	}
+
+	names := certNames(leafCert)
+	errText := fmt.Sprintf(
+		"Incorrect validation certificate for %s challenge. "+
+			"Requested %s from %s. Received %d certificate(s), "+
+			"first certificate had names %q",
+		challenge.Type, sanAName, hostPort, len(certs), strings.Join(names, ", "))
+	va.log.Info(fmt.Sprintf("Remote host failed to give %s challenge name. host: %s", challenge.Type, identifier))
+	return validationRecords, probs.Unauthorized(errText)
+}
+
+func (va *ValidationAuthorityImpl) getTLSSNICerts(hostPort string, identifier core.AcmeIdentifier, challenge core.Challenge, zName string) ([]*x509.Certificate, *probs.ProblemDetails) {
 	va.log.Info(fmt.Sprintf("%s [%s] Attempting to validate for %s %s", challenge.Type, identifier, hostPort, zName))
 	conn, err := tls.DialWithDialer(&net.Dialer{Timeout: validationTimeout}, "tcp", hostPort, &tls.Config{
 		ServerName:         zName,
 		InsecureSkipVerify: true,
 	})
 
 	if err != nil {
-		va.log.Info(fmt.Sprintf("TLS-01 connection failure for %s. err=[%#v] errStr=[%s]", identifier, err, err))
-		return validationRecords,
-			parseHTTPConnError(fmt.Sprintf("Failed to connect to %s for TLS-SNI-01 challenge", hostPort), err)
+		va.log.Info(fmt.Sprintf("%s connection failure for %s. err=[%#v] errStr=[%s]", challenge.Type, identifier, err, err))
+		return nil,
+			parseHTTPConnError(fmt.Sprintf("Failed to connect to %s for %s challenge", hostPort, challenge.Type), err)
 	}
 	// close errors are not important here
 	defer func() {
@@ -339,28 +419,14 @@ func (va *ValidationAuthorityImpl) validateTLSWithZName(ctx context.Context, ide
 	// Check that zName is a dNSName SAN in the server's certificate
 	certs := conn.ConnectionState().PeerCertificates
 	if len(certs) == 0 {
-		va.log.Info(fmt.Sprintf("TLS-SNI-01 challenge for %s resulted in no certificates", identifier.Value))
-		return validationRecords, probs.Unauthorized("No certs presented for TLS SNI challenge")
+		va.log.Info(fmt.Sprintf("%s challenge for %s resulted in no certificates", challenge.Type, identifier.Value))
+		return nil, probs.Unauthorized(fmt.Sprintf("No certs presented for %s challenge", challenge.Type))
 	}
 	for i, cert := range certs {
-		va.log.AuditInfo(fmt.Sprintf("TLS-SNI-01 challenge for %s received certificate (%d of %d): cert=[%s]",
-			identifier.Value, i+1, len(certs), hex.EncodeToString(cert.Raw)))
-	}
-	leafCert := certs[0]
-	for _, name := range leafCert.DNSNames {
-		if subtle.ConstantTimeCompare([]byte(name), []byte(zName)) == 1 {
-			return validationRecords, nil
-		}
+		va.log.AuditInfo(fmt.Sprintf("%s challenge for %s received certificate (%d of %d): cert=[%s]",
+			challenge.Type, identifier.Value, i+1, len(certs), hex.EncodeToString(cert.Raw)))
 	}
-
-	names := certNames(leafCert)
-	errText := fmt.Sprintf(
-		"Incorrect validation certificate for TLS-SNI-01 challenge. "+
-			"Requested %s from %s. Received %d certificate(s), "+
-			"first certificate had names %q",
-		zName, hostPort, len(certs), strings.Join(names, ", "))
-	va.log.Info(fmt.Sprintf("Remote host failed to give TLS-01 challenge name. host: %s", identifier))
-	return validationRecords, probs.Unauthorized(errText)
+	return certs, nil
 }
 
 func (va *ValidationAuthorityImpl) validateHTTP01(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) {
@@ -390,17 +456,38 @@ func (va *ValidationAuthorityImpl) validateHTTP01(ctx context.Context, identifie
 
 func (va *ValidationAuthorityImpl) validateTLSSNI01(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) {
 	if identifier.Type != "dns" {
-		va.log.Info(fmt.Sprintf("Identifier type for TLS-SNI was not DNS: %s", identifier))
-		return nil, probs.Malformed("Identifier type for TLS-SNI was not DNS")
+		va.log.Info(fmt.Sprintf("Identifier type for TLS-SNI-01 was not DNS: %s", identifier))
+		return nil, probs.Malformed("Identifier type for TLS-SNI-01 was not DNS")
 	}
 
 	// Compute the digest that will appear in the certificate
-	h := sha256.New()
-	h.Write([]byte(challenge.ProvidedKeyAuthorization))
-	Z := hex.EncodeToString(h.Sum(nil))
+	h := sha256.Sum256([]byte(challenge.ProvidedKeyAuthorization))
+	Z := hex.EncodeToString(h[:])
 	ZName := fmt.Sprintf("%s.%s.%s", Z[:32], Z[32:], core.TLSSNISuffix)
 
-	return va.validateTLSWithZName(ctx, identifier, challenge, ZName)
+	return va.validateTLSSNI01WithZName(ctx, identifier, challenge, ZName)
+}
+
+func (va *ValidationAuthorityImpl) validateTLSSNI02(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) {
+	if identifier.Type != "dns" {
+		va.log.Info(fmt.Sprintf("Identifier type for TLS-SNI-02 was not DNS: %s", identifier))
+		return nil, probs.Malformed("Identifier type for TLS-SNI-02 was not DNS")
+	}
+
+	const tlsSNITokenID = "token"
+	const tlsSNIKaID = "ka"
+
+	// Compute the digest for the SAN b that will appear in the certificate
+	ha := sha256.Sum256([]byte(challenge.Token))
+	za := hex.EncodeToString(ha[:])
+	sanAName := fmt.Sprintf("%s.%s.%s.%s", za[:32], za[32:], tlsSNITokenID, core.TLSSNISuffix)
+
+	// Compute the digest for the SAN B that will appear in the certificate
+	hb := sha256.Sum256([]byte(challenge.ProvidedKeyAuthorization))
+	zb := hex.EncodeToString(hb[:])
+	sanBName := fmt.Sprintf("%s.%s.%s.%s", zb[:32], zb[32:], tlsSNIKaID, core.TLSSNISuffix)
+
+	return va.validateTLSSNI02WithZNames(ctx, identifier, challenge, sanAName, sanBName)
 }
 
 // badTLSHeader contains the string 'HTTP /' which is returned when
@@ -549,6 +636,8 @@ func (va *ValidationAuthorityImpl) validateChallenge(ctx context.Context, identi
 		return va.validateHTTP01(ctx, identifier, challenge)
 	case core.ChallengeTypeTLSSNI01:
 		return va.validateTLSSNI01(ctx, identifier, challenge)
+	case core.ChallengeTypeTLSSNI02:
+		return va.validateTLSSNI02(ctx, identifier, challenge)
 	case core.ChallengeTypeDNS01:
 		return va.validateDNS01(ctx, identifier, challenge)
 	}
diff --git a/va/va_test.go b/va/va_test.go

Original file line number	Diff line number	Diff line change
`@@ -13,12 +13,17 @@ func HTTPChallenge01() Challenge {`
`13`	`13`	`return newChallenge(ChallengeTypeHTTP01)`
`14`	`14`	`}`
`15`	`15`
`16`		`-// TLSSNIChallenge01 constructs a random tls-sni-00 challenge`
	`16`	`+// TLSSNIChallenge01 constructs a random tls-sni-01 challenge`
`17`	`17`	`func TLSSNIChallenge01() Challenge {`
`18`	`18`	`return newChallenge(ChallengeTypeTLSSNI01)`
`19`	`19`	`}`
`20`	`20`
`21`		`-// DNSChallenge01 constructs a random DNS challenge`
	`21`	`+// TLSSNIChallenge02 constructs a random tls-sni-02 challenge`
	`22`	`+func TLSSNIChallenge02() Challenge {`
	`23`	`+ return newChallenge(ChallengeTypeTLSSNI02)`
	`24`	`+}`
	`25`	`+`
	`26`	`+// DNSChallenge01 constructs a random dns-01 challenge`
`22`	`27`	`func DNSChallenge01() Challenge {`
`23`	`28`	`return newChallenge(ChallengeTypeDNS01)`
`24`	`29`	`}`
Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,7 @@ import (`
`12`	`12`
`13`	`13`	`"gopkg.in/square/go-jose.v1"`
`14`	`14`
	`15`	`+ "github.com/letsencrypt/boulder/features"`
`15`	`16`	`"github.com/letsencrypt/boulder/probs"`
`16`	`17`	`"github.com/letsencrypt/boulder/revocation"`
`17`	`18`	`)`
`@@ -69,6 +70,7 @@ const (`
`69`	`70`	`const (`
`70`	`71`	`ChallengeTypeHTTP01 = "http-01"`
`71`	`72`	`ChallengeTypeTLSSNI01 = "tls-sni-01"`
	`73`	`+ ChallengeTypeTLSSNI02 = "tls-sni-02"`
`72`	`74`	`ChallengeTypeDNS01 = "dns-01"`
`73`	`75`	`)`
`74`	`76`
`@@ -81,6 +83,8 @@ func ValidChallenge(name string) bool {`
`81`	`83`	`fallthrough`
`82`	`84`	`case ChallengeTypeDNS01:`
`83`	`85`	`return true`
	`86`	`+ case ChallengeTypeTLSSNI02:`
	`87`	`+ return features.Enabled(features.AllowTLS02Challenges)`
`84`	`88`
`85`	`89`	`default:`
`86`	`90`	`return false`
`@@ -261,6 +265,8 @@ func (ch Challenge) RecordsSane() bool {`
`261`	`265`	`}`
`262`	`266`	`}`
`263`	`267`	`case ChallengeTypeTLSSNI01:`
	`268`	`+ fallthrough`
	`269`	`+ case ChallengeTypeTLSSNI02:`
`264`	`270`	`if len(ch.ValidationRecord) > 1 {`
`265`	`271`	`return false`
`266`	`272`	`}`
Original file line number	Diff line number	Diff line change
`@@ -18,6 +18,7 @@ const (`
`18`	`18`	`ResubmitMissingSCTsOnly`
`19`	`19`	`GoogleSafeBrowsingV4`
`20`	`20`	`UseAIAIssuerURL`
	`21`	`+ AllowTLS02Challenges`
`21`	`22`	`)`
`22`	`23`
`23`	`24`	`// List of features and their default value, protected by fMu`
`@@ -29,6 +30,7 @@ var features = map[FeatureFlag]bool{`
`29`	`30`	`ResubmitMissingSCTsOnly: false,`
`30`	`31`	`GoogleSafeBrowsingV4: false,`
`31`	`32`	`UseAIAIssuerURL: false,`
	`33`	`+ AllowTLS02Challenges: false,`
`32`	`34`	`}`
`33`	`35`
`34`	`36`	`var fMu = new(sync.RWMutex)`
Original file line number	Diff line number	Diff line change
`@@ -286,6 +286,10 @@ func (pa *AuthorityImpl) ChallengesFor(identifier core.AcmeIdentifier) ([]core.C`
`286`	`286`	`challenges = append(challenges, core.TLSSNIChallenge01())`
`287`	`287`	`}`
`288`	`288`
	`289`	`+ if features.Enabled(features.AllowTLS02Challenges) && pa.enabledChallenges[core.ChallengeTypeTLSSNI02] {`
	`290`	`+ challenges = append(challenges, core.TLSSNIChallenge02())`
	`291`	`+ }`
	`292`	`+`
`289`	`293`	`if pa.enabledChallenges[core.ChallengeTypeDNS01] {`
`290`	`294`	`challenges = append(challenges, core.DNSChallenge01())`
`291`	`295`	`}`
Original file line number	Diff line number	Diff line change
`@@ -134,7 +134,8 @@`
`134`	`134`	`"serviceQueue": "CA.server"`
`135`	`135`	`},`
`136`	`136`	`"features": {`
`137`		`- "IDNASupport": true`
	`137`	`+ "IDNASupport": true,`
	`138`	`+ "AllowTLS02Challenges": true`
`138`	`139`	`}`
`139`	`140`	`},`
`140`	`141`
Original file line number	Diff line number	Diff line change
`@@ -46,7 +46,8 @@`
`46`	`46`	`},`
`47`	`47`	`"features": {`
`48`	`48`	`"IDNASupport": true,`
`49`		`- "AllowKeyRollover": true`
	`49`	`+ "AllowKeyRollover": true,`
	`50`	`+ "AllowTLS02Challenges": true`
`50`	`51`	`}`
`51`	`52`	`},`
`52`	`53`