crypto/x509: reject IPv4-mapped addresses during parsing We've received a number of security reports that IPv4-mapped IPv6 SANs can bypass IPv4 constraints. We don't consider this a security issue, since IPv4-mapped addresses _should never appear_ in certificates. They are intended to be a system local representation intended to allow the kernel to support v4 addresses using v6 semantics. A CA which allows these addresses to leak into certificates is doing something _very wrong_. In order to stem the misconception that this is actually a problem, we are just going to reject these types of addresses during parsing, for both SANs and constraints. For CreateCertificate, it turns out we don't actually have to do anything. We already converted IPv4-mapped SANs to their IPv4 form when encoding certificates. Just for the sake of my future sanity, document that we do this. For constraints, we are saved by a combination of two factors. Firstly, net.ParseCIDR already returns a net.IPNet with the IP address in it's masked form, so a CIDR such as "::ffff:192.0.2.1/32" already results in a v4 form address. One caveat though is that we do encode IPv4-mapped CIDRs with masks >96 as IPv4-mapped. This seems like user error, but we reject them anyway because why not. Updates #79776 Change-Id: I6b2be42f2af5c40760309aebd6efd68e1d88e663 Reviewed-on: https://go-review.googlesource.com/c/go/+/769022 Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: Carlos Amedee <carlos@golang.org> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/src/crypto/x509/name_constraints_test.go b/src/crypto/x509/name_constraints_test.go index 8a50d36..803ab17 100644 --- a/src/crypto/x509/name_constraints_test.go +++ b/src/crypto/x509/name_constraints_test.go
@@ -1039,22 +1039,6 @@ }, }, { - name: "IPv4-mapped-IPv6 exclusion does not affect IPv4", - roots: []constraintsSpec{ - { - bad: []string{"ip:::ffff:1.2.3.4/128"}, - }, - }, - intermediates: [][]constraintsSpec{ - { - {}, - }, - }, - leaf: leafSpec{ - sans: []string{"ip:1.2.3.4"}, - }, - }, - { name: "URI constraint not matched by URN", roots: []constraintsSpec{ {
diff --git a/src/crypto/x509/parser.go b/src/crypto/x509/parser.go index 57a1584..4a1416e 100644 --- a/src/crypto/x509/parser.go +++ b/src/crypto/x509/parser.go
@@ -496,7 +496,12 @@ uris = append(uris, uri) case nameTypeIP: switch len(data) { - case net.IPv4len, net.IPv6len: + case net.IPv6len: + if net.IP(data).To4() != nil { + return errors.New("x509: SAN iPAddress contains IPv4-mapped IPv6 address") + } + ipAddresses = append(ipAddresses, data) + case net.IPv4len: ipAddresses = append(ipAddresses, data) default: return errors.New("x509: cannot parse IP address of length " + strconv.Itoa(len(data))) @@ -683,6 +688,10 @@ return nil, nil, nil, nil, fmt.Errorf("x509: IP constraint contained invalid mask %x", mask) } + if len(ip) == net.IPv6len && net.IP(ip).To4() != nil { + return nil, nil, nil, nil, errors.New("x509: IP constraint contained IPv4-mapped IPv6 address") + } + ips = append(ips, &net.IPNet{IP: net.IP(ip), Mask: net.IPMask(mask)}) case emailTag:
diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go index dff55a8..755007f 100644 --- a/src/crypto/x509/x509.go +++ b/src/crypto/x509/x509.go
@@ -1398,12 +1398,17 @@ ret[n].Id = oidExtensionNameConstraints ret[n].Critical = template.PermittedDNSDomainsCritical - ipAndMask := func(ipNet *net.IPNet) []byte { + ipAndMask := func(ipNet *net.IPNet) ([]byte, error) { maskedIP := ipNet.IP.Mask(ipNet.Mask) + // This is extremely unlikely to actually happen, but lets save people from doing something they + // probably shouldn't. + if len(maskedIP) == net.IPv6len && maskedIP.To4() != nil { + return nil, errors.New("x509: IP constraint contained IPv4-mapped IPv6 address with a IPv6 mask") + } ipAndMask := make([]byte, 0, len(maskedIP)+len(ipNet.Mask)) ipAndMask = append(ipAndMask, maskedIP...) ipAndMask = append(ipAndMask, ipNet.Mask...) - return ipAndMask + return ipAndMask, nil } serialiseConstraints := func(dns []string, ips []*net.IPNet, emails []string, uriDomains []string) (der []byte, err error) { @@ -1422,9 +1427,13 @@ } for _, ipNet := range ips { + encodedIPNet, err := ipAndMask(ipNet) + if err != nil { + return nil, err + } b.AddASN1(cryptobyte_asn1.SEQUENCE, func(b *cryptobyte.Builder) { b.AddASN1(cryptobyte_asn1.Tag(7).ContextSpecific(), func(b *cryptobyte.Builder) { - b.AddBytes(ipAndMask(ipNet)) + b.AddBytes(encodedIPNet) }) }) } @@ -1792,6 +1801,9 @@ // be marshaled instead of the Policies field. This changed in Go 1.24. The Policies field can // be used to marshal policy OIDs which have components that are larger than 31 // bits. +// +// IP addresses in IPAddresses which are in their IPv4-mapped IPv6 form will always be encoded +// in their IPv4 form. func CreateCertificate(rand io.Reader, template, parent *Certificate, pub, priv any) ([]byte, error) { key, ok := priv.(crypto.Signer) if !ok {
diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go index fbf73ea..53735bd 100644 --- a/src/crypto/x509/x509_test.go +++ b/src/crypto/x509/x509_test.go
@@ -5377,3 +5377,83 @@ AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAQMEhUdHiUs -----END CERTIFICATE----- ` + +const ipv4MappedSANCert = "308201a33082010ca003020102020101300d06092a864886f70d01010b05003000301e170d3236303431393135353233345a170d3236303432313135353233345a300030819f300d06092a864886f70d010101050003818d0030818902818100b1a1e0945b9289c4d3f1329f8a982c4a2dcd59bfd372fb8085a9c517554607ebd2f7990eef216ac9f4605f71a03b04f42a5255b158cf8e0844191f5119348baa44c35056e20609bcf9510f30ead4b481c81d7865fb27b8e0090e112b717f3ee08cdfc4012da1f1f7cf2a1bc34c73a54a12b06372d09714742dd7895eadde4aa50203010001a32d302b30290603551d110101ff041f301d82096c6f63616c686f7374871000000000000000000000ffffc0000201300d06092a864886f70d01010b0500038181002d2cdad074598049ad1c119711adbb0616495c5f27eb22e0df54fe9b35c3c4d379e27bf65b47c4ecd46eda09019dd0e57311ce13cd88a0fbff0d056a39ab69c6665303fd1e754777d86b57d12491be9f9200b4067ca1d5fc1988c3f09a4fe950c69e924b7bfd167b4c38ead525471ba9b1852ba6f5d783c4ca34a93aa70c23bf" + +const ipv4MappedConstraintCert = "308201c23082012ba003020102020101300d06092a864886f70d01010b05003000301e170d3236303431393135333435315a170d3236303432313135333435315a300030819f300d06092a864886f70d010101050003818d0030818902818100b1a1e0945b9289c4d3f1329f8a982c4a2dcd59bfd372fb8085a9c517554607ebd2f7990eef216ac9f4605f71a03b04f42a5255b158cf8e0844191f5119348baa44c35056e20609bcf9510f30ead4b481c81d7865fb27b8e0090e112b717f3ee08cdfc4012da1f1f7cf2a1bc34c73a54a12b06372d09714742dd7895eadde4aa50203010001a34c304a30170603551d110101ff040d300b82096c6f63616c686f7374302f0603551d1e04283026a1243022872000000000000000000000ffffc0000201ffffffffffffffffffffffffffffffff300d06092a864886f70d01010b050003818100973c4ba96f538b2fbd38a71be95e9953777b949f93ea6e4de09aeef6d19bb20cf07daf2896d4662f22ca8cfb239df6ab723b8cb0b80486ff1242124b61d5eca92824aa8cc2b8bf58e2a237cbf4db43ebfd01c4960dc546f5e139612db0059c9c413c565facc191af601ef968c17acd9476e5393778656c51a0979d66508e77e6" + +func TestIPv4MappedIPsParse(t *testing.T) { + for _, tc := range []struct { + name string + der string + }{ + { + name: "SAN", + der: ipv4MappedSANCert, + }, + { + name: "constraint", + der: ipv4MappedConstraintCert, + }, + } { + t.Run(tc.name, func(t *testing.T) { + der, err := hex.DecodeString(tc.der) + if err != nil { + t.Fatalf("hex.DecodeString() unexpected error: %v", err) + } + + expectedErrSuffix := "IPv4-mapped IPv6 address" + + _, err = ParseCertificate(der) + if err == nil { + t.Fatal("expected error when parsing certificate containing IPv4-mapped IPv6 address") + } else if !strings.Contains(err.Error(), expectedErrSuffix) { + t.Fatalf("unexpected error: %v, want suffix: %s", err, expectedErrSuffix) + } + }) + } +} + +func TestIPv4MappedIPsSANCreate(t *testing.T) { + tmpl := Certificate{ + SerialNumber: big.NewInt(1), + NotBefore: time.Now().Add(-24 * time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + DNSNames: []string{"localhost"}, + IPAddresses: []net.IP{net.ParseIP("::ffff:192.0.2.1")}, + } + + der, err := CreateCertificate(rand.Reader, &tmpl, &tmpl, rsaPrivateKey.Public(), rsaPrivateKey) + if err != nil { + t.Fatalf("unexpected error creating certificate: %v", err) + } + + cert, err := ParseCertificate(der) + if err != nil { + t.Fatalf("unexpected error parsing certificate: %v", err) + } + + if len(cert.IPAddresses) != 1 { + t.Fatalf("unexpected number of IP addresses in parsed certificate, got %d, want 1", len(cert.IPAddresses)) + } + + if len(cert.IPAddresses[0]) != net.IPv4len { + t.Fatalf("unexpected IP address length in parsed certificate, got %d, want %d", len(cert.IPAddresses[0]), net.IPv4len) + } +} + +func TestIPv4MappedIPsConstraintCreate(t *testing.T) { + _, mappedNetwork, _ := net.ParseCIDR("::ffff:192.0.2.1/128") + tmpl := Certificate{ + SerialNumber: big.NewInt(1), + NotBefore: time.Now().Add(-24 * time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + DNSNames: []string{"localhost"}, + PermittedIPRanges: []*net.IPNet{mappedNetwork}, + } + + _, err := CreateCertificate(rand.Reader, &tmpl, &tmpl, rsaPrivateKey.Public(), rsaPrivateKey) + if err == nil { + t.Fatalf("unexpected success creating certificate with IPv4-mapped IPv6 address constraint") + } +}