Move "Combinations" support to WFE1. (letsencrypt#4155)

Early ACME drafts supported a notion of "combinations" of challenges
that had to be completed together. This was removed from subsequent
drafts. Boulder has only ever supported "combinations" that exactly map
to the list of challenges, 1 for 1.

This removes all the plumbing for combinations, and adds a list of
combinations to the authz JSON right before marshaling it in WFE1.

core/interfaces.go

@@ -108,7 +108,7 @@ type CertificateAuthority interface {
 type PolicyAuthority interface {
 	WillingToIssue(domain AcmeIdentifier) error
 	WillingToIssueWildcard(domain AcmeIdentifier) error
-	ChallengesFor(domain AcmeIdentifier) (challenges []Challenge, validCombinations [][]int, err error)
+	ChallengesFor(domain AcmeIdentifier) ([]Challenge, error)
 	ChallengeTypeEnabled(t string) bool
 }
 

core/objects.go

@@ -389,8 +389,7 @@ type Authorization struct {
 	// slice and the order of these challenges may not be predictable.
 	Challenges []Challenge `json:"challenges,omitempty" db:"-"`
 
-	// The server may suggest combinations of challenges if it
-	// requires more than one challenge to be completed.
+	// This field is deprecated. It's filled in by WFE for the ACMEv1 API.
 	Combinations [][]int `json:"combinations,omitempty" db:"combinations"`
 
 	// Wildcard is a Boulder-specific Authorization field that indicates the

core/proto/core.proto

@@ -61,7 +61,7 @@ message Authorization {
         optional string status = 4;
         optional int64 expires = 5; // Unix timestamp (nanoseconds)
         repeated core.Challenge challenges = 6;
-        optional bytes combinations = 7;
+        optional bytes combinations = 7; // deprecated
         optional bool v2 = 8;
 }
 

csr/csr_test.go

@@ -23,7 +23,7 @@ var testingPolicy = &goodkey.KeyPolicy{
 
 type mockPA struct{}
 
-func (pa *mockPA) ChallengesFor(identifier core.AcmeIdentifier) (challenges []core.Challenge, combinations [][]int, err error) {
+func (pa *mockPA) ChallengesFor(identifier core.AcmeIdentifier) (challenges []core.Challenge, err error) {
 	return
 }
 

grpc/pb-marshalling.go

@@ -6,7 +6,6 @@
 package grpc
 
 import (
-	"encoding/json"
 	"net"
 	"time"
 
@@ -352,10 +351,6 @@ func AuthzToPB(authz core.Authorization) (*corepb.Authorization, error) {
 		}
 		challs[i] = pbChall
 	}
-	comboBytes, err := json.Marshal(authz.Combinations)
-	if err != nil {
-		return nil, err
-	}
 	status := string(authz.Status)
 	var expires int64
 	if authz.Expires != nil {
@@ -368,7 +363,6 @@ func AuthzToPB(authz core.Authorization) (*corepb.Authorization, error) {
 		Status:         &status,
 		Expires:        &expires,
 		Challenges:     challs,
-		Combinations:   comboBytes,
 		V2:             &authz.V2,
 	}, nil
 }
@@ -382,11 +376,6 @@ func PBToAuthz(pb *corepb.Authorization) (core.Authorization, error) {
 		}
 		challs[i] = chall
 	}
-	var combos [][]int
-	err := json.Unmarshal(pb.Combinations, &combos)
-	if err != nil {
-		return core.Authorization{}, err
-	}
 	expires := time.Unix(0, *pb.Expires).UTC()
 	v2 := false
 	if pb.V2 != nil {
@@ -398,7 +387,6 @@ func PBToAuthz(pb *corepb.Authorization) (core.Authorization, error) {
 		Status:         core.AcmeStatus(*pb.Status),
 		Expires:        &expires,
 		Challenges:     challs,
-		Combinations:   combos,
 		V2:             v2,
 	}
 	if pb.Id != nil {

grpc/pb-marshalling_test.go

@@ -268,8 +268,6 @@ func TestRegistration(t *testing.T) {
 func TestAuthz(t *testing.T) {
 	exp := time.Now().AddDate(0, 0, 1).UTC()
 	identifier := core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "example.com"}
-	combos := make([][]int, 1)
-	combos[0] = []int{0, 1}
 	challA := core.Challenge{
 		ID:                       10,
 		Type:                     core.ChallengeTypeDNS01,
@@ -291,7 +289,6 @@ func TestAuthz(t *testing.T) {
 		Status:         core.StatusPending,
 		Expires:        &exp,
 		Challenges:     []core.Challenge{challA, challB},
-		Combinations:   combos,
 	}
 
 	pbAuthz, err := AuthzToPB(inAuthz)

policy/pa.go

@@ -376,9 +376,9 @@ func (pa *AuthorityImpl) checkHostLists(domain string) error {
 	return nil
 }
 
-// ChallengesFor makes a decision of what challenges, and combinations, are
-// acceptable for the given identifier.
-func (pa *AuthorityImpl) ChallengesFor(identifier core.AcmeIdentifier) ([]core.Challenge, [][]int, error) {
+// ChallengesFor makes a decision of what challenges are acceptable for
+// the given identifier.
+func (pa *AuthorityImpl) ChallengesFor(identifier core.AcmeIdentifier) ([]core.Challenge, error) {
 	challenges := []core.Challenge{}
 
 	// If we are using the new authorization storage schema we only use a single
@@ -394,7 +394,7 @@ func (pa *AuthorityImpl) ChallengesFor(identifier core.AcmeIdentifier) ([]core.C
 		// We must have the DNS-01 challenge type enabled to create challenges for
 		// a wildcard identifier per LE policy.
 		if !pa.ChallengeTypeEnabled(core.ChallengeTypeDNS01) {
-			return nil, nil, fmt.Errorf(
+			return nil, fmt.Errorf(
 				"Challenges requested for wildcard identifier but DNS-01 " +
 					"challenge type is not enabled")
 		}
@@ -415,24 +415,17 @@ func (pa *AuthorityImpl) ChallengesFor(identifier core.AcmeIdentifier) ([]core.C
 		}
 	}
 
-	// We shuffle the challenges and combinations to prevent ACME clients from
-	// relying on the specific order that boulder returns them in.
+	// We shuffle the challenges to prevent ACME clients from relying on the
+	// specific order that boulder returns them in.
 	shuffled := make([]core.Challenge, len(challenges))
-	combinations := make([][]int, len(challenges))
 
 	pa.rngMu.Lock()
 	defer pa.rngMu.Unlock()
 	for i, challIdx := range pa.pseudoRNG.Perm(len(challenges)) {
 		shuffled[i] = challenges[challIdx]
-		combinations[i] = []int{i}
 	}
 
-	shuffledCombos := make([][]int, len(combinations))
-	for i, comboIdx := range pa.pseudoRNG.Perm(len(combinations)) {
-		shuffledCombos[i] = combinations[comboIdx]
-	}
-
-	return shuffled, shuffledCombos, nil
+	return shuffled, nil
 }
 
 // ChallengeTypeEnabled returns whether the specified challenge type is enabled

policy/pa_test.go

@@ -312,23 +312,19 @@ var accountKeyJSON = `{
 func TestChallengesFor(t *testing.T) {
 	pa := paImpl(t)
 
-	challenges, combinations, err := pa.ChallengesFor(core.AcmeIdentifier{})
+	challenges, err := pa.ChallengesFor(core.AcmeIdentifier{})
 	test.AssertNotError(t, err, "ChallengesFor failed")
 
 	test.Assert(t, len(challenges) == len(enabledChallenges), "Wrong number of challenges returned")
-	test.Assert(t, len(combinations) == len(enabledChallenges), "Wrong number of combinations returned")
 
 	seenChalls := make(map[string]bool)
-	// Expected only if the pseudo-RNG is seeded with 99.
-	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
 
 		test.Assert(t, enabledChallenges[challenge.Type], "Unsupported challenge returned")
 	}
 	test.AssertEquals(t, len(seenChalls), len(enabledChallenges))
-	test.AssertDeepEquals(t, expectedCombos, combinations)
 
 }
 
@@ -352,7 +348,7 @@ func TestChallengesForWildcard(t *testing.T) {
 		core.ChallengeTypeDNS01:  false,
 	}
 	pa := mustConstructPA(t, enabledChallenges)
-	_, _, err := pa.ChallengesFor(wildcardIdent)
+	_, 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 "+
@@ -362,10 +358,9 @@ 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)
+	challenges, err := pa.ChallengesFor(wildcardIdent)
 	test.AssertNotError(t, err, "ChallengesFor errored for a wildcard ident "+
 		"unexpectedly")
-	test.AssertEquals(t, len(combinations), 1)
 	test.AssertEquals(t, len(challenges), 1)
 	test.AssertEquals(t, challenges[0].Type, core.ChallengeTypeDNS01)
 }

ra/ra.go

@@ -2,7 +2,6 @@ package ra
 
 import (
 	"crypto/x509"
-	"encoding/json"
 	"fmt"
 	"net"
 	"net/mail"
@@ -1646,29 +1645,15 @@ func (ra *RegistrationAuthorityImpl) AdministrativelyRevokeCertificate(ctx conte
 // onValidationUpdate saves a validation's new status after receiving an
 // authorization back from the VA.
 func (ra *RegistrationAuthorityImpl) onValidationUpdate(ctx context.Context, authz core.Authorization) error {
-	// Consider validation successful if any of the combinations
+	// Consider validation successful if any of the challenges
 	// specified in the authorization has been fulfilled
-	validated := map[int]bool{}
-	for i, ch := range authz.Challenges {
+	for _, ch := range authz.Challenges {
 		if ch.Status == core.StatusValid {
-			validated[i] = true
-		}
-	}
-	for _, combo := range authz.Combinations {
-		comboValid := true
-		for _, i := range combo {
-			if !validated[i] {
-				comboValid = false
-				break
-			}
-		}
-		if comboValid {
 			authz.Status = core.StatusValid
 		}
 	}
 
 	// If no validation succeeded, then the authorization is invalid
-	// NOTE: This only works because we only ever do one validation
 	if authz.Status != core.StatusValid {
 		authz.Status = core.StatusInvalid
 	} else {
@@ -1917,7 +1902,7 @@ func (ra *RegistrationAuthorityImpl) createPendingAuthz(ctx context.Context, reg
 	}
 
 	// Create challenges. The WFE will update them with URIs before sending them out.
-	challenges, combinations, err := ra.PA.ChallengesFor(identifier)
+	challenges, 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
@@ -1938,11 +1923,6 @@ func (ra *RegistrationAuthorityImpl) createPendingAuthz(ctx context.Context, reg
 		}
 		authz.Challenges = append(authz.Challenges, challPB)
 	}
-	comboBytes, err := json.Marshal(combinations)
-	if err != nil {
-		return nil, err
-	}
-	authz.Combinations = comboBytes
 	return authz, nil
 }
 

ra/ra_test.go

@@ -138,7 +138,6 @@ var (
 		Identifier:     core.AcmeIdentifier{Type: "dns", Value: "not-example.com"},
 		RegistrationID: 1,
 		Status:         "pending",
-		Combinations:   [][]int{{0}, {1}},
 	}
 	AuthzFinal = core.Authorization{}
 
@@ -287,9 +286,8 @@ func initAuthorities(t *testing.T) (*DummyValidationAuthority, *sa.SQLStorageAut
 
 	AuthzInitial.RegistrationID = Registration.ID
 
-	challenges, combinations, _ := pa.ChallengesFor(AuthzInitial.Identifier)
+	challenges, _ := pa.ChallengesFor(AuthzInitial.Identifier)
 	AuthzInitial.Challenges = challenges
-	AuthzInitial.Combinations = combinations
 
 	AuthzFinal = AuthzInitial
 	AuthzFinal.Status = "valid"
@@ -2401,7 +2399,6 @@ func (sa *mockSAUnsafeAuthzReuse) GetAuthorizations(
 			ID:             "bad-bad-not-good",
 			Identifier:     core.AcmeIdentifier{Type: "dns", Value: "*.zombo.com"},
 			RegistrationID: *req.RegistrationID,
-			Combinations:   [][]int{{0}, {1}},
 			// Authz is valid
 			Status: "valid",
 			Challenges: []core.Challenge{
@@ -2422,7 +2419,6 @@ func (sa *mockSAUnsafeAuthzReuse) GetAuthorizations(
 			ID:             "reused-valid-authz",
 			Identifier:     core.AcmeIdentifier{Type: "dns", Value: "zombo.com"},
 			RegistrationID: *req.RegistrationID,
-			Combinations:   [][]int{{0}, {1}},
 			// Authz is valid
 			Status: "valid",
 			Challenges: []core.Challenge{
@@ -2731,7 +2727,6 @@ func (sa *mockSANearExpiredAuthz) GetAuthorizations(
 					Status: core.StatusValid,
 				},
 			},
-			Combinations: [][]int{{0}, {1}},
 		},
 	}
 	// We can't easily access sa.authzMapToPB so we "inline" it for the mock :-)

sa/model.go

@@ -70,13 +70,13 @@ func selectPendingAuthz(s dbOneSelector, q string, args ...interface{}) (*pendin
 	var model pendingauthzModel
 	err := s.SelectOne(
 		&model,
-		"SELECT id, identifier, registrationID, status, expires, combinations, LockCol FROM pendingAuthorizations "+q,
+		"SELECT id, identifier, registrationID, status, expires, LockCol FROM pendingAuthorizations "+q,
 		args...,
 	)
 	return &model, err
 }
 
-const authzFields = "id, identifier, registrationID, status, expires, combinations"
+const authzFields = "id, identifier, registrationID, status, expires"
 
 // selectAuthz selects all fields of one authorization model
 func selectAuthz(s dbOneSelector, q string, args ...interface{}) (*authzModel, error) {

sa/sa_test.go

@@ -193,12 +193,9 @@ func TestAddAuthorization(t *testing.T) {
 	expectedPa := core.Authorization{ID: PA.ID}
 	test.AssertMarshaledEquals(t, dbPa.ID, expectedPa.ID)
 
-	combos := make([][]int, 1)
-	combos[0] = []int{0, 1}
-
 	exp := time.Now().AddDate(0, 0, 1)
 	identifier := core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "wut.com"}
-	newPa := core.Authorization{ID: PA.ID, Identifier: identifier, RegistrationID: reg.ID, Status: core.StatusPending, Expires: &exp, Combinations: combos}
+	newPa := core.Authorization{ID: PA.ID, Identifier: identifier, RegistrationID: reg.ID, Status: core.StatusPending, Expires: &exp}
 
 	newPa.Status = core.StatusValid
 	err = sa.FinalizeAuthorization(ctx, newPa)
@@ -271,17 +268,13 @@ func CreateDomainAuth(t *testing.T, domainName string, sa *SQLStorageAuthority)
 func CreateDomainAuthWithRegID(t *testing.T, domainName string, sa *SQLStorageAuthority, regID int64) (authz core.Authorization) {
 	exp := sa.clk.Now().AddDate(0, 0, 1) // expire in 1 day
 
-	combos := make([][]int, 1)
-	combos[0] = []int{0, 1}
-
 	// create pending auth
 	authz, err := sa.NewPendingAuthorization(ctx, core.Authorization{
 		Status:         core.StatusPending,
 		Expires:        &exp,
 		Identifier:     core.AcmeIdentifier{Type: core.IdentifierDNS, Value: domainName},
 		RegistrationID: regID,
 		Challenges:     []core.Challenge{{Type: "simpleHttp", Status: core.StatusValid, URI: domainName, Token: "THISWOULDNTBEAGOODTOKEN"}},
-		Combinations:   combos,
 	})
 	if err != nil {
 		t.Fatalf("Couldn't create new pending authorization: %s", err)
@@ -1070,9 +1063,6 @@ func TestDeactivateAuthorization(t *testing.T) {
 	expectedPa := core.Authorization{ID: PA.ID}
 	test.AssertMarshaledEquals(t, dbPa.ID, expectedPa.ID)
 
-	combos := make([][]int, 1)
-	combos[0] = []int{0, 1}
-
 	exp := time.Now().AddDate(0, 0, 1)
 	identifier := core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "wut.com"}
 	newPa := core.Authorization{
@@ -1081,7 +1071,6 @@ func TestDeactivateAuthorization(t *testing.T) {
 		RegistrationID: reg.ID,
 		Status:         core.StatusPending,
 		Expires:        &exp,
-		Combinations:   combos,
 	}
 
 	newPa.Status = core.StatusValid
@@ -1646,7 +1635,6 @@ func TestGetAuthorizations(t *testing.T) {
 		Identifier:     core.AcmeIdentifier{Type: core.IdentifierDNS, Value: identA},
 		Status:         core.StatusPending,
 		Expires:        &exp,
-		Combinations:   [][]int{[]int{0, 1}},
 	}
 
 	// Add the template to create pending authorization A
@@ -1758,7 +1746,6 @@ func TestAddPendingAuthorizations(t *testing.T) {
 	expires := fc.Now().Add(time.Hour).UnixNano()
 	identA := `a`
 	identB := `a`
-	combo := []byte(`[[0]]`)
 	status := string(core.StatusPending)
 	empty := ""
 	authz := []*corepb.Authorization{
@@ -1768,15 +1755,13 @@ func TestAddPendingAuthorizations(t *testing.T) {
 			RegistrationID: &reg.ID,
 			Status:         &status,
 			Expires:        &expires,
-			Combinations:   combo,
 		},
 		&corepb.Authorization{
 			Id:             &empty,
 			Identifier:     &identB,
 			RegistrationID: &reg.ID,
 			Status:         &status,
 			Expires:        &expires,
-			Combinations:   combo,
 		},
 	}