diff --git a/internal/xds/rbac/matchers.go b/internal/xds/rbac/matchers.go index c4ae7b3004de..e7fb95b816bf 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,15 @@ func newRemoteIPMatcher(cidrRange *v3corepb.CidrRange) (*remoteIPMatcher, error) } 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 newLocalIPMatcher(cidrRange *v3corepb.CidrRange) (*localIPMatcher, error) { } 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 fe84bf249d35..9534feaf4fec 100644 --- a/internal/xds/rbac/rbac_engine_test.go +++ b/internal/xds/rbac/rbac_engine_test.go @@ -760,7 +760,7 @@ func (s) TestChainEngine(t *testing.T) { 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 @@ func (s) TestChainEngine(t *testing.T) { 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 @@ func (s) TestChainEngine(t *testing.T) { 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 @@ func (s) TestChainEngine(t *testing.T) { 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 @@ func (s) TestChainEngine(t *testing.T) { { 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 @@ func (s) TestChainEngine(t *testing.T) { { 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 @@ func (s) TestChainEngine(t *testing.T) { { 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 @@ func (s) TestChainEngine(t *testing.T) { { 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 @@ func (s) TestChainEngine(t *testing.T) { 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 @@ func (s) TestChainEngine(t *testing.T) { 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 @@ func (s) TestChainEngine(t *testing.T) { 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 @@ func (s) TestChainEngine(t *testing.T) { { 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 @@ func (s) TestChainEngine(t *testing.T) { 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 @@ func (s) TestChainEngine(t *testing.T) { { 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 @@ func (s) TestChainEngine(t *testing.T) { 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 @@ func (s) TestChainEngine(t *testing.T) { { 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 @@ func (s) TestChainEngine(t *testing.T) { 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 @@ func (s) TestChainEngine(t *testing.T) { { 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 40d2b362baa0..dcb749f30611 100644 --- a/test/xds/xds_server_rbac_test.go +++ b/test/xds/xds_server_rbac_test.go @@ -605,6 +605,48 @@ func (s) TestRBACHTTPFilter(t *testing.T) { 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