identifier: Add identifiers package; rename NewDNS & NewIP

Add a new `identifiers` package for functions that handle slices of `ACMEIdentifier`. We plan to add several such functions (e.g. in #7961). Without this change, they'd need a clumsy prefix/suffix to differentiate them from their sibling functions that handle single idents. The current `NewDNS` / `FromDNSNames` are the first example of this.

Rename `NewDNS` and `NewIP` to use a more standardized `From` prefix, since they're doing conversions. Move and rename `FromDNSNames`, standardizing `identifier.FromDNS` as the singular and `identifiers.FromDNS` as the plural (i.e. slice).

Add a named type `ACMEIdentifiers` so that we can add methods to slices. We will have a lot of slice handling code coming up, which this will make more elegant and readable.

Part of #7311

Author: jprenken

cmd/cert-checker/main.go

@@ -412,7 +412,7 @@ func (c *certChecker) checkCert(ctx context.Context, cert core.Certificate) ([]s
 		//
 		// TODO(#7311): We'll need to iterate over IP address identifiers too.
 		for _, name := range parsedCert.DNSNames {
-			err = c.pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewDNS(name)})
+			err = c.pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.FromDNS(name)})
 			if err != nil {
 				problems = append(problems, fmt.Sprintf("Policy Authority isn't willing to issue for '%s': %s", name, err))
 			} else {

cmd/reversed-hostname-checker/main.go

@@ -51,7 +51,7 @@ func main() {
 	var errors bool
 	for scanner.Scan() {
 		n := sa.ReverseName(scanner.Text())
-		err := pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewDNS(n)})
+		err := pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.FromDNS(n)})
 		if err != nil {
 			errors = true
 			fmt.Printf("%s: %s\n", n, err)

csr/csr.go

@@ -10,7 +10,7 @@ import (
 	"github.com/letsencrypt/boulder/core"
 	berrors "github.com/letsencrypt/boulder/errors"
 	"github.com/letsencrypt/boulder/goodkey"
-	"github.com/letsencrypt/boulder/identifier"
+	"github.com/letsencrypt/boulder/identifiers"
 )
 
 // maxCNLength is the maximum length allowed for the common name as specified in RFC 5280
@@ -82,7 +82,7 @@ func VerifyCSR(ctx context.Context, csr *x509.CertificateRequest, maxNames int,
 		return berrors.BadCSRError("CSR contains more than %d DNS names", maxNames)
 	}
 
-	err = pa.WillingToIssue(identifier.FromDNSNames(names.SANs))
+	err = pa.WillingToIssue(identifiers.FromDNS(names.SANs))
 	if err != nil {
 		return err
 	}

errors/errors_test.go

@@ -17,14 +17,14 @@ func TestWithSubErrors(t *testing.T) {
 
 	subErrs := []SubBoulderError{
 		{
-			Identifier: identifier.NewDNS("example.com"),
+			Identifier: identifier.FromDNS("example.com"),
 			BoulderError: &BoulderError{
 				Type:   RateLimit,
 				Detail: "everyone uses this example domain",
 			},
 		},
 		{
-			Identifier: identifier.NewDNS("what about example.com"),
+			Identifier: identifier.FromDNS("what about example.com"),
 			BoulderError: &BoulderError{
 				Type:   RateLimit,
 				Detail: "try a real identifier value next time",
@@ -39,7 +39,7 @@ func TestWithSubErrors(t *testing.T) {
 	test.AssertDeepEquals(t, outResult.SubErrors, subErrs)
 	// Adding another suberr shouldn't squash the original sub errors
 	anotherSubErr := SubBoulderError{
-		Identifier: identifier.NewDNS("another ident"),
+		Identifier: identifier.FromDNS("another ident"),
 		BoulderError: &BoulderError{
 			Type:   RateLimit,
 			Detail: "another rate limit err",

grpc/errors_test.go

@@ -97,7 +97,7 @@ func TestSubErrorWrapping(t *testing.T) {
 
 	subErrors := []berrors.SubBoulderError{
 		{
-			Identifier: identifier.NewDNS("chillserver.com"),
+			Identifier: identifier.FromDNS("chillserver.com"),
 			BoulderError: &berrors.BoulderError{
 				Type:   berrors.RejectedIdentifier,
 				Detail: "2 ill 2 chill",

grpc/pb-marshalling.go

@@ -315,7 +315,7 @@ func PBToAuthz(pb *corepb.Authorization) (core.Authorization, error) {
 	}
 	authz := core.Authorization{
 		ID:                     pb.Id,
-		Identifier:             identifier.NewDNS(pb.DnsName),
+		Identifier:             identifier.FromDNS(pb.DnsName),
 		RegistrationID:         pb.RegistrationID,
 		Status:                 core.AcmeStatus(pb.Status),
 		Expires:                expires,

grpc/pb-marshalling_test.go

@@ -227,7 +227,7 @@ func TestRegistration(t *testing.T) {
 
 func TestAuthz(t *testing.T) {
 	exp := time.Now().AddDate(0, 0, 1).UTC()
-	ident := identifier.NewDNS("example.com")
+	ident := identifier.FromDNS("example.com")
 	challA := core.Challenge{
 		Type:   core.ChallengeTypeDNS01,
 		Status: core.StatusPending,

identifier/identifier.go

@@ -1,6 +1,6 @@
-// The identifier package defines types for RFC 8555 ACME identifiers.
-// It exists as a separate package to prevent an import loop between the core
-// and probs packages.
+// The identifier package defines types and functions for handling singular RFC
+// 8555 ACME identifiers. It exists as a separate package to prevent an import
+// loop between the core and probs packages.
 package identifier
 
 import (
@@ -50,33 +50,23 @@ func FromProto(ident *corepb.Identifier) ACMEIdentifier {
 // RPCs. TODO(#8023)
 func FromProtoWithDefault(ident *corepb.Identifier, name string) ACMEIdentifier {
 	if ident == nil {
-		return NewDNS(name)
+		return FromDNS(name)
 	}
 	return FromProto(ident)
 }
 
-// NewDNS is a convenience function for creating an ACMEIdentifier with Type
+// FromDNS is a convenience function for creating an ACMEIdentifier with Type
 // "dns" for a given domain name.
-func NewDNS(domain string) ACMEIdentifier {
+func FromDNS(domain string) ACMEIdentifier {
 	return ACMEIdentifier{
 		Type:  TypeDNS,
 		Value: domain,
 	}
 }
 
-// FromDNSNames is a convenience function for creating a slice of ACMEIdentifier
-// with Type "dns" for a given slice of domain names.
-func FromDNSNames(input []string) []ACMEIdentifier {
-	var out []ACMEIdentifier
-	for _, in := range input {
-		out = append(out, NewDNS(in))
-	}
-	return out
-}
-
-// NewIP is a convenience function for creating an ACMEIdentifier with Type "ip"
+// FromIP is a convenience function for creating an ACMEIdentifier with Type "ip"
 // for a given IP address.
-func NewIP(ip netip.Addr) ACMEIdentifier {
+func FromIP(ip netip.Addr) ACMEIdentifier {
 	return ACMEIdentifier{
 		Type: TypeIP,
 		// RFC 8738, Sec. 3: The identifier value MUST contain the textual form

identifiers/identifiers.go

@@ -0,0 +1,25 @@
+// The identifier package defines types and functions for handling plural RFC
+// 8555 ACME identifiers. It exists as a separate package to prevent an import
+// loop between the core and probs packages, and to improve code readability so
+// that it's clear when you're calling a function to handle/return a slice of
+// identifiers, as opposed to a single identifier.
+package identifiers
+
+import (
+	//	corepb "github.com/letsencrypt/boulder/core/proto"
+	"github.com/letsencrypt/boulder/identifier"
+)
+
+// ACMEIdentifiers is a named type for a slice of ACME identifiers, so that
+// methods can be applied to these slices.
+type ACMEIdentifiers []identifier.ACMEIdentifier
+
+// FromDNS is a convenience function for creating a slice of ACMEIdentifier with
+// Type "dns" for a given slice of domain names.
+func FromDNS(input []string) ACMEIdentifiers {
+	var out ACMEIdentifiers
+	for _, in := range input {
+		out = append(out, identifier.FromDNS(in))
+	}
+	return out
+}

mocks/sa.go

@@ -450,7 +450,7 @@ func (sa *StorageAuthorityReadOnly) GetValidAuthorizations2(ctx context.Context,
 			Status:         core.StatusValid,
 			RegistrationID: req.RegistrationID,
 			Expires:        &exp,
-			Identifier:     identifier.NewDNS(name),
+			Identifier:     identifier.FromDNS(name),
 			Challenges: []core.Challenge{
 				{
 					Status:    core.StatusValid,

policy/pa_test.go

@@ -110,7 +110,7 @@ func TestWellFormedIdentifiers(t *testing.T) {
 
 	// Test syntax errors
 	for _, tc := range testCases {
-		err := WellFormedIdentifiers([]identifier.ACMEIdentifier{identifier.NewDNS(tc.domain)})
+		err := WellFormedIdentifiers([]identifier.ACMEIdentifier{identifier.FromDNS(tc.domain)})
 		if tc.err == nil {
 			test.AssertNil(t, err, fmt.Sprintf("Unexpected error for domain %q, got %s", tc.domain, err))
 		} else {
@@ -121,7 +121,7 @@ func TestWellFormedIdentifiers(t *testing.T) {
 		}
 	}
 
-	err := WellFormedIdentifiers([]identifier.ACMEIdentifier{identifier.NewIP(netip.MustParseAddr("9.9.9.9"))})
+	err := WellFormedIdentifiers([]identifier.ACMEIdentifier{identifier.FromIP(netip.MustParseAddr("9.9.9.9"))})
 	test.AssertError(t, err, "Expected error for IP, but got none")
 	var berr *berrors.BoulderError
 	test.AssertErrorWraps(t, err, &berr)
@@ -183,22 +183,22 @@ func TestWillingToIssue(t *testing.T) {
 	test.AssertNotError(t, err, "Couldn't load rules")
 
 	// Invalid encoding
-	err = pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewDNS("www.xn--m.com")})
+	err = pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.FromDNS("www.xn--m.com")})
 	test.AssertError(t, err, "WillingToIssue didn't fail on a malformed IDN")
 	// Invalid identifier type
-	err = pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewIP(netip.MustParseAddr("1.1.1.1"))})
+	err = pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.FromIP(netip.MustParseAddr("1.1.1.1"))})
 	test.AssertError(t, err, "WillingToIssue didn't fail on an IP address")
 	// Valid encoding
-	err = pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewDNS("www.xn--mnich-kva.com")})
+	err = pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.FromDNS("www.xn--mnich-kva.com")})
 	test.AssertNotError(t, err, "WillingToIssue failed on a properly formed IDN")
 	// IDN TLD
-	err = pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewDNS("xn--example--3bhk5a.xn--p1ai")})
+	err = pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.FromDNS("xn--example--3bhk5a.xn--p1ai")})
 	test.AssertNotError(t, err, "WillingToIssue failed on a properly formed domain with IDN TLD")
 	features.Reset()
 
 	// Test expected blocked domains
 	for _, domain := range shouldBeBlocked {
-		err := pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewDNS(domain)})
+		err := pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.FromDNS(domain)})
 		test.AssertError(t, err, "domain was not correctly forbidden")
 		var berr *berrors.BoulderError
 		test.AssertErrorWraps(t, err, &berr)
@@ -207,7 +207,7 @@ func TestWillingToIssue(t *testing.T) {
 
 	// Test acceptance of good names
 	for _, domain := range shouldBeAccepted {
-		err := pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewDNS(domain)})
+		err := pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.FromDNS(domain)})
 		test.AssertNotError(t, err, "domain was incorrectly forbidden")
 	}
 }
@@ -293,7 +293,7 @@ func TestWillingToIssue_Wildcards(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.Name, func(t *testing.T) {
-			err := pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewDNS(tc.Domain)})
+			err := pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.FromDNS(tc.Domain)})
 			if tc.ExpectedErr == nil {
 				test.AssertNil(t, err, fmt.Sprintf("Unexpected error for domain %q, got %s", tc.Domain, err))
 			} else {
@@ -329,11 +329,11 @@ func TestWillingToIssue_SubErrors(t *testing.T) {
 
 	// Test multiple malformed domains and one banned domain; only the malformed ones will generate errors
 	err = pa.WillingToIssue([]identifier.ACMEIdentifier{
-		identifier.NewDNS("perfectly-fine.com"),      // fine
-		identifier.NewDNS("letsdecrypt_org"),         // malformed
-		identifier.NewDNS("example.comm"),            // malformed
-		identifier.NewDNS("letsdecrypt.org"),         // banned
-		identifier.NewDNS("also-perfectly-fine.com"), // fine
+		identifier.FromDNS("perfectly-fine.com"),      // fine
+		identifier.FromDNS("letsdecrypt_org"),         // malformed
+		identifier.FromDNS("example.comm"),            // malformed
+		identifier.FromDNS("letsdecrypt.org"),         // banned
+		identifier.FromDNS("also-perfectly-fine.com"), // fine
 	})
 	test.AssertDeepEquals(t, err,
 		&berrors.BoulderError{
@@ -345,24 +345,24 @@ func TestWillingToIssue_SubErrors(t *testing.T) {
 						Type:   berrors.Malformed,
 						Detail: "Domain name contains an invalid character",
 					},
-					Identifier: identifier.NewDNS("letsdecrypt_org"),
+					Identifier: identifier.FromDNS("letsdecrypt_org"),
 				},
 				{
 					BoulderError: &berrors.BoulderError{
 						Type:   berrors.Malformed,
 						Detail: "Domain name does not end with a valid public suffix (TLD)",
 					},
-					Identifier: identifier.NewDNS("example.comm"),
+					Identifier: identifier.FromDNS("example.comm"),
 				},
 			},
 		})
 
 	// Test multiple banned domains.
 	err = pa.WillingToIssue([]identifier.ACMEIdentifier{
-		identifier.NewDNS("perfectly-fine.com"),      // fine
-		identifier.NewDNS("letsdecrypt.org"),         // banned
-		identifier.NewDNS("example.com"),             // banned
-		identifier.NewDNS("also-perfectly-fine.com"), // fine
+		identifier.FromDNS("perfectly-fine.com"),      // fine
+		identifier.FromDNS("letsdecrypt.org"),         // banned
+		identifier.FromDNS("example.com"),             // banned
+		identifier.FromDNS("also-perfectly-fine.com"), // fine
 	})
 	test.AssertError(t, err, "Expected err from WillingToIssueWildcards")
 
@@ -376,20 +376,20 @@ func TestWillingToIssue_SubErrors(t *testing.T) {
 						Type:   berrors.RejectedIdentifier,
 						Detail: "The ACME server refuses to issue a certificate for this domain name, because it is forbidden by policy",
 					},
-					Identifier: identifier.NewDNS("letsdecrypt.org"),
+					Identifier: identifier.FromDNS("letsdecrypt.org"),
 				},
 				{
 					BoulderError: &berrors.BoulderError{
 						Type:   berrors.RejectedIdentifier,
 						Detail: "The ACME server refuses to issue a certificate for this domain name, because it is forbidden by policy",
 					},
-					Identifier: identifier.NewDNS("example.com"),
+					Identifier: identifier.FromDNS("example.com"),
 				},
 			},
 		})
 
 	// Test willing to issue with only *one* bad identifier.
-	err = pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.NewDNS("letsdecrypt.org")})
+	err = pa.WillingToIssue([]identifier.ACMEIdentifier{identifier.FromDNS("letsdecrypt.org")})
 	test.AssertDeepEquals(t, err,
 		&berrors.BoulderError{
 			Type:   berrors.RejectedIdentifier,
@@ -409,21 +409,21 @@ func TestChallengeTypesFor(t *testing.T) {
 	}{
 		{
 			name:  "dns",
-			ident: identifier.NewDNS("example.com"),
+			ident: identifier.FromDNS("example.com"),
 			wantChalls: []core.AcmeChallenge{
 				core.ChallengeTypeHTTP01, core.ChallengeTypeDNS01, core.ChallengeTypeTLSALPN01,
 			},
 		},
 		{
 			name:  "wildcard",
-			ident: identifier.NewDNS("*.example.com"),
+			ident: identifier.FromDNS("*.example.com"),
 			wantChalls: []core.AcmeChallenge{
 				core.ChallengeTypeDNS01,
 			},
 		},
 		{
 			name:    "other",
-			ident:   identifier.NewIP(netip.MustParseAddr("1.2.3.4")),
+			ident:   identifier.FromIP(netip.MustParseAddr("1.2.3.4")),
 			wantErr: "unrecognized identifier type",
 		},
 	}
@@ -514,23 +514,23 @@ func TestCheckAuthzChallenges(t *testing.T) {
 		{
 			name: "no challenges",
 			authz: core.Authorization{
-				Identifier: identifier.NewDNS("example.com"),
+				Identifier: identifier.FromDNS("example.com"),
 				Challenges: []core.Challenge{},
 			},
 			wantErr: "has no challenges",
 		},
 		{
 			name: "no valid challenges",
 			authz: core.Authorization{
-				Identifier: identifier.NewDNS("example.com"),
+				Identifier: identifier.FromDNS("example.com"),
 				Challenges: []core.Challenge{{Type: core.ChallengeTypeDNS01, Status: core.StatusPending}},
 			},
 			wantErr: "not solved by any challenge",
 		},
 		{
 			name: "solved by disabled challenge",
 			authz: core.Authorization{
-				Identifier: identifier.NewDNS("example.com"),
+				Identifier: identifier.FromDNS("example.com"),
 				Challenges: []core.Challenge{{Type: core.ChallengeTypeDNS01, Status: core.StatusValid}},
 			},
 			enabled: map[core.AcmeChallenge]bool{core.ChallengeTypeHTTP01: true},
@@ -539,15 +539,15 @@ func TestCheckAuthzChallenges(t *testing.T) {
 		{
 			name: "solved by wrong kind of challenge",
 			authz: core.Authorization{
-				Identifier: identifier.NewDNS("*.example.com"),
+				Identifier: identifier.FromDNS("*.example.com"),
 				Challenges: []core.Challenge{{Type: core.ChallengeTypeHTTP01, Status: core.StatusValid}},
 			},
 			wantErr: "inapplicable challenge type",
 		},
 		{
 			name: "valid authz",
 			authz: core.Authorization{
-				Identifier: identifier.NewDNS("example.com"),
+				Identifier: identifier.FromDNS("example.com"),
 				Challenges: []core.Challenge{{Type: core.ChallengeTypeTLSALPN01, Status: core.StatusValid}},
 			},
 		},