Implement CAA issueWild enforcement for wildcard names (#3266)

This commit implements RFC 6844's description of the "CAA issuewild
property" for CAA records.

We check CAA in two places: at the time of validation, and at the time
of issuance when an authorization is more than 8hours old. Both
locations have been updated to properly enforce issuewild when checking
CAA for a domain corresponding to a wildcard name in a certificate
order.

Resolves https://github.com/letsencrypt/boulder/issues/3211

ra/ra.go

@@ -696,7 +696,10 @@ func (ra *RegistrationAuthorityImpl) checkAuthorizationsCAA(
 	authzs map[string]*core.Authorization,
 	regID int64,
 	now time.Time) error {
-	var badNames, recheckNames []string
+	// badNames contains the names that were unauthorized
+	var badNames []string
+	// recheckNames is a list of names that must have their CAA records rechecked
+	var recheckNames []string
 	// Per Baseline Requirements, CAA must be checked within 8 hours of issuance.
 	// CAA is checked when an authorization is validated, so as long as that was
 	// less than 8 hours ago, we're fine. If it was more than 8 hours ago
@@ -715,6 +718,7 @@ func (ra *RegistrationAuthorityImpl) checkAuthorizationsCAA(
 		} else if authz.Expires.Before(now) {
 			badNames = append(badNames, name)
 		} else if authz.Expires.Before(caaRecheckTime) {
+			// Ensure that CAA is rechecked for this name
 			recheckNames = append(recheckNames, name)
 		}
 	}
@@ -733,6 +737,10 @@ func (ra *RegistrationAuthorityImpl) checkAuthorizationsCAA(
 	return nil
 }
 
+// recheckCAA accepts a list of of names that need to have their CAA records
+// rechecked because their associated authorizations are sufficiently old and
+// performs the CAA checks required for each. If any of the rechecks fail an
+// error is returned.
 func (ra *RegistrationAuthorityImpl) recheckCAA(ctx context.Context, names []string) error {
 	ra.stats.Inc("recheck_caa", 1)
 	ra.stats.Inc("recheck_caa_names", int64(len(names)))

ra/ra_test.go

@@ -1948,10 +1948,33 @@ func (m *mockSAWithRecentAndOlder) GetValidAuthorizations(
 	registrationID int64,
 	names []string,
 	now time.Time) (map[string]*core.Authorization, error) {
+	makeIdentifier := func(name string) core.AcmeIdentifier {
+		return core.AcmeIdentifier{
+			Type:  core.IdentifierDNS,
+			Value: name,
+		}
+	}
 	return map[string]*core.Authorization{
-		"recent.com": &core.Authorization{Expires: &m.recent},
+		"recent.com": &core.Authorization{
-		"older.com":  &core.Authorization{Expires: &m.older},
+			Identifier: makeIdentifier("recent.com"),
-		"older2.com": &core.Authorization{Expires: &m.older},
+			Expires:    &m.recent,
+		},
+		"older.com": &core.Authorization{
+			Identifier: makeIdentifier("older.com"),
+			Expires:    &m.older,
+		},
+		"older2.com": &core.Authorization{
+			Identifier: makeIdentifier("older2.com"),
+			Expires:    &m.older,
+		},
+		"wildcard.com": &core.Authorization{
+			Identifier: makeIdentifier("wildcard.com"),
+			Expires:    &m.older,
+		},
+		"*.wildcard.com": &core.Authorization{
+			Identifier: makeIdentifier("*.wildcard.com"),
+			Expires:    &m.older,
+		},
 	}, nil
 }
 
@@ -1967,20 +1990,42 @@ func TestRecheckCAADates(t *testing.T) {
 		recent: fc.Now().Add(15 * time.Hour),
 		older:  fc.Now().Add(5 * time.Hour),
 	}
-	names := []string{"recent.com", "older.com", "older2.com"}
+
+	// NOTE: The names provided here correspond to authorizations in the
+	// `mockSAWithRecentAndOlder`
+	names := []string{"recent.com", "older.com", "older2.com", "wildcard.com", "*.wildcard.com"}
 	err := ra.checkAuthorizations(context.Background(), names, 999)
+	// We expect that there is no error rechecking authorizations for these names
 	if err != nil {
 		t.Errorf("expected nil err, got %s", err)
 	}
-	if recorder.names["recent.com"] {
+
+	// We expect that "recent.com" is not checked because its mock authorization
+	// isn't expired
+	if _, present := recorder.names["recent.com"]; present {
 		t.Errorf("Rechecked CAA unnecessarily for recent.com")
 	}
-	if !recorder.names["older.com"] {
+
-		t.Errorf("Failed to recheck CAA for older.com %#v", recorder.names)
+	// We expect that "older.com" is checked
-	}
+	if _, present := recorder.names["older.com"]; !present {
-	if !recorder.names["older2.com"] {
 		t.Errorf("Failed to recheck CAA for older.com")
 	}
+
+	// We expect that "older2.com" is checked
+	if _, present := recorder.names["older2.com"]; !present {
+		t.Errorf("Failed to recheck CAA for older2.com")
+	}
+
+	// We expect that the "wildcard.com" domain (without the `*.` prefix) is checked.
+	if _, present := recorder.names["wildcard.com"]; !present {
+		t.Errorf("Failed to recheck CAA for wildcard.com")
+	}
+
+	// We expect that "*.wildcard.com" is checked (with the `*.` prefix, because
+	// it is stripped at a lower layer than we are testing)
+	if _, present := recorder.names["*.wildcard.com"]; !present {
+		t.Errorf("Failed to recheck CAA for *.wildcard.com")
+	}
 }
 
 type caaFailer struct{}

va/caa.go

@@ -35,11 +35,11 @@ func (va *ValidationAuthorityImpl) IsCAAValid(
 	return &vapb.IsCAAValidResponse{}, nil
 }
 
-// TODO(@cpu): `checkCAA` needs to be updated to accept an authorization instead
+// checkCAA performs a CAA lookup & validation for the provided identifier. If
-// of a challenge. Subsequently we should also update the function to check CAA
+// the CAA lookup & validation fail a problem is returned.
-// IssueWild if the authorization's identifier's value has a `*.` prefix (See
+func (va *ValidationAuthorityImpl) checkCAA(
-// #3211)
+	ctx context.Context,
-func (va *ValidationAuthorityImpl) checkCAA(ctx context.Context, identifier core.AcmeIdentifier) *probs.ProblemDetails {
+	identifier core.AcmeIdentifier) *probs.ProblemDetails {
 	present, valid, err := va.checkCAARecords(ctx, identifier)
 	if err != nil {
 		return probs.ConnectionFailure(err.Error())
@@ -155,17 +155,38 @@ func (va *ValidationAuthorityImpl) getCAASet(ctx context.Context, hostname strin
 	return parseResults(results)
 }
 
-func (va *ValidationAuthorityImpl) checkCAARecords(ctx context.Context, identifier core.AcmeIdentifier) (present, valid bool, err error) {
+// checkCAARecords fetches the CAA records for the given identifier and then
+// validates them. If the identifier argument's value has a wildcard prefix then
+// the prefix is stripped and validation will be performed against the base
+// domain, honouring any issueWild CAA records encountered as apppropriate.
+// checkCAARecords returns three values: the first is a bool indicating whether
+// CAA records were present. The second is a bool indicating whether issuance
+// for the identifier is valid. Any errors encountered are returned as the third
+// return value (or nil).
+func (va *ValidationAuthorityImpl) checkCAARecords(
+	ctx context.Context,
+	identifier core.AcmeIdentifier) (present, valid bool, err error) {
 	hostname := strings.ToLower(identifier.Value)
+	// If this is a wildcard name, remove the prefix
+	var wildcard bool
+	if strings.HasPrefix(hostname, `*.`) {
+		hostname = strings.TrimPrefix(identifier.Value, `*.`)
+		wildcard = true
+	}
 	caaSet, err := va.getCAASet(ctx, hostname)
 	if err != nil {
 		return false, false, err
 	}
-	present, valid = va.validateCAASet(caaSet)
+	present, valid = va.validateCAASet(caaSet, wildcard)
 	return present, valid, nil
 }
 
-func (va *ValidationAuthorityImpl) validateCAASet(caaSet *CAASet) (present, valid bool) {
+// validateCAASet checks a provided *CAASet. When the wildcard argument is true
+// this means the CAASet's issueWild records must be validated as well. This
+// function returns two booleans: the first indicates whether the CAASet was
+// empty, the second indicates whether the CAASet is valid for issuance to
+// proceed.
+func (va *ValidationAuthorityImpl) validateCAASet(caaSet *CAASet, wildcard bool) (present, valid bool) {
 	if caaSet == nil {
 		// No CAA records found, can issue
 		va.stats.Inc("CAA.None", 1)
@@ -187,7 +208,7 @@ func (va *ValidationAuthorityImpl) validateCAASet(caaSet *CAASet) (present, vali
 		va.stats.Inc("CAA.WithUnknownNoncritical", 1)
 	}
 
-	if len(caaSet.Issue) == 0 {
+	if len(caaSet.Issue) == 0 && !wildcard {
 		// Although CAA records exist, none of them pertain to issuance in this case.
 		// (e.g. there is only an issuewild directive, but we are checking for a
 		// non-wildcard identifier, or there is only an iodef or non-critical unknown
@@ -196,12 +217,22 @@ func (va *ValidationAuthorityImpl) validateCAASet(caaSet *CAASet) (present, vali
 		return true, true
 	}
 
+	// Per RFC 6844 Section 5.3 "issueWild properties MUST be ignored when
+	// processing a request for a domain that is not a wildcard domain" so we
+	// default to checking the `caaSet.Issue` records and only check
+	// `caaSet.Issuewild` when `wildcard` is true and there is >0 `Issuewild`
+	// records.
+	records := caaSet.Issue
+	if wildcard && len(caaSet.Issuewild) > 0 {
+		records = caaSet.Issuewild
+	}
+
 	// There are CAA records pertaining to issuance in our case. Note that this
 	// includes the case of the unsatisfiable CAA record value ";", used to
 	// prevent issuance by any CA under any circumstance.
 	//
 	// Our CAA identity must be found in the chosen checkSet.
-	for _, caa := range caaSet.Issue {
+	for _, caa := range records {
 		if extractIssuerDomain(caa) == va.issuerDomain {
 			va.stats.Inc("CAA.Authorized", 1)
 			return true, true

va/caa_test.go

@@ -89,6 +89,43 @@ func (mock caaMockDNS) LookupCAA(_ context.Context, domain string) ([]*dns.CAA,
 		record.Tag = "issue"
 		record.Value = ";"
 		results = append(results, &record)
+	case "unsatisfiable-wildcard.com":
+		// Forbidden issuance - issuewild doesn't contain LE
+		record.Tag = "issuewild"
+		record.Value = ";"
+		results = append(results, &record)
+	case "unsatisfiable-wildcard-override.com":
+		// Forbidden issuance - issue allows LE, issuewild overrides and does not
+		record.Tag = "issue"
+		record.Value = "letsencrypt.org"
+		results = append(results, &record)
+		secondRecord := record
+		secondRecord.Tag = "issuewild"
+		secondRecord.Value = "ca.com"
+		results = append(results, &secondRecord)
+	case "satisfiable-wildcard-override.com":
+		// Ok issuance - issue doesn't allow LE, issuewild overrides and does
+		record.Tag = "issue"
+		record.Value = "ca.com"
+		results = append(results, &record)
+		secondRecord := record
+		secondRecord.Tag = "issuewild"
+		secondRecord.Value = "letsencrypt.org"
+		results = append(results, &secondRecord)
+	case "satisfiable-multi-wildcard.com":
+		// Ok issuance - first issuewild doesn't permit LE but second does
+		record.Tag = "issuewild"
+		record.Value = "ca.com"
+		results = append(results, &record)
+		secondRecord := record
+		secondRecord.Tag = "issuewild"
+		secondRecord.Value = "letsencrypt.org"
+		results = append(results, &secondRecord)
+	case "satisfiable-wildcard.com":
+		// Ok issuance - issuewild allows LE
+		record.Tag = "issuewild"
+		record.Value = "letsencrypt.org"
+		results = append(results, &record)
 	}
 	return results, nil
 }
@@ -107,39 +144,133 @@ func TestCAATimeout(t *testing.T) {
 }
 
 func TestCAAChecking(t *testing.T) {
-	type CAATest struct {
+
+	testCases := []struct {
+		Name    string
 		Domain  string
 		Present bool
 		Valid   bool
-	}
+	}{
-	tests := []CAATest{
+		{
-		// Reserved
+			Name:    "Bad (Reserved)",
-		{"reserved.com", true, false},
+			Domain:  "reserved.com",
-		// Critical
+			Present: true,
-		{"critical.com", true, false},
+			Valid:   false,
-		{"nx.critical.com", true, false},
+		},
-		// Good (absent)
+		{
-		{"absent.com", false, true},
+			Name:    "Bad (Critical)",
-		{"example.co.uk", false, true},
+			Domain:  "critical.com",
-		// Good (present)
+			Present: true,
-		{"present.com", true, true},
+			Valid:   false,
-		{"present.servfail.com", true, true},
+		},
-		// Good (multiple critical, one matching)
+		{
-		{"multi-crit-present.com", true, true},
+			Name:    "Bad (NX Critical)",
-		// Bad (unknown critical)
+			Domain:  "nx.critical.com",
-		{"unknown-critical.com", true, false},
+			Present: true,
-		{"unknown-critical2.com", true, false},
+			Valid:   false,
-		// Good (unknown noncritical, no issue/issuewild records)
+		},
-		{"unknown-noncritical.com", true, true},
+		{
-		// Good (issue record with unknown parameters)
+			Name:    "Good (absent)",
-		{"present-with-parameter.com", true, true},
+			Domain:  "absent.com",
-		// Bad (unsatisfiable issue record)
+			Present: false,
-		{"unsatisfiable.com", true, false},
+			Valid:   true,
+		},
+		{
+			Name:    "Good (Example.co.uk, absent)",
+			Domain:  "example.co.uk",
+			Present: false,
+			Valid:   true,
+		},
+		{
+			Name:    "Good (present and valid)",
+			Domain:  "present.com",
+			Present: true,
+			Valid:   true,
+		},
+		{
+			Name:    "Good (Present w/ servfail exception?)",
+			Domain:  "present.servfail.com",
+			Present: true,
+			Valid:   true,
+		},
+		{
+			Name:    "Good (multiple critical, one matching)",
+			Domain:  "multi-crit-present.com",
+			Present: true,
+			Valid:   true,
+		},
+		{
+			Name:    "Bad (unknown critical)",
+			Domain:  "unknown-critical.com",
+			Present: true,
+			Valid:   false,
+		},
+		{
+			Name:    "Bad (unknown critical 2)",
+			Domain:  "unknown-critical2.com",
+			Present: true,
+			Valid:   false,
+		},
+		{
+			Name:    "Good (unknown non-critical, no issue/issuewild)",
+			Domain:  "unknown-noncritical.com",
+			Present: true,
+			Valid:   true,
+		},
+		{
+			Name:    "Good (issue rec with unknown params)",
+			Domain:  "present-with-parameter.com",
+			Present: true,
+			Valid:   true,
+		},
+		{
+			Name:    "Bad (unsatisfiable issue record)",
+			Domain:  "unsatisfiable.com",
+			Present: true,
+			Valid:   false,
+		},
+		{
+			Name:    "Bad (unsatisfiable issue, wildcard)",
+			Domain:  "*.unsatisfiable.com",
+			Present: true,
+			Valid:   false,
+		},
+		{
+			Name:    "Bad (unsatisfiable wildcard)",
+			Domain:  "*.unsatisfiable-wildcard.com",
+			Present: true,
+			Valid:   false,
+		},
+		{
+			Name:    "Bad (unsatisfiable wildcard override)",
+			Domain:  "*.unsatisfiable-wildcard-override.com",
+			Present: true,
+			Valid:   false,
+		},
+		{
+			Name:    "Good (satisfiable wildcard)",
+			Domain:  "*.satisfiable-wildcard.com",
+			Present: true,
+			Valid:   true,
+		},
+		{
+			Name:    "Good (multiple issuewild, one satisfiable)",
+			Domain:  "*.satisfiable-multi-wildcard.com",
+			Present: true,
+			Valid:   true,
+		},
+		{
+			Name:    "Good (satisfiable wildcard override)",
+			Domain:  "*.satisfiable-wildcard-override.com",
+			Present: true,
+			Valid:   true,
+		},
 	}
 
 	va, _ := setup(nil, 0)
 	va.dnsClient = caaMockDNS{}
-	for _, caaTest := range tests {
+	for _, caaTest := range testCases {
+		t.Run(caaTest.Name, func(t *testing.T) {
 			present, valid, err := va.checkCAARecords(ctx, core.AcmeIdentifier{Type: "dns", Value: caaTest.Domain})
 			if err != nil {
 				t.Errorf("checkCAARecords error for %s: %s", caaTest.Domain, err)
@@ -150,6 +281,7 @@ func TestCAAChecking(t *testing.T) {
 			if valid != caaTest.Valid {
 				t.Errorf("checkCAARecords validity mismatch for %s: got %t expected %t", caaTest.Domain, valid, caaTest.Valid)
 			}
+		})
 	}
 
 	present, valid, err := va.checkCAARecords(ctx, core.AcmeIdentifier{Type: "dns", Value: "servfail.com"})

va/proto/va.pb.go

@@ -41,6 +41,7 @@ var _ = math.Inf
 const _ = proto1.ProtoPackageIsVersion2 // please upgrade the proto package
 
 type IsCAAValidRequest struct {
+	// NOTE: Domain may be a name with a wildcard prefix (e.g. `*.example.com`)
 	Domain           *string `protobuf:"bytes,1,opt,name=domain" json:"domain,omitempty"`
 	XXX_unrecognized []byte  `json:"-"`
 }

va/proto/va.proto

@@ -15,6 +15,7 @@ service CAA {
 }
 
 message IsCAAValidRequest {
+	// NOTE: Domain may be a name with a wildcard prefix (e.g. `*.example.com`)
 	optional string domain = 1;
 }
 

va/va.go

@@ -768,18 +768,34 @@ func (va *ValidationAuthorityImpl) validateDNS01(ctx context.Context, identifier
 	return nil, probs.Unauthorized("Correct value not found for DNS challenge")
 }
 
-// TODO(@cpu): `validateChallengeAndCAA` needs to be updated to accept an
+// validateChallengeAndCAA performs a challenge validation and CAA validation
-// authorization instead of a challenge. Subsequently we should also update the
+// for the provided identifier and a corresponding challenge. If the validation
-// function to check CAA IssueWild if the authorization's identifier's value has
+// or CAA lookup fail a problem is returned along with the validation records
-// a `*.` prefix (See #3211)
+// created during the validation attempt.
-func (va *ValidationAuthorityImpl) validateChallengeAndCAA(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) {
+func (va *ValidationAuthorityImpl) validateChallengeAndCAA(
+	ctx context.Context,
+	identifier core.AcmeIdentifier,
+	challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) {
+
+	// If the identifier is a wildcard domain we need to validate the base
+	// domain by removing the "*." wildcard prefix. We create a separate
+	// `baseIdentifier` here before starting the `va.checkCAA` goroutine with the
+	// `identifier` to avoid a data race.
+	baseIdentifier := identifier
+	if strings.HasPrefix(identifier.Value, "*.") {
+		baseIdentifier.Value = strings.TrimPrefix(identifier.Value, "*.")
+	}
+
+	// va.checkCAA accepts wildcard identifiers and handles them appropriately so
+	// we can dispatch `checkCAA` with the provided `identifier` instead of
+	// `baseIdentifier`
 	ch := make(chan *probs.ProblemDetails, 1)
 	go func() {
 		ch <- va.checkCAA(ctx, identifier)
 	}()
 
 	// TODO(#1292): send into another goroutine
-	validationRecords, err := va.validateChallenge(ctx, identifier, challenge)
+	validationRecords, err := va.validateChallenge(ctx, baseIdentifier, challenge)
 	if err != nil {
 		return validationRecords, err
 	}
@@ -879,19 +895,16 @@ func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, domain
 	}
 	vStart := va.clk.Now()
 
-	// If the identifier is a wildcard domain we need to validate the base
-	// domain by removing the "*." wildcard prefix.
-	if strings.HasPrefix(domain, "*.") {
-		domain = strings.TrimPrefix(domain, "*.")
-	}
-
 	var remoteError chan *probs.ProblemDetails
 	if len(va.remoteVAs) > 0 {
 		remoteError = make(chan *probs.ProblemDetails, 1)
 		go va.performRemoteValidation(ctx, domain, challenge, authz, remoteError)
 	}
 
-	records, prob := va.validateChallengeAndCAA(ctx, core.AcmeIdentifier{Type: "dns", Value: domain}, challenge)
+	records, prob := va.validateChallengeAndCAA(
+		ctx,
+		core.AcmeIdentifier{Type: "dns", Value: domain},
+		challenge)
 
 	logEvent.ValidationRecords = records
 	challenge.ValidationRecord = records