diff --git a/va/caa.go b/va/caa.go index 475aa57b6eb..0e4860df8ba 100644 --- a/va/caa.go +++ b/va/caa.go @@ -130,7 +130,7 @@ func (va *ValidationAuthorityImpl) checkCAA( return errors.New("expected validationMethod or accountURIID not provided to checkCAA") } - foundAt, valid, response, err := va.checkCAARecords(ctx, ident, params) + foundAt, valid, response, denyDetail, err := va.checkCAARecords(ctx, ident, params) if err != nil { return berrors.DNSError("%s", err) } @@ -145,6 +145,9 @@ func (va *ValidationAuthorityImpl) checkCAA( "response": response, }) if !valid { + if denyDetail != "" { + return berrors.CAAError("CAA record for %s prevents issuance: %s", foundAt, denyDetail) + } return berrors.CAAError("CAA record for %s prevents issuance", foundAt) } return nil @@ -292,7 +295,7 @@ func (va *ValidationAuthorityImpl) getCAA(ctx context.Context, hostname string) func (va *ValidationAuthorityImpl) checkCAARecords( ctx context.Context, ident identifier.ACMEIdentifier, - params *caaParams) (string, bool, string, error) { + params *caaParams) (string, bool, string, string, error) { hostname := strings.ToLower(ident.Value) // If this is a wildcard name, remove the prefix var wildcard bool @@ -302,14 +305,14 @@ func (va *ValidationAuthorityImpl) checkCAARecords( } caaSet, err := va.getCAA(ctx, hostname) if err != nil { - return "", false, "", err + return "", false, "", "", err } raw := "" if caaSet != nil { raw = caaSet.dig } - valid, foundAt := va.validateCAA(caaSet, wildcard, params) - return foundAt, valid, raw, nil + valid, foundAt, denyDetail := va.validateCAA(caaSet, wildcard, params) + return foundAt, valid, raw, denyDetail, nil } // validateCAA checks a provided *caaResult. When the wildcard argument is true @@ -318,17 +321,17 @@ func (va *ValidationAuthorityImpl) checkCAARecords( // records, and a string indicating the name at which the CAA records allowing // issuance were found (if any -- since finding no records at all allows // issuance). -func (va *ValidationAuthorityImpl) validateCAA(caaSet *caaResult, wildcard bool, params *caaParams) (bool, string) { +func (va *ValidationAuthorityImpl) validateCAA(caaSet *caaResult, wildcard bool, params *caaParams) (bool, string, string) { if caaSet == nil { // No CAA records found, can issue va.metrics.caaCounter.WithLabelValues("no records").Inc() - return true, "" + return true, "", "" } if caaSet.criticalUnknown { // Contains unknown critical directives va.metrics.caaCounter.WithLabelValues("record with unknown critical directive").Inc() - return false, caaSet.name + return false, caaSet.name, "" } // Per RFC 8659 Section 5.3: @@ -351,7 +354,23 @@ func (va *ValidationAuthorityImpl) validateCAA(caaSet *caaResult, wildcard bool, // non-wildcard identifier, or there is only an iodef or non-critical unknown // directive.) va.metrics.caaCounter.WithLabelValues("no relevant records").Inc() - return true, caaSet.name + return true, caaSet.name, "" + } + + nonStandardCapitalizationDetail := func(badTag, expectedLower string) string { + return fmt.Sprintf("CAA parameter tag %q has invalid capitalization; use %q", badTag, expectedLower) + } + + nonStandardCapitalization := func(parsedParams []caaParameter) (badTag string, expectedLower string, ok bool) { + for _, param := range parsedParams { + if strings.EqualFold(param.tag, "accounturi") && param.tag != "accounturi" { + return param.tag, "accounturi", true + } + if strings.EqualFold(param.tag, "validationmethods") && param.tag != "validationmethods" { + return param.tag, "validationmethods", true + } + } + return "", "", false } // There are CAA records pertaining to issuance in our case. Note that this @@ -369,6 +388,11 @@ func (va *ValidationAuthorityImpl) validateCAA(caaSet *caaResult, wildcard bool, continue } + if badTag, expectedLower, ok := nonStandardCapitalization(parsedParams); ok { + va.metrics.caaCounter.WithLabelValues("invalid parameter tag capitalization").Inc() + return false, caaSet.name, nonStandardCapitalizationDetail(badTag, expectedLower) + } + if !caaAccountURIMatches(parsedParams, va.accountURIPrefixes, params.accountURIID) { continue } @@ -378,12 +402,12 @@ func (va *ValidationAuthorityImpl) validateCAA(caaSet *caaResult, wildcard bool, } va.metrics.caaCounter.WithLabelValues("authorized").Inc() - return true, caaSet.name + return true, caaSet.name, "" } // The list of authorized issuers is non-empty, but we are not in it. Fail. va.metrics.caaCounter.WithLabelValues("unauthorized").Inc() - return false, caaSet.name + return false, caaSet.name, "" } // caaParameter is a key-value pair parsed from a single CAA RR. diff --git a/va/caa_test.go b/va/caa_test.go index a7cdf2b64d5..f6e73ba43e5 100644 --- a/va/caa_test.go +++ b/va/caa_test.go @@ -100,6 +100,10 @@ func (mock *caaFakeDNS) LookupCAA(_ context.Context, domain string) (*bdns.Resul record.Tag = "issue" record.Value = "letsencrypt.org; validationmethods=dns-01" results = append(results, &record) + case "present-mixedcase-validationmethods.com": + record.Tag = "issue" + record.Value = "letsencrypt.org; ValidationMethods=dns-01" + results = append(results, &record) case "present-http-only.com": record.Tag = "issue" record.Value = "letsencrypt.org; validationmethods=http-01" @@ -120,6 +124,10 @@ func (mock *caaFakeDNS) LookupCAA(_ context.Context, domain string) (*bdns.Resul record.Tag = "issue" record.Value = "letsencrypt.org; accounturi=https://letsencrypt.org/acct/reg/321; validationmethods=http-01" results = append(results, &record) + case "present-mixedcase-accounturi.com": + record.Tag = "issue" + record.Value = "letsencrypt.org; AccountURI=https://letsencrypt.org/acct/reg/321" + results = append(results, &record) case "present-correct-accounturi.com": record.Tag = "issue" record.Value = "letsencrypt.org; accounturi=https://letsencrypt.org/acct/reg/123" @@ -128,6 +136,10 @@ func (mock *caaFakeDNS) LookupCAA(_ context.Context, domain string) (*bdns.Resul record.Tag = "issue" record.Value = "letsencrypt.org; accounturi=https://letsencrypt.org/acct/reg/321" results = append(results, &record) + case "present-wildcard-mixedcase-validationmethods.com": + record.Tag = "issuewild" + record.Value = "letsencrypt.org; ValidationMethods=dns-01" + results = append(results, &record) case "present-multiple-accounturi.com": record.Tag = "issue" record.Value = "letsencrypt.org; accounturi=https://letsencrypt.org/acct/reg/321" @@ -310,6 +322,12 @@ func TestCAAChecking(t *testing.T) { FoundAt: "present-dns-only.com", Valid: false, }, + { + Name: "Bad (restricts to dns-01, mixed-case validationmethods tag)", + Domain: "present-mixedcase-validationmethods.com", + FoundAt: "present-mixedcase-validationmethods.com", + Valid: false, + }, { Name: "Good (restricts to http-01, tested with http-01)", Domain: "present-http-only.com", @@ -352,6 +370,12 @@ func TestCAAChecking(t *testing.T) { FoundAt: "present-incorrect-accounturi.com", Valid: false, }, + { + Name: "Bad (restricts to accounturi, mixed-case accounturi tag)", + Domain: "present-mixedcase-accounturi.com", + FoundAt: "present-mixedcase-accounturi.com", + Valid: false, + }, { Name: "Good (restricts to multiple accounturi, tested with a correct account)", Domain: "present-multiple-accounturi.com", @@ -388,6 +412,12 @@ func TestCAAChecking(t *testing.T) { FoundAt: "satisfiable-wildcard.com", Valid: true, }, + { + Name: "Bad (restricts to dns-01, mixed-case validationmethods tag, wildcard)", + Domain: "*.present-wildcard-mixedcase-validationmethods.com", + FoundAt: "present-wildcard-mixedcase-validationmethods.com", + Valid: false, + }, { Name: "Good (multiple issuewild, one satisfiable)", Domain: "*.satisfiable-multi-wildcard.com", @@ -414,7 +444,7 @@ func TestCAAChecking(t *testing.T) { defer mockLog.Clear() t.Run(caaTest.Name, func(t *testing.T) { ident := identifier.NewDNS(caaTest.Domain) - foundAt, valid, _, err := va.checkCAARecords(ctx, ident, params) + foundAt, valid, _, _, err := va.checkCAARecords(ctx, ident, params) if err != nil { t.Errorf("checkCAARecords error for %s: %s", caaTest.Domain, err) } @@ -517,6 +547,35 @@ func TestCAALogging(t *testing.T) { } } +func TestCAAMisCapitalizedStandardParameterTagFailsClosedWithHelpfulError(t *testing.T) { + va, _ := setup(nil, "", nil, &caaFakeDNS{}) + va.accountURIPrefixes = []string{"https://letsencrypt.org/acct/reg/"} + + resp, err := va.DoCAA(ctx, &vapb.IsCAAValidRequest{ + Identifier: identifier.NewDNS("present-mixedcase-accounturi.com").ToProto(), + ValidationMethod: string(core.ChallengeTypeHTTP01), + AccountURIID: 123, + }) + test.AssertNotError(t, err, "Unexpected error calling DoCAA") + test.AssertNotNil(t, resp, "Response to IsCAAValidRequest was nil") + test.AssertNotNil(t, resp.Problem, "Response Problem was nil") + test.AssertContains(t, resp.Problem.Detail, "invalid capitalization") + test.AssertContains(t, resp.Problem.Detail, "AccountURI") + test.AssertContains(t, resp.Problem.Detail, "accounturi") + + resp, err = va.DoCAA(ctx, &vapb.IsCAAValidRequest{ + Identifier: identifier.NewDNS("present-mixedcase-validationmethods.com").ToProto(), + ValidationMethod: string(core.ChallengeTypeHTTP01), + AccountURIID: 123, + }) + test.AssertNotError(t, err, "Unexpected error calling DoCAA") + test.AssertNotNil(t, resp, "Response to IsCAAValidRequest was nil") + test.AssertNotNil(t, resp.Problem, "Response Problem was nil") + test.AssertContains(t, resp.Problem.Detail, "invalid capitalization") + test.AssertContains(t, resp.Problem.Detail, "ValidationMethods") + test.AssertContains(t, resp.Problem.Detail, "validationmethods") +} + // TestDoCAAErrMessage tests that an error result from `va.IsCAAValid` // includes the domain name that was being checked in the failure detail. func TestDoCAAErrMessage(t *testing.T) {