From 41853ffc5b44c00567d4c4cabbb0ec438cb5d019 Mon Sep 17 00:00:00 2001 From: Cameron Sparr Date: Mon, 29 Jul 2024 10:15:22 -0700 Subject: [PATCH] Validate interface name passed in ECS_OFFHOST_INTROSPECTION_INTERFACE_NAME --- ecs-init/exec/iptables/iptables.go | 33 +++++++- ecs-init/exec/iptables/iptables_test.go | 104 +++++++++++++++++++++++- 2 files changed, 132 insertions(+), 5 deletions(-) diff --git a/ecs-init/exec/iptables/iptables.go b/ecs-init/exec/iptables/iptables.go index c5ce753a244..6dbdee357a8 100644 --- 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 index 343fa0677a7..a819fb79450 100644 --- 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)()