From 3edbfbfa41d127dbac486edf05ceca16e6e98b0b Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Thu, 29 Sep 2022 10:05:06 +0530 Subject: [PATCH] fix localhost address check in xfcc authenticator (#41189) * fix localhost address check in xfcc authenticator Signed-off-by: Rama Chavali * fix log Signed-off-by: Rama Chavali * fix ut Signed-off-by: Rama Chavali * move up Signed-off-by: Rama Chavali Signed-off-by: Rama Chavali --- .../ca/authenticate/xfcc_authenticator.go | 10 ++++++++-- .../authenticate/xfcc_authenticator_test.go | 19 ++++++++++++++----- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/security/pkg/server/ca/authenticate/xfcc_authenticator.go b/security/pkg/server/ca/authenticate/xfcc_authenticator.go index 6b1d548bf436..bfa5fa3a7eef 100644 --- a/security/pkg/server/ca/authenticate/xfcc_authenticator.go +++ b/security/pkg/server/ca/authenticate/xfcc_authenticator.go @@ -26,6 +26,7 @@ import ( "istio.io/istio/pilot/pkg/features" "istio.io/istio/pkg/security" + "istio.io/pkg/log" ) const ( @@ -92,13 +93,18 @@ func buildSecurityCaller(xfccHeader string) (*security.Caller, error) { } func isTrustedAddress(addr string, trustedCidrs []string) bool { + ip, _, err := net.SplitHostPort(addr) + if err != nil { + log.Warnf("peer address %s can not be split in to proper host and port", addr) + return false + } for _, cidr := range trustedCidrs { - if isInRange(addr, cidr) { + if isInRange(ip, cidr) { return true } } // Always trust local host addresses. - return net.ParseIP(addr).IsLoopback() + return net.ParseIP(ip).IsLoopback() } func isInRange(addr, cidr string) bool { diff --git a/security/pkg/server/ca/authenticate/xfcc_authenticator_test.go b/security/pkg/server/ca/authenticate/xfcc_authenticator_test.go index 30b1b55f4779..139a63e0baca 100644 --- a/security/pkg/server/ca/authenticate/xfcc_authenticator_test.go +++ b/security/pkg/server/ca/authenticate/xfcc_authenticator_test.go @@ -16,6 +16,7 @@ package authenticate import ( "net" + "net/netip" "reflect" "strings" "testing" @@ -36,11 +37,18 @@ func TestIsTrustedAddress(t *testing.T) { trusted bool }{ { - name: "localhost client", + name: "localhost client with port", cidr: "", - peer: "127.0.0.1", + peer: "127.0.0.1:9901", trusted: true, }, + { + // Should never happen, added test case for testing it. + name: "localhost client without port", + cidr: "", + peer: "127.0.0.1", + trusted: false, + }, { name: "external client without trusted cidr", cidr: "", @@ -50,13 +58,13 @@ func TestIsTrustedAddress(t *testing.T) { { name: "cidr in range", cidr: "172.17.0.0/16,192.17.0.0/16", - peer: "172.17.0.2", + peer: "172.17.0.2:9901", trusted: true, }, { name: "cidr in range with both ipv6 and ipv4", cidr: "172.17.0.0/16,2001:db8:1234:1a00::/56", - peer: "2001:0db8:1234:1aff:ffff:ffff:ffff:ffff", + peer: "[2001:0db8:1234:1a00:0000:0000:0000:0000]:80", trusted: true, }, { @@ -130,7 +138,8 @@ func TestXfccAuthenticator(t *testing.T) { if len(tt.xfccHeader) > 0 { md.Append(xfccparser.ForwardedClientCertHeader, tt.xfccHeader) } - ctx := peer.NewContext(context.Background(), &peer.Peer{Addr: &net.IPAddr{IP: net.ParseIP("127.0.0.1").To4()}}) + addr := net.TCPAddrFromAddrPort(netip.MustParseAddrPort("127.0.0.1:2301")) + ctx := peer.NewContext(context.Background(), &peer.Peer{Addr: addr}) ctx = metadata.NewIncomingContext(ctx, md) result, err := auth.Authenticate(security.AuthContext{GrpcContext: ctx}) if len(tt.authenticateErrMsg) > 0 {