From e13541598aa1eb5aaee49f21bae9edbe2221cc7a Mon Sep 17 00:00:00 2001 From: instantraaamen Date: Fri, 20 Mar 2026 21:51:12 +0900 Subject: [PATCH 1/3] xds/rbac: use net.SplitHostPort in IP matchers to handle ip:port addresses remoteIPMatcher.match() and localIPMatcher.match() call netip.ParseAddr() on net.Addr.String(), which returns "ip:port" format in production (*net.TCPAddr). Since netip.ParseAddr() only accepts bare IP addresses, the parse always fails, causing all IP-based RBAC rules to silently not match. Use net.SplitHostPort() to extract the host before parsing, consistent with how rbac_engine.go already handles address parsing for port matching. Add test cases using *net.TCPAddr to verify correct behavior with ip:port format addresses. Fixes #8991 Made-with: Cursor --- internal/xds/rbac/ip_matcher_test.go | 131 +++++++++++++++++++++++++++ internal/xds/rbac/matchers.go | 15 ++- 2 files changed, 144 insertions(+), 2 deletions(-) create mode 100644 internal/xds/rbac/ip_matcher_test.go diff --git a/internal/xds/rbac/ip_matcher_test.go b/internal/xds/rbac/ip_matcher_test.go new file mode 100644 index 000000000000..01b571f70ae1 --- /dev/null +++ b/internal/xds/rbac/ip_matcher_test.go @@ -0,0 +1,131 @@ +/* + * Copyright 2026 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package rbac + +import ( + "net" + "testing" + + v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" + "google.golang.org/grpc/peer" + "google.golang.org/protobuf/types/known/wrapperspb" +) + +// TestRemoteIPMatcherWithTCPAddr verifies that remoteIPMatcher works correctly +// when peer.Addr is a *net.TCPAddr, whose String() returns "ip:port" format. +func (s) TestRemoteIPMatcherWithTCPAddr(t *testing.T) { + cidr := &v3corepb.CidrRange{ + AddressPrefix: "10.0.0.0", + PrefixLen: &wrapperspb.UInt32Value{Value: 8}, + } + matcher, err := newRemoteIPMatcher(cidr) + if err != nil { + t.Fatalf("newRemoteIPMatcher() error: %v", err) + } + + tests := []struct { + name string + addr net.Addr + wantMatch bool + }{ + { + name: "bare IP matching CIDR", + addr: &addr{ipAddress: "10.0.0.5"}, + wantMatch: true, + }, + { + name: "TCPAddr matching CIDR", + addr: &net.TCPAddr{IP: net.ParseIP("10.0.0.5"), Port: 8080}, + wantMatch: true, + }, + { + name: "bare IP not matching CIDR", + addr: &addr{ipAddress: "192.168.1.1"}, + wantMatch: false, + }, + { + name: "TCPAddr not matching CIDR", + addr: &net.TCPAddr{IP: net.ParseIP("192.168.1.1"), Port: 443}, + wantMatch: false, + }, + { + name: "IPv6 TCPAddr matching CIDR", + addr: &net.TCPAddr{IP: net.ParseIP("10.0.0.1"), Port: 0}, + wantMatch: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + data := &rpcData{ + peerInfo: &peer.Peer{Addr: tt.addr}, + } + if got := matcher.match(data); got != tt.wantMatch { + t.Errorf("remoteIPMatcher.match() = %v, want %v (addr.String() = %q)", got, tt.wantMatch, tt.addr.String()) + } + }) + } +} + +// TestLocalIPMatcherWithTCPAddr verifies that localIPMatcher works correctly +// when localAddr is a *net.TCPAddr, whose String() returns "ip:port" format. +func (s) TestLocalIPMatcherWithTCPAddr(t *testing.T) { + cidr := &v3corepb.CidrRange{ + AddressPrefix: "172.16.0.0", + PrefixLen: &wrapperspb.UInt32Value{Value: 12}, + } + matcher, err := newLocalIPMatcher(cidr) + if err != nil { + t.Fatalf("newLocalIPMatcher() error: %v", err) + } + + tests := []struct { + name string + addr net.Addr + wantMatch bool + }{ + { + name: "bare IP matching CIDR", + addr: &addr{ipAddress: "172.16.5.1"}, + wantMatch: true, + }, + { + name: "TCPAddr matching CIDR", + addr: &net.TCPAddr{IP: net.ParseIP("172.16.5.1"), Port: 9090}, + wantMatch: true, + }, + { + name: "bare IP not matching CIDR", + addr: &addr{ipAddress: "192.168.1.1"}, + wantMatch: false, + }, + { + name: "TCPAddr not matching CIDR", + addr: &net.TCPAddr{IP: net.ParseIP("192.168.1.1"), Port: 443}, + wantMatch: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + data := &rpcData{localAddr: tt.addr} + if got := matcher.match(data); got != tt.wantMatch { + t.Errorf("localIPMatcher.match() = %v, want %v (addr.String() = %q)", got, tt.wantMatch, tt.addr.String()) + } + }) + } +} diff --git a/internal/xds/rbac/matchers.go b/internal/xds/rbac/matchers.go index c4ae7b3004de..a160a0f60ace 100644 --- a/internal/xds/rbac/matchers.go +++ b/internal/xds/rbac/matchers.go @@ -19,6 +19,7 @@ package rbac import ( "errors" "fmt" + "net" "net/netip" "regexp" @@ -344,7 +345,12 @@ func newRemoteIPMatcher(cidrRange *v3corepb.CidrRange) (*remoteIPMatcher, error) } func (sim *remoteIPMatcher) match(data *rpcData) bool { - ip, _ := netip.ParseAddr(data.peerInfo.Addr.String()) + addrStr := data.peerInfo.Addr.String() + host, _, err := net.SplitHostPort(addrStr) + if err != nil { + host = addrStr + } + ip, _ := netip.ParseAddr(host) return sim.ipNet.Contains(ip) } @@ -362,7 +368,12 @@ func newLocalIPMatcher(cidrRange *v3corepb.CidrRange) (*localIPMatcher, error) { } func (dim *localIPMatcher) match(data *rpcData) bool { - ip, _ := netip.ParseAddr(data.localAddr.String()) + addrStr := data.localAddr.String() + host, _, err := net.SplitHostPort(addrStr) + if err != nil { + host = addrStr + } + ip, _ := netip.ParseAddr(host) return dim.ipNet.Contains(ip) } From bf5253e9747fe1a564ce17f2c9be67767d4bfac1 Mon Sep 17 00:00:00 2001 From: instantraaamen Date: Fri, 20 Mar 2026 22:06:37 +0900 Subject: [PATCH 2/3] xds/rbac: replace net.ParseIP with netip.MustParseAddr in tests Replace net.ParseIP (disallowed by vet.sh) with netip.MustParseAddr(...).AsSlice() in ip_matcher_test.go. Made-with: Cursor --- internal/xds/rbac/ip_matcher_test.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/internal/xds/rbac/ip_matcher_test.go b/internal/xds/rbac/ip_matcher_test.go index 01b571f70ae1..482f57191dc1 100644 --- a/internal/xds/rbac/ip_matcher_test.go +++ b/internal/xds/rbac/ip_matcher_test.go @@ -18,6 +18,7 @@ package rbac import ( "net" + "net/netip" "testing" v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" @@ -49,7 +50,7 @@ func (s) TestRemoteIPMatcherWithTCPAddr(t *testing.T) { }, { name: "TCPAddr matching CIDR", - addr: &net.TCPAddr{IP: net.ParseIP("10.0.0.5"), Port: 8080}, + addr: &net.TCPAddr{IP: netip.MustParseAddr("10.0.0.5").AsSlice(), Port: 8080}, wantMatch: true, }, { @@ -59,12 +60,12 @@ func (s) TestRemoteIPMatcherWithTCPAddr(t *testing.T) { }, { name: "TCPAddr not matching CIDR", - addr: &net.TCPAddr{IP: net.ParseIP("192.168.1.1"), Port: 443}, - wantMatch: false, + addr: &net.TCPAddr{IP: netip.MustParseAddr("192.168.1.1").AsSlice(), Port: 443}, + wantMatch: false, }, { name: "IPv6 TCPAddr matching CIDR", - addr: &net.TCPAddr{IP: net.ParseIP("10.0.0.1"), Port: 0}, + addr: &net.TCPAddr{IP: netip.MustParseAddr("10.0.0.1").AsSlice(), Port: 0}, wantMatch: true, }, } @@ -105,7 +106,7 @@ func (s) TestLocalIPMatcherWithTCPAddr(t *testing.T) { }, { name: "TCPAddr matching CIDR", - addr: &net.TCPAddr{IP: net.ParseIP("172.16.5.1"), Port: 9090}, + addr: &net.TCPAddr{IP: netip.MustParseAddr("172.16.5.1").AsSlice(), Port: 9090}, wantMatch: true, }, { @@ -115,7 +116,7 @@ func (s) TestLocalIPMatcherWithTCPAddr(t *testing.T) { }, { name: "TCPAddr not matching CIDR", - addr: &net.TCPAddr{IP: net.ParseIP("192.168.1.1"), Port: 443}, + addr: &net.TCPAddr{IP: netip.MustParseAddr("192.168.1.1").AsSlice(), Port: 443}, wantMatch: false, }, } From 2dd19b3ef5cfb6595c8c605cdb0dddd30ffbda2c Mon Sep 17 00:00:00 2001 From: instantraaamen Date: Fri, 20 Mar 2026 22:10:04 +0900 Subject: [PATCH 3/3] xds/rbac: fix gofmt indentation in ip_matcher_test.go Made-with: Cursor --- internal/xds/rbac/ip_matcher_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/xds/rbac/ip_matcher_test.go b/internal/xds/rbac/ip_matcher_test.go index 482f57191dc1..f716f057982b 100644 --- a/internal/xds/rbac/ip_matcher_test.go +++ b/internal/xds/rbac/ip_matcher_test.go @@ -61,7 +61,7 @@ func (s) TestRemoteIPMatcherWithTCPAddr(t *testing.T) { { name: "TCPAddr not matching CIDR", addr: &net.TCPAddr{IP: netip.MustParseAddr("192.168.1.1").AsSlice(), Port: 443}, - wantMatch: false, + wantMatch: false, }, { name: "IPv6 TCPAddr matching CIDR",