xds/rbac: IP address matching doesn't handle ports

[arjan-bal]

During a PR review, Gemini raised a comment about a potential bug in the xDS RBAC implementation: #8908 (comment)

The remoteIPMatcher gets the peer address from the peer.Peer set by the transport:

func (sim *remoteIPMatcher) match(data *rpcData) bool {
    ip, _ := netip.ParseAddr(data.peerInfo.Addr.String())
    return sim.ipNet.Contains(net.IP(ip.AsSlice()))
    return sim.ipNet.Contains(ip)
}

All the non-test usages get the peerInfo.Addr field from the net.Conn. In most cases, this should contain the port. This means that netip.ParseAddr will fail and we need to use netip.ParseAddrPort instead. All the RBAC tests set the peer address to an IP address without a port, example

grpc-go/internal/xds/rbac/rbac_engine_test.go

Line 730 in c1a9239

Addr: &addr{ipAddress: "0.0.0.0"},

There doesn't seem to be any code that ensures the addresses stores in the rpcData always have the port stripped. This seems like a bug due to which address matching would always fail in the real world.

[easwars]

I added the following test table entry to TestRBACHTTPFilter in test/xds/xds_server_rbac_test.go and was able to reproduce the problem.

		{
			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,
		},

I was also subsequently able to confirm with a change to the remoteIPMatcher that using netip.ParseAddrPort works.

This bug not only affects the remote IP matcher, but should affect the local IP matcher.