[go] crypto/x509: normalize IP name constraints with non-zero host bits
4 views
Skip to first unread message
p4p3r (Gerrit)
Jun 4, 2026, 8:44:20 PM (5 days ago) Jun 4
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
p4p3r has uploaded the change for review![]( "Open in Gerrit")
Commit message
crypto/x509: normalize IP name constraints with non-zero host bits
Name constraint IP subtrees are encoded as a network address followed by
a mask. RFC 5280 permits the network address to carry non-zero host bits
(e.g. 10.10.10.10/16, denoting 10.10.0.0/16), and some CA tooling emits
this verbatim. parseNameConstraintsExtension stored the address exactly
as encoded.
The sub-quadratic constraint matcher added in Go 1.26 (constraints.go)
binary-searches the sorted constraint set using the raw stored IP and
then checks only the nearest lower-bound entry with net.IPNet.Contains.
That relies on the stored IP being the canonical masked network address.
When the address has host bits set, the containing network can sort above
the target while a lower-addressed constraint occupies the neighbor slot,
so the containing network is never tested and search reports no match.
Because this only ever yields false negatives, an excluded constraint
with host bits set could fail to match an address inside its network,
silently accepting a certificate the constraint should have rejected.
Mask off the host bits at parse time so the stored address is always the
canonical network address, restoring the matcher's invariant. This also
makes the parsed ranges and constraint error messages canonical.
Fixes #79833
Change-Id: I7ec6b0452871d32a2f3db2eb9d46802ce64ecd78
Change diff
diff --git a/src/crypto/x509/name_constraints_test.go b/src/crypto/x509/name_constraints_test.go
index 803ab17..42680fd 100644
--- a/src/crypto/x509/name_constraints_test.go
+++ b/src/crypto/x509/name_constraints_test.go
@@ -2397,3 +2397,88 @@
}
}
}
+
+// TestExcludedIPConstraintWithHostBits checks that an excluded iPAddress
+// constraint encoded with non-zero host bits (e.g. 10.10.10.10/16) is still
+// enforced. See go.dev/issue/79833.
+func TestExcludedIPConstraintWithHostBits(t *testing.T) {
+ // nameConstraintsExt hand-encodes a NameConstraints extension excluding the
+ // given IPv4 (ip||mask) subtrees, preserving host bits (CreateCertificate
+ // would mask them off).
+ nameConstraintsExt := func(excluded [][8]byte) pkix.Extension {
+ var subtrees []byte
+ for _, e := range excluded {
+ gn := append([]byte{0x87, 0x08}, e[:]...) // [7] IMPLICIT OCTET STRING
+ subtrees = append(subtrees, append([]byte{0x30, byte(len(gn))}, gn...)...)
+ }
+ excl := append([]byte{0xA1, byte(len(subtrees))}, subtrees...) // [1] excludedSubtrees
+ val := append([]byte{0x30, byte(len(excl))}, excl...) // NameConstraints SEQUENCE
+ return pkix.Extension{Id: []int{2, 5, 29, 30}, Critical: true, Value: val}
+ }
+
+ caKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+ if err != nil {
+ t.Fatal(err)
+ }
+ leafKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ caTmpl := &Certificate{
+ SerialNumber: big.NewInt(1),
+ Subject: pkix.Name{CommonName: "Constrained CA"},
+ NotBefore: time.Now().Add(-time.Hour),
+ NotAfter: time.Now().Add(time.Hour),
+ IsCA: true,
+ BasicConstraintsValid: true,
+ KeyUsage: KeyUsageCertSign,
+ ExtraExtensions: []pkix.Extension{
+ nameConstraintsExt([][8]byte{
+ {10, 0, 0, 0, 255, 255, 255, 252}, // 10.0.0.0/30, occupies the lower neighbor slot
+ {10, 10, 10, 10, 255, 255, 0, 0}, // 10.10.10.10/16: host bits set, network 10.10.0.0/16
+ }),
+ },
+ }
+ caDER, err := CreateCertificate(rand.Reader, caTmpl, caTmpl, &caKey.PublicKey, caKey)
+ if err != nil {
+ t.Fatal(err)
+ }
+ ca, err := ParseCertificate(caDER)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ // The parsed excluded range must be the masked network address.
+ if got := ca.ExcludedIPRanges[1].String(); got != "10.10.0.0/16" {
+ t.Errorf("excluded range not normalized: got %q, want %q", got, "10.10.0.0/16")
+ }
+
+ // 10.10.0.1 is inside the excluded 10.10.0.0/16 network but numerically
+ // below the encoded 10.10.10.10, which is what triggered the bypass.
+ leafTmpl := &Certificate{
+ SerialNumber: big.NewInt(2),
+ Subject: pkix.Name{CommonName: "leaf"},
+ NotBefore: time.Now().Add(-time.Hour),
+ NotAfter: time.Now().Add(time.Hour),
+ IPAddresses: []net.IP{net.IPv4(10, 10, 0, 1)},
+ KeyUsage: KeyUsageDigitalSignature,
+ ExtKeyUsage: []ExtKeyUsage{ExtKeyUsageServerAuth},
+ }
+ leafDER, err := CreateCertificate(rand.Reader, leafTmpl, ca, &leafKey.PublicKey, caKey)
+ if err != nil {
+ t.Fatal(err)
+ }
+ leaf, err := ParseCertificate(leafDER)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ roots := NewCertPool()
+ roots.AddCert(ca)
+ if _, err := leaf.Verify(VerifyOptions{Roots: roots}); err == nil {
+ t.Error("leaf with IP SAN inside an excluded range was accepted; constraint bypassed")
+ } else if !strings.Contains(err.Error(), "excluded by constraint") {
+ t.Errorf("unexpected verify error: %v", err)
+ }
+}
diff --git a/src/crypto/x509/parser.go b/src/crypto/x509/parser.go
index 4a1416e..9654512 100644
--- a/src/crypto/x509/parser.go
+++ b/src/crypto/x509/parser.go
@@ -692,7 +692,11 @@
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)})
+ ipnet := &net.IPNet{IP: net.IP(ip), Mask: net.IPMask(mask)}
+ // Mask off host bits; the matcher requires the canonical
+ // network address (see constraints.go).
+ ipnet.IP = ipnet.IP.Mask(ipnet.Mask)
+ ips = append(ips, ipnet)
case emailTag:
constraint := string(value)
Change information
Files:
- M src/crypto/x509/name_constraints_test.go
- M src/crypto/x509/parser.go
Change size: M
Delta: 2 files changed, 90 insertions(+), 1 deletion(-)
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gopher Robot (Gerrit)
Jun 4, 2026, 8:47:03 PM (5 days ago) Jun 4
to p4p3r, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Message from Gopher Robot![]( "Open in Gerrit")
Congratulations on opening your first change. Thank you for your contribution!
Next steps:
A maintainer will review your change and provide feedback. See
https://go.dev/doc/contribute#review for more info and tips to get your
patch through code review.
Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.
During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://go.dev/s/release
for more details.
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Roland Shoemaker (Gerrit)
Jun 5, 2026, 11:23:07 AM (5 days ago) Jun 5
to p4p3r, goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Attention needed from p4p3r
Roland Shoemaker voted and added 1 comment![]( "Open in Gerrit")
Votes added by Roland Shoemaker
| Commit-Queue | +1 |
1 comment
Patchset-level comments
File-level comment, Patchset 1 (Latest):
Roland Shoemaker . resolved
This isn't a blocking comment, but I do wonder if we should do the masking as part of the constraint code, instead of in the parser, so that we maintain the actual CIDR notation that is encoded in the certificate, which _someone_ might be expecting?
Related details
Attention is currently required from:
- p4p3r
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Roland Shoemaker (Gerrit)
Jun 5, 2026, 11:46:20 AM (5 days ago) Jun 5
to p4p3r, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Gopher Robot, golang-co...@googlegroups.com
Related details
Attention is currently required from:
- p4p3r
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
p4p3r (Gerrit)
Jun 5, 2026, 12:01:44 PM (5 days ago) Jun 5
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Roland Shoemaker and p4p3r
p4p3r uploaded new patchset![]( "Open in Gerrit")
p4p3r uploaded patch set #2 to this change.
Following approvals got outdated and were removed:
- Code-Review: +2 by Roland Shoemaker
- TryBots-Pass: LUCI-TryBot-Result+1 by golang...@luci-project-accounts.iam.gserviceaccount.com
Related details
Attention is currently required from:
- Roland Shoemaker
- p4p3r
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
p4p3r (Gerrit)
Jun 9, 2026, 11:20:28 PM (7 hours ago) Jun 9
to goph...@pubsubhelper.golang.org, Ian Alexander, Roland Shoemaker, golang...@luci-project-accounts.iam.gserviceaccount.com, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Ian Alexander and Roland Shoemaker
p4p3r added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Roland Shoemaker . resolved
This isn't a blocking comment, but I do wonder if we should do the masking as part of the constraint code, instead of in the parser, so that we maintain the actual CIDR notation that is encoded in the certificate, which _someone_ might be expecting?
p4p3r
Done in PS2 — I moved the masking logic into newIPNetConstraints instead of the parser, so parsed IP ranges now preserve the CIDR exactly as encoded.
Only the sort/search keys needed a masked address (Contains and the subset check were already correct), so newIPNetConstraints now operates on a masked copy while aliasing the original when it is already canonical.
I also added a test that verifies both constraint enforcement and preservation of the original encoded range.
Related details
Attention is currently required from:
- Ian Alexander
- Roland Shoemaker
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages