xds/rbac: add additional handling for addresses with ports (#8990)
This PR enhances the rbac matcher to handle IP address string with a
port attached.
The fix introduces the `net.SplitHostPort` utility function, ensuring
the port is properly stripped out of the underlying
`peerInfo.Addr.String()` and `localAddr.String()` values before parsing
them with `netip.ParseAddr`. A fallback mechanism is also included in
case `SplitHostPort` fails due to a missing port.
RELEASE NOTES:
* xds/rbac: Add additional handling for addresses with ports
diff --git a/internal/xds/rbac/matchers.go b/internal/xds/rbac/matchers.go
index c4ae7b3..e7fb95b 100644
--- a/internal/xds/rbac/matchers.go
+++ b/internal/xds/rbac/matchers.go
@@ -19,6 +19,7 @@
import (
"errors"
"fmt"
+ "net"
"net/netip"
"regexp"
@@ -344,7 +345,15 @@
}
func (sim *remoteIPMatcher) match(data *rpcData) bool {
- ip, _ := netip.ParseAddr(data.peerInfo.Addr.String())
+ host, _, err := net.SplitHostPort(data.peerInfo.Addr.String())
+ if err != nil {
+ // Fallback for addresses without a port.
+ host = data.peerInfo.Addr.String()
+ }
+ ip, err := netip.ParseAddr(host)
+ if err != nil {
+ return false
+ }
return sim.ipNet.Contains(ip)
}
@@ -362,7 +371,15 @@
}
func (dim *localIPMatcher) match(data *rpcData) bool {
- ip, _ := netip.ParseAddr(data.localAddr.String())
+ host, _, err := net.SplitHostPort(data.localAddr.String())
+ if err != nil {
+ // Fallback for addresses without a port.
+ host = data.localAddr.String()
+ }
+ ip, err := netip.ParseAddr(host)
+ if err != nil {
+ return false
+ }
return dim.ipNet.Contains(ip)
}
diff --git a/internal/xds/rbac/rbac_engine_test.go b/internal/xds/rbac/rbac_engine_test.go
index fe84bf2..9534fea 100644
--- a/internal/xds/rbac/rbac_engine_test.go
+++ b/internal/xds/rbac/rbac_engine_test.go
@@ -760,7 +760,7 @@
rpcData: &rpcData{
fullMethod: "localhost-fan-page",
peerInfo: &peer.Peer{
- Addr: &addr{ipAddress: "0.0.0.0"},
+ Addr: &addr{ipAddress: "0.0.0.0:8080"},
},
},
wantStatusCode: codes.OK,
@@ -823,7 +823,7 @@
rpcData: &rpcData{
fullMethod: "some method",
peerInfo: &peer.Peer{
- Addr: &addr{ipAddress: "0.0.0.0"},
+ Addr: &addr{ipAddress: "0.0.0.0:8080"},
AuthInfo: credentials.TLSInfo{
State: tls.ConnectionState{
PeerCertificates: []*x509.Certificate{
@@ -855,7 +855,7 @@
rpcData: &rpcData{
fullMethod: "get-product-list",
peerInfo: &peer.Peer{
- Addr: &addr{ipAddress: "0.0.0.0"},
+ Addr: &addr{ipAddress: "0.0.0.0:8080"},
},
},
wantStatusCode: codes.PermissionDenied,
@@ -907,7 +907,7 @@
rpcData: &rpcData{
fullMethod: "/regular-content",
peerInfo: &peer.Peer{
- Addr: &addr{ipAddress: "0.0.0.0"},
+ Addr: &addr{ipAddress: "0.0.0.0:8080"},
},
},
wantStatusCode: codes.OK,
@@ -945,7 +945,7 @@
{
rpcData: &rpcData{
peerInfo: &peer.Peer{
- Addr: &addr{ipAddress: "0.0.0.0"},
+ Addr: &addr{ipAddress: "0.0.0.0:8080"},
},
},
wantStatusCode: codes.OK,
@@ -995,7 +995,7 @@
{
rpcData: &rpcData{
peerInfo: &peer.Peer{
- Addr: &addr{ipAddress: "10.0.0.0"},
+ Addr: &addr{ipAddress: "10.0.0.0:8080"},
},
},
wantStatusCode: codes.PermissionDenied,
@@ -1072,7 +1072,7 @@
{
rpcData: &rpcData{
peerInfo: &peer.Peer{
- Addr: &addr{ipAddress: "0.0.0.0"},
+ Addr: &addr{ipAddress: "0.0.0.0:8080"},
},
},
wantStatusCode: codes.OK,
@@ -1093,7 +1093,7 @@
{
rpcData: &rpcData{
peerInfo: &peer.Peer{
- Addr: &addr{ipAddress: "10.0.0.0"},
+ Addr: &addr{ipAddress: "10.0.0.0:8080"},
},
},
wantStatusCode: codes.PermissionDenied,
@@ -1139,7 +1139,7 @@
rpcData: &rpcData{
fullMethod: "some method",
peerInfo: &peer.Peer{
- Addr: &addr{ipAddress: "0.0.0.0"},
+ Addr: &addr{ipAddress: "0.0.0.0:8080"},
AuthInfo: credentials.TLSInfo{
State: tls.ConnectionState{
PeerCertificates: []*x509.Certificate{
@@ -1218,7 +1218,7 @@
rpcData: &rpcData{
fullMethod: "some method",
peerInfo: &peer.Peer{
- Addr: &addr{ipAddress: "0.0.0.0"},
+ Addr: &addr{ipAddress: "0.0.0.0:8080"},
},
},
wantStatusCode: codes.OK,
@@ -1328,7 +1328,7 @@
rpcData: &rpcData{
fullMethod: "localhost-fan-page",
peerInfo: &peer.Peer{
- Addr: &addr{ipAddress: "0.0.0.0"},
+ Addr: &addr{ipAddress: "0.0.0.0:8080"},
},
},
wantStatusCode: codes.PermissionDenied,
@@ -1338,7 +1338,7 @@
{
rpcData: &rpcData{
peerInfo: &peer.Peer{
- Addr: &addr{ipAddress: "10.0.0.0"},
+ Addr: &addr{ipAddress: "10.0.0.0:8080"},
},
},
wantStatusCode: codes.PermissionDenied,
@@ -1427,7 +1427,7 @@
rpcData: &rpcData{
fullMethod: "localhost-fan-page",
peerInfo: &peer.Peer{
- Addr: &addr{ipAddress: "0.0.0.0"},
+ Addr: &addr{ipAddress: "0.0.0.0:8080"},
AuthInfo: credentials.TLSInfo{
State: tls.ConnectionState{
PeerCertificates: []*x509.Certificate{
@@ -1460,7 +1460,7 @@
{
rpcData: &rpcData{
peerInfo: &peer.Peer{
- Addr: &addr{ipAddress: "10.0.0.0"},
+ Addr: &addr{ipAddress: "10.0.0.0:8080"},
},
},
wantStatusCode: codes.PermissionDenied,
@@ -1566,7 +1566,7 @@
rpcData: &rpcData{
fullMethod: "localhost-fan-page",
peerInfo: &peer.Peer{
- Addr: &addr{ipAddress: "0.0.0.0"},
+ Addr: &addr{ipAddress: "0.0.0.0:8080"},
},
},
wantStatusCode: codes.PermissionDenied,
@@ -1577,7 +1577,7 @@
{
rpcData: &rpcData{
peerInfo: &peer.Peer{
- Addr: &addr{ipAddress: "10.0.0.0"},
+ Addr: &addr{ipAddress: "10.0.0.0:8080"},
},
},
wantStatusCode: codes.PermissionDenied,
@@ -1675,7 +1675,7 @@
rpcData: &rpcData{
fullMethod: "localhost-fan-page",
peerInfo: &peer.Peer{
- Addr: &addr{ipAddress: "0.0.0.0"},
+ Addr: &addr{ipAddress: "0.0.0.0:8080"},
},
},
wantStatusCode: codes.PermissionDenied,
@@ -1694,7 +1694,7 @@
{
rpcData: &rpcData{
peerInfo: &peer.Peer{
- Addr: &addr{ipAddress: "10.0.0.0"},
+ Addr: &addr{ipAddress: "10.0.0.0:8080"},
},
},
wantStatusCode: codes.PermissionDenied,
diff --git a/test/xds/xds_server_rbac_test.go b/test/xds/xds_server_rbac_test.go
index 40d2b36..dcb749f 100644
--- a/test/xds/xds_server_rbac_test.go
+++ b/test/xds/xds_server_rbac_test.go
@@ -605,6 +605,48 @@
wantStatusEmptyCall: codes.PermissionDenied,
wantStatusUnaryCall: codes.PermissionDenied,
},
+ {
+ name: "match-on-principal-remote-ip",
+ rbacCfg: &rpb.RBAC{
+ Rules: &v3rbacpb.RBAC{
+ Action: v3rbacpb.RBAC_ALLOW,
+ Policies: map[string]*v3rbacpb.Policy{
+ "match-on-principal-remote-ip": {
+ Permissions: []*v3rbacpb.Permission{
+ {
+ Rule: &v3rbacpb.Permission_Header{
+ Header: &v3routepb.HeaderMatcher{
+ Name: "host",
+ HeaderMatchSpecifier: &v3routepb.HeaderMatcher_PrefixMatch{PrefixMatch: "my-service-fallback"},
+ },
+ },
+ },
+ },
+ Principals: []*v3rbacpb.Principal{
+ {
+ Identifier: &v3rbacpb.Principal_RemoteIp{
+ RemoteIp: &v3corepb.CidrRange{
+ AddressPrefix: "127.0.0.0",
+ PrefixLen: &wrapperspb.UInt32Value{Value: 8},
+ },
+ },
+ },
+ {
+ Identifier: &v3rbacpb.Principal_RemoteIp{
+ RemoteIp: &v3corepb.CidrRange{
+ AddressPrefix: "::1",
+ PrefixLen: &wrapperspb.UInt32Value{Value: 128},
+ },
+ },
+ },
+ },
+ },
+ },
+ },
+ },
+ wantStatusEmptyCall: codes.OK,
+ wantStatusUnaryCall: codes.OK,
+ },
// This test tests that RBAC ignores the TE: trailers header (which is
// hardcoded in http2_client.go for every RPC). Since the RBAC
// Configuration says to only ALLOW RPC's with a TE: Trailers, every RPC
