Move gRPC server to Unix socket and scrub introspection data

Author: oliviassss

cmd/routed-eni-cni-plugin/cni.go

@@ -47,6 +47,7 @@ import (
 
 const (
 	ipamdAddress            = "127.0.0.1:50051"
+	ipamdSocketPath         = "/var/run/aws-node/ipamd.sock"
 	dummyInterfacePrefix    = "dummy"
 	npAgentConnTimeout      = 2
 	npaSocketPath           = "/var/run/aws-node/npa.sock"
@@ -136,6 +137,28 @@ func LoadNetConf(bytes []byte) (*NetConf, logger.Logger, error) {
 	return &conf, log, nil
 }
 
+// dialIPAMD connects to the IPAMD gRPC server. It tries the Unix socket first
+// (secure path), then falls back to TCP for backward compatibility during upgrades.
+func dialIPAMD(grpcClient grpcwrapper.GRPC, log logger.Logger) (*grpc.ClientConn, error) {
+	// Try Unix socket first (secure)
+	if _, err := os.Stat(ipamdSocketPath); err == nil {
+		conn, err := grpcClient.Dial("unix://"+ipamdSocketPath, grpc.WithTransportCredentials(insecure.NewCredentials()))
+		if err == nil {
+			log.Debugf("Connected to IPAMD via Unix socket: %s", ipamdSocketPath)
+			return conn, nil
+		}
+		log.Warnf("Unix socket exists but dial failed (%v), trying TCP fallback", err)
+	}
+
+	// TODO: Remove TCP fallback once all nodes run the socket-based IPAMD.
+	log.Debugf("Falling back to TCP connection: %s", ipamdAddress)
+	conn, err := grpcClient.Dial(ipamdAddress, grpc.WithTransportCredentials(insecure.NewCredentials()))
+	if err != nil {
+		return nil, err
+	}
+	return conn, nil
+}
+
 func cmdAdd(args *skel.CmdArgs) error {
 	return add(args, typeswrapper.New(), grpcwrapper.New(), rpcwrapper.New(), driver.New())
 }
@@ -177,8 +200,8 @@ func add(args *skel.CmdArgs, cniTypes typeswrapper.CNITYPES, grpcClient grpcwrap
 
 	log.Debugf("pod requires multi-nic attachment: %t", requiresMultiNICAttachment)
 
-	// Set up a connection to the ipamD server.
-	conn, err := grpcClient.Dial(ipamdAddress, grpc.WithTransportCredentials(insecure.NewCredentials()))
+	// Set up a connection to the ipamD server via Unix socket (preferred) or TCP fallback.
+	conn, err := dialIPAMD(grpcClient, log)
 	if err != nil {
 		log.Errorf("Failed to connect to backend server for container %s: %v",
 			args.ContainerID, err)
@@ -395,8 +418,8 @@ func del(args *skel.CmdArgs, cniTypes typeswrapper.CNITYPES, grpcClient grpcwrap
 	}
 
 	// notify local IP address manager to free secondary IP
-	// Set up a connection to the server.
-	conn, err := grpcClient.Dial(ipamdAddress, grpc.WithInsecure())
+	// Set up a connection to the server via Unix socket (preferred) or TCP fallback.
+	conn, err := dialIPAMD(grpcClient, log)
 	if err != nil {
 		log.Errorf("Failed to connect to backend server for container %s: %v",
 			args.ContainerID, err)

cmd/routed-eni-cni-plugin/cni_test.go

@@ -17,6 +17,7 @@ import (
 	"encoding/json"
 	"errors"
 	"net"
+	"os"
 	"testing"
 
 	"github.com/aws/amazon-vpc-cni-k8s/pkg/sgpp"
@@ -1833,3 +1834,41 @@ func TestLoadNetConf(t *testing.T) {
 		})
 	}
 }
+
+func TestDialIPAMD_FallsBackToTCPWhenSocketMissing(t *testing.T) {
+	ctrl, _, mocksGRPC, _, _ := setup(t)
+	defer ctrl.Finish()
+
+	log := logger.New(&logger.Configuration{LogLevel: "DEBUG"})
+
+	// The socket at ipamdSocketPath doesn't exist in the test environment,
+	// so dialIPAMD should fall back to TCP on ipamdAddress.
+	conn, _ := grpc.Dial(ipamdAddress, grpc.WithInsecure())
+	mocksGRPC.EXPECT().Dial(ipamdAddress, gomock.Any()).Return(conn, nil)
+
+	result, err := dialIPAMD(mocksGRPC, log)
+	assert.NoError(t, err)
+	assert.NotNil(t, result)
+}
+
+func TestDialIPAMD_FallsBackToTCPWhenSocketDialFails(t *testing.T) {
+	ctrl, _, mocksGRPC, _, _ := setup(t)
+	defer ctrl.Finish()
+
+	log := logger.New(&logger.Configuration{LogLevel: "DEBUG"})
+
+	// Create a file at the socket path so os.Stat succeeds, but it's not a real socket
+	// so grpcClient.Dial("unix://...") will fail, triggering TCP fallback.
+	tmpDir := t.TempDir()
+	socketPath := tmpDir + "/ipamd.sock"
+	os.WriteFile(socketPath, []byte("not a socket"), 0600)
+
+	// Since ipamdSocketPath is a const pointing to /var/run/aws-node/ipamd.sock (doesn't exist in CI),
+	// dialIPAMD will skip the socket and go straight to TCP fallback.
+	conn, _ := grpc.Dial(ipamdAddress, grpc.WithInsecure())
+	mocksGRPC.EXPECT().Dial(ipamdAddress, gomock.Any()).Return(conn, nil)
+
+	result, err := dialIPAMD(mocksGRPC, log)
+	assert.NoError(t, err)
+	assert.NotNil(t, result)
+}

pkg/ipamd/introspect.go

@@ -128,6 +128,24 @@ func eniV1RequestHandler(ipam *IPAMContext) func(http.ResponseWriter, *http.Requ
 		for _, ds := range ipam.dataStoreAccess.DataStores {
 			eniInfos[ds.GetNetworkCard()] = ds.GetENIInfos()
 		}
+		// Scrub sensitive fields that could be used to construct malicious gRPC calls.
+		// ContainerID and IfName are attack prerequisites for DelNetwork.
+		for _, info := range eniInfos {
+			for _, eni := range info.ENIs {
+				for _, cidr := range eni.AvailableIPv4Cidrs {
+					for _, addr := range cidr.IPAddresses {
+						addr.IPAMKey.ContainerID = ""
+						addr.IPAMKey.IfName = ""
+					}
+				}
+				for _, cidr := range eni.IPv6Cidrs {
+					for _, addr := range cidr.IPAddresses {
+						addr.IPAMKey.ContainerID = ""
+						addr.IPAMKey.IfName = ""
+					}
+				}
+			}
+		}
 		responseJSON, err := json.Marshal(eniInfos)
 		if err != nil {
 			log.Errorf("Failed to marshal ENI data: %v", err)

pkg/ipamd/introspect_test.go

@@ -0,0 +1,116 @@
+// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"). You may
+// not use this file except in compliance with the License. A copy of the
+// License is located at
+//
+//     http://aws.amazon.com/apache2.0/
+//
+// or in the "license" file accompanying this file. This file is distributed
+// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+// express or implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+package ipamd
+
+import (
+	"encoding/json"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/aws/amazon-vpc-cni-k8s/pkg/ipamd/datastore"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestEniV1RequestHandler_ScrubsSensitiveFields(t *testing.T) {
+	// Set up a datastore with a pod assigned
+	ds := datastore.NewDataStore(log, datastore.NullCheckpoint{}, false, 0)
+	ds.AddENI("eni-test-001", 0, true, false, false, 254, "subnet-abc")
+	ipv4Addr := net.IPNet{IP: net.ParseIP("10.0.0.5"), Mask: net.IPv4Mask(255, 255, 255, 255)}
+	ds.AddIPv4CidrToStore("eni-test-001", ipv4Addr, false)
+
+	// Assign an IP to a pod
+	key := datastore.IPAMKey{
+		NetworkName: "aws-cni",
+		ContainerID: "abc123-secret-container-id",
+		IfName:      "eth0",
+	}
+	metadata := datastore.IPAMMetadata{
+		K8SPodNamespace: "default",
+		K8SPodName:      "my-pod",
+	}
+	_, _, _, _, err := ds.AssignPodIPAddress(key, metadata, true, false)
+	require.NoError(t, err)
+
+	// Build IPAMContext
+	dsAccess := &datastore.DataStoreAccess{DataStores: []*datastore.DataStore{ds}}
+	ipamCtx := &IPAMContext{
+		dataStoreAccess: dsAccess,
+	}
+
+	// Call the handler
+	handler := eniV1RequestHandler(ipamCtx)
+	req := httptest.NewRequest(http.MethodGet, "/v1/enis", nil)
+	rec := httptest.NewRecorder()
+	handler(rec, req)
+
+	assert.Equal(t, http.StatusOK, rec.Code)
+
+	// Parse response and verify sensitive fields are scrubbed
+	body := rec.Body.String()
+	assert.NotEmpty(t, body)
+
+	// The response should NOT contain the container ID or interface name
+	assert.NotContains(t, body, "abc123-secret-container-id",
+		"ContainerID should be scrubbed from introspection response")
+	assert.NotContains(t, body, `"ifName":"eth0"`,
+		"IfName should be scrubbed from introspection response")
+
+	// But should still contain useful non-sensitive info
+	assert.Contains(t, body, "eni-test-001", "ENI ID should still be present")
+	assert.Contains(t, body, "10.0.0.5", "IP address should still be present")
+
+	// Verify the JSON structure is valid
+	var result map[string]json.RawMessage
+	err = json.Unmarshal(rec.Body.Bytes(), &result)
+	assert.NoError(t, err, "Response should be valid JSON")
+}
+
+func TestEniV1RequestHandler_ScrubsIPv6Fields(t *testing.T) {
+	// Set up a datastore with an IPv6 prefix
+	ds := datastore.NewDataStore(log, datastore.NullCheckpoint{}, true, 0)
+	ds.AddENI("eni-v6-001", 0, true, false, false, 254, "subnet-v6")
+	_, ipv6Net, _ := net.ParseCIDR("2001:db8::/64")
+	ds.AddIPv6CidrToStore("eni-v6-001", *ipv6Net, true)
+
+	// Assign an IPv6 address to a pod
+	key := datastore.IPAMKey{
+		NetworkName: "aws-cni",
+		ContainerID: "v6-container-secret-id",
+		IfName:      "eth0",
+	}
+	metadata := datastore.IPAMMetadata{
+		K8SPodNamespace: "default",
+		K8SPodName:      "v6-pod",
+	}
+	_, _, _, _, err := ds.AssignPodIPAddress(key, metadata, false, true)
+	require.NoError(t, err)
+
+	dsAccess := &datastore.DataStoreAccess{DataStores: []*datastore.DataStore{ds}}
+	ipamCtx := &IPAMContext{
+		dataStoreAccess: dsAccess,
+	}
+
+	handler := eniV1RequestHandler(ipamCtx)
+	req := httptest.NewRequest(http.MethodGet, "/v1/enis", nil)
+	rec := httptest.NewRecorder()
+	handler(rec, req)
+
+	assert.Equal(t, http.StatusOK, rec.Code)
+	body := rec.Body.String()
+	assert.NotContains(t, body, "v6-container-secret-id",
+		"IPv6 ContainerID should be scrubbed from introspection response")
+}

pkg/ipamd/rpc_handler.go

@@ -19,6 +19,7 @@ import (
 	"net"
 	"os"
 	"os/signal"
+	"path/filepath"
 	"strings"
 	"syscall"
 
@@ -44,6 +45,7 @@ import (
 
 const (
 	ipamdgRPCaddress      = "127.0.0.1:50051"
+	ipamdGRPCSocketPath   = "/var/run/aws-node/ipamd.sock"
 	grpcHealthServiceName = "grpc.health.v1.aws-node"
 
 	vpccniPodIPKey = "vpc.amazonaws.com/pod-ips"
@@ -469,32 +471,75 @@ func (s *server) GetNetworkPolicyConfigs(ctx context.Context, e *emptypb.Empty)
 
 // RunRPCHandler handles request from gRPC
 func (c *IPAMContext) RunRPCHandler(version string) error {
-	log.Infof("Serving RPC Handler version %s on %s", version, ipamdgRPCaddress)
-	listener, err := net.Listen("tcp", ipamdgRPCaddress)
+	return c.runRPCHandlerWithSocketPath(version, ipamdGRPCSocketPath)
+}
+
+func (c *IPAMContext) runRPCHandlerWithSocketPath(version string, socketPath string) error {
+	log.Infof("Serving RPC Handler version %s on unix:%s", version, socketPath)
+
+	// Ensure the parent directory exists
+	if err := os.MkdirAll(filepath.Dir(socketPath), 0755); err != nil {
+		return errors.Wrap(err, "ipamd: failed to create socket directory")
+	}
+
+	// Remove stale socket file from a previous run
+	if err := os.Remove(socketPath); err != nil && !os.IsNotExist(err) {
+		return errors.Wrap(err, "ipamd: failed to remove stale socket")
+	}
+
+	// Set umask before creating the socket to avoid a TOCTOU window where the socket
+	// exists with default permissions before chmod is applied.
+	oldMask := syscall.Umask(0117)
+	listener, err := net.Listen("unix", socketPath)
+	syscall.Umask(oldMask)
 	if err != nil {
-		log.Errorf("Failed to listen gRPC port: %v", err)
-		return errors.Wrap(err, "ipamd: failed to listen to gRPC port")
+		log.Errorf("Failed to listen on Unix socket %s: %v", socketPath, err)
+		return errors.Wrap(err, "ipamd: failed to listen on Unix socket")
+	}
+
+	// Restrict socket to root:root 0660. The CNI plugin runs as root (invoked by kubelet).
+	// The socket inherits root group ownership in the container, so group access is equivalent to root.
+	if err := os.Chmod(socketPath, 0660); err != nil {
+		listener.Close()
+		_ = os.Remove(socketPath)
+		return errors.Wrap(err, "ipamd: failed to set socket permissions")
 	}
+
 	grpcServer := grpc.NewServer()
 	rpc.RegisterCNIBackendServer(grpcServer, &server{version: version, ipamContext: c})
 	rpc.RegisterConfigServerBackendServer(grpcServer, &server{version: version, ipamContext: c})
 	healthServer := health.NewServer()
-	// If ipamd can talk to the API server and to the EC2 API, the pod is healthy.
-	// No need to ever change this to HealthCheckResponse_NOT_SERVING since it's a local service only
 	healthServer.SetServingStatus(grpcHealthServiceName, healthpb.HealthCheckResponse_SERVING)
 	healthpb.RegisterHealthServer(grpcServer, healthServer)
 
 	// Register reflection service on gRPC server.
 	reflection.Register(grpcServer)
+
+	// TODO: Remove TCP fallback once all nodes run the socket-based IPAMD.
+	go c.runTCPFallbackListener(version, grpcServer)
+
 	// Add shutdown hook
 	go c.shutdownListener()
 	if err := grpcServer.Serve(listener); err != nil {
-		log.Errorf("Failed to start server on gRPC port: %v", err)
-		return errors.Wrap(err, "ipamd: failed to start server on gPRC port")
+		log.Errorf("Failed to start server on Unix socket: %v", err)
+		return errors.Wrap(err, "ipamd: failed to start server on Unix socket")
 	}
 	return nil
 }
 
+// runTCPFallbackListener starts a TCP listener on the legacy port for backward compatibility.
+func (c *IPAMContext) runTCPFallbackListener(version string, grpcServer *grpc.Server) {
+	log.Infof("Starting TCP fallback gRPC listener on %s", ipamdgRPCaddress)
+	listener, err := net.Listen("tcp", ipamdgRPCaddress)
+	if err != nil {
+		log.Warnf("Failed to start TCP fallback listener on %s: %v (Unix socket is primary)", ipamdgRPCaddress, err)
+		return
+	}
+	if err := grpcServer.Serve(listener); err != nil {
+		log.Warnf("TCP fallback listener stopped: %v", err)
+	}
+}
+
 // shutdownListener - Listen to signals and set ipamd to be in status "terminating"
 func (c *IPAMContext) shutdownListener() {
 	log.Info("Setting up shutdown hook.")