Skip to content

Commit cb94164

Browse files
authored
policy: Add initial Identifier support (#8064)
Change WillingToIssue and WellFormedDomainNames to use Identifiers, and (for now) reject non-DNS identifiers. Part of #7311
1 parent edc3c7f commit cb94164

File tree

11 files changed

+94
-56
lines changed

11 files changed

+94
-56
lines changed

cmd/cert-checker/main.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"github.com/letsencrypt/boulder/features"
3030
"github.com/letsencrypt/boulder/goodkey"
3131
"github.com/letsencrypt/boulder/goodkey/sagoodkey"
32+
"github.com/letsencrypt/boulder/identifier"
3233
"github.com/letsencrypt/boulder/linter"
3334
blog "github.com/letsencrypt/boulder/log"
3435
"github.com/letsencrypt/boulder/policy"
@@ -408,8 +409,10 @@ func (c *certChecker) checkCert(ctx context.Context, cert core.Certificate) ([]s
408409
// Check that the PA is still willing to issue for each name in DNSNames.
409410
// We do not check the CommonName here, as (if it exists) we already checked
410411
// that it is identical to one of the DNSNames in the SAN.
412+
//
413+
// TODO(#7311): We'll need to iterate over IP address identifiers too.
411414
for _, name := range parsedCert.DNSNames {
412-
err = c.pa.WillingToIssue([]string{name})
415+
err = c.pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewDNS(name)})
413416
if err != nil {
414417
problems = append(problems, fmt.Sprintf("Policy Authority isn't willing to issue for '%s': %s", name, err))
415418
} else {

cmd/reversed-hostname-checker/main.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"os"
1313

1414
"github.com/letsencrypt/boulder/cmd"
15+
"github.com/letsencrypt/boulder/identifier"
1516
"github.com/letsencrypt/boulder/policy"
1617
"github.com/letsencrypt/boulder/sa"
1718
)
@@ -50,7 +51,7 @@ func main() {
5051
var errors bool
5152
for scanner.Scan() {
5253
n := sa.ReverseName(scanner.Text())
53-
err := pa.WillingToIssue([]string{n})
54+
err := pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewDNS(n)})
5455
if err != nil {
5556
errors = true
5657
fmt.Printf("%s: %s\n", n, err)

core/interfaces.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77
// PolicyAuthority defines the public interface for the Boulder PA
88
// TODO(#5891): Move this interface to a more appropriate location.
99
type PolicyAuthority interface {
10-
WillingToIssue([]string) error
10+
WillingToIssue([]identifier.ACMEIdentifier) error
1111
ChallengeTypesFor(identifier.ACMEIdentifier) ([]AcmeChallenge, error)
1212
ChallengeTypeEnabled(AcmeChallenge) bool
1313
CheckAuthzChallenges(*Authorization) error

csr/csr.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/letsencrypt/boulder/core"
1111
berrors "github.com/letsencrypt/boulder/errors"
1212
"github.com/letsencrypt/boulder/goodkey"
13+
"github.com/letsencrypt/boulder/identifier"
1314
)
1415

1516
// maxCNLength is the maximum length allowed for the common name as specified in RFC 5280
@@ -34,7 +35,7 @@ var (
3435
invalidSig = berrors.BadCSRError("invalid signature on CSR")
3536
invalidEmailPresent = berrors.BadCSRError("CSR contains one or more email address fields")
3637
invalidIPPresent = berrors.BadCSRError("CSR contains one or more IP address fields")
37-
invalidNoDNS = berrors.BadCSRError("at least one DNS name is required")
38+
invalidNoIdent = berrors.BadCSRError("at least one identifier is required")
3839
)
3940

4041
// VerifyCSR checks the validity of a x509.CertificateRequest. It uses
@@ -72,7 +73,7 @@ func VerifyCSR(ctx context.Context, csr *x509.CertificateRequest, maxNames int,
7273
names := NamesFromCSR(csr)
7374

7475
if len(names.SANs) == 0 && names.CN == "" {
75-
return invalidNoDNS
76+
return invalidNoIdent
7677
}
7778
if len(names.CN) > maxCNLength {
7879
return berrors.BadCSRError("CN was longer than %d bytes", maxCNLength)
@@ -81,7 +82,7 @@ func VerifyCSR(ctx context.Context, csr *x509.CertificateRequest, maxNames int,
8182
return berrors.BadCSRError("CSR contains more than %d DNS names", maxNames)
8283
}
8384

84-
err = pa.WillingToIssue(names.SANs)
85+
err = pa.WillingToIssue(identifier.FromDNSNames(names.SANs))
8586
if err != nil {
8687
return err
8788
}

csr/csr_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ func (pa *mockPA) ChallengeTypesFor(ident identifier.ACMEIdentifier) ([]core.Acm
2626
return []core.AcmeChallenge{}, nil
2727
}
2828

29-
func (pa *mockPA) WillingToIssue(domains []string) error {
30-
for _, domain := range domains {
31-
if domain == "bad-name.com" || domain == "other-bad-name.com" {
29+
func (pa *mockPA) WillingToIssue(idents []identifier.ACMEIdentifier) error {
30+
for _, ident := range idents {
31+
if ident.Value == "bad-name.com" || ident.Value == "other-bad-name.com" {
3232
return errors.New("policy forbids issuing for identifier")
3333
}
3434
}
@@ -103,7 +103,7 @@ func TestVerifyCSR(t *testing.T) {
103103
signedReq,
104104
100,
105105
&mockPA{},
106-
invalidNoDNS,
106+
invalidNoIdent,
107107
},
108108
{
109109
signedReqWithLongCN,

identifier/identifier.go

+10
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,16 @@ func NewDNS(domain string) ACMEIdentifier {
6464
}
6565
}
6666

67+
// FromDNSNames is a convenience function for creating a slice of ACMEIdentifier
68+
// with Type "dns" for a given slice of domain names.
69+
func FromDNSNames(input []string) []ACMEIdentifier {
70+
var out []ACMEIdentifier
71+
for _, in := range input {
72+
out = append(out, NewDNS(in))
73+
}
74+
return out
75+
}
76+
6777
// NewIP is a convenience function for creating an ACMEIdentifier with Type "ip"
6878
// for a given IP address.
6979
func NewIP(ip netip.Addr) ACMEIdentifier {

policy/pa.go

+36-23
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ var (
180180
errMalformedWildcard = berrors.MalformedError("Domain name contains an invalid wildcard. A wildcard is only permitted before the first dot in a domain name")
181181
errICANNTLDWildcard = berrors.MalformedError("Domain name is a wildcard for an ICANN TLD")
182182
errWildcardNotSupported = berrors.MalformedError("Wildcard domain names are not supported")
183+
errUnsupportedIdent = berrors.MalformedError("invalid non-DNS type identifier")
183184
)
184185

185186
// validNonWildcardDomain checks that a domain isn't:
@@ -353,16 +354,16 @@ func ValidEmail(address string) error {
353354
}
354355

355356
// subError returns an appropriately typed error based on the input error
356-
func subError(name string, err error) berrors.SubBoulderError {
357+
func subError(ident identifier.ACMEIdentifier, err error) berrors.SubBoulderError {
357358
var bErr *berrors.BoulderError
358359
if errors.As(err, &bErr) {
359360
return berrors.SubBoulderError{
360-
Identifier: identifier.NewDNS(name),
361+
Identifier: ident,
361362
BoulderError: bErr,
362363
}
363364
} else {
364365
return berrors.SubBoulderError{
365-
Identifier: identifier.NewDNS(name),
366+
Identifier: ident,
366367
BoulderError: &berrors.BoulderError{
367368
Type: berrors.RejectedIdentifier,
368369
Detail: err.Error(),
@@ -372,47 +373,53 @@ func subError(name string, err error) berrors.SubBoulderError {
372373
}
373374

374375
// WillingToIssue determines whether the CA is willing to issue for the provided
375-
// domain names.
376+
// identifiers.
376377
//
377-
// It checks the criteria checked by `WellFormedDomainNames`, and additionally checks
378-
// whether any domain is on a blocklist.
378+
// It checks the criteria checked by `WellFormedIdentifiers`, and additionally
379+
// checks whether any identifier is on a blocklist.
379380
//
380-
// If multiple domains are invalid, the error will contain suberrors specific to
381-
// each domain.
381+
// If multiple identifiers are invalid, the error will contain suberrors
382+
// specific to each identifier.
382383
//
383-
// Precondition: all input domain names must be in lowercase.
384-
func (pa *AuthorityImpl) WillingToIssue(domains []string) error {
385-
err := WellFormedDomainNames(domains)
384+
// Precondition: all input identifier values must be in lowercase.
385+
func (pa *AuthorityImpl) WillingToIssue(idents []identifier.ACMEIdentifier) error {
386+
err := WellFormedIdentifiers(idents)
386387
if err != nil {
387388
return err
388389
}
389390

390391
var subErrors []berrors.SubBoulderError
391-
for _, domain := range domains {
392-
if strings.Count(domain, "*") > 0 {
392+
for _, ident := range idents {
393+
if ident.Type != identifier.TypeDNS {
394+
subErrors = append(subErrors, subError(ident, errUnsupportedIdent))
395+
continue
396+
}
397+
398+
if strings.Count(ident.Value, "*") > 0 {
393399
// The base domain is the wildcard request with the `*.` prefix removed
394-
baseDomain := strings.TrimPrefix(domain, "*.")
400+
baseDomain := strings.TrimPrefix(ident.Value, "*.")
395401

396402
// The base domain can't be in the wildcard exact blocklist
397403
err = pa.checkWildcardHostList(baseDomain)
398404
if err != nil {
399-
subErrors = append(subErrors, subError(domain, err))
405+
subErrors = append(subErrors, subError(ident, err))
400406
continue
401407
}
402408
}
403409

404410
// For both wildcard and non-wildcard domains, check whether any parent domain
405411
// name is on the regular blocklist.
406-
err := pa.checkHostLists(domain)
412+
err := pa.checkHostLists(ident.Value)
407413
if err != nil {
408-
subErrors = append(subErrors, subError(domain, err))
414+
subErrors = append(subErrors, subError(ident, err))
409415
continue
410416
}
411417
}
412418
return combineSubErrors(subErrors)
413419
}
414420

415-
// WellFormedDomainNames returns an error if any of the provided domains do not meet these criteria:
421+
// WellFormedIdentifiers returns an error if any of the provided domains do not
422+
// meet these criteria:
416423
//
417424
// - MUST contains only lowercase characters, numbers, hyphens, and dots
418425
// - MUST NOT have more than maxLabels labels
@@ -434,12 +441,18 @@ func (pa *AuthorityImpl) WillingToIssue(domains []string) error {
434441
//
435442
// If multiple domains are invalid, the error will contain suberrors specific to
436443
// each domain.
437-
func WellFormedDomainNames(domains []string) error {
444+
func WellFormedIdentifiers(idents []identifier.ACMEIdentifier) error {
438445
var subErrors []berrors.SubBoulderError
439-
for _, domain := range domains {
440-
err := ValidDomain(domain)
441-
if err != nil {
442-
subErrors = append(subErrors, subError(domain, err))
446+
for _, ident := range idents {
447+
// TODO(#7311): When this gets a third case for TypeIP, this will be
448+
// more elegant as a switch/case.
449+
if ident.Type == identifier.TypeDNS {
450+
err := ValidDomain(ident.Value)
451+
if err != nil {
452+
subErrors = append(subErrors, subError(ident, err))
453+
}
454+
} else {
455+
subErrors = append(subErrors, subError(ident, errUnsupportedIdent))
443456
}
444457
}
445458
return combineSubErrors(subErrors)

policy/pa_test.go

+29-20
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func paImpl(t *testing.T) *AuthorityImpl {
3030
return pa
3131
}
3232

33-
func TestWellFormedDomainNames(t *testing.T) {
33+
func TestWellFormedIdentifiers(t *testing.T) {
3434
testCases := []struct {
3535
domain string
3636
err error
@@ -110,7 +110,7 @@ func TestWellFormedDomainNames(t *testing.T) {
110110

111111
// Test syntax errors
112112
for _, tc := range testCases {
113-
err := WellFormedDomainNames([]string{tc.domain})
113+
err := WellFormedIdentifiers([]identifier.ACMEIdentifier{identifier.NewDNS(tc.domain)})
114114
if tc.err == nil {
115115
test.AssertNil(t, err, fmt.Sprintf("Unexpected error for domain %q, got %s", tc.domain, err))
116116
} else {
@@ -120,6 +120,12 @@ func TestWellFormedDomainNames(t *testing.T) {
120120
test.AssertContains(t, berr.Error(), tc.err.Error())
121121
}
122122
}
123+
124+
err := WellFormedIdentifiers([]identifier.ACMEIdentifier{identifier.NewIP(netip.MustParseAddr("9.9.9.9"))})
125+
test.AssertError(t, err, "Expected error for IP, but got none")
126+
var berr *berrors.BoulderError
127+
test.AssertErrorWraps(t, err, &berr)
128+
test.AssertContains(t, berr.Error(), errUnsupportedIdent.Error())
123129
}
124130

125131
func TestWillingToIssue(t *testing.T) {
@@ -177,19 +183,22 @@ func TestWillingToIssue(t *testing.T) {
177183
test.AssertNotError(t, err, "Couldn't load rules")
178184

179185
// Invalid encoding
180-
err = pa.WillingToIssue([]string{"www.xn--m.com"})
186+
err = pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewDNS("www.xn--m.com")})
181187
test.AssertError(t, err, "WillingToIssue didn't fail on a malformed IDN")
188+
// Invalid identifier type
189+
err = pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewIP(netip.MustParseAddr("1.1.1.1"))})
190+
test.AssertError(t, err, "WillingToIssue didn't fail on an IP address")
182191
// Valid encoding
183-
err = pa.WillingToIssue([]string{"www.xn--mnich-kva.com"})
192+
err = pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewDNS("www.xn--mnich-kva.com")})
184193
test.AssertNotError(t, err, "WillingToIssue failed on a properly formed IDN")
185194
// IDN TLD
186-
err = pa.WillingToIssue([]string{"xn--example--3bhk5a.xn--p1ai"})
195+
err = pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewDNS("xn--example--3bhk5a.xn--p1ai")})
187196
test.AssertNotError(t, err, "WillingToIssue failed on a properly formed domain with IDN TLD")
188197
features.Reset()
189198

190199
// Test expected blocked domains
191200
for _, domain := range shouldBeBlocked {
192-
err := pa.WillingToIssue([]string{domain})
201+
err := pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewDNS(domain)})
193202
test.AssertError(t, err, "domain was not correctly forbidden")
194203
var berr *berrors.BoulderError
195204
test.AssertErrorWraps(t, err, &berr)
@@ -198,7 +207,7 @@ func TestWillingToIssue(t *testing.T) {
198207

199208
// Test acceptance of good names
200209
for _, domain := range shouldBeAccepted {
201-
err := pa.WillingToIssue([]string{domain})
210+
err := pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewDNS(domain)})
202211
test.AssertNotError(t, err, "domain was incorrectly forbidden")
203212
}
204213
}
@@ -284,7 +293,7 @@ func TestWillingToIssue_Wildcards(t *testing.T) {
284293

285294
for _, tc := range testCases {
286295
t.Run(tc.Name, func(t *testing.T) {
287-
err := pa.WillingToIssue([]string{tc.Domain})
296+
err := pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewDNS(tc.Domain)})
288297
if tc.ExpectedErr == nil {
289298
test.AssertNil(t, err, fmt.Sprintf("Unexpected error for domain %q, got %s", tc.Domain, err))
290299
} else {
@@ -319,12 +328,12 @@ func TestWillingToIssue_SubErrors(t *testing.T) {
319328
test.AssertNotError(t, err, "Couldn't load policy contents from file")
320329

321330
// Test multiple malformed domains and one banned domain; only the malformed ones will generate errors
322-
err = pa.WillingToIssue([]string{
323-
"perfectly-fine.com", // fine
324-
"letsdecrypt_org", // malformed
325-
"example.comm", // malformed
326-
"letsdecrypt.org", // banned
327-
"also-perfectly-fine.com", // fine
331+
err = pa.WillingToIssue([]identifier.ACMEIdentifier{
332+
identifier.NewDNS("perfectly-fine.com"), // fine
333+
identifier.NewDNS("letsdecrypt_org"), // malformed
334+
identifier.NewDNS("example.comm"), // malformed
335+
identifier.NewDNS("letsdecrypt.org"), // banned
336+
identifier.NewDNS("also-perfectly-fine.com"), // fine
328337
})
329338
test.AssertDeepEquals(t, err,
330339
&berrors.BoulderError{
@@ -349,11 +358,11 @@ func TestWillingToIssue_SubErrors(t *testing.T) {
349358
})
350359

351360
// Test multiple banned domains.
352-
err = pa.WillingToIssue([]string{
353-
"perfectly-fine.com", // fine
354-
"letsdecrypt.org", // banned
355-
"example.com", // banned
356-
"also-perfectly-fine.com", // fine
361+
err = pa.WillingToIssue([]identifier.ACMEIdentifier{
362+
identifier.NewDNS("perfectly-fine.com"), // fine
363+
identifier.NewDNS("letsdecrypt.org"), // banned
364+
identifier.NewDNS("example.com"), // banned
365+
identifier.NewDNS("also-perfectly-fine.com"), // fine
357366
})
358367
test.AssertError(t, err, "Expected err from WillingToIssueWildcards")
359368

@@ -380,7 +389,7 @@ func TestWillingToIssue_SubErrors(t *testing.T) {
380389
})
381390

382391
// Test willing to issue with only *one* bad identifier.
383-
err = pa.WillingToIssue([]string{"letsdecrypt.org"})
392+
err = pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewDNS("letsdecrypt.org")})
384393
test.AssertDeepEquals(t, err,
385394
&berrors.BoulderError{
386395
Type: berrors.RejectedIdentifier,

ra/ra.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -2287,7 +2287,7 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New
22872287
}
22882288

22892289
// Validate that our policy allows issuing for each of the names in the order
2290-
err = ra.PA.WillingToIssue(newOrder.DnsNames)
2290+
err = ra.PA.WillingToIssue(identifier.FromDNSNames(newOrder.DnsNames))
22912291
if err != nil {
22922292
return nil, err
22932293
}

ratelimits/names.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"strconv"
77
"strings"
88

9+
"github.com/letsencrypt/boulder/identifier"
910
"github.com/letsencrypt/boulder/policy"
1011
)
1112

@@ -199,7 +200,7 @@ func validateFQDNSet(id string) error {
199200
return fmt.Errorf(
200201
"invalid fqdnSet, %q must be formatted 'fqdnSet'", id)
201202
}
202-
return policy.WellFormedDomainNames(domains)
203+
return policy.WellFormedIdentifiers(identifier.FromDNSNames(domains))
203204
}
204205

205206
func validateIdForName(name Name, id string) error {

wfe2/wfe.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -2287,7 +2287,7 @@ func (wfe *WebFrontEndImpl) NewOrder(
22872287
}
22882288

22892289
names = core.UniqueLowerNames(names)
2290-
err = policy.WellFormedDomainNames(names)
2290+
err = policy.WellFormedIdentifiers(identifier.FromDNSNames(names))
22912291
if err != nil {
22922292
wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Invalid identifiers requested"), nil)
22932293
return

0 commit comments

Comments
 (0)