diff --git a/ca/ca.go b/ca/ca.go index 23d0410de..20abb3839 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -35,6 +35,7 @@ import ( csrlib "github.com/letsencrypt/boulder/csr" berrors "github.com/letsencrypt/boulder/errors" "github.com/letsencrypt/boulder/goodkey" + "github.com/letsencrypt/boulder/identifier" "github.com/letsencrypt/boulder/issuance" "github.com/letsencrypt/boulder/linter" blog "github.com/letsencrypt/boulder/log" @@ -417,11 +418,17 @@ func (ca *certificateAuthorityImpl) issueCertificateForPrecertificate(ctx contex } ca.log.AuditObject("Signing cert", logEvent) + var ipStrings []string + for _, ip := range issuanceReq.IPAddresses { + ipStrings = append(ipStrings, ip.String()) + } + _, span := ca.tracer.Start(ctx, "signing cert", trace.WithAttributes( attribute.String("serial", serialHex), attribute.String("issuer", issuer.Name()), attribute.String("certProfileName", certProfile.name), attribute.StringSlice("names", issuanceReq.DNSNames), + attribute.StringSlice("ipAddresses", ipStrings), )) certDER, err := issuer.Issue(issuanceToken) if err != nil { @@ -536,13 +543,18 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context serialHex := core.SerialToString(serialBigInt) - names := csrlib.NamesFromCSR(csr) + dnsNames, ipAddresses, err := identifier.FromCSR(csr).ToValues() + if err != nil { + return nil, nil, err + } + req := &issuance.IssuanceRequest{ PublicKey: issuance.MarshalablePublicKey{PublicKey: csr.PublicKey}, SubjectKeyId: subjectKeyId, Serial: serialBigInt.Bytes(), - DNSNames: names.SANs, - CommonName: names.CN, + DNSNames: dnsNames, + IPAddresses: ipAddresses, + CommonName: csrlib.NamesFromCSR(csr).CN, IncludeCTPoison: true, IncludeMustStaple: issuance.ContainsMustStaple(csr.Extensions), NotBefore: notBefore, @@ -584,11 +596,17 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context } ca.log.AuditObject("Signing precert", logEvent) + var ipStrings []string + for _, ip := range csr.IPAddresses { + ipStrings = append(ipStrings, ip.String()) + } + _, span := ca.tracer.Start(ctx, "signing precert", trace.WithAttributes( attribute.String("serial", serialHex), attribute.String("issuer", issuer.Name()), attribute.String("certProfileName", certProfile.name), attribute.StringSlice("names", csr.DNSNames), + attribute.StringSlice("ipAddresses", ipStrings), )) certDER, err := issuer.Issue(issuanceToken) if err != nil { diff --git a/identifier/identifier.go b/identifier/identifier.go index ab6b930ba..6c9a86f18 100644 --- a/identifier/identifier.go +++ b/identifier/identifier.go @@ -194,6 +194,30 @@ func Normalize(idents ACMEIdentifiers) ACMEIdentifiers { return slices.Compact(idents) } +// ToValues returns a slice of DNS names and a slice of IP addresses in the +// input. If an identifier type or IP address is invalid, it returns an error. +func (idents ACMEIdentifiers) ToValues() ([]string, []net.IP, error) { + var dnsNames []string + var ipAddresses []net.IP + + for _, ident := range idents { + switch ident.Type { + case TypeDNS: + dnsNames = append(dnsNames, ident.Value) + case TypeIP: + ip := net.ParseIP(ident.Value) + if ip == nil { + return nil, nil, fmt.Errorf("parsing IP address: %s", ident.Value) + } + ipAddresses = append(ipAddresses, ip) + default: + return nil, nil, fmt.Errorf("evaluating identifier type: %s for %s", ident.Type, ident.Value) + } + } + + return dnsNames, ipAddresses, nil +} + // hasIdentifier matches any protobuf struct that has both Identifier and // DnsName fields, like Authorization, Order, or many SA requests. This lets us // convert these to ACMEIdentifier, vice versa, etc. diff --git a/identifier/identifier_test.go b/identifier/identifier_test.go index b2a7b3000..2d7ad4a28 100644 --- a/identifier/identifier_test.go +++ b/identifier/identifier_test.go @@ -5,6 +5,7 @@ import ( "crypto/x509/pkix" "net" "net/netip" + "reflect" "slices" "testing" @@ -262,3 +263,85 @@ func TestNormalize(t *testing.T) { }) } } + +func TestToValues(t *testing.T) { + cases := []struct { + name string + idents ACMEIdentifiers + wantErr string + wantDnsNames []string + wantIpAddresses []net.IP + }{ + { + name: "DNS names and IP addresses", + // These are deliberately out of alphabetical and type order, to + // ensure ToValues doesn't do normalization, which ought to be done + // explicitly. + idents: ACMEIdentifiers{ + {Type: TypeDNS, Value: "beta.example.com"}, + {Type: TypeIP, Value: "fe80::cafe"}, + {Type: TypeDNS, Value: "alpha.example.com"}, + {Type: TypeIP, Value: "127.0.0.1"}, + }, + wantErr: "", + wantDnsNames: []string{"beta.example.com", "alpha.example.com"}, + wantIpAddresses: []net.IP{net.ParseIP("fe80::cafe"), net.ParseIP("127.0.0.1")}, + }, + { + name: "DNS names only", + idents: ACMEIdentifiers{ + {Type: TypeDNS, Value: "alpha.example.com"}, + {Type: TypeDNS, Value: "beta.example.com"}, + }, + wantErr: "", + wantDnsNames: []string{"alpha.example.com", "beta.example.com"}, + wantIpAddresses: nil, + }, + { + name: "IP addresses only", + idents: ACMEIdentifiers{ + {Type: TypeIP, Value: "127.0.0.1"}, + {Type: TypeIP, Value: "fe80::cafe"}, + }, + wantErr: "", + wantDnsNames: nil, + wantIpAddresses: []net.IP{net.ParseIP("127.0.0.1"), net.ParseIP("fe80::cafe")}, + }, + { + name: "invalid IP address", + idents: ACMEIdentifiers{ + {Type: TypeIP, Value: "fe80::c0ffee"}, + }, + wantErr: "parsing IP address: fe80::c0ffee", + wantDnsNames: nil, + wantIpAddresses: nil, + }, + { + name: "invalid identifier type", + idents: ACMEIdentifiers{ + {Type: "fnord", Value: "panic.example.com"}, + }, + wantErr: "evaluating identifier type: fnord for panic.example.com", + wantDnsNames: nil, + wantIpAddresses: nil, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + gotDnsNames, gotIpAddresses, gotErr := tc.idents.ToValues() + if !slices.Equal(gotDnsNames, tc.wantDnsNames) { + t.Errorf("Got DNS names %#v, but want %#v", gotDnsNames, tc.wantDnsNames) + } + if !reflect.DeepEqual(gotIpAddresses, tc.wantIpAddresses) { + t.Errorf("Got IP addresses %#v, but want %#v", gotIpAddresses, tc.wantIpAddresses) + } + if tc.wantErr != "" && (gotErr.Error() != tc.wantErr) { + t.Errorf("Got error %#v, but want %#v", gotErr.Error(), tc.wantErr) + } + if tc.wantErr == "" && gotErr != nil { + t.Errorf("Got error %#v, but didn't want one", gotErr.Error()) + } + }) + } +} diff --git a/issuance/cert.go b/issuance/cert.go index 13cc7f1ce..0ef51546a 100644 --- a/issuance/cert.go +++ b/issuance/cert.go @@ -13,6 +13,7 @@ import ( "errors" "fmt" "math/big" + "net" "sync" "time" @@ -296,8 +297,9 @@ type IssuanceRequest struct { NotBefore time.Time NotAfter time.Time - CommonName string - DNSNames []string + CommonName string + DNSNames []string + IPAddresses []net.IP IncludeMustStaple bool IncludeCTPoison bool @@ -359,6 +361,7 @@ func (i *Issuer) Prepare(prof *Profile, req *IssuanceRequest) ([]byte, *issuance template.Subject.CommonName = req.CommonName } template.DNSNames = req.DNSNames + template.IPAddresses = req.IPAddresses switch req.PublicKey.PublicKey.(type) { case *rsa.PublicKey: @@ -490,6 +493,7 @@ func RequestFromPrecert(precert *x509.Certificate, scts []ct.SignedCertificateTi NotAfter: precert.NotAfter, CommonName: precert.Subject.CommonName, DNSNames: precert.DNSNames, + IPAddresses: precert.IPAddresses, IncludeMustStaple: ContainsMustStaple(precert.Extensions), sctList: scts, precertDER: precert.Raw, diff --git a/issuance/cert_test.go b/issuance/cert_test.go index 07518c5e8..90d3435f5 100644 --- a/issuance/cert_test.go +++ b/issuance/cert_test.go @@ -10,6 +10,7 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/base64" + "net" "reflect" "strings" "testing" @@ -375,6 +376,7 @@ func TestIssue(t *testing.T) { SubjectKeyId: goodSKID, Serial: []byte{1, 2, 3, 4, 5, 6, 7, 8, 9}, DNSNames: []string{"example.com"}, + IPAddresses: []net.IP{net.ParseIP("128.101.101.101"), net.ParseIP("3fff:aaa:a:c0ff:ee:a:bad:deed")}, NotBefore: fc.Now(), NotAfter: fc.Now().Add(time.Hour - time.Second), IncludeCTPoison: true, @@ -389,6 +391,14 @@ func TestIssue(t *testing.T) { err = cert.CheckSignatureFrom(issuerCert.Certificate) test.AssertNotError(t, err, "signature validation failed") test.AssertDeepEquals(t, cert.DNSNames, []string{"example.com"}) + // net.ParseIP always returns a 16-byte address; IPv4 addresses are + // returned in IPv4-mapped IPv6 form. But RFC 5280, Sec. 4.2.1.6 + // requires that IPv4 addresses be encoded as 4 bytes. + // + // The issuance pipeline calls x509.marshalSANs, which reduces IPv4 + // addresses back to 4 bytes. Adding .To4() both allows this test to + // succeed, and covers this requirement. + test.AssertDeepEquals(t, cert.IPAddresses, []net.IP{net.ParseIP("128.101.101.101").To4(), net.ParseIP("3fff:aaa:a:c0ff:ee:a:bad:deed")}) test.AssertByteEquals(t, cert.SerialNumber.Bytes(), []byte{1, 2, 3, 4, 5, 6, 7, 8, 9}) test.AssertDeepEquals(t, cert.PublicKey, pk.Public()) test.AssertEquals(t, len(cert.Extensions), 9) // Constraints, KU, EKU, SKID, AKID, AIA, SAN, Policies, Poison @@ -400,6 +410,84 @@ func TestIssue(t *testing.T) { } } +func TestIssueDNSNamesOnly(t *testing.T) { + fc := clock.NewFake() + signer, err := newIssuer(defaultIssuerConfig(), issuerCert, issuerSigner, fc) + if err != nil { + t.Fatalf("newIssuer: %s", err) + } + pk, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("ecdsa.GenerateKey: %s", err) + } + _, issuanceToken, err := signer.Prepare(defaultProfile(), &IssuanceRequest{ + PublicKey: MarshalablePublicKey{pk.Public()}, + SubjectKeyId: goodSKID, + Serial: []byte{1, 2, 3, 4, 5, 6, 7, 8, 9}, + DNSNames: []string{"example.com"}, + NotBefore: fc.Now(), + NotAfter: fc.Now().Add(time.Hour - time.Second), + IncludeCTPoison: true, + }) + if err != nil { + t.Fatalf("signer.Prepare: %s", err) + } + certBytes, err := signer.Issue(issuanceToken) + if err != nil { + t.Fatalf("signer.Issue: %s", err) + } + cert, err := x509.ParseCertificate(certBytes) + if err != nil { + t.Fatalf("x509.ParseCertificate: %s", err) + } + if !reflect.DeepEqual(cert.DNSNames, []string{"example.com"}) { + t.Errorf("got DNSNames %s, wanted example.com", cert.DNSNames) + } + // BRs 7.1.2.7.12 requires iPAddress, if present, to contain an entry. + if cert.IPAddresses != nil { + t.Errorf("got IPAddresses %s, wanted nil", cert.IPAddresses) + } +} + +func TestIssueIPAddressesOnly(t *testing.T) { + fc := clock.NewFake() + signer, err := newIssuer(defaultIssuerConfig(), issuerCert, issuerSigner, fc) + if err != nil { + t.Fatalf("newIssuer: %s", err) + } + pk, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("ecdsa.GenerateKey: %s", err) + } + _, issuanceToken, err := signer.Prepare(defaultProfile(), &IssuanceRequest{ + PublicKey: MarshalablePublicKey{pk.Public()}, + SubjectKeyId: goodSKID, + Serial: []byte{1, 2, 3, 4, 5, 6, 7, 8, 9}, + IPAddresses: []net.IP{net.ParseIP("128.101.101.101"), net.ParseIP("3fff:aaa:a:c0ff:ee:a:bad:deed")}, + NotBefore: fc.Now(), + NotAfter: fc.Now().Add(time.Hour - time.Second), + IncludeCTPoison: true, + }) + if err != nil { + t.Fatalf("signer.Prepare: %s", err) + } + certBytes, err := signer.Issue(issuanceToken) + if err != nil { + t.Fatalf("signer.Issue: %s", err) + } + cert, err := x509.ParseCertificate(certBytes) + if err != nil { + t.Fatalf("x509.ParseCertificate: %s", err) + } + // BRs 7.1.2.7.12 requires dNSName, if present, to contain an entry. + if cert.DNSNames != nil { + t.Errorf("got DNSNames %s, wanted nil", cert.DNSNames) + } + if !reflect.DeepEqual(cert.IPAddresses, []net.IP{net.ParseIP("128.101.101.101").To4(), net.ParseIP("3fff:aaa:a:c0ff:ee:a:bad:deed")}) { + t.Errorf("got IPAddresses %s, wanted 128.101.101.101 (4-byte) & 3fff:aaa:a:c0ff:ee:a:bad:deed (16-byte)", cert.IPAddresses) + } +} + func TestIssueWithCRLDP(t *testing.T) { fc := clock.NewFake() issuerConfig := defaultIssuerConfig()