Change in implementation design for blocking IMDS.

Instead of creating Windows Firewall rules, create a loopback route for IMDS inside the task namespace.

agent/ecscni/namespace_helper.go

@@ -21,25 +21,19 @@ import (
 	"github.com/containernetworking/cni/pkg/types/current"
 )
 
-// execCmdExecutorFnType is the method signature for execCmdExecutorFn.
-type execCmdExecutorFnType func(commands []string, separator string) error
-
 // NamespaceHelper defines the methods for performing additional actions to setup/clean the task namespace.
 // Task namespace in awsvpc network mode is configured using pause container which is the first container
 // launched for the task. These commands are executed inside that container.
 type NamespaceHelper interface {
 	ConfigureTaskNamespaceRouting(ctx context.Context, taskENI *apieni.ENI, config *Config, result *current.Result) error
-	ConfigureFirewallForTaskNSSetup(taskENI *apieni.ENI, config *Config) error
-	ConfigureFirewallForTaskNSCleanup(taskENI *apieni.ENI, config *Config) error
 }
 
 // helper is the client for executing methods of NamespaceHelper interface.
 type helper struct {
-	dockerClient    dockerapi.DockerClient
-	execCmdExecutor execCmdExecutorFnType
+	dockerClient dockerapi.DockerClient
 }
 
 // NewNamespaceHelper returns a new instance of NamespaceHelper interface.
 func NewNamespaceHelper(client dockerapi.DockerClient) NamespaceHelper {
-	return &helper{dockerClient: client, execCmdExecutor: execCmdExecutorFn}
+	return &helper{dockerClient: client}
 }

agent/ecscni/namespace_helper_linux.go

@@ -22,22 +22,8 @@ import (
 	"github.com/containernetworking/cni/pkg/types/current"
 )
 
-var execCmdExecutorFn execCmdExecutorFnType = nil
-
 // ConfigureTaskNamespaceRouting executes the commands required for setting up appropriate routing inside task namespace.
 // This is applicable only for Windows.
 func (nsHelper *helper) ConfigureTaskNamespaceRouting(ctx context.Context, taskENI *apieni.ENI, config *Config, result *current.Result) error {
 	return nil
 }
-
-// ConfigureFirewallForTaskNSSetup executes the commands, if required, to setup firewall rules for disabling IMDS access from task.
-// This is applicable only for Windows.
-func (nsHelper *helper) ConfigureFirewallForTaskNSSetup(taskENI *apieni.ENI, config *Config) error {
-	return nil
-}
-
-// ConfigureFirewallForTaskNSCleanup executes the commands, if required, to cleanup the firewall rules created during setup.
-// This is applicable only for Windows.
-func (nsHelper *helper) ConfigureFirewallForTaskNSCleanup(taskENI *apieni.ENI, config *Config) error {
-	return nil
-}

agent/ecscni/namespace_helper_unsupported.go

@@ -22,22 +22,8 @@ import (
 	"github.com/containernetworking/cni/pkg/types/current"
 )
 
-var execCmdExecutorFn execCmdExecutorFnType = nil
-
 // ConfigureTaskNamespaceRouting executes the commands required for setting up appropriate routing inside task namespace.
 // This is applicable only for Windows.
 func (nsHelper *helper) ConfigureTaskNamespaceRouting(ctx context.Context, taskENI *apieni.ENI, config *Config, result *current.Result) error {
 	return nil
 }
-
-// ConfigureFirewallForTaskNSSetup executes the commands, if required, to setup firewall rules for disabling IMDS access from task.
-// This is applicable only for Windows.
-func (nsHelper *helper) ConfigureFirewallForTaskNSSetup(taskENI *apieni.ENI, config *Config) error {
-	return nil
-}
-
-// ConfigureFirewallForTaskNSCleanup executes the commands, if required, to cleanup the firewall rules created during setup.
-// This is applicable only for Windows.
-func (nsHelper *helper) ConfigureFirewallForTaskNSCleanup(taskENI *apieni.ENI, config *Config) error {
-	return nil
-}

agent/ecscni/namespace_helper_windows.go

@@ -16,11 +16,9 @@
 package ecscni
 
 import (
-	"bytes"
 	"context"
 	"fmt"
 	"net"
-	"os/exec"
 	"strings"
 
 	apieni "github.com/aws/amazon-ecs-agent/agent/api/eni"
@@ -41,31 +39,24 @@ const (
 	// imdsEndpointIPAddress is the IP address of the endpoint for accessing IMDS.
 	imdsEndpointIPAddress = "169.254.169.254/32"
 	// ecsBridgeEndpointNameFormat is the name format of the ecs-bridge endpoint in the task namespace.
-	ecsBridgeEndpointNameFormat = "%s-ep-%s"
+	ecsBridgeEndpointNameFormat = "vEthernet (%s-ep-%s)"
 	// taskPrimaryEndpointNameFormat is the name format of the primary endpoint in the task namespace.
-	taskPrimaryEndpointNameFormat = "%sbr%s-ep-%s"
-	// blockIMDSFirewallRuleNameFormat is the format of firewall rule name for blocking IMDS from task namespace.
-	blockIMDSFirewallRuleNameFormat = "Disable IMDS for %s"
-	// ecsBridgeRouteAddCmdFormat is the format of command for adding route entry through ECS Bridge.
-	ecsBridgeRouteAddCmdFormat = `netsh interface ipv4 add route prefix=%s interface="vEthernet (%s)"`
-	// ecsBridgeRouteDeleteCmdFormat is the format of command for deleting route entry of ECS bridge endpoint.
-	ecsBridgeRouteDeleteCmdFormat = `netsh interface ipv4 delete route prefix=%s interface="vEthernet (%s)"`
-	// checkExistingFirewallRuleCmdFormat is the format of the command to check if the firewall rule exists.
-	checkExistingFirewallRuleCmdFormat = `netsh advfirewall firewall show rule name="%s" >nul`
-	// addFirewallRuleCmdFormat is the format of command for creating firewall rule on Windows.
-	addFirewallRuleCmdFormat = `netsh advfirewall firewall add rule name="%s" dir=out localip=%s remoteip=%s action=block`
-	// deleteFirewallRuleCmdFormat is the format of the command to delete a firewall rule on Windows.
-	deleteFirewallRuleCmdFormat = `netsh advfirewall firewall delete rule name="%s" dir=out`
+	taskPrimaryEndpointNameFormat = "vEthernet (%sbr%s-ep-%s)"
+	// loopbackInterfaceName is the name of the loopback interface.
+	loopbackInterfaceName = "Loopback"
+	// windowsRouteAddCmdFormat is the format of command for adding route entry on Windows.
+	windowsRouteAddCmdFormat = `netsh interface ipv4 add route prefix=%s interface="%s"`
+	// windowsRouteDeleteCmdFormat is the format of command for deleting route entry on Windowsx.
+	windowsRouteDeleteCmdFormat = `netsh interface ipv4 delete route prefix=%s interface="%s"`
 )
 
-var execCmdExecutorFn execCmdExecutorFnType = execCmdExecutor
-
 // ConfigureTaskNamespaceRouting executes the commands required for setting up appropriate routing inside task namespace.
 // The commands currently executed are-
 // netsh interface ipv4 delete route prefix=0.0.0.0/0 interface="vEthernet (nat-ep-<container_id>)
 // netsh interface ipv4 delete route prefix=<ecs-bridge-subnet-cidr> interface="vEthernet (nat-ep-<container_id>)
 // netsh interface ipv4 add route prefix=169.254.170.2/32 interface="vEthernet (nat-ep-<container_id>)
 // netsh interface ipv4 add route prefix=169.254.169.254/32 interface="vEthernet (task-br-<mac>-ep-<container_id>)
+// netsh interface ipv4 add route prefix=169.254.169.254/32 interface="Loopback"
 // netsh interface ipv4 add route prefix=<local-route> interface="vEthernet (nat-ep-<container_id>)
 func (nsHelper *helper) ConfigureTaskNamespaceRouting(ctx context.Context, taskENI *apieni.ENI, config *Config, result *current.Result) error {
 	// Obtain the ecs-bridge endpoint's subnet IP address from the CNI plugin execution result.
@@ -76,19 +67,25 @@ func (nsHelper *helper) ConfigureTaskNamespaceRouting(ctx context.Context, taskE
 	ecsBridgeEndpointName := fmt.Sprintf(ecsBridgeEndpointNameFormat, ECSBridgeNetworkName, config.ContainerID)
 
 	// Prepare the commands to be executed inside task namespace to setup the ECS Bridge.
-	defaultRouteDeletionCmd := fmt.Sprintf(ecsBridgeRouteDeleteCmdFormat, windowsDefaultRoute, ecsBridgeEndpointName)
-	defaultSubnetRouteDeletionCmd := fmt.Sprintf(ecsBridgeRouteDeleteCmdFormat, ecsBridgeSubnetIPAddress.String(),
+	defaultRouteDeletionCmd := fmt.Sprintf(windowsRouteDeleteCmdFormat, windowsDefaultRoute, ecsBridgeEndpointName)
+	defaultSubnetRouteDeletionCmd := fmt.Sprintf(windowsRouteDeleteCmdFormat, ecsBridgeSubnetIPAddress.String(),
 		ecsBridgeEndpointName)
-	credentialsAddressRouteAdditionCmd := fmt.Sprintf(ecsBridgeRouteAddCmdFormat, credentialsEndpointRoute, ecsBridgeEndpointName)
+	credentialsAddressRouteAdditionCmd := fmt.Sprintf(windowsRouteAddCmdFormat, credentialsEndpointRoute, ecsBridgeEndpointName)
 	commands := []string{defaultRouteDeletionCmd, defaultSubnetRouteDeletionCmd, credentialsAddressRouteAdditionCmd}
 
-	if !config.BlockInstanceMetadata {
+	// For blocking instance metadata, create a black hole route inside the task namespace.
+	// This route will redirect all the packets sent to IMDS endpoint through its loopback interface.
+	// If IMDS is required, then create an explicit route through the primary interface of the task.
+	if config.BlockInstanceMetadata {
+		blockIMDSRouteCommand := fmt.Sprintf(windowsRouteAddCmdFormat, imdsEndpointIPAddress, loopbackInterfaceName)
+		commands = append(commands, blockIMDSRouteCommand)
+	} else {
 		// This naming convention is drawn from the way CNI plugin names the endpoints.
 		// https://github.com/aws/amazon-vpc-cni-plugins/blob/master/plugins/vpc-eni/network/network_windows.go
 		taskPrimaryEndpointId := strings.Replace(strings.ToLower(taskENI.MacAddress), ":", "", -1)
 		taskPrimaryEndpointName := fmt.Sprintf(taskPrimaryEndpointNameFormat, TaskHNSNetworkNamePrefix,
 			taskPrimaryEndpointId, config.ContainerID)
-		imdsRouteAdditionCmd := fmt.Sprintf(ecsBridgeRouteAddCmdFormat, imdsEndpointIPAddress, taskPrimaryEndpointName)
+		imdsRouteAdditionCmd := fmt.Sprintf(windowsRouteAddCmdFormat, imdsEndpointIPAddress, taskPrimaryEndpointName)
 		commands = append(commands, imdsRouteAdditionCmd)
 	}
 
@@ -98,7 +95,7 @@ func (nsHelper *helper) ConfigureTaskNamespaceRouting(ctx context.Context, taskE
 			IP:   route.IP,
 			Mask: route.Mask,
 		}
-		additionalRouteAdditionCmd := fmt.Sprintf(ecsBridgeRouteAddCmdFormat, ipRoute.String(), ecsBridgeEndpointName)
+		additionalRouteAdditionCmd := fmt.Sprintf(windowsRouteAddCmdFormat, ipRoute.String(), ecsBridgeEndpointName)
 		commands = append(commands, additionalRouteAdditionCmd)
 	}
 
@@ -110,56 +107,6 @@ func (nsHelper *helper) ConfigureTaskNamespaceRouting(ctx context.Context, taskE
 	return nil
 }
 
-// ConfigureFirewallForTaskNSSetup executes the commands, if required, to setup firewall rules for disabling IMDS access from task.
-// The commands executed are-
-// netsh advfirewall firewall add rule name="Disable IMDS for <ENI IP>" dir=out localip=<ENI IP> remoteip=169.254.170.2/32 action=block
-func (nsHelper *helper) ConfigureFirewallForTaskNSSetup(taskENI *apieni.ENI, config *Config) error {
-	if config.BlockInstanceMetadata {
-		if taskENI == nil {
-			return errors.New("failed to configure firewall due to invalid task eni")
-		}
-
-		firewallRuleName := fmt.Sprintf(blockIMDSFirewallRuleNameFormat, taskENI.GetPrimaryIPv4Address())
-
-		checkExistingFirewallRule := fmt.Sprintf(checkExistingFirewallRuleCmdFormat, firewallRuleName)
-		blockIMDSFirewallRuleCreationCmd := fmt.Sprintf(addFirewallRuleCmdFormat, firewallRuleName,
-			taskENI.GetPrimaryIPv4Address(), imdsEndpointIPAddress)
-
-		// Invoke the generated command on the host to add the firewall rule.
-		// Separator is "||" as either the firewall rule should exist or a new one should be created.
-		err := nsHelper.invokeCommandsOnHost([]string{checkExistingFirewallRule, blockIMDSFirewallRuleCreationCmd}, " || ")
-		if err != nil {
-			return errors.Wrapf(err, "failed to create firewall rule to disable imds")
-		}
-	}
-
-	return nil
-}
-
-// ConfigureFirewallForTaskNSCleanup executes the commands, if required, to cleanup the firewall rules created during setup.
-// The commands executed are-
-// netsh advfirewall firewall delete rule name="Disable IMDS for <ENI IP>" dir=out
-func (nsHelper *helper) ConfigureFirewallForTaskNSCleanup(taskENI *apieni.ENI, config *Config) error {
-	if config.BlockInstanceMetadata {
-		if taskENI == nil {
-			return errors.New("failed to configure firewall due to invalid task eni")
-		}
-
-		firewallRuleName := fmt.Sprintf(blockIMDSFirewallRuleNameFormat, taskENI.GetPrimaryIPv4Address())
-
-		// Delete the firewall rule created for blocking IMDS access by the task.
-		checkExistingFirewallRule := fmt.Sprintf(checkExistingFirewallRuleCmdFormat, firewallRuleName)
-		blockIMDSFirewallRuleDeletionCmd := fmt.Sprintf(deleteFirewallRuleCmdFormat, firewallRuleName)
-
-		// The separator would be "&&" to ensure if the firewall rule exists then delete it.
-		// An error at this point means that the firewall rule was not present and was therefore not deleted.
-		// Hence, skip returning the error as it is redundant.
-		nsHelper.invokeCommandsOnHost([]string{checkExistingFirewallRule, blockIMDSFirewallRuleDeletionCmd}, " && ")
-	}
-
-	return nil
-}
-
 // invokeCommandsInsideContainer executes a set of commands inside the container namespace.
 func (nsHelper *helper) invokeCommandsInsideContainer(ctx context.Context, containerID string, commands []string, separator string) error {
 
@@ -202,28 +149,3 @@ func (nsHelper *helper) invokeCommandsInsideContainer(ctx context.Context, conta
 
 	return nil
 }
-
-// invokeCommandsOnHost invokes given commands on the host instance using the executeFn.
-func (nsHelper *helper) invokeCommandsOnHost(commands []string, separator string) error {
-	return nsHelper.execCmdExecutor(commands, separator)
-}
-
-// execCmdExecutor invokes given commands on the host instance.
-func execCmdExecutor(commands []string, separator string) error {
-	seelog.Debugf("[ECSCNI] Executing commands on host: %v", commands)
-
-	// Concatenate all the commands into a single command.
-	execCommands := strings.Join(commands, separator)
-
-	cmd := exec.Command("cmd", "/C", execCommands)
-	var stdout bytes.Buffer
-	cmd.Stdout = &stdout
-
-	err := cmd.Run()
-	if err != nil {
-		seelog.Errorf("[ECSCNI] Failed to execute command on host: %v: %s", err, stdout.String())
-		return err
-	}
-
-	return nil
-}

agent/ecscni/namespace_helper_windows_test.go

@@ -60,19 +60,19 @@ func TestConfigureTaskNamespaceRouting(t *testing.T) {
 	taskENI := getTaskENI()
 
 	cniConig.AdditionalLocalRoutes = append(cniConig.AdditionalLocalRoutes, cnitypes.IPNet{
-		net.ParseIP("10.0.0.0"),
-		net.CIDRMask(24, 32),
+		IP:   net.ParseIP("10.0.0.0"),
+		Mask: net.CIDRMask(24, 32),
 	})
 
 	bridgeEpName := fmt.Sprintf(ecsBridgeEndpointNameFormat, ECSBridgeNetworkName, containerID)
 	taskEpId := strings.Replace(strings.ToLower(taskENI.MacAddress), ":", "", -1)
 	taskEPName := fmt.Sprintf(taskPrimaryEndpointNameFormat, TaskHNSNetworkNamePrefix, taskEpId, containerID)
 
-	cmd1 := fmt.Sprintf(ecsBridgeRouteDeleteCmdFormat, windowsDefaultRoute, bridgeEpName)
-	cmd2 := fmt.Sprintf(ecsBridgeRouteDeleteCmdFormat, "10.0.0.0/24", bridgeEpName)
-	cmd3 := fmt.Sprintf(ecsBridgeRouteAddCmdFormat, credentialsEndpointRoute, bridgeEpName)
-	cmd4 := fmt.Sprintf(ecsBridgeRouteAddCmdFormat, imdsEndpointIPAddress, taskEPName)
-	cmd5 := fmt.Sprintf(ecsBridgeRouteAddCmdFormat, "10.0.0.0/24", bridgeEpName)
+	cmd1 := fmt.Sprintf(windowsRouteDeleteCmdFormat, windowsDefaultRoute, bridgeEpName)
+	cmd2 := fmt.Sprintf(windowsRouteDeleteCmdFormat, "10.0.0.0/24", bridgeEpName)
+	cmd3 := fmt.Sprintf(windowsRouteAddCmdFormat, credentialsEndpointRoute, bridgeEpName)
+	cmd4 := fmt.Sprintf(windowsRouteAddCmdFormat, imdsEndpointIPAddress, taskEPName)
+	cmd5 := fmt.Sprintf(windowsRouteAddCmdFormat, "10.0.0.0/24", bridgeEpName)
 	finalCmd := strings.Join([]string{cmd1, cmd2, cmd3, cmd4, cmd5}, " && ")
 
 	gomock.InOrder(
@@ -99,46 +99,3 @@ func TestConfigureTaskNamespaceRouting(t *testing.T) {
 	err := nsHelper.ConfigureTaskNamespaceRouting(ctx, taskENI, cniConig, getECSBridgeResult())
 	assert.NoError(t, err)
 }
-
-func TestConfigureFirewallForTaskNSSetup(t *testing.T) {
-	taskENI := getTaskENI()
-	cniConfig := getCNIConfig()
-	cniConfig.BlockInstanceMetadata = true
-
-	firewallRuleName := fmt.Sprintf(blockIMDSFirewallRuleNameFormat, taskENI.GetPrimaryIPv4Address())
-	checkExistingFirewallRule := fmt.Sprintf(checkExistingFirewallRuleCmdFormat, firewallRuleName)
-	blockIMDSFirewallRuleCreationCmd := fmt.Sprintf(addFirewallRuleCmdFormat, firewallRuleName,
-		taskENI.GetPrimaryIPv4Address(), imdsEndpointIPAddress)
-
-	nsHelper := &helper{}
-	nsHelper.execCmdExecutor = func(commands []string, separator string) error {
-		assert.Equal(t, checkExistingFirewallRule, commands[0])
-		assert.Equal(t, blockIMDSFirewallRuleCreationCmd, commands[1])
-		assert.Equal(t, " || ", separator)
-		return nil
-	}
-
-	err := nsHelper.ConfigureFirewallForTaskNSSetup(taskENI, cniConfig)
-	assert.NoError(t, err)
-}
-
-func TestConfigureFirewallForTaskNSCleanup(t *testing.T) {
-	taskENI := getTaskENI()
-	cniConfig := getCNIConfig()
-	cniConfig.BlockInstanceMetadata = true
-
-	firewallRuleName := fmt.Sprintf(blockIMDSFirewallRuleNameFormat, taskENI.GetPrimaryIPv4Address())
-	checkExistingFirewallRule := fmt.Sprintf(checkExistingFirewallRuleCmdFormat, firewallRuleName)
-	blockIMDSFirewallRuleDeletionCmd := fmt.Sprintf(deleteFirewallRuleCmdFormat, firewallRuleName)
-
-	nsHelper := &helper{}
-	nsHelper.execCmdExecutor = func(commands []string, separator string) error {
-		assert.Equal(t, checkExistingFirewallRule, commands[0])
-		assert.Equal(t, blockIMDSFirewallRuleDeletionCmd, commands[1])
-		assert.Equal(t, " && ", separator)
-		return nil
-	}
-
-	err := nsHelper.ConfigureFirewallForTaskNSCleanup(taskENI, cniConfig)
-	assert.NoError(t, err)
-}

agent/engine/docker_task_engine.go

@@ -1460,18 +1460,6 @@ func (engine *DockerTaskEngine) provisionContainerResources(task *apitask.Task,
 		}
 	}
 
-	// Invoke additional commands, if required, to configure the firewall for disabling IMDS access from task.
-	err = engine.namespaceHelper.ConfigureFirewallForTaskNSSetup(task.GetPrimaryENI(), cniConfig)
-	if err != nil {
-		seelog.Errorf("Task engine [%s]: unable to configure pause container namespace: %v",
-			task.Arn, err)
-		return dockerapi.DockerContainerMetadata{
-			DockerID: cniConfig.ContainerID,
-			Error: ContainerNetworkingError{errors.Wrapf(err,
-				"container resource provisioning: failed to setup network namespace")},
-		}
-	}
-
 	return dockerapi.DockerContainerMetadata{
 		DockerID: cniConfig.ContainerID,
 	}
@@ -1526,12 +1514,6 @@ func (engine *DockerTaskEngine) cleanupPauseContainerNetwork(task *apitask.Task,
 		return err
 	}
 
-	// Invoke additional command, if required, to cleanup the firewall rule created during ns setup.
-	err = engine.namespaceHelper.ConfigureFirewallForTaskNSCleanup(task.GetPrimaryENI(), cniConfig)
-	if err != nil {
-		return err
-	}
-
 	container.SetContainerTornDown(true)
 	seelog.Infof("Task engine [%s]: cleaned pause container network namespace", task.Arn)
 	return nil