-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Description
What version of gRPC are you using?
v1.81.0-dev (master, commit 12e91dd) — the bug exists in all versions since the RBAC engine was introduced.
What version of Go are you using (go version)?
go version go1.24.1 darwin/arm64
What operating system (Linux, Windows, …) and version?
macOS 15.3 (darwin/arm64)
What did you do?
I reviewed the RBAC IP matcher implementation in internal/xds/rbac/matchers.go and found that remoteIPMatcher.match() and localIPMatcher.match() call netip.ParseAddr() on net.Addr.String(), which returns an ip:port formatted string (e.g. "10.0.0.5:8080") in production. netip.ParseAddr() only accepts bare IP addresses (e.g. "10.0.0.5"), so the parse always fails on real TCP connections, returning a zero-value netip.Addr. The error is silently discarded.
As a result, ipNet.Contains(zeroAddr) always returns false, causing all IP-based RBAC rules to silently fail.
Root Cause
In internal/xds/rbac/matchers.go (lines 346–348):
func (sim *remoteIPMatcher) match(data *rpcData) bool {
ip, _ := netip.ParseAddr(data.peerInfo.Addr.String())
return sim.ipNet.Contains(ip)
}And similarly in lines 364–366:
func (dim *localIPMatcher) match(data *rpcData) bool {
ip, _ := netip.ParseAddr(data.localAddr.String())
return dim.ipNet.Contains(ip)
}In production, peer.Peer.Addr is a *net.TCPAddr whose String() returns "ip:port":
netip.ParseAddr("10.0.0.5:8080")
// error: ParseAddr("10.0.0.5:8080"): unexpected character (at ":8080")
// ip = invalid IP (zero-value)Notably, the same file (rbac_engine.go) already uses net.SplitHostPort() correctly for port matching at line 232:
_, dPort, err := net.SplitHostPort(conn.LocalAddr().String())Why Unit Tests Don't Catch This
The existing tests use a custom addr struct that returns only the IP without a port (rbac_engine_test.go lines 62–67):
type addr struct {
ipAddress string
}
func (addr) Network() string { return "" }
func (a *addr) String() string { return a.ipAddress }
// returns "0.0.0.0", not "0.0.0.0:8080"In production, peer.Peer.Addr is *net.TCPAddr, whose String() includes the port.
Local Verification
I wrote test cases using *net.TCPAddr (matching production behavior) and confirmed the bug:
Before fix (current code):
=== RUN TestRemoteIPMatcherWithTCPAddr
=== RUN TestRemoteIPMatcherWithTCPAddr/custom_addr_without_port_(test-only)_-_10.0.0.5
=== RUN TestRemoteIPMatcherWithTCPAddr/net.TCPAddr_with_port_(production)_-_10.0.0.5:8080
ip_matcher_bug_test.go:66: remoteIPMatcher.match() = false, want true (addr.String() = "10.0.0.5:8080")
--- FAIL: TestRemoteIPMatcherWithTCPAddr (0.00s)
--- PASS: .../custom_addr_without_port_(test-only)_-_10.0.0.5 (0.00s)
--- FAIL: .../net.TCPAddr_with_port_(production)_-_10.0.0.5:8080 (0.00s)
=== RUN TestLocalIPMatcherWithTCPAddr
=== RUN TestLocalIPMatcherWithTCPAddr/net.TCPAddr_with_port_(production)_-_172.16.5.1:9090
ip_matcher_bug_test.go:108: localIPMatcher.match() = false, want true (addr.String() = "172.16.5.1:9090")
--- FAIL: TestLocalIPMatcherWithTCPAddr (0.00s)
--- PASS: .../custom_addr_without_port_(test-only)_-_172.16.5.1 (0.00s)
--- FAIL: .../net.TCPAddr_with_port_(production)_-_172.16.5.1:9090 (0.00s)
FAIL
The custom addr struct (test-only) passes, but *net.TCPAddr (production) fails — confirming the bug.
After fix (using net.SplitHostPort):
=== RUN TestRemoteIPMatcherWithTCPAddr
--- PASS: TestRemoteIPMatcherWithTCPAddr (0.00s)
--- PASS: .../custom_addr_without_port_(test-only)_-_10.0.0.5 (0.00s)
--- PASS: .../net.TCPAddr_with_port_(production)_-_10.0.0.5:8080 (0.00s)
=== RUN TestLocalIPMatcherWithTCPAddr
--- PASS: TestLocalIPMatcherWithTCPAddr (0.00s)
--- PASS: .../custom_addr_without_port_(test-only)_-_172.16.5.1 (0.00s)
--- PASS: .../net.TCPAddr_with_port_(production)_-_172.16.5.1:9090 (0.00s)
PASS
ok google.golang.org/grpc/internal/xds/rbac 0.322s
All tests pass, including the full existing test suite.
Test code used for verification
func 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: "custom addr without port (test-only) - 10.0.0.5",
addr: &addr{ipAddress: "10.0.0.5"},
wantMatch: true,
},
{
name: "net.TCPAddr with port (production) - 10.0.0.5:8080",
addr: &net.TCPAddr{IP: net.ParseIP("10.0.0.5"), Port: 8080},
wantMatch: true,
},
{
name: "custom addr without port (test-only) - 192.168.1.1",
addr: &addr{ipAddress: "192.168.1.1"},
wantMatch: false,
},
{
name: "net.TCPAddr with port (production) - 192.168.1.1:443",
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{
peerInfo: &peer.Peer{Addr: tt.addr},
}
got := matcher.match(data)
if got != tt.wantMatch {
t.Errorf("remoteIPMatcher.match() = %v, want %v (addr.String() = %q)",
got, tt.wantMatch, tt.addr.String())
}
})
}
}What did you expect to see?
IP-based RBAC rules (DirectRemoteIp, RemoteIp, DestinationIp matchers) should correctly match against the client's or server's IP address, regardless of whether the net.Addr.String() representation includes a port number.
For example, a DENY rule for 10.0.0.0/8 should block a request from 10.0.0.5:8080.
What did you see instead?
All IP-based RBAC rules silently fail because netip.ParseAddr() receives "10.0.0.5:8080" instead of "10.0.0.5". The parse error is discarded, and ipNet.Contains(zeroAddr) always returns false.
Impact:
- DENY rules: Traffic from blocked IP ranges is incorrectly allowed through (authorization bypass).
- ALLOW rules: All traffic is denied because the IP never matches (fail-closed denial of service).
- Affected: All gRPC-Go servers using
google.golang.org/grpc/authzwith IP-based RBAC policies, including xDS/Envoy service mesh configurations.
Suggested Fix
Use net.SplitHostPort() to extract the host before parsing, with a fallback for addresses that don't include a port:
func (sim *remoteIPMatcher) match(data *rpcData) bool {
addrStr := data.peerInfo.Addr.String()
host, _, err := net.SplitHostPort(addrStr)
if err != nil {
host = addrStr
}
ip, _ := netip.ParseAddr(host)
return sim.ipNet.Contains(ip)
}
func (dim *localIPMatcher) match(data *rpcData) bool {
addrStr := data.localAddr.String()
host, _, err := net.SplitHostPort(addrStr)
if err != nil {
host = addrStr
}
ip, _ := netip.ParseAddr(host)
return dim.ipNet.Contains(ip)
}I'm happy to submit a PR with this fix and updated tests.