Validate interface name passed in ECS_OFFHOST_INTROSPECTION_INTERFACE…

…_NAME
aws · Sep 14, 2024 · b02155d · b02155d
1 parent 027c3ae
commit b02155d
Show file tree

Hide file tree

Showing 2 changed files with 132 additions and 5 deletions.
diff --git a/ecs-init/exec/iptables/iptables.go b/ecs-init/exec/iptables/iptables.go
@@ -20,6 +20,7 @@ import (
 	"os"
 	"strconv"
 	"strings"
+	"unicode"
 
 	"github.com/aws/amazon-ecs-agent/ecs-init/exec"
 	log "github.com/cihub/seelog"
@@ -54,6 +55,8 @@ const (
 	ipv4ZeroAddrInHex                     = "00000000"
 	loopbackInterfaceName                 = "lo"
 	fallbackOffhostIntrospectionInterface = "eth0"
+
+	ifNameSize = 16
 )
 
 var (
@@ -213,10 +216,38 @@ func getBlockIntrospectionOffhostAccessInputChainArgs() []string {
 	}
 }
 
+// checkValidIfname checks that the string is a valid eth interface name. Returns
+// an error if the interface name is invalid.
+// see kernel dev_valid_name function for reference.
+// https://github.com/torvalds/linux/blob/d7e78951a8b8b53e4d52c689d927a6887e6cfadf/net/core/dev.c#L1056-L1079
+func checkValidIfname(name string) error {
+	if name == "" {
+		return fmt.Errorf("Invalid ifname [%s]: empty string not allowed", name)
+	}
+	if len(name) >= ifNameSize {
+		return fmt.Errorf("Invalid ifname [%s]: length of name must be less than %d chars", name, ifNameSize)
+	}
+	if name == "." || name == ".." {
+		return fmt.Errorf("Invalid ifname [%s]: '.' or '..' not allowed", name)
+	}
+
+	for _, char := range name {
+		if char == '/' || char == ':' || unicode.IsSpace(char) {
+			return fmt.Errorf("Invalid ifname [%s]: invalid character found: space, ':', and '/' not allowed", name)
+		}
+	}
+	return nil
+}
+
 func getOffhostIntrospectionInterface() (string, error) {
 	s := os.Getenv(offhostIntrospectonAccessInterfaceEnv)
 	if s != "" {
-		return s, nil
+		err := checkValidIfname(s)
+		if err != nil {
+			log.Errorf("ECS_OFFHOST_INTROSPECTION_INTERFACE_NAME interface name is invalid, falling back to default. %s", err)
+		} else {
+			return s, nil
+		}
 	}
 	return getDefaultNetworkInterfaceIPv4()
 }

diff --git a/ecs-init/exec/iptables/iptables_test.go b/ecs-init/exec/iptables/iptables_test.go
@@ -62,9 +62,9 @@ var (
 		"--to-ports", localhostCredentialsProxyPort,
 	}
 
-	testIPV4RouteInput = `Iface	Destination	Gateway 	Flags	RefCnt	Use	Metric	Mask		MTU	Window	IRTT                                                       
-ens5	00000000	01201FAC	0003	0	0	0	00000000	0	0	0                                                                               
-ens5	FEA9FEA9	00000000	0005	0	0	0	FFFFFFFF	0	0	0                                                                               
+	testIPV4RouteInput = `Iface	Destination	Gateway 	Flags	RefCnt	Use	Metric	Mask		MTU	Window	IRTT
+ens5	00000000	01201FAC	0003	0	0	0	00000000	0	0	0
+ens5	FEA9FEA9	00000000	0005	0	0	0	FFFFFFFF	0	0	0
 ens5	00201FAC	00000000	0001	0	0	0	00F0FFFF	0	0	0
 `
 )
@@ -83,6 +83,91 @@ func overrideIPRouteInput(ipv4RouteInput string) func() {
 	}
 }
 
+func TestValidIfName(t *testing.T) {
+	testCases := []struct {
+		ifname      string
+		expectValid bool
+	}{
+		{
+			ifname:      "eth0",
+			expectValid: true,
+		},
+		{
+			ifname:      "eth1",
+			expectValid: true,
+		},
+		{
+			ifname:      "eth24",
+			expectValid: true,
+		},
+		{
+			ifname:      "eth255",
+			expectValid: true,
+		},
+		{
+			ifname:      "myinterfacename",
+			expectValid: true,
+		},
+		{
+			ifname:      "12345",
+			expectValid: true,
+		},
+		{
+			ifname:      "e",
+			expectValid: true,
+		},
+		{
+			ifname:      "ens5",
+			expectValid: true,
+		},
+		{
+			ifname:      "invalidchar",
+			expectValid: true,
+		},
+		{
+			ifname:      "invalid char",
+			expectValid: false,
+		},
+		{
+			ifname:      "invalid:char",
+			expectValid: false,
+		},
+		{
+			ifname:      "invalid/char",
+			expectValid: false,
+		},
+		{
+			ifname:      "ethnameistoolong",
+			expectValid: false,
+		},
+		{
+			ifname:      "ethnameiswaywaywaywaytoolong",
+			expectValid: false,
+		},
+		{
+			ifname:      "",
+			expectValid: false,
+		},
+		{
+			ifname:      ".",
+			expectValid: false,
+		},
+		{
+			ifname:      "..",
+			expectValid: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		err := checkValidIfname(tc.ifname)
+		if tc.expectValid {
+			require.NoError(t, err)
+		} else {
+			require.Error(t, err)
+		}
+	}
+}
+
 func TestNewNetfilterRouteFailsWhenExecutableNotFound(t *testing.T) {
 	defer overrideIPRouteInput(testIPV4RouteInput)()
 	ctrl := gomock.NewController(t)
@@ -565,7 +650,7 @@ func TestScanIPv4RoutesNoDefaultRoute(t *testing.T) {
 }
 
 func TestScanIPv4RoutesNoDefaultRouteExceptLoopback(t *testing.T) {
-	var testInput = `Iface	Destination	Gateway 	Flags	RefCnt	Use	Metric	Mask		MTU	Window	IRTT                                                       
+	var testInput = `Iface	Destination	Gateway 	Flags	RefCnt	Use	Metric	Mask		MTU	Window	IRTT
 lo	00000000	01201FAC	0003	0	0	0	00000000	0	0	0
 `
 	iface, err := scanIPv4RoutesForDefaultInterface(strings.NewReader(testInput))
@@ -582,6 +667,17 @@ func TestGetOffhostIntrospectionInterfaceWithEnvOverride(t *testing.T) {
 	assert.Equal(t, "test_iface", iface)
 }
 
+func TestGetOffhostIntrospectionInterfaceWithEnvOverride_InvalidIfname(t *testing.T) {
+	os.Setenv(offhostIntrospectonAccessInterfaceEnv, "invalid ifname")
+	defer os.Unsetenv(offhostIntrospectonAccessInterfaceEnv)
+	defer overrideIPRouteInput(testIPV4RouteInput)()
+
+	iface, err := getOffhostIntrospectionInterface()
+	assert.NoError(t, err)
+	// falls back to default
+	assert.Equal(t, offhostIntrospectionInterface, iface)
+}
+
 func TestGetOffhostIntrospectionInterfaceUseDefaultV4(t *testing.T) {
 	defer overrideIPRouteInput(testIPV4RouteInput)()