Remove TLS-SNI-01 (#4114)

* Remove the challenge whitelist
* Reduce the signature for ChallengesFor and ChallengeTypeEnabled
* Some unit tests in the VA were changed from testing TLS-SNI to testing the same behavior
  in TLS-ALPN, when that behavior wasn't already tested. For instance timeouts during connect 
  are now tested.

Fixes #4109

README.md

@@ -104,7 +104,7 @@ Alternatively, you can override the docker-compose.yml default with an environme
 
     docker-compose run --use-aliases -e FAKE_DNS=172.17.0.1 --service-ports boulder ./start.py
 
-Boulder's default VA configuration (`test/config/va.json`) is configured to connect to port 5002 to validate HTTP-01 challenges and port 5001 to validate TLS-SNI-01/TLS-ALPN-01 challenges. If you want to solve challenges with a client running on your host you should make sure it uses these ports to respond to validation requests, or update the VA configuration's `portConfig` to use ports 80 and 443 to match how the VA operates in production and staging environments. If you use a host-based firewall (e.g. `ufw` or `iptables`) make sure you allow connections from the Docker instance to your host on the required ports.
+Boulder's default VA configuration (`test/config/va.json`) is configured to connect to port 5002 to validate HTTP-01 challenges and port 5001 to validate TLS-ALPN-01 challenges. If you want to solve challenges with a client running on your host you should make sure it uses these ports to respond to validation requests, or update the VA configuration's `portConfig` to use ports 80 and 443 to match how the VA operates in production and staging environments. If you use a host-based firewall (e.g. `ufw` or `iptables`) make sure you allow connections from the Docker instance to your host on the required ports.
 
 If a base image changes (i.e. `letsencrypt/boulder-tools`) you will need to rebuild images for both the boulder and bhsm containers and re-create them. The quickest way to do this is with this command:
 

cmd/boulder-ra/main.go

@@ -133,13 +133,6 @@ func main() {
 	err = pa.SetHostnamePolicyFile(c.RA.HostnamePolicyFile)
 	cmd.FailOnError(err, "Couldn't load hostname policy file")
 
-	if c.PA.ChallengesWhitelistFile != "" {
-		err = pa.SetChallengesWhitelistFile(c.PA.ChallengesWhitelistFile)
-		cmd.FailOnError(err, "Couldn't load challenges whitelist file")
-	} else {
-		logger.Info("No challengesWhitelistFile given, not loading")
-	}
-
 	if features.Enabled(features.RevokeAtRA) && (c.RA.AkamaiPurgerService == nil || c.RA.IssuerCertPath == "") {
 		cmd.Fail("If the RevokeAtRA feature is enabled the AkamaiPurgerService and IssuerCertPath config fields must be populated")
 	}

cmd/config.go

@@ -75,9 +75,7 @@ type SMTPConfig struct {
 // it should offer.
 type PAConfig struct {
 	DBConfig
-	EnforcePolicyWhitelist  bool
 	Challenges map[string]bool
-	ChallengesWhitelistFile string
 }
 
 // HostnamePolicyConfig specifies a file from which to load a policy regarding

core/challenges.go

@@ -17,12 +17,6 @@ func HTTPChallenge01(token string) Challenge {
 	return newChallenge(ChallengeTypeHTTP01, token)
 }
 
-// TLSSNIChallenge01 constructs a random tls-sni-01 challenge. If token is empty a random token
-// will be generated, otherwise the provided token is used.
-func TLSSNIChallenge01(token string) Challenge {
-	return newChallenge(ChallengeTypeTLSSNI01, token)
-}
-
 // DNSChallenge01 constructs a random dns-01 challenge. If token is empty a random token
 // will be generated, otherwise the provided token is used.
 func DNSChallenge01(token string) Challenge {

core/core_test.go

@@ -27,9 +27,6 @@ func TestChallenges(t *testing.T) {
 	http01 := HTTPChallenge01("")
 	test.AssertNotError(t, http01.CheckConsistencyForClientOffer(), "CheckConsistencyForClientOffer returned an error")
 
-	tlssni01 := TLSSNIChallenge01("")
-	test.AssertNotError(t, tlssni01.CheckConsistencyForClientOffer(), "CheckConsistencyForClientOffer returned an error")
-
 	dns01 := DNSChallenge01("")
 	test.AssertNotError(t, dns01.CheckConsistencyForClientOffer(), "CheckConsistencyForClientOffer returned an error")
 
@@ -37,7 +34,6 @@ func TestChallenges(t *testing.T) {
 	test.AssertNotError(t, tlsalpn01.CheckConsistencyForClientOffer(), "CheckConsistencyForClientOffer returned an error")
 
 	test.Assert(t, ValidChallenge(ChallengeTypeHTTP01), "Refused valid challenge")
-	test.Assert(t, ValidChallenge(ChallengeTypeTLSSNI01), "Refused valid challenge")
 	test.Assert(t, ValidChallenge(ChallengeTypeDNS01), "Refused valid challenge")
 	test.Assert(t, ValidChallenge(ChallengeTypeTLSALPN01), "Refused valid challenge")
 	test.Assert(t, !ValidChallenge("nonsense-71"), "Accepted invalid challenge")

core/interfaces.go

@@ -108,8 +108,8 @@ type CertificateAuthority interface {
 type PolicyAuthority interface {
 	WillingToIssue(domain AcmeIdentifier) error
 	WillingToIssueWildcard(domain AcmeIdentifier) error
-	ChallengesFor(domain AcmeIdentifier, registrationID int64, revalidation bool) (challenges []Challenge, validCombinations [][]int, err error)
+	ChallengesFor(domain AcmeIdentifier) (challenges []Challenge, validCombinations [][]int, err error)
-	ChallengeTypeEnabled(t string, registrationID int64) bool
+	ChallengeTypeEnabled(t string) bool
 }
 
 // StorageGetter are the Boulder SA's read-only methods

core/objects.go

@@ -70,7 +70,6 @@ const (
 // These types are the available challenges
 const (
 	ChallengeTypeHTTP01    = "http-01"
-	ChallengeTypeTLSSNI01  = "tls-sni-01"
 	ChallengeTypeDNS01     = "dns-01"
 	ChallengeTypeTLSALPN01 = "tls-alpn-01"
 )
@@ -79,7 +78,6 @@ const (
 func ValidChallenge(name string) bool {
 	switch name {
 	case ChallengeTypeHTTP01,
-		ChallengeTypeTLSSNI01,
 		ChallengeTypeDNS01,
 		ChallengeTypeTLSALPN01:
 		return true
@@ -88,9 +86,6 @@ func ValidChallenge(name string) bool {
 	}
 }
 
-// TLSSNISuffix is appended to pseudo-domain names in DVSNI challenges
-const TLSSNISuffix = "acme.invalid"
-
 // DNSPrefix is attached to DNS names in DNS challenges
 const DNSPrefix = "_acme-challenge"
 
@@ -284,7 +279,7 @@ func (ch Challenge) RecordsSane() bool {
 				return false
 			}
 		}
-	case ChallengeTypeTLSSNI01, ChallengeTypeTLSALPN01:
+	case ChallengeTypeTLSALPN01:
 		if len(ch.ValidationRecord) > 1 {
 			return false
 		}

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, ChallengeTypeTLSALPN01}
+	types := []string{ChallengeTypeHTTP01, ChallengeTypeDNS01, ChallengeTypeTLSALPN01}
 	for _, challengeType := range types {
 		chall := Challenge{
 			Type:   challengeType,

csr/csr_test.go

@@ -23,7 +23,7 @@ var testingPolicy = &goodkey.KeyPolicy{
 
 type mockPA struct{}
 
-func (pa *mockPA) ChallengesFor(identifier core.AcmeIdentifier, registrationID int64, revalidation bool) (challenges []core.Challenge, combinations [][]int, err error) {
+func (pa *mockPA) ChallengesFor(identifier core.AcmeIdentifier) (challenges []core.Challenge, combinations [][]int, err error) {
 	return
 }
 
@@ -38,7 +38,7 @@ func (pa *mockPA) WillingToIssueWildcard(id core.AcmeIdentifier) error {
 	return nil
 }
 
-func (pa *mockPA) ChallengeTypeEnabled(t string, registrationID int64) bool {
+func (pa *mockPA) ChallengeTypeEnabled(t string) bool {
 	return true
 }
 

docs/acme-divergences.md

@@ -108,10 +108,6 @@ Boulder uses an HTTP status code 202 (Accepted) response for correct challenge r
 
 Boulder does not implement the ability to retry challenges or the `Retry-After` header.
 
-## [Section 8.4](https://tools.ietf.org/html/draft-ietf-acme-acme-07#section-8.4)
-
-Boulder implements `tls-sni-01` from [draft-ietf-acme-01 Section 7.3](https://tools.ietf.org/html/draft-ietf-acme-acme-01#section-7.3) instead of the `tls-sni-02` validation method.
-
 ## [Section 8.6](https://tools.ietf.org/html/draft-ietf-acme-acme-07#section-8.6)
 
 Boulder does not implement the `oob-01` validation method.

features/featureflag_string.go

@@ -4,9 +4,9 @@ package features
 
 import "strconv"
 
-const _FeatureFlag_name = "unusedPerformValidationRPCACME13KeyRolloverSimplifiedVAHTTPAllowRenewalFirstRLTLSSNIRevalidationCAAValidationMethodsCAAAccountURIProbeCTLogsHeadNonceStatusOKNewAuthorizationSchemaRevokeAtRASetIssuedNamesRenewalBitEarlyOrderRateLimit"
+const _FeatureFlag_name = "unusedPerformValidationRPCACME13KeyRolloverSimplifiedVAHTTPTLSSNIRevalidationAllowRenewalFirstRLCAAValidationMethodsCAAAccountURIProbeCTLogsHeadNonceStatusOKNewAuthorizationSchemaRevokeAtRASetIssuedNamesRenewalBitEarlyOrderRateLimit"
 
-var _FeatureFlag_index = [...]uint8{0, 6, 26, 43, 59, 78, 96, 116, 129, 140, 157, 179, 189, 213, 232}
+var _FeatureFlag_index = [...]uint8{0, 6, 26, 43, 59, 77, 96, 116, 129, 140, 157, 179, 189, 213, 232}
 
 func (i FeatureFlag) String() string {
 	if i < 0 || i >= FeatureFlag(len(_FeatureFlag_index)-1) {

features/features.go

@@ -15,11 +15,10 @@ const (
 	PerformValidationRPC
 	ACME13KeyRollover
 	SimplifiedVAHTTP
+	TLSSNIRevalidation
 
 	//   Currently in-use features
 	AllowRenewalFirstRL
-	// Allow TLS-SNI in new-authz that are revalidating for previous issuance
-	TLSSNIRevalidation
 	// Check CAA and respect validationmethods parameter.
 	CAAValidationMethods
 	// Check CAA and respect accounturi parameter.

policy/pa.go

@@ -32,7 +32,6 @@ type AuthorityImpl struct {
 	blacklistMu            sync.RWMutex
 
 	enabledChallenges map[string]bool
-	enabledChallengesWhitelist map[string]map[int64]bool
 	pseudoRNG         *rand.Rand
 	rngMu             sync.Mutex
 }
@@ -111,42 +110,6 @@ func (pa *AuthorityImpl) loadHostnamePolicy(b []byte) error {
 	return nil
 }
 
-// SetChallengesWhitelistFile will load the given whitelist file, returning error if it
-// fails. It will also start a reloader in case the file changes.
-func (pa *AuthorityImpl) SetChallengesWhitelistFile(f string) error {
-	_, err := reloader.New(f, pa.loadChallengesWhitelist, pa.challengesWhitelistLoadError)
-	return err
-}
-
-func (pa *AuthorityImpl) challengesWhitelistLoadError(err error) {
-	pa.log.AuditErrf("error loading challenges whitelist: %s", err)
-}
-
-func (pa *AuthorityImpl) loadChallengesWhitelist(b []byte) error {
-	hash := sha256.Sum256(b)
-	pa.log.Infof("loading challenges whitelist, sha256: %s", hex.EncodeToString(hash[:]))
-	var wl map[string][]int64
-	err := json.Unmarshal(b, &wl)
-	if err != nil {
-		return err
-	}
-
-	chalWl := make(map[string]map[int64]bool)
-
-	for k, v := range wl {
-		chalWl[k] = make(map[int64]bool)
-		for _, i := range v {
-			chalWl[k][i] = true
-		}
-	}
-
-	pa.blacklistMu.Lock()
-	pa.enabledChallengesWhitelist = chalWl
-	pa.blacklistMu.Unlock()
-
-	return nil
-}
-
 const (
 	maxLabels = 10
 
@@ -414,10 +377,8 @@ func (pa *AuthorityImpl) checkHostLists(domain string) error {
 }
 
 // ChallengesFor makes a decision of what challenges, and combinations, are
-// acceptable for the given identifier. If the TLSSNIRevalidation feature flag
+// acceptable for the given identifier.
-// is set, create TLS-SNI-01 challenges for revalidation requests even if
+func (pa *AuthorityImpl) ChallengesFor(identifier core.AcmeIdentifier) ([]core.Challenge, [][]int, error) {
-// TLS-SNI-01 is not among the configured challenges.
-func (pa *AuthorityImpl) ChallengesFor(identifier core.AcmeIdentifier, regID int64, revalidation bool) ([]core.Challenge, [][]int, error) {
 	challenges := []core.Challenge{}
 
 	// If we are using the new authorization storage schema we only use a single
@@ -432,7 +393,7 @@ func (pa *AuthorityImpl) ChallengesFor(identifier core.AcmeIdentifier, regID int
 	if strings.HasPrefix(identifier.Value, "*.") {
 		// We must have the DNS-01 challenge type enabled to create challenges for
 		// a wildcard identifier per LE policy.
-		if !pa.ChallengeTypeEnabled(core.ChallengeTypeDNS01, regID) {
+		if !pa.ChallengeTypeEnabled(core.ChallengeTypeDNS01) {
 			return nil, nil, fmt.Errorf(
 				"Challenges requested for wildcard identifier but DNS-01 " +
 					"challenge type is not enabled")
@@ -441,22 +402,15 @@ func (pa *AuthorityImpl) ChallengesFor(identifier core.AcmeIdentifier, regID int
 		challenges = []core.Challenge{core.DNSChallenge01(token)}
 	} else {
 		// Otherwise we collect up challenges based on what is enabled.
-		if pa.ChallengeTypeEnabled(core.ChallengeTypeHTTP01, regID) {
+		if pa.ChallengeTypeEnabled(core.ChallengeTypeHTTP01) {
 			challenges = append(challenges, core.HTTPChallenge01(token))
 		}
 
-		// Add a TLS-SNI challenge, if either (a) the challenge is enabled, or (b)
+		if pa.ChallengeTypeEnabled(core.ChallengeTypeTLSALPN01) {
-		// the TLSSNIRevalidation feature flag is on and this is a revalidation.
-		if pa.ChallengeTypeEnabled(core.ChallengeTypeTLSSNI01, regID) ||
-			(features.Enabled(features.TLSSNIRevalidation) && revalidation) {
-			challenges = append(challenges, core.TLSSNIChallenge01(token))
-		}
-
-		if pa.ChallengeTypeEnabled(core.ChallengeTypeTLSALPN01, regID) {
 			challenges = append(challenges, core.TLSALPNChallenge01(token))
 		}
 
-		if pa.ChallengeTypeEnabled(core.ChallengeTypeDNS01, regID) {
+		if pa.ChallengeTypeEnabled(core.ChallengeTypeDNS01) {
 			challenges = append(challenges, core.DNSChallenge01(token))
 		}
 	}
@@ -482,9 +436,8 @@ func (pa *AuthorityImpl) ChallengesFor(identifier core.AcmeIdentifier, regID int
 }
 
 // ChallengeTypeEnabled returns whether the specified challenge type is enabled
-func (pa *AuthorityImpl) ChallengeTypeEnabled(t string, regID int64) bool {
+func (pa *AuthorityImpl) ChallengeTypeEnabled(t string) bool {
 	pa.blacklistMu.RLock()
 	defer pa.blacklistMu.RUnlock()
-	return pa.enabledChallenges[t] ||
+	return pa.enabledChallenges[t]
-		(pa.enabledChallengesWhitelist[t] != nil && pa.enabledChallengesWhitelist[t][regID])
 }

policy/pa_test.go

@@ -16,7 +16,6 @@ var log = blog.UseMock()
 
 var enabledChallenges = map[string]bool{
 	core.ChallengeTypeHTTP01: true,
-	core.ChallengeTypeTLSSNI01: true,
 	core.ChallengeTypeDNS01:  true,
 }
 
@@ -313,7 +312,7 @@ var accountKeyJSON = `{
 func TestChallengesFor(t *testing.T) {
 	pa := paImpl(t)
 
-	challenges, combinations, err := pa.ChallengesFor(core.AcmeIdentifier{}, testRegID, false)
+	challenges, combinations, err := pa.ChallengesFor(core.AcmeIdentifier{})
 	test.AssertNotError(t, err, "ChallengesFor failed")
 
 	test.Assert(t, len(challenges) == len(enabledChallenges), "Wrong number of challenges returned")
@@ -321,7 +320,7 @@ func TestChallengesFor(t *testing.T) {
 
 	seenChalls := make(map[string]bool)
 	// Expected only if the pseudo-RNG is seeded with 99.
-	expectedCombos := [][]int{{1}, {2}, {0}}
+	expectedCombos := [][]int{{1}, {0}}
 	for _, challenge := range challenges {
 		test.Assert(t, !seenChalls[challenge.Type], "should not already have seen this type")
 		seenChalls[challenge.Type] = true
@@ -333,35 +332,6 @@ func TestChallengesFor(t *testing.T) {
 
 }
 
-func TestChallengesForWhitelist(t *testing.T) {
-	enabledChallenges[core.ChallengeTypeTLSSNI01] = false
-
-	var enabledChallengesWhitelist = map[string][]int64{
-		core.ChallengeTypeHTTP01:   []int64{},
-		core.ChallengeTypeTLSSNI01: []int64{testRegIDWhitelisted},
-		core.ChallengeTypeDNS01:    []int64{},
-	}
-
-	pa := paImpl(t)
-
-	wlBytes, err := json.Marshal(enabledChallengesWhitelist)
-	test.AssertNotError(t, err, "Couldn't serialize whitelist")
-	f, _ := ioutil.TempFile("", "test-challenges-whitelist.json")
-	defer os.Remove(f.Name())
-	err = ioutil.WriteFile(f.Name(), wlBytes, 0640)
-	test.AssertNotError(t, err, "Couldn't write serialized whitelist to file")
-	err = pa.SetChallengesWhitelistFile(f.Name())
-	test.AssertNotError(t, err, "Couldn't load policy contents from file")
-
-	challenges, _, err := pa.ChallengesFor(core.AcmeIdentifier{}, testRegID, false)
-	test.AssertNotError(t, err, "ChallengesFor failed")
-	test.Assert(t, len(challenges) == len(enabledChallenges)-1, "Wrong number of challenges returned")
-
-	challenges, _, err = pa.ChallengesFor(core.AcmeIdentifier{}, testRegIDWhitelisted, false)
-	test.AssertNotError(t, err, "ChallengesFor failed")
-	test.Assert(t, len(challenges) == len(enabledChallenges), "Wrong number of challenges returned")
-}
-
 func TestChallengesForWildcard(t *testing.T) {
 	// wildcardIdent is an identifier for a wildcard domain name
 	wildcardIdent := core.AcmeIdentifier{
@@ -379,11 +349,10 @@ func TestChallengesForWildcard(t *testing.T) {
 	// DNS-01 challenge type enabled. This should produce an error
 	var enabledChallenges = map[string]bool{
 		core.ChallengeTypeHTTP01: true,
-		core.ChallengeTypeTLSSNI01: true,
 		core.ChallengeTypeDNS01:  false,
 	}
 	pa := mustConstructPA(t, enabledChallenges)
-	_, _, err := pa.ChallengesFor(wildcardIdent, testRegID, false)
+	_, _, err := pa.ChallengesFor(wildcardIdent)
 	test.AssertError(t, err, "ChallengesFor did not error for a wildcard ident "+
 		"when DNS-01 was disabled")
 	test.AssertEquals(t, err.Error(), "Challenges requested for wildcard "+
@@ -393,7 +362,7 @@ func TestChallengesForWildcard(t *testing.T) {
 	// should return only one DNS-01 type challenge
 	enabledChallenges[core.ChallengeTypeDNS01] = true
 	pa = mustConstructPA(t, enabledChallenges)
-	challenges, combinations, err := pa.ChallengesFor(wildcardIdent, testRegID, false)
+	challenges, combinations, err := pa.ChallengesFor(wildcardIdent)
 	test.AssertNotError(t, err, "ChallengesFor errored for a wildcard ident "+
 		"unexpectedly")
 	test.AssertEquals(t, len(combinations), 1)

ra/ra.go

@@ -1433,22 +1433,9 @@ func (ra *RegistrationAuthorityImpl) PerformValidation(
 
 	ch := &authz.Challenges[challIndex]
 
-	// If TLSSNIRevalidation is enabled, find out whether this was a revalidation
+	// This challenge type may have been disabled since the challenge was created.
-	// (previous certificate existed) or not. If it is a revalidation, we can
+	if !ra.PA.ChallengeTypeEnabled(ch.Type) {
-	// proceed with validation even though the challenge type is currently
+		return nil, berrors.MalformedError("challenge type %q no longer allowed", ch.Type)
-	// disabled.
-	if !ra.PA.ChallengeTypeEnabled(ch.Type, authz.RegistrationID) && features.Enabled(features.TLSSNIRevalidation) {
-		existsResp, err := ra.SA.PreviousCertificateExists(ctx, &sapb.PreviousCertificateExistsRequest{
-			Domain: &authz.Identifier.Value,
-			RegID:  &authz.RegistrationID,
-		})
-		if err != nil {
-			return nil, err
-		}
-		if !*existsResp.Exists {
-			return nil,
-				berrors.MalformedError("challenge type %q no longer allowed", ch.Type)
-		}
 	}
 
 	// When configured with `reuseValidAuthz` we can expect some clients to try
@@ -1929,23 +1916,8 @@ func (ra *RegistrationAuthorityImpl) createPendingAuthz(ctx context.Context, reg
 		Expires:        &expires,
 	}
 
-	// If TLSSNIRevalidation is enabled, find out whether this was a revalidation
-	// (previous certificate existed) or not. If it is a revalidation, we'll tell
-	// the PA about that so it can include the TLS-SNI-01 challenge.
-	var previousCertificateExists bool
-	if features.Enabled(features.TLSSNIRevalidation) {
-		existsResp, err := ra.SA.PreviousCertificateExists(ctx, &sapb.PreviousCertificateExistsRequest{
-			Domain: &identifier.Value,
-			RegID:  &reg,
-		})
-		if err != nil {
-			return nil, err
-		}
-		previousCertificateExists = *existsResp.Exists
-	}
-
 	// Create challenges. The WFE will update them with URIs before sending them out.
-	challenges, combinations, err := ra.PA.ChallengesFor(identifier, reg, previousCertificateExists)
+	challenges, combinations, err := ra.PA.ChallengesFor(identifier)
 	if err != nil {
 		// The only time ChallengesFor errors it is a fatal configuration error
 		// where challenges required by policy for an identifier are not enabled. We
@@ -1979,7 +1951,7 @@ func (ra *RegistrationAuthorityImpl) createPendingAuthz(ctx context.Context, reg
 func (ra *RegistrationAuthorityImpl) authzValidChallengeEnabled(authz *core.Authorization) bool {
 	for _, chall := range authz.Challenges {
 		if chall.Status == core.StatusValid {
-			return ra.PA.ChallengeTypeEnabled(chall.Type, authz.RegistrationID)
+			return ra.PA.ChallengeTypeEnabled(chall.Type)
 		}
 	}
 	return false

ra/ra_test.go

@@ -81,7 +81,6 @@ func (dva *DummyValidationAuthority) IsSafeDomain(ctx context.Context, req *vaPB
 var (
 	SupportedChallenges = map[string]bool{
 		core.ChallengeTypeHTTP01: true,
-		core.ChallengeTypeTLSSNI01: true,
 		core.ChallengeTypeDNS01:  true,
 	}
 
@@ -296,7 +295,7 @@ func initAuthorities(t *testing.T) (*DummyValidationAuthority, *sa.SQLStorageAut
 
 	AuthzInitial.RegistrationID = Registration.ID
 
-	challenges, combinations, _ := pa.ChallengesFor(AuthzInitial.Identifier, Registration.ID, false)
+	challenges, combinations, _ := pa.ChallengesFor(AuthzInitial.Identifier)
 	AuthzInitial.Challenges = challenges
 	AuthzInitial.Combinations = combinations
 
@@ -647,12 +646,6 @@ func TestReuseValidAuthorization(t *testing.T) {
 	test.AssertEquals(t, httpChallenge.Type, core.ChallengeTypeHTTP01)
 	test.AssertEquals(t, httpChallenge.Status, core.StatusValid)
 
-	// It should have one SNI challenge that is pending
-	sniIndex := httpIndex + 1
-	sniChallenge := secondAuthz.Challenges[sniIndex]
-	test.AssertEquals(t, sniChallenge.Type, core.ChallengeTypeTLSSNI01)
-	test.AssertEquals(t, sniChallenge.Status, core.StatusPending)
-
 	// Sending an update to this authz for an already valid challenge should do
 	// nothing (but produce no error), since it is already a valid authz
 	authzPB, err := bgrpc.AuthzToPB(secondAuthz)
@@ -667,7 +660,7 @@ func TestReuseValidAuthorization(t *testing.T) {
 	test.AssertEquals(t, finalAuthz.ID, secondAuthz.ID)
 	test.AssertEquals(t, secondAuthz.Status, core.StatusValid)
 
-	challIndex = int64(sniIndex)
+	challIndex = int64(httpIndex)
 	authzPB, err = ra.PerformValidation(ctx, &rapb.PerformValidationRequest{
 		Authz:          authzPB,
 		ChallengeIndex: &challIndex})
@@ -2399,13 +2392,13 @@ func TestNewOrderReuseInvalidAuthz(t *testing.T) {
 }
 
 // mockSAUnsafeAuthzReuse has a GetAuthorizations implementation that returns
-// a TLS-SNI-01 validated wildcard authz.
+// an HTTP-01 validated wildcard authz.
 type mockSAUnsafeAuthzReuse struct {
 	mocks.StorageAuthority
 }
 
 // GetAuthorizations returns a _bizarre_ authorization for "*.zombo.com" that
-// was validated by TLS-SNI-01. This should never happen in real life since the
+// was validated by HTTP-01. This should never happen in real life since the
 // name is a wildcard. We use this mock to test that we reject this bizarre
 // situation correctly.
 func (sa *mockSAUnsafeAuthzReuse) GetAuthorizations(
@@ -2421,9 +2414,9 @@ func (sa *mockSAUnsafeAuthzReuse) GetAuthorizations(
 			// Authz is valid
 			Status: "valid",
 			Challenges: []core.Challenge{
-				// TLS-SNI-01 challenge is valid
+				// HTTP-01 challenge is valid
 				core.Challenge{
-					Type:   core.ChallengeTypeTLSSNI01, // The dreaded TLS-SNI-01! X__X
+					Type:   core.ChallengeTypeHTTP01, // The dreaded HTTP-01! X__X
 					Status: core.StatusValid,
 				},
 				// DNS-01 challenge is pending
@@ -2442,9 +2435,9 @@ func (sa *mockSAUnsafeAuthzReuse) GetAuthorizations(
 			// Authz is valid
 			Status: "valid",
 			Challenges: []core.Challenge{
-				// TLS-SNI-01 challenge is valid
+				// HTTP-01 challenge is valid
 				core.Challenge{
-					Type:   core.ChallengeTypeTLSSNI01,
+					Type:   core.ChallengeTypeHTTP01,
 					Status: core.StatusValid,
 				},
 				// DNS-01 challenge is pending
@@ -2496,7 +2489,7 @@ func TestNewOrderAuthzReuseSafety(t *testing.T) {
 	regA := int64(1)
 	names := []string{"*.zombo.com"}
 
-	// Use a mock SA that always returns a valid TLS-SNI-01 authz for the name
+	// Use a mock SA that always returns a valid HTTP-01 authz for the name
 	// "zombo.com"
 	ra.SA = &mockSAUnsafeAuthzReuse{}
 
@@ -2524,7 +2517,7 @@ func TestNewOrderAuthzReuseDisabled(t *testing.T) {
 	regA := int64(1)
 	names := []string{"zombo.com"}
 
-	// Use a mock SA that always returns a valid TLS-SNI-01 authz for the name
+	// Use a mock SA that always returns a valid HTTP-01 authz for the name
 	// "zombo.com"
 	ra.SA = &mockSAUnsafeAuthzReuse{}
 
@@ -2564,7 +2557,6 @@ func TestNewOrderWildcard(t *testing.T) {
 	// DNS-01
 	supportedChallenges := map[string]bool{
 		core.ChallengeTypeHTTP01: true,
-		core.ChallengeTypeTLSSNI01: true,
 		core.ChallengeTypeDNS01:  true,
 	}
 	pa, err := policy.New(supportedChallenges)
@@ -2606,7 +2598,7 @@ func TestNewOrderWildcard(t *testing.T) {
 			test.AssertEquals(t, authz.Challenges[0].Type, core.ChallengeTypeDNS01)
 		case "example.com":
 			// If the authz is for example.com, we expect it has normal challenges
-			test.AssertEquals(t, len(authz.Challenges), 3)
+			test.AssertEquals(t, len(authz.Challenges), 2)
 		default:
 			t.Fatalf("Received an authorization for a name not requested: %q", name)
 		}
@@ -2644,7 +2636,7 @@ func TestNewOrderWildcard(t *testing.T) {
 		case "zombo.com":
 			// We expect that the base domain identifier auth has the normal number of
 			// challenges
-			test.AssertEquals(t, len(authz.Challenges), 3)
+			test.AssertEquals(t, len(authz.Challenges), 2)
 		case "*.zombo.com":
 			// We expect that the wildcard identifier auth has only a pending
 			// DNS-01 type challenge
@@ -2677,7 +2669,7 @@ func TestNewOrderWildcard(t *testing.T) {
 	// We expect the authz is for the identifier the correct domain
 	test.AssertEquals(t, authz.Identifier.Value, "everything.is.possible.zombo.com")
 	// We expect the authz has the normal # of challenges
-	test.AssertEquals(t, len(authz.Challenges), 3)
+	test.AssertEquals(t, len(authz.Challenges), 2)
 
 	// Now submit an order request for a wildcard of the domain we just created an
 	// order for. We should **NOT** reuse the authorization from the previous
@@ -3194,7 +3186,6 @@ func TestFinalizeOrderWildcard(t *testing.T) {
 	// DNS-01 or DNS-01-Wildcard
 	supportedChallenges := map[string]bool{
 		core.ChallengeTypeHTTP01: true,
-		core.ChallengeTypeTLSSNI01: true,
 		core.ChallengeTypeDNS01:  true,
 	}
 	pa, err := policy.New(supportedChallenges)
@@ -3318,20 +3309,17 @@ func TestIssueCertificateAuditLog(t *testing.T) {
 		// Create challenges
 		httpChal := core.HTTPChallenge01("")
 		dnsChal := core.DNSChallenge01("")
-		tlsChal := core.TLSSNIChallenge01("")
 		// Set the selected challenge to valid
 		switch chalType {
 		case "http-01":
 			httpChal.Status = core.StatusValid
 		case "dns-01":
 			dnsChal.Status = core.StatusValid
-		case "tls-sni-01":
-			tlsChal.Status = core.StatusValid
 		default:
 			t.Fatalf("Invalid challenge type used with authzForChalType: %q", chalType)
 		}
 		// Set the template's challenges
-		template.Challenges = []core.Challenge{httpChal, dnsChal, tlsChal}
+		template.Challenges = []core.Challenge{httpChal, dnsChal}
 		// Set the overall authz to valid
 		template.Status = "valid"
 		template.Expires = &exp
@@ -3351,7 +3339,7 @@ func TestIssueCertificateAuditLog(t *testing.T) {
 
 	// Make some valid authorizations for some names using different challenge types
 	names := []string{"not-example.com", "www.not-example.com", "still.not-example.com", "definitely.not-example.com"}
-	chalTypes := []string{"http-01", "dns-01", "tls-sni-01", "dns-01"}
+	chalTypes := []string{"http-01", "dns-01", "http-01", "dns-01"}
 	var authzs []core.Authorization
 	var authzIDs []string
 	for i, name := range names {
@@ -3526,93 +3514,18 @@ func (ms *mockSAPreexistingCertificate) GetPendingAuthorization(ctx context.Cont
 	return nil, berrors.NotFoundError("no pending authorization found")
 }
 
-// With TLS-SNI-01 disabled, an account that previously issued a certificate for
-// example.com should still be able to get a new authorization.
-func TestNewAuthzTLSSNIRevalidation(t *testing.T) {
-	_, _, ra, _, cleanUp := initAuthorities(t)
-	defer cleanUp()
-
-	challenges := map[string]bool{
-		core.ChallengeTypeHTTP01: true,
-	}
-	_ = features.Set(map[string]bool{
-		"TLSSNIRevalidation": true,
-	})
-	pa, err := policy.New(challenges)
-	test.AssertNotError(t, err, "Couldn't create PA")
-	err = pa.SetHostnamePolicyFile("../test/hostname-policy.json")
-	test.AssertNotError(t, err, "Couldn't set hostname policy")
-	ra.PA = pa
-
-	ra.SA = &mockSAPreexistingCertificate{}
-
-	// Test with a reg ID and hostname that have a previous issuance, expect to
-	// see TLS-SNI-01.
-	authz, err := ra.NewAuthorization(context.Background(),
-		core.Authorization{
-			Identifier: core.AcmeIdentifier{
-				Type:  core.IdentifierDNS,
-				Value: previousIssuanceDomain,
-			},
-		}, previousIssuanceRegId)
-	test.AssertNotError(t, err, "creating authz with domain for revalidation")
-
-	hasTLSSNI := func(challenges []core.Challenge) bool {
-		var foundTLSSNI bool
-		for _, c := range challenges {
-			if c.Type == core.ChallengeTypeTLSSNI01 {
-				foundTLSSNI = true
-			}
-		}
-		return foundTLSSNI
-	}
-	if !hasTLSSNI(authz.Challenges) {
-		t.Errorf("TLS-SNI challenge was not created during revalidation.")
-	}
-
-	// Test with a different reg ID, expect no TLS-SNI-01.
-	authz, err = ra.NewAuthorization(context.Background(),
-		core.Authorization{
-			Identifier: core.AcmeIdentifier{
-				Type:  core.IdentifierDNS,
-				Value: previousIssuanceDomain,
-			},
-		}, 1234)
-	test.AssertNotError(t, err, "creating authz with domain for revalidation")
-	if hasTLSSNI(authz.Challenges) {
-		t.Errorf("TLS-SNI challenge was created during non-revalidation new-authz " +
-			"(different regID).")
-	}
-
-	// Test with a different domain, expect no TLS-SNI-01.
-	authz, err = ra.NewAuthorization(context.Background(),
-		core.Authorization{
-			Identifier: core.AcmeIdentifier{
-				Type:  core.IdentifierDNS,
-				Value: "not.example.com",
-			},
-		}, previousIssuanceRegId)
-	test.AssertNotError(t, err, "creating authz with domain for revalidation")
-	if hasTLSSNI(authz.Challenges) {
-		t.Errorf("TLS-SNI challenge was created during non-revalidation new-authz " +
-			"(different domain).")
-	}
-}
-
 func TestValidChallengeStillGood(t *testing.T) {
 	_, _, ra, _, cleanUp := initAuthorities(t)
 	defer cleanUp()
 	pa, err := policy.New(map[string]bool{
-		core.ChallengeTypeTLSSNI01: true,
+		core.ChallengeTypeHTTP01: true,
 	})
 	test.AssertNotError(t, err, "Couldn't create PA")
 	ra.PA = pa
 
 	test.Assert(t, !ra.authzValidChallengeEnabled(&core.Authorization{}), "ra.authzValidChallengeEnabled didn't fail with empty authorization")
 	test.Assert(t, !ra.authzValidChallengeEnabled(&core.Authorization{Challenges: []core.Challenge{{Status: core.StatusPending}}}), "ra.authzValidChallengeEnabled didn't fail with no valid challenges")
-	test.Assert(t, !ra.authzValidChallengeEnabled(&core.Authorization{Challenges: []core.Challenge{{Status: core.StatusValid, Type: core.ChallengeTypeHTTP01}}}), "ra.authzValidChallengeEnabled didn't fail with disabled challenge")
+	test.Assert(t, !ra.authzValidChallengeEnabled(&core.Authorization{Challenges: []core.Challenge{{Status: core.StatusValid, Type: core.ChallengeTypeDNS01}}}), "ra.authzValidChallengeEnabled didn't fail with disabled challenge")
-
-	test.Assert(t, ra.authzValidChallengeEnabled(&core.Authorization{Challenges: []core.Challenge{{Status: core.StatusValid, Type: core.ChallengeTypeTLSSNI01}}}), "ra.authzValidChallengeEnabled failed with enabled challenge")
 }
 
 func TestPerformValidationBadChallengeType(t *testing.T) {
@@ -3627,7 +3540,7 @@ func TestPerformValidationBadChallengeType(t *testing.T) {
 		Challenges: []core.Challenge{
 			core.Challenge{
 				Status: core.StatusValid,
-				Type:   core.ChallengeTypeTLSSNI01},
+				Type:   core.ChallengeTypeHTTP01},
 		},
 		Expires: &exp,
 	}
@@ -3640,7 +3553,7 @@ func TestPerformValidationBadChallengeType(t *testing.T) {
 		ChallengeIndex: &challIndex,
 	})
 	test.AssertError(t, err, "ra.PerformValidation allowed a update to a authorization")
-	test.AssertEquals(t, err.Error(), "challenge type \"tls-sni-01\" no longer allowed")
+	test.AssertEquals(t, err.Error(), "challenge type \"http-01\" no longer allowed")
 }
 
 type timeoutPub struct {

sa/model.go

@@ -403,16 +403,14 @@ func modelToOrder(om *orderModel) (*corepb.Order, error) {
 
 var challTypeToUint = map[string]uint{
 	"http-01":     0,
-	"tls-sni-01":  1,
+	"dns-01":      1,
-	"dns-01":      2,
+	"tls-alpn-01": 2,
-	"tls-alpn-01": 3,
 }
 
 var uintToChallType = map[uint]string{
 	0: "http-01",
-	1: "tls-sni-01",
+	1: "dns-01",
-	2: "dns-01",
+	2: "tls-alpn-01",
-	3: "tls-alpn-01",
 }
 
 var identifierTypeToUint = map[string]uint{

test/challenges-whitelist.json

@@ -1,5 +0,0 @@
-{
-      "http-01": [],
-      "tls-sni-01": [1000],
-      "dns-01": []
-}

test/config-next/ca-a.json

@@ -135,7 +135,6 @@
   "pa": {
     "challenges": {
       "http-01": true,
-      "tls-sni-01": true,
       "dns-01": true,
       "tls-alpn-01": true
     }

test/config-next/ca-b.json

@@ -135,7 +135,6 @@
   "pa": {
     "challenges": {
       "http-01": true,
-      "tls-sni-01": true,
       "dns-01": true,
       "tls-alpn-01": true
     }

test/config-next/cert-checker.json

@@ -8,7 +8,6 @@
   "pa": {
     "challenges": {
       "http-01": true,
-      "tls-sni-01": true,
       "dns-01": true,
       "tls-alpn-01": true
     }

test/config-next/ra.json

@@ -116,8 +116,7 @@
       "http-01": true,
       "dns-01": true,
       "tls-alpn-01": true
-    },
+    }
-    "challengesWhitelistFile": "test/challenges-whitelist.json"
   },
 
   "syslog": {

test/config/ca-a.json

@@ -137,7 +137,6 @@
   "pa": {
     "challenges": {
       "http-01": true,
-      "tls-sni-01": true,
       "dns-01": true
     }
   },

test/config/ca-b.json

@@ -137,7 +137,6 @@
   "pa": {
     "challenges": {
       "http-01": true,
-      "tls-sni-01": true,
       "dns-01": true
     }
   },

test/config/cert-checker.json

@@ -8,7 +8,6 @@
   "pa": {
     "challenges": {
       "http-01": true,
-      "tls-sni-01": true,
       "dns-01": true
     }
   },

test/config/ra.json

@@ -94,8 +94,7 @@
       "http-01": true,
       "dns-01": true,
       "tls-alpn-01": true
-    },
+    }
-    "challengesWhitelistFile": "test/challenges-whitelist.json"
   },
 
   "syslog": {

va/caa_test.go

@@ -583,8 +583,8 @@ func TestIsCAAValidErrMessage(t *testing.T) {
 }
 
 func TestCAAFailure(t *testing.T) {
-	chall := createChallenge(core.ChallengeTypeTLSSNI01)
+	chall := createChallenge(core.ChallengeTypeHTTP01)
-	hs := tlssni01Srv(t, chall)
+	hs := httpSrv(t, chall.Token)
 	defer hs.Close()
 
 	va, _ := setup(hs, 0)

va/va.go

@@ -197,7 +197,7 @@ func NewValidationAuthorityImpl(
 		// singleDialTimeout specifies how long an individual `DialContext` operation may take
 		// before timing out. This timeout ignores the base RPC timeout and is strictly
 		// used for the DialContext operations that take place during an
-		// HTTP-01/TLS-SNI-[01|02] challenge validation.
+		// HTTP-01 challenge validation.
 		singleDialTimeout: 10 * time.Second,
 	}, nil
 }
@@ -321,28 +321,6 @@ func (va *ValidationAuthorityImpl) tryGetTLSCerts(ctx context.Context,
 	return certs, cs, validationRecords, err
 }
 
-func (va *ValidationAuthorityImpl) validateTLSSNI01WithZName(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge, zName string) ([]core.ValidationRecord, *probs.ProblemDetails) {
-	certs, _, validationRecords, problem := va.tryGetTLSCerts(ctx, identifier, challenge, &tls.Config{ServerName: 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
-		}
-	}
-
-	hostPort := net.JoinHostPort(validationRecords[0].AddressUsed.String(), validationRecords[0].Port)
-	names := certNames(leafCert)
-	problem = probs.Unauthorized("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.Infof("Remote host failed to give %s challenge name. host: %s", challenge.Type, identifier)
-	return validationRecords, problem
-}
-
 func (va *ValidationAuthorityImpl) getTLSCerts(
 	ctx context.Context,
 	hostPort string,
@@ -426,20 +404,6 @@ func (va *ValidationAuthorityImpl) validateHTTP01(ctx context.Context, identifie
 	return validationRecords, nil
 }
 
-func (va *ValidationAuthorityImpl) validateTLSSNI01(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) {
-	if identifier.Type != "dns" {
-		va.log.Infof("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.Sum256([]byte(challenge.ProvidedKeyAuthorization))
-	Z := hex.EncodeToString(h[:])
-	ZName := fmt.Sprintf("%s.%s.%s", Z[:32], Z[32:], core.TLSSNISuffix)
-
-	return va.validateTLSSNI01WithZName(ctx, identifier, challenge, ZName)
-}
-
 func (va *ValidationAuthorityImpl) validateTLSALPN01(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-ALPN-01 was not DNS: %s", identifier))
@@ -519,7 +483,7 @@ func (va *ValidationAuthorityImpl) validateTLSALPN01(ctx context.Context, identi
 var badTLSHeader = []byte{0x48, 0x54, 0x54, 0x50, 0x2f}
 
 // detailedError returns a ProblemDetails corresponding to an error
-// that occurred during HTTP-01 or TLS-SNI domain validation. Specifically it
+// that occurred during HTTP-01 or TLS-ALPN domain validation. Specifically it
 // tries to unwrap known Go error types and present something a little more
 // meaningful. It additionally handles `berrors.ConnectionFailure` errors by
 // passing through the detailed message.
@@ -678,8 +642,6 @@ func (va *ValidationAuthorityImpl) validateChallenge(ctx context.Context, identi
 	switch challenge.Type {
 	case core.ChallengeTypeHTTP01:
 		return va.validateHTTP01(ctx, identifier, challenge)
-	case core.ChallengeTypeTLSSNI01:
-		return va.validateTLSSNI01(ctx, identifier, challenge)
 	case core.ChallengeTypeDNS01:
 		return va.validateDNS01(ctx, identifier, challenge)
 	case core.ChallengeTypeTLSALPN01:

va/va_test.go

@@ -9,7 +9,6 @@ import (
 	"crypto/x509/pkix"
 	"encoding/asn1"
 	"encoding/base64"
-	"encoding/hex"
 	"errors"
 	"fmt"
 	"math/big"
@@ -35,7 +34,6 @@ import (
 	"github.com/letsencrypt/boulder/core"
 	blog "github.com/letsencrypt/boulder/log"
 	"github.com/letsencrypt/boulder/metrics"
-	"github.com/letsencrypt/boulder/metrics/mock_metrics"
 	"github.com/letsencrypt/boulder/probs"
 	"github.com/letsencrypt/boulder/test"
 	vaPB "github.com/letsencrypt/boulder/va/proto"
@@ -174,14 +172,6 @@ func httpSrv(t *testing.T, token string) *httptest.Server {
 	return server
 }
 
-func tlssni01Srv(t *testing.T, chall core.Challenge) *httptest.Server {
-	h := sha256.Sum256([]byte(chall.ProvidedKeyAuthorization))
-	Z := hex.EncodeToString(h[:])
-	ZName := fmt.Sprintf("%s.%s.acme.invalid", Z[:32], Z[32:])
-
-	return tlssniSrvWithNames(t, chall, ZName)
-}
-
 func tlsCertTemplate(names []string) *x509.Certificate {
 	return &x509.Certificate{
 		SerialNumber: big.NewInt(1337),
@@ -208,6 +198,7 @@ func makeACert(names []string) *tls.Certificate {
 	}
 }
 
+// tlssniSrvWithNames is kept around for the use of TestValidateTLSALPN01UnawareSrv
 func tlssniSrvWithNames(t *testing.T, chall core.Challenge, names ...string) *httptest.Server {
 	cert := makeACert(names)
 	tlsConfig := &tls.Config{
@@ -594,29 +585,13 @@ func getPort(hs *httptest.Server) int {
 	return int(port)
 }
 
-func TestTLSSNI01Success(t *testing.T) {
+func TestTLSALPN01FailIP(t *testing.T) {
-	chall := createChallenge(core.ChallengeTypeTLSSNI01)
+	chall := createChallenge(core.ChallengeTypeTLSALPN01)
-	hs := tlssni01Srv(t, chall)
+	hs := tlsalpn01Srv(t, chall, IdPeAcmeIdentifier, "localhost")
-	va, log := setup(hs, 0)
-
-	_, prob := va.validateTLSSNI01(ctx, dnsi("localhost"), chall)
-	if prob != nil {
-		t.Fatalf("Unexpected failure in validate TLS-SNI-01: %s", prob)
-	}
-	test.AssertEquals(t, len(log.GetAllMatching(`Resolved addresses for localhost: \[127.0.0.1\]`)), 1)
-	if len(log.GetAllMatching(`challenge for localhost received certificate \(1 of 1\): cert=\[`)) != 1 {
-		t.Errorf("Didn't get log message with validated certificate. Instead got:\n%s",
-			strings.Join(log.GetAllMatching(".*"), "\n"))
-	}
-}
-
-func TestTLSSNI01FailIP(t *testing.T) {
-	chall := createChallenge(core.ChallengeTypeTLSSNI01)
-	hs := tlssni01Srv(t, chall)
 	va, _ := setup(hs, 0)
 
 	port := getPort(hs)
-	_, prob := va.validateTLSSNI01(ctx, core.AcmeIdentifier{
+	_, prob := va.validateTLSALPN01(ctx, core.AcmeIdentifier{
 		Type:  core.IdentifierType("ip"),
 		Value: net.JoinHostPort("127.0.0.1", strconv.Itoa(port)),
 	}, chall)
@@ -626,49 +601,6 @@ func TestTLSSNI01FailIP(t *testing.T) {
 	test.AssertEquals(t, prob.Type, probs.MalformedProblem)
 }
 
-func TestTLSSNI01Invalid(t *testing.T) {
-	chall := createChallenge(core.ChallengeTypeTLSSNI01)
-	hs := tlssni01Srv(t, chall)
-	va, _ := setup(hs, 0)
-
-	_, prob := va.validateTLSSNI01(ctx, core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "always.invalid"}, chall)
-	if prob == nil {
-		t.Fatalf("Domain name was supposed to be invalid.")
-	}
-	test.AssertEquals(t, prob.Type, probs.UnknownHostProblem)
-	expected := "No valid IP addresses found for always.invalid"
-	if prob.Detail != expected {
-		t.Errorf("Got wrong error detail. Expected %q, got %q",
-			expected, prob.Detail)
-	}
-}
-
-// TestTLSSNI01BadUTFSrv tests that validating TLS-SNI-01 against
-// a host that returns a certificate with a SAN/CN that contains invalid UTF-8
-// will result in a problem with the invalid UTF-8 replaced.
-func TestTLSSNI01BadUTFSrv(t *testing.T) {
-	chall := createChallenge(core.ChallengeTypeTLSSNI01)
-	hs := tlssniSrvWithNames(t, chall, "localhost", "\xf0\x28\x8c\xbc")
-	port := getPort(hs)
-	va, _ := setup(hs, 0)
-
-	// Construct the zName so we know what to expect in the error message
-	h := sha256.Sum256([]byte(chall.ProvidedKeyAuthorization))
-	z := hex.EncodeToString(h[:])
-	zName := fmt.Sprintf("%s.%s.acme.invalid", z[:32], z[32:])
-
-	_, prob := va.validateTLSSNI01(ctx, dnsi("localhost"), chall)
-	if prob == nil {
-		t.Fatalf("TLS-SNI-01 validation should have failed.")
-	}
-	test.AssertEquals(t, prob.Type, probs.UnauthorizedProblem)
-	test.AssertEquals(t, prob.Detail, fmt.Sprintf(
-		"Incorrect validation certificate for tls-sni-01 challenge. "+
-			"Requested %s from 127.0.0.1:%d. Received 1 certificate(s), "+
-			`first certificate had names "localhost, %s"`,
-		zName, port, "\ufffd(\ufffd\ufffd"))
-}
-
 func slowTLSSrv() *httptest.Server {
 	server := httptest.NewUnstartedServer(http.DefaultServeMux)
 	server.TLS = &tls.Config{
@@ -681,8 +613,8 @@ func slowTLSSrv() *httptest.Server {
 	return server
 }
 
-func TestTLSSNI01TimeoutAfterConnect(t *testing.T) {
+func TestTLSALPNTimeoutAfterConnect(t *testing.T) {
-	chall := createChallenge(core.ChallengeTypeTLSSNI01)
+	chall := createChallenge(core.ChallengeTypeTLSALPN01)
 	hs := slowTLSSrv()
 	va, _ := setup(hs, 0)
 
@@ -691,7 +623,7 @@ func TestTLSSNI01TimeoutAfterConnect(t *testing.T) {
 	defer cancel()
 
 	started := time.Now()
-	_, prob := va.validateTLSSNI01(ctx, dnsi("slow.server"), chall)
+	_, prob := va.validateTLSALPN01(ctx, dnsi("slow.server"), chall)
 	if prob == nil {
 		t.Fatalf("Validation should've failed")
 	}
@@ -717,8 +649,8 @@ func TestTLSSNI01TimeoutAfterConnect(t *testing.T) {
 	}
 }
 
-func TestTLSSNI01DialTimeout(t *testing.T) {
+func TestTLSALPN01DialTimeout(t *testing.T) {
-	chall := createChallenge(core.ChallengeTypeTLSSNI01)
+	chall := createChallenge(core.ChallengeTypeTLSALPN01)
 	hs := slowTLSSrv()
 	va, _ := setup(hs, 0)
 	va.dnsClient = dnsMockReturnsUnroutable{&bdns.MockDNSClient{}}
@@ -734,7 +666,7 @@ func TestTLSSNI01DialTimeout(t *testing.T) {
 	// that, just retry until we get something other than "Network unreachable".
 	var prob *probs.ProblemDetails
 	for i := 0; i < 20; i++ {
-		_, prob = va.validateTLSSNI01(ctx, dnsi("unroutable.invalid"), chall)
+		_, prob = va.validateTLSALPN01(ctx, dnsi("unroutable.invalid"), chall)
 		if prob != nil && strings.Contains(prob.Detail, "Network unreachable") {
 			continue
 		} else {
@@ -766,45 +698,31 @@ func TestTLSSNI01DialTimeout(t *testing.T) {
 	}
 }
 
-func TestTLSSNI01InvalidResponse(t *testing.T) {
+func TestTLSALPN01Refused(t *testing.T) {
-	chall := createChallenge(core.ChallengeTypeTLSSNI01)
+	chall := createChallenge(core.ChallengeTypeTLSALPN01)
-	hs := tlssni01Srv(t, chall)
+	hs := tlsalpn01Srv(t, chall, IdPeAcmeIdentifier, "localhost")
-	va, _ := setup(hs, 0)
-
-	differentChall := createChallenge(core.ChallengeTypeTLSSNI01)
-	differentChall.ProvidedKeyAuthorization = "invalid.keyAuthorization"
-
-	_, prob := va.validateTLSSNI01(ctx, dnsi("localhost"), differentChall)
-	if prob == nil {
-		t.Fatalf("Validation should've failed")
-	}
-	expected := "Incorrect validation certificate for tls-sni-01 challenge."
-	if !strings.HasPrefix(prob.Detail, expected) {
-		t.Errorf("Wrong error detail. Expected %q, got %q", expected, prob.Detail)
-	}
-}
-
-func TestTLSSNI01Refused(t *testing.T) {
-	chall := createChallenge(core.ChallengeTypeTLSSNI01)
-	hs := tlssni01Srv(t, chall)
 	va, _ := setup(hs, 0)
 	// Take down validation server and check that validation fails.
 	hs.Close()
-	_, prob := va.validateTLSSNI01(ctx, dnsi("localhost"), chall)
+	_, prob := va.validateTLSALPN01(ctx, dnsi("localhost"), chall)
 	if prob == nil {
 		t.Fatalf("Server's down; expected refusal. Where did we connect?")
 	}
 	test.AssertEquals(t, prob.Type, probs.ConnectionProblem)
+	expected := "Connection refused"
+	if prob.Detail != expected {
+		t.Errorf("Wrong error detail. Expected %q, got %q", expected, prob.Detail)
+	}
 }
 
-func TestTLSSNI01TalkingToHTTP(t *testing.T) {
+func TestTLSALPN01TalkingToHTTP(t *testing.T) {
-	chall := createChallenge(core.ChallengeTypeTLSSNI01)
+	chall := createChallenge(core.ChallengeTypeTLSALPN01)
-	hs := tlssni01Srv(t, chall)
+	hs := tlsalpn01Srv(t, chall, IdPeAcmeIdentifier, "localhost")
 	va, _ := setup(hs, 0)
 	httpOnly := httpSrv(t, "")
 	va.tlsPort = getPort(httpOnly)
 
-	_, prob := va.validateTLSSNI01(ctx, dnsi("localhost"), chall)
+	_, prob := va.validateTLSALPN01(ctx, dnsi("localhost"), chall)
 	test.AssertError(t, prob, "TLS-SNI-01 validation passed when talking to a HTTP-only server")
 	expected := "Server only speaks HTTP, not TLS"
 	if !strings.HasSuffix(prob.Detail, expected) {
@@ -824,12 +742,12 @@ func brokenTLSSrv() *httptest.Server {
 }
 
 func TestTLSError(t *testing.T) {
-	chall := createChallenge(core.ChallengeTypeTLSSNI01)
+	chall := createChallenge(core.ChallengeTypeTLSALPN01)
 	hs := brokenTLSSrv()
 
 	va, _ := setup(hs, 0)
 
-	_, prob := va.validateTLSSNI01(ctx, dnsi("localhost"), chall)
+	_, prob := va.validateTLSALPN01(ctx, dnsi("localhost"), chall)
 	if prob == nil {
 		t.Fatalf("TLS validation should have failed: What cert was used?")
 	}
@@ -839,37 +757,6 @@ func TestTLSError(t *testing.T) {
 	}
 }
 
-// misconfiguredTLSSrv is a TLS HTTP test server that returns a certificate
-// chain with more than one cert, none of which will solve a TLS SNI challenge
-func misconfiguredTLSSrv() *httptest.Server {
-	template := &x509.Certificate{
-		SerialNumber:          big.NewInt(1337),
-		NotBefore:             time.Now(),
-		NotAfter:              time.Now().AddDate(0, 0, 1),
-		KeyUsage:              x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
-		ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
-		BasicConstraintsValid: true,
-
-		Subject: pkix.Name{
-			CommonName: "hello.world",
-		},
-		DNSNames: []string{"goodbye.world", "hello.world"},
-	}
-
-	certBytes, _ := x509.CreateCertificate(rand.Reader, template, template, &TheKey.PublicKey, &TheKey)
-	cert := &tls.Certificate{
-		Certificate: [][]byte{certBytes, certBytes},
-		PrivateKey:  &TheKey,
-	}
-
-	server := httptest.NewUnstartedServer(http.DefaultServeMux)
-	server.TLS = &tls.Config{
-		Certificates: []tls.Certificate{*cert},
-	}
-	server.StartTLS()
-	return server
-}
-
 func TestCertNames(t *testing.T) {
 	// We duplicate names inside the SAN set
 	names := []string{
@@ -907,28 +794,6 @@ func TestCertNames(t *testing.T) {
 	test.AssertDeepEquals(t, actual, expected)
 }
 
-// TestSNIErrInvalidChain sets up a TLS server with two certificates, neither of
-// which validate the SNI challenge.
-func TestSNIErrInvalidChain(t *testing.T) {
-	chall := createChallenge(core.ChallengeTypeTLSSNI01)
-	hs := misconfiguredTLSSrv()
-
-	va, _ := setup(hs, 0)
-
-	// Validate the SNI challenge with the test server, expecting it to fail
-	_, prob := va.validateTLSSNI01(ctx, dnsi("localhost"), chall)
-	if prob == nil {
-		t.Fatalf("TLS validation should have failed")
-	}
-
-	// We expect that the error message will say 2 certificates were received, and
-	// we expect the error to contain a deduplicated list of domain names from the
-	// subject CN and SANs of the leaf cert
-	expected := "Received 2 certificate(s), first certificate had names \"goodbye.world, hello.world\""
-	test.AssertEquals(t, prob.Type, probs.UnauthorizedProblem)
-	test.AssertContains(t, prob.Detail, expected)
-}
-
 func TestValidateHTTP(t *testing.T) {
 	chall := core.HTTPChallenge01("")
 	setChallengeToken(&chall, core.NewToken())
@@ -997,31 +862,7 @@ func setChallengeToken(ch *core.Challenge, token string) {
 	ch.ProvidedKeyAuthorization = token + ".9jg46WB3rR_AHD-EBXdN7cBkH1WOu0tA3M9fm21mqTI"
 }
 
-func TestValidateTLSSNI01(t *testing.T) {
+func TestTLSALPN01Success(t *testing.T) {
-	chall := createChallenge(core.ChallengeTypeTLSSNI01)
-	hs := tlssni01Srv(t, chall)
-	defer hs.Close()
-
-	va, _ := setup(hs, 0)
-
-	_, prob := va.validateChallenge(ctx, dnsi("localhost"), chall)
-
-	test.Assert(t, prob == nil, "validation failed")
-}
-
-func TestValidateTLSSNI01NotSane(t *testing.T) {
-	va, _ := setup(nil, 0)
-
-	chall := createChallenge(core.ChallengeTypeTLSSNI01)
-
-	chall.Token = "not sane"
-
-	_, prob := va.validateChallenge(ctx, dnsi("localhost"), chall)
-
-	test.AssertEquals(t, prob.Type, probs.MalformedProblem)
-}
-
-func TestValidateTLSALPN01(t *testing.T) {
 	chall := createChallenge(core.ChallengeTypeTLSALPN01)
 	hs := tlsalpn01Srv(t, chall, IdPeAcmeIdentifier, "localhost")
 
@@ -1490,60 +1331,6 @@ func TestAvailableAddresses(t *testing.T) {
 	}
 }
 
-func TestFallbackTLS(t *testing.T) {
-	// Create a new challenge to use for the httpSrv
-	chall := createChallenge(core.ChallengeTypeTLSSNI01)
-
-	// Create a TLS SNI 01 test server, this will be bound on 127.0.0.1 (e.g. IPv4
-	// only!)
-	hs := tlssni01Srv(t, chall)
-	defer hs.Close()
-
-	// Create a test VA
-	va, _ := setup(hs, 0)
-
-	ctrl := gomock.NewController(t)
-	defer ctrl.Finish()
-	scope := mock_metrics.NewMockScope(ctrl)
-	va.stats = scope
-
-	// We expect the IPV4 Fallback stat to be incremented
-	scope.EXPECT().Inc("IPv4Fallback", int64(1))
-
-	// The validation is expected to succeed  by the fallback to the IPv4 address
-	// that has a test server waiting
-	ident := dnsi("ipv4.and.ipv6.localhost")
-	records, prob := va.validateChallenge(ctx, ident, chall)
-	test.Assert(t, prob == nil, "validation failed with IPv6 fallback to IPv4")
-	// We expect one validation record to be present
-	test.AssertEquals(t, len(records), 1)
-	// We expect that the address eventually used was the IPv4 localhost address
-	test.AssertEquals(t, records[0].AddressUsed.String(), "127.0.0.1")
-	// We expect that one address was tried before the address used
-	test.AssertEquals(t, len(records[0].AddressesTried), 1)
-	// We expect that IPv6 localhost address was tried before the address used
-	test.AssertEquals(t, records[0].AddressesTried[0].String(), "::1")
-
-	// Now try a validation for an IPv6 only host. E.g. one without an IPv4
-	// address. The IPv6 will fail without a server and we expect the overall
-	// validation to fail since there is no IPv4 address/listener to fall back to.
-	ident = dnsi("ipv6.localhost")
-	va.stats = metrics.NewNoopScope()
-	records, prob = va.validateChallenge(ctx, ident, chall)
-	test.Assert(t, prob != nil, "validation succeeded with broken IPv6 and no IPv4 fallback")
-	// We expect that the problem has the correct error message about nothing to fallback to
-	test.AssertEquals(t, prob.Detail,
-		"Unable to contact \"ipv6.localhost\" at \"::1\", no IPv4 addresses to try as fallback")
-	// We expect one validation record to be present
-	test.AssertEquals(t, len(records), 1)
-	// We expect that the address eventually used was the IPv6 localhost address
-	test.AssertEquals(t, records[0].AddressUsed.String(), "::1")
-	// We expect that one address was tried
-	test.AssertEquals(t, len(records[0].AddressesTried), 1)
-	// We expect that IPv6 localhost address was tried
-	test.AssertEquals(t, records[0].AddressesTried[0].String(), "::1")
-}
-
 type multiSrv struct {
 	*httptest.Server
 

wfe/wfe_test.go

@@ -269,7 +269,7 @@ func (ra *MockRegistrationAuthority) FinalizeOrder(ctx context.Context, _ *rapb.
 
 type mockPA struct{}
 
-func (pa *mockPA) ChallengesFor(identifier core.AcmeIdentifier, registrationID int64, revalidation bool) (challenges []core.Challenge, combinations [][]int, err error) {
+func (pa *mockPA) ChallengesFor(identifier core.AcmeIdentifier) (challenges []core.Challenge, combinations [][]int, err error) {
 	return
 }
 
@@ -281,7 +281,7 @@ func (pa *mockPA) WillingToIssueWildcard(id core.AcmeIdentifier) error {
 	return nil
 }
 
-func (pa *mockPA) ChallengeTypeEnabled(t string, registrationID int64) bool {
+func (pa *mockPA) ChallengeTypeEnabled(t string) bool {
 	return true
 }