From 14c3b5e619f5506a16d0967ec2b75b4589c680fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arnar=20P=C3=A1ll=20Arnarsson?= Date: Thu, 27 Aug 2026 13:46:57 +0000 Subject: [PATCH] Trim whitespace from ENI DNS fields sent by ACS An ENI's DomainNameServers and DomainName values originate from the VPC DHCP option set, which stores each value exactly as it was typed and performs no validation of its own. An option set configured as domain-name-servers 10.0.0.2, 10.0.0.3 yields a second value with a leading space. That value was copied verbatim into NetworkInterface and surfaced by the task metadata endpoint, where " 10.0.0.3" is not a valid IP address. A consumer that parses the response strictly fails the whole document rather than the single field, losing unrelated metadata such as the task ARN, cluster and family. Retries do not help because the payload never changes. Trim surrounding whitespace off each value in InterfaceFromACS, and in the V2N tunnel and VETH pair constructors, which read the same fields from the same payload. --- .../networkinterface/networkinterface.go | 30 ++++--- .../networkinterface/networkinterface_test.go | 82 +++++++++++++++++++ 2 files changed, 102 insertions(+), 10 deletions(-) diff --git a/ecs-agent/netlib/model/networkinterface/networkinterface.go b/ecs-agent/netlib/model/networkinterface/networkinterface.go index 84e25eaccd3..99437a8857e 100644 --- a/ecs-agent/netlib/model/networkinterface/networkinterface.go +++ b/ecs-agent/netlib/model/networkinterface/networkinterface.go @@ -477,16 +477,26 @@ func InterfaceFromACS(acsENI *ecsacs.ElasticNetworkInterface) (*NetworkInterface ni.InterfaceAssociationProtocol = VLANInterfaceAssociationProtocol } - for _, nameserverIP := range acsENI.DomainNameServers { - ni.DomainNameServers = append(ni.DomainNameServers, aws.ToString(nameserverIP)) - } - for _, nameserverDomain := range acsENI.DomainName { - ni.DomainNameSearchList = append(ni.DomainNameSearchList, aws.ToString(nameserverDomain)) - } + ni.DomainNameServers = trimSpaceAll(acsENI.DomainNameServers) + ni.DomainNameSearchList = trimSpaceAll(acsENI.DomainName) return ni, nil } +// trimSpaceAll returns the given values with surrounding whitespace stripped from each. +// +// The DNS fields of an ENI originate from the VPC DHCP option set, which stores each value exactly +// as it was typed and performs no validation of its own. An option set configured as +// "domain-name-servers 10.0.0.2, 10.0.0.3" therefore yields a second value with a leading space, +// which is not a valid IP address by the time it reaches the task metadata response. +func trimSpaceAll(values []*string) []string { + var trimmed []string + for _, value := range values { + trimmed = append(trimmed, strings.TrimSpace(aws.ToString(value))) + } + return trimmed +} + // ValidateENI validates the NetworkInterface information sent from ACS. func ValidateENI(acsENI *ecsacs.ElasticNetworkInterface) error { var validateSubnetGatewayAddr = func(version, addr string) error { @@ -689,8 +699,8 @@ func v2nTunnelFromACS(acsENI *ecsacs.ElasticNetworkInterface) (*NetworkInterface }, }, - DomainNameServers: aws.ToStringSlice(acsENI.DomainNameServers), - DomainNameSearchList: aws.ToStringSlice(acsENI.DomainName), + DomainNameServers: trimSpaceAll(acsENI.DomainNameServers), + DomainNameSearchList: trimSpaceAll(acsENI.DomainName), TunnelProperties: &TunnelProperties{ ID: aws.ToString(acsTunnelProperties.TunnelId), DestinationIPAddress: aws.ToString(acsTunnelProperties.InterfaceIpAddress), @@ -726,8 +736,8 @@ func vethPairFromACS( // DNS related data for VETH interface will be copied from the peer interface's DNS data. // This is because if default traffic of the container needs to use the VETH interface, // domain name resolution will be based on the DNS config of the peer interface. - DomainNameServers: aws.ToStringSlice(peerInterface.DomainNameServers), - DomainNameSearchList: aws.ToStringSlice(peerInterface.DomainName), + DomainNameServers: trimSpaceAll(peerInterface.DomainNameServers), + DomainNameSearchList: trimSpaceAll(peerInterface.DomainName), VETHProperties: &VETHProperties{ PeerInterfaceName: aws.ToString(acsENI.InterfaceVethProperties.PeerInterface), }, diff --git a/ecs-agent/netlib/model/networkinterface/networkinterface_test.go b/ecs-agent/netlib/model/networkinterface/networkinterface_test.go index 867b28acc35..19f19df699d 100644 --- a/ecs-agent/netlib/model/networkinterface/networkinterface_test.go +++ b/ecs-agent/netlib/model/networkinterface/networkinterface_test.go @@ -213,3 +213,85 @@ func TestSetDeviceNameDefaultInterfaceNotFound(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "unable to find device name") } + +// getTestACSInterface returns a minimal ACS ENI that passes ValidateENI, for tests that only +// care about how InterfaceFromACS handles the DNS fields. +func getTestACSInterface() *ecsacs.ElasticNetworkInterface { + return &ecsacs.ElasticNetworkInterface{ + Ec2Id: aws.String("eni-1"), + MacAddress: aws.String(testconst.RandomMAC), + Ipv4Addresses: []*ecsacs.IPv4AddressAssignment{ + {Primary: aws.Bool(true), PrivateAddress: aws.String("1.2.3.4")}, + }, + SubnetGatewayIpv4Address: aws.String("1.2.3.1/20"), + } +} + +// TestInterfaceFromACSTrimsWhitespaceFromDomainNameServers tests that surrounding whitespace on a +// nameserver from ACS is stripped. The VPC DHCP option set stores nameservers as typed, so an +// option set configured as "domain-name-servers 10.0.0.2, 10.0.0.3" yields a padded second value. +func TestInterfaceFromACSTrimsWhitespaceFromDomainNameServers(t *testing.T) { + acsENI := getTestACSInterface() + acsENI.DomainNameServers = []*string{aws.String("10.0.0.2"), aws.String(" 10.0.0.3 ")} + + ni, err := InterfaceFromACS(acsENI) + + assert.NoError(t, err) + assert.Equal(t, []string{"10.0.0.2", "10.0.0.3"}, ni.DomainNameServers) +} + +// TestInterfaceFromACSTrimsWhitespaceFromDomainNameSearchList tests that surrounding whitespace on +// a search domain from ACS is stripped. Search domains come from the same DHCP option set as the +// nameservers and are stored just as literally. +func TestInterfaceFromACSTrimsWhitespaceFromDomainNameSearchList(t *testing.T) { + acsENI := getTestACSInterface() + acsENI.DomainName = []*string{aws.String(" us-west-2.compute.internal ")} + + ni, err := InterfaceFromACS(acsENI) + + assert.NoError(t, err) + assert.Equal(t, []string{"us-west-2.compute.internal"}, ni.DomainNameSearchList) +} + +// TestV2NTunnelFromACSTrimsWhitespaceFromDNSFields tests that a V2N tunnel interface gets the same +// whitespace trimming as a regular interface, since its DNS data comes from the same ACS payload. +func TestV2NTunnelFromACSTrimsWhitespaceFromDNSFields(t *testing.T) { + acsENI := &ecsacs.ElasticNetworkInterface{ + Ec2Id: aws.String("eni-1"), + DomainNameServers: []*string{aws.String(" 10.0.0.2")}, + DomainName: []*string{aws.String(" us-west-2.compute.internal")}, + InterfaceTunnelProperties: &ecsacs.NetworkInterfaceTunnelProperties{ + TunnelId: aws.String("42"), + InterfaceIpAddress: aws.String("10.1.2.3"), + }, + } + + ni, err := v2nTunnelFromACS(acsENI) + + assert.NoError(t, err) + assert.Equal(t, []string{"10.0.0.2"}, ni.DomainNameServers) + assert.Equal(t, []string{"us-west-2.compute.internal"}, ni.DomainNameSearchList) +} + +// TestVETHPairFromACSTrimsWhitespaceFromDNSFields tests that a VETH interface gets the same +// whitespace trimming as its peer, whose DNS data it copies from the same ACS payload. +func TestVETHPairFromACSTrimsWhitespaceFromDNSFields(t *testing.T) { + peer := &ecsacs.ElasticNetworkInterface{ + Ec2Id: aws.String("eni-1"), + Name: aws.String("peer"), + DomainNameServers: []*string{aws.String(" 10.0.0.2")}, + DomainName: []*string{aws.String(" us-west-2.compute.internal")}, + } + acsENI := &ecsacs.ElasticNetworkInterface{ + Ec2Id: aws.String("eni-2"), + InterfaceVethProperties: &ecsacs.NetworkInterfaceVethProperties{ + PeerInterface: aws.String("peer"), + }, + } + + ni, err := vethPairFromACS(acsENI, []*ecsacs.ElasticNetworkInterface{peer}) + + assert.NoError(t, err) + assert.Equal(t, []string{"10.0.0.2"}, ni.DomainNameServers) + assert.Equal(t, []string{"us-west-2.compute.internal"}, ni.DomainNameSearchList) +}