letsencrypt
/
boulder
mirror of https://github.com/letsencrypt/boulder.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

caa: Reject multiple "accounturi" and "validationmethods" Parameter Tags (#7565) 

Prevent misconfigured DNS CAA entries from passing CAA checks.
Previously, boulder would take the last `accounturi` and
`validationmethods` in the event there were multiple, cause by DNS
misconfiguration. An example of what that looks like is below.
```
example.com CAA 0 issue "example.net; accounturi=https://example.net/acme/acct/123; accounturi=https://example.net/acme/acct/456;"
example.com CAA 0 issue "example.net; validationmethods=dns-01,http-01; validationmethods=tls-alpn-01;"
```

[RFC 8567 Section
3](https://www.rfc-editor.org/rfc/rfc8657#name-extensions-to-the-caa-recor)
states the following. The RFC does not explicitly state how to handle
multiple `validationmethods`, but I chose to be consistent with
`accounturi` and reject multiple parameter tags there too.
> A Property without an "accounturi" parameter matches any account. A
Property with an invalid or unrecognized "accounturi" parameter is
unsatisfiable. A Property with multiple "accounturi" parameters is
unsatisfiable.
This commit is contained in:
Phil Porada 2024-07-03 09:54:50 -04:00 committed by GitHub
parent cb6fcc2889
commit 55ea152381
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 260 additions and 109 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

92
va/caa.go
View File

@ -532,13 +532,19 @@ func (va *ValidationAuthorityImpl) validateCAA(caaSet *caaResult, wildcard bool,
return false, caaSet.name
}
// caaParameter is a key-value pair parsed from a single CAA RR.
type caaParameter struct {
tag string
val string
}
// parseCAARecord extracts the domain and parameters (if any) from a
// issue/issuewild CAA record. This follows RFC 8659 Section 4.2 and Section 4.3
// (https://www.rfc-editor.org/rfc/rfc8659.html#section-4). It returns the
// domain name (which may be the empty string if the record forbids issuance)
// and a tag-value map of CAA parameters, or a descriptive error if the record
// is malformed.
func parseCAARecord(caa *dns.CAA) (string, map[string]string, error) {
// and a slice of CAA parameters, or a descriptive error if the record is
// malformed.
func parseCAARecord(caa *dns.CAA) (string, []caaParameter, error) {
isWSP := func(r rune) bool {
return r == '\t' || r == ' '
}
@ -546,16 +552,21 @@ func parseCAARecord(caa *dns.CAA) (string, map[string]string, error) {
// Semi-colons (ASCII 0x3B) are prohibited from being specified in the
// parameter tag or value, hence we can simply split on semi-colons.
parts := strings.Split(caa.Value, ";")
domain := strings.TrimFunc(parts[0], isWSP)
// See https://www.rfc-editor.org/rfc/rfc8659.html#section-4.2
//
// issuer-domain-name = label *("." label)
// label = (ALPHA / DIGIT) *( *("-") (ALPHA / DIGIT))
issuerDomainName := strings.TrimFunc(parts[0], isWSP)
paramList := parts[1:]
parameters := make(map[string]string)
// Handle the case where a semi-colon is specified following the domain
// but no parameters are given.
if len(paramList) == 1 && strings.TrimFunc(paramList[0], isWSP) == "" {
return domain, parameters, nil
return issuerDomainName, nil, nil
}
var caaParameters []caaParameter
for _, parameter := range paramList {
// A parameter tag cannot include equal signs (ASCII 0x3D),
// however they are permitted in the value itself.
@ -584,10 +595,13 @@ func parseCAARecord(caa *dns.CAA) (string, map[string]string, error) {
}
}
parameters[tag] = value
caaParameters = append(caaParameters, caaParameter{
tag: tag,
val: value,
})
}
return domain, parameters, nil
return issuerDomainName, caaParameters, nil
}
// caaDomainMatches checks that the issuer domain name listed in the parsed
@ -599,10 +613,26 @@ func caaDomainMatches(caaDomain string, issuerDomain string) bool {
// caaAccountURIMatches checks that the accounturi CAA parameter, if present,
// matches one of the specific account URIs we expect. We support multiple
// account URI prefixes to handle accounts which were registered under ACMEv1.
// We accept only a single "accounturi" parameter and will fail if multiple are
// found in the CAA RR.
// See RFC 8657 Section 3: https://www.rfc-editor.org/rfc/rfc8657.html#section-3
func caaAccountURIMatches(caaParams map[string]string, accountURIPrefixes []string, accountID int64) bool {
accountURI, ok := caaParams["accounturi"]
if !ok {
func caaAccountURIMatches(caaParams []caaParameter, accountURIPrefixes []string, accountID int64) bool {
var found bool
var accountURI string
for _, c := range caaParams {
if c.tag == "accounturi" {
if found {
// A Property with multiple "accounturi" parameters is
// unsatisfiable.
return false
}
accountURI = c.val
found = true
}
}
if !found {
// A Property without an "accounturi" parameter matches any account.
return true
}
@ -624,17 +654,39 @@ var validationMethodRegexp = regexp.MustCompile(`^[[:alnum:]-]+$`)
// caaValidationMethodMatches checks that the validationmethods CAA parameter,
// if present, contains the exact name of the ACME validation method used to
// validate this domain.
// See RFC 8657 Section 4: https://www.rfc-editor.org/rfc/rfc8657.html#section-4
func caaValidationMethodMatches(caaParams map[string]string, method core.AcmeChallenge) bool {
commaSeparatedMethods, ok := caaParams["validationmethods"]
if !ok {
// validate this domain. We accept only a single "validationmethods" parameter
// and will fail if multiple are found in the CAA RR, even if all tag-value
// pairs would be valid. See RFC 8657 Section 4:
// https://www.rfc-editor.org/rfc/rfc8657.html#section-4.
func caaValidationMethodMatches(caaParams []caaParameter, method core.AcmeChallenge) bool {
var validationMethods string
var found bool
for _, param := range caaParams {
if param.tag == "validationmethods" {
if found {
// RFC 8657 does not define what behavior to take when multiple
// "validationmethods" parameters exist, but we make the
// conscious choice to fail validation similar to how multiple
// "accounturi" parameters are "unsatisfiable". Subscribers
// should be aware of RFC 8657 Section 5.8:
// https://www.rfc-editor.org/rfc/rfc8657.html#section-5.8
return false
}
validationMethods = param.val
found = true
}
}
if !found {
return true
}
for _, m := range strings.Split(commaSeparatedMethods, ",") {
// If any listed method does not match the ABNF 1*(ALPHA / DIGIT / "-"),
// immediately reject the whole record.
for _, m := range strings.Split(validationMethods, ",") {
// The value of the "validationmethods" parameter MUST comply with the
// following ABNF [RFC5234]:
//
// value = [*(label ",") label]
// label = 1*(ALPHA / DIGIT / "-")
if !validationMethodRegexp.MatchString(m) {
return false
}
@ -643,10 +695,10 @@ func caaValidationMethodMatches(caaParams map[string]string, method core.AcmeCha
if !caaMethod.IsValid() {
continue
}
if caaMethod == method {
return true
}
}
return false
}

277
va/caa_test.go
View File

@ -1109,16 +1109,17 @@ func TestSelectCAA(t *testing.T) {
}
func TestAccountURIMatches(t *testing.T) {
t.Parallel()
tests := []struct {
name string
params map[string]string
params []caaParameter
prefixes []string
id int64
want bool
}{
{
name: "empty accounturi",
params: map[string]string{},
params: nil,
prefixes: []string{
"https://acme-v01.api.letsencrypt.org/acme/reg/",
},
@ -1126,10 +1127,17 @@ func TestAccountURIMatches(t *testing.T) {
want: true,
},
{
name: "non-uri accounturi",
params: map[string]string{
"accounturi": "\\invalid 😎/123456",
name: "no accounturi in rr, but other parameters exist",
params: []caaParameter{{tag: "validationmethods", val: "tls-alpn-01"}},
prefixes: []string{
"https://acme-v02.api.letsencrypt.org/acme/reg/",
},
id: 123456,
want: true,
},
{
name: "non-uri accounturi",
params: []caaParameter{{tag: "accounturi", val: "\\invalid 😎/123456"}},
prefixes: []string{
"\\invalid 😎",
},
@ -1137,10 +1145,8 @@ func TestAccountURIMatches(t *testing.T) {
want: false,
},
{
name: "simple match",
params: map[string]string{
"accounturi": "https://acme-v01.api.letsencrypt.org/acme/reg/123456",
},
name: "simple match",
params: []caaParameter{{tag: "accounturi", val: "https://acme-v01.api.letsencrypt.org/acme/reg/123456"}},
prefixes: []string{
"https://acme-v01.api.letsencrypt.org/acme/reg/",
},
@ -1148,10 +1154,17 @@ func TestAccountURIMatches(t *testing.T) {
want: true,
},
{
name: "accountid mismatch",
params: map[string]string{
"accounturi": "https://acme-v01.api.letsencrypt.org/acme/reg/123456",
name: "simple match, but has a friend",
params: []caaParameter{{tag: "validationmethods", val: "dns-01"}, {tag: "accounturi", val: "https://acme-v01.api.letsencrypt.org/acme/reg/123456"}},
prefixes: []string{
"https://acme-v01.api.letsencrypt.org/acme/reg/",
},
id: 123456,
want: true,
},
{
name: "accountid mismatch",
params: []caaParameter{{tag: "accounturi", val: "https://acme-v01.api.letsencrypt.org/acme/reg/123456"}},
prefixes: []string{
"https://acme-v01.api.letsencrypt.org/acme/reg/",
},
@ -1159,10 +1172,53 @@ func TestAccountURIMatches(t *testing.T) {
want: false,
},
{
name: "multiple prefixes, match first",
params: map[string]string{
"accounturi": "https://acme-staging.api.letsencrypt.org/acme/reg/123456",
name: "single parameter, no value",
params: []caaParameter{{tag: "accounturi", val: ""}},
prefixes: []string{
"https://acme-v02.api.letsencrypt.org/acme/reg/",
},
id: 123456,
want: false,
},
{
name: "multiple parameters, each with no value",
params: []caaParameter{{tag: "accounturi", val: ""}, {tag: "accounturi", val: ""}},
prefixes: []string{
"https://acme-v02.api.letsencrypt.org/acme/reg/",
},
id: 123456,
want: false,
},
{
name: "multiple parameters, one with no value",
params: []caaParameter{{tag: "accounturi", val: ""}, {tag: "accounturi", val: "https://acme-v02.api.letsencrypt.org/acme/reg/123456"}},
prefixes: []string{
"https://acme-v02.api.letsencrypt.org/acme/reg/",
},
id: 123456,
want: false,
},
{
name: "multiple parameters, each with an identical value",
params: []caaParameter{{tag: "accounturi", val: "https://acme-v02.api.letsencrypt.org/acme/reg/123456"}, {tag: "accounturi", val: "https://acme-v02.api.letsencrypt.org/acme/reg/123456"}},
prefixes: []string{
"https://acme-v02.api.letsencrypt.org/acme/reg/",
},
id: 123456,
want: false,
},
{
name: "multiple parameters, each with a different value",
params: []caaParameter{{tag: "accounturi", val: "https://acme-v02.api.letsencrypt.org/acme/reg/69"}, {tag: "accounturi", val: "https://acme-v02.api.letsencrypt.org/acme/reg/420"}},
prefixes: []string{
"https://acme-v02.api.letsencrypt.org/acme/reg/",
},
id: 69,
want: false,
},
{
name: "multiple prefixes, match first",
params: []caaParameter{{tag: "accounturi", val: "https://acme-staging.api.letsencrypt.org/acme/reg/123456"}},
prefixes: []string{
"https://acme-staging.api.letsencrypt.org/acme/reg/",
"https://acme-staging-v02.api.letsencrypt.org/acme/acct/",
@ -1171,10 +1227,8 @@ func TestAccountURIMatches(t *testing.T) {
want: true,
},
{
name: "multiple prefixes, match second",
params: map[string]string{
"accounturi": "https://acme-v02.api.letsencrypt.org/acme/acct/123456",
},
name: "multiple prefixes, match second",
params: []caaParameter{{tag: "accounturi", val: "https://acme-v02.api.letsencrypt.org/acme/acct/123456"}},
prefixes: []string{
"https://acme-v01.api.letsencrypt.org/acme/reg/",
"https://acme-v02.api.letsencrypt.org/acme/acct/",
@ -1183,10 +1237,8 @@ func TestAccountURIMatches(t *testing.T) {
want: true,
},
{
name: "multiple prefixes, match none",
params: map[string]string{
"accounturi": "https://acme-v02.api.letsencrypt.org/acme/acct/123456",
},
name: "multiple prefixes, match none",
params: []caaParameter{{tag: "accounturi", val: "https://acme-v02.api.letsencrypt.org/acme/acct/123456"}},
prefixes: []string{
"https://acme-v01.api.letsencrypt.org/acme/acct/",
"https://acme-v03.api.letsencrypt.org/acme/acct/",
@ -1195,10 +1247,8 @@ func TestAccountURIMatches(t *testing.T) {
want: false,
},
{
name: "three prefixes",
params: map[string]string{
"accounturi": "https://acme-v02.api.letsencrypt.org/acme/acct/123456",
},
name: "three prefixes",
params: []caaParameter{{tag: "accounturi", val: "https://acme-v02.api.letsencrypt.org/acme/acct/123456"}},
prefixes: []string{
"https://acme-v01.api.letsencrypt.org/acme/reg/",
"https://acme-v02.api.letsencrypt.org/acme/acct/",
@ -1208,10 +1258,8 @@ func TestAccountURIMatches(t *testing.T) {
want: true,
},
{
name: "multiple prefixes, wrong accountid",
params: map[string]string{
"accounturi": "https://acme-v02.api.letsencrypt.org/acme/acct/123456",
},
name: "multiple prefixes, wrong accountid",
params: []caaParameter{{tag: "accounturi", val: "https://acme-v02.api.letsencrypt.org/acme/acct/123456"}},
prefixes: []string{
"https://acme-v01.api.letsencrypt.org/acme/reg/",
"https://acme-v02.api.letsencrypt.org/acme/acct/",
@ -1222,7 +1270,10 @@ func TestAccountURIMatches(t *testing.T) {
}
for _, tc := range tests {
// TODO(#7454) Remove this rebinding
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
got := caaAccountURIMatches(tc.params, tc.prefixes, tc.id)
test.AssertEquals(t, got, tc.want)
})
@ -1230,86 +1281,116 @@ func TestAccountURIMatches(t *testing.T) {
}
func TestValidationMethodMatches(t *testing.T) {
t.Parallel()
tests := []struct {
name string
params map[string]string
params []caaParameter
method core.AcmeChallenge
want bool
}{
{
name: "empty validationmethods",
params: map[string]string{},
params: nil,
method: core.ChallengeTypeHTTP01,
want: true,
},
{
name: "only comma",
params: map[string]string{
"validationmethods": ",",
},
method: core.ChallengeTypeHTTP01,
want: false,
},
{
name: "malformed method",
params: map[string]string{
"validationmethods": "howdy !",
},
method: core.ChallengeTypeHTTP01,
want: false,
},
{
name: "invalid method",
params: map[string]string{
"validationmethods": "tls-sni-01",
},
method: core.ChallengeTypeHTTP01,
want: false,
},
{
name: "simple match",
params: map[string]string{
"validationmethods": "http-01",
},
name: "no validationmethods in rr, but other parameters exist", // validationmethods is not mandatory
params: []caaParameter{{tag: "accounturi", val: "ph1LwuzHere"}},
method: core.ChallengeTypeHTTP01,
want: true,
},
{
name: "simple mismatch",
params: map[string]string{
"validationmethods": "dns-01",
},
name: "no value",
params: []caaParameter{{tag: "validationmethods", val: ""}}, // equivalent to forbidding issuance
method: core.ChallengeTypeHTTP01,
want: false,
},
{
name: "multiple choices, match first",
params: map[string]string{
"validationmethods": "http-01,dns-01",
},
name: "only comma",
params: []caaParameter{{tag: "validationmethods", val: ","}},
method: core.ChallengeTypeHTTP01,
want: false,
},
{
name: "malformed method",
params: []caaParameter{{tag: "validationmethods", val: "howdy !"}},
method: core.ChallengeTypeHTTP01,
want: false,
},
{
name: "invalid method",
params: []caaParameter{{tag: "validationmethods", val: "tls-sni-01"}},
method: core.ChallengeTypeHTTP01,
want: false,
},
{
name: "simple match",
params: []caaParameter{{tag: "validationmethods", val: "http-01"}},
method: core.ChallengeTypeHTTP01,
want: true,
},
{
name: "multiple choices, match second",
params: map[string]string{
"validationmethods": "http-01,dns-01",
},
name: "simple match, but has a friend",
params: []caaParameter{{tag: "accounturi", val: "https://example.org"}, {tag: "validationmethods", val: "http-01"}},
method: core.ChallengeTypeHTTP01,
want: true,
},
{
name: "multiple validationmethods, each with no value",
params: []caaParameter{{tag: "validationmethods", val: ""}, {tag: "validationmethods", val: ""}},
method: core.ChallengeTypeHTTP01,
want: false,
},
{
name: "multiple validationmethods, one with no value",
params: []caaParameter{{tag: "validationmethods", val: ""}, {tag: "validationmethods", val: "http-01"}},
method: core.ChallengeTypeHTTP01,
want: false,
},
{
name: "multiple validationmethods, each with an identical value",
params: []caaParameter{{tag: "validationmethods", val: "http-01"}, {tag: "validationmethods", val: "http-01"}},
method: core.ChallengeTypeHTTP01,
want: false,
},
{
name: "multiple validationmethods, each with a different value",
params: []caaParameter{{tag: "validationmethods", val: "http-01"}, {tag: "validationmethods", val: "dns-01"}},
method: core.ChallengeTypeHTTP01,
want: false,
},
{
name: "simple mismatch",
params: []caaParameter{{tag: "validationmethods", val: "dns-01"}},
method: core.ChallengeTypeHTTP01,
want: false,
},
{
name: "multiple choices, match first",
params: []caaParameter{{tag: "validationmethods", val: "http-01,dns-01"}},
method: core.ChallengeTypeHTTP01,
want: true,
},
{
name: "multiple choices, match second",
params: []caaParameter{{tag: "validationmethods", val: "http-01,dns-01"}},
method: core.ChallengeTypeDNS01,
want: true,
},
{
name: "multiple choices, match none",
params: map[string]string{
"validationmethods": "http-01,dns-01",
},
name: "multiple choices, match none",
params: []caaParameter{{tag: "validationmethods", val: "http-01,dns-01"}},
method: core.ChallengeTypeTLSALPN01,
want: false,
},
}
for _, tc := range tests {
// TODO(#7454) Remove this rebinding
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
got := caaValidationMethodMatches(tc.params, tc.method)
test.AssertEquals(t, got, tc.want)
})
@ -1317,81 +1398,96 @@ func TestValidationMethodMatches(t *testing.T) {
}
func TestExtractIssuerDomainAndParameters(t *testing.T) {
t.Parallel()
tests := []struct {
name string
value string
wantDomain string
wantParameters map[string]string
wantParameters []caaParameter
expectErrSubstr string
}{
{
name: "empty record is valid",
value: "",
wantDomain: "",
wantParameters: map[string]string{},
wantParameters: nil,
expectErrSubstr: "",
},
{
name: "only semicolon is valid",
value: ";",
wantDomain: "",
wantParameters: map[string]string{},
wantParameters: nil,
expectErrSubstr: "",
},
{
name: "only semicolon and whitespace is valid",
value: " ; ",
wantDomain: "",
wantParameters: map[string]string{},
wantParameters: nil,
expectErrSubstr: "",
},
{
name: "only domain is valid",
value: "letsencrypt.org",
wantDomain: "letsencrypt.org",
wantParameters: map[string]string{},
wantParameters: nil,
expectErrSubstr: "",
},
{
name: "only domain with trailing semicolon is valid",
value: "letsencrypt.org;",
wantDomain: "letsencrypt.org",
wantParameters: map[string]string{},
wantParameters: nil,
expectErrSubstr: "",
},
{
name: "only domain with semicolon and trailing whitespace is valid",
value: "letsencrypt.org; ",
wantDomain: "letsencrypt.org",
wantParameters: nil,
expectErrSubstr: "",
},
{
name: "domain with params and whitespace is valid",
value: " letsencrypt.org ;foo=bar;baz=bar",
wantDomain: "letsencrypt.org",
wantParameters: map[string]string{"foo": "bar", "baz": "bar"},
wantParameters: []caaParameter{{tag: "foo", val: "bar"}, {tag: "baz", val: "bar"}},
expectErrSubstr: "",
},
{
name: "domain with params and different whitespace is valid",
value: " letsencrypt.org ;foo=bar;baz=bar",
wantDomain: "letsencrypt.org",
wantParameters: map[string]string{"foo": "bar", "baz": "bar"},
wantParameters: []caaParameter{{tag: "foo", val: "bar"}, {tag: "baz", val: "bar"}},
expectErrSubstr: "",
},
{
name: "empty params are valid",
value: "letsencrypt.org; foo=; baz = bar",
wantDomain: "letsencrypt.org",
wantParameters: map[string]string{"foo": "", "baz": "bar"},
wantParameters: []caaParameter{{tag: "foo", val: ""}, {tag: "baz", val: "bar"}},
expectErrSubstr: "",
},
{
name: "whitespace around params is valid",
value: "letsencrypt.org; foo= ; baz = bar",
wantDomain: "letsencrypt.org",
wantParameters: map[string]string{"foo": "", "baz": "bar"},
wantParameters: []caaParameter{{tag: "foo", val: ""}, {tag: "baz", val: "bar"}},
expectErrSubstr: "",
},
{
name: "comma-separated param values are valid",
value: "letsencrypt.org; foo=b1,b2,b3 ; baz = a=b ",
wantDomain: "letsencrypt.org",
wantParameters: map[string]string{"foo": "b1,b2,b3", "baz": "a=b"},
wantParameters: []caaParameter{{tag: "foo", val: "b1,b2,b3"}, {tag: "baz", val: "a=b"}},
expectErrSubstr: "",
},
{
name: "duplicate tags are valid",
value: "letsencrypt.org; foo=b1,b2,b3 ; foo= b1,b2,b3 ",
wantDomain: "letsencrypt.org",
wantParameters: []caaParameter{{tag: "foo", val: "b1,b2,b3"}, {tag: "foo", val: "b1,b2,b3"}},
expectErrSubstr: "",
},
{
@ -1413,7 +1509,7 @@ func TestExtractIssuerDomainAndParameters(t *testing.T) {
name: "hyphens in param values are valid",
value: "letsencrypt.org; 1=2; baz=a-b",
wantDomain: "letsencrypt.org",
wantParameters: map[string]string{"1": "2", "baz": "a-b"},
wantParameters: []caaParameter{{tag: "1", val: "2"}, {tag: "baz", val: "a-b"}},
expectErrSubstr: "",
},
{
@ -1443,7 +1539,10 @@ func TestExtractIssuerDomainAndParameters(t *testing.T) {
},
}
for _, tc := range tests {
// TODO(#7454) Remove this rebinding
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
gotDomain, gotParameters, gotErr := parseCAARecord(&dns.CAA{Value: tc.value})
if tc.expectErrSubstr == "" {