Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate interface name passed in ECS_OFFHOST_INTROSPECTION_INTERFACE_NAME #4275

Merged
merged 1 commit into from
Sep 14, 2024
Merged

Validate interface name passed in ECS_OFFHOST_INTROSPECTION_INTERFACE_NAME #4275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion ecs-init/exec/iptables/iptables.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"os"
"strconv"
"strings"
"unicode"

"github.com/aws/amazon-ecs-agent/ecs-init/exec"
log "github.com/cihub/seelog"
Expand Down Expand Up @@ -54,6 +55,8 @@ const (
ipv4ZeroAddrInHex = "00000000"
loopbackInterfaceName = "lo"
fallbackOffhostIntrospectionInterface = "eth0"

ifNameSize = 16
)

var (
Expand Down Expand Up @@ -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()
}
Expand Down
104 changes: 100 additions & 4 deletions ecs-init/exec/iptables/iptables_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
`
)
Expand All @@ -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)
Expand Down Expand Up @@ -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))
Expand All @@ -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)()

Expand Down
Loading