From 4d4c01ae22dc8527ef15f8938b51ca7f17a84608 Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Tue, 10 Oct 2023 09:31:34 +0100 Subject: [PATCH 1/2] AWS OIDC: Only consider Linux/UNIX when listing EC2 instances Listing EC2 instances is part of the EICE flow, where Teleport connects to the instance's SSH server. We were returning Windows instances, but those don't have an SSH Server, thus not compatible with the feature. This PR adds a new filter to only return Linux/UNIX instances. --- lib/integrations/awsoidc/listec2.go | 15 +++++++++++++++ lib/integrations/awsoidc/listec2_test.go | 14 ++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/lib/integrations/awsoidc/listec2.go b/lib/integrations/awsoidc/listec2.go index a59adec52a7a5..6ec1975684110 100644 --- a/lib/integrations/awsoidc/listec2.go +++ b/lib/integrations/awsoidc/listec2.go @@ -35,6 +35,10 @@ const ( // https://docs.aws.amazon.com/cli/latest/reference/ec2/describe-instances.html // Used for filtering instances. awsInstanceStateName = "instance-state-name" + + // describeEC2PlatformDetailsFilter is a filter for EC2 DescribeInstances operation that filters per Platform Details. + describeEC2PlatformDetailsFilter = "platform-details" + describeEC2PlatformDetailsFilterLinuxUNIX = "Linux/UNIX" ) var ( @@ -43,6 +47,15 @@ var ( Name: aws.String(awsInstanceStateName), Values: []string{string(ec2Types.InstanceStateNameRunning)}, } + + // filterEC2PlatformLinuxUNIX is an EC2 DescribeInstances Filter to filter for Linux/UNIX instances. + // AWS Docs have multiple values for identifying Linux hosts. + // However, from our tests only the values Linux/UNIX and Windows are returned. + // ListEC2 is only meant for accessing EC2 instances using SSH, so our only supported platform is Linux. + filterEC2PlatformLinuxUNIX = ec2Types.Filter{ + Name: aws.String(describeEC2PlatformDetailsFilter), + Values: []string{describeEC2PlatformDetailsFilterLinuxUNIX}, + } ) // ListEC2Request contains the required fields to list AWS EC2 Instances. @@ -122,6 +135,7 @@ func NewListEC2Client(ctx context.Context, req *AWSClientRequest) (ListEC2Client // ListEC2 calls the following AWS API: // https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_DescribeInstances.html // It returns a list of EC2 Instances and an optional NextToken that can be used to fetch the next page +// Only PlatformDetails=Linux/UNIX and State=Running instances are returned. func ListEC2(ctx context.Context, clt ListEC2Client, req ListEC2Request) (*ListEC2Response, error) { if err := req.CheckAndSetDefaults(); err != nil { return nil, trace.Wrap(err) @@ -139,6 +153,7 @@ func ListEC2(ctx context.Context, clt ListEC2Client, req ListEC2Request) (*ListE describeEC2Instances := &ec2.DescribeInstancesInput{ Filters: []ec2Types.Filter{ filterRunningEC2Instance, + filterEC2PlatformLinuxUNIX, }, } if req.NextToken != "" { diff --git a/lib/integrations/awsoidc/listec2_test.go b/lib/integrations/awsoidc/listec2_test.go index f21bcad3941cb..f8fe4d4749a5f 100644 --- a/lib/integrations/awsoidc/listec2_test.go +++ b/lib/integrations/awsoidc/listec2_test.go @@ -52,6 +52,20 @@ func (m *mockListEC2Client) GetCallerIdentity(ctx context.Context, params *sts.G func (m mockListEC2Client) DescribeInstances(ctx context.Context, params *ec2.DescribeInstancesInput, optFns ...func(*ec2.Options)) (*ec2.DescribeInstancesOutput, error) { requestedPage := 1 + stateFilter := false + platformFilter := false + for _, filter := range params.Filters { + if *filter.Name == "instance-state-name" && len(filter.Values) == 1 && filter.Values[0] == "running" { + stateFilter = true + } + if *filter.Name == "platform-details" && len(filter.Values) == 1 && filter.Values[0] == "Linux/UNIX" { + platformFilter = true + } + } + if !stateFilter || !platformFilter { + return nil, trace.BadParameter("instance-state-name and platform-details filters were not included") + } + totalInstances := len(m.ec2Instances) if params.NextToken != nil { From 15798e3c8de7589e440213ca9f9c2491fceaf110 Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Mon, 16 Oct 2023 12:17:11 +0100 Subject: [PATCH 2/2] use aws.ToString to check nil strings --- lib/integrations/awsoidc/listec2.go | 6 ++---- lib/integrations/awsoidc/listec2_test.go | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/integrations/awsoidc/listec2.go b/lib/integrations/awsoidc/listec2.go index 6ec1975684110..d4e05537619da 100644 --- a/lib/integrations/awsoidc/listec2.go +++ b/lib/integrations/awsoidc/listec2.go @@ -165,10 +165,8 @@ func ListEC2(ctx context.Context, clt ListEC2Client, req ListEC2Request) (*ListE return nil, trace.Wrap(err) } - ret := &ListEC2Response{} - - if aws.ToString(ec2Instances.NextToken) != "" { - ret.NextToken = *ec2Instances.NextToken + ret := &ListEC2Response{ + NextToken: aws.ToString(ec2Instances.NextToken), } ret.Servers = make([]types.Server, 0, len(ec2Instances.Reservations)) diff --git a/lib/integrations/awsoidc/listec2_test.go b/lib/integrations/awsoidc/listec2_test.go index f8fe4d4749a5f..348dfd2fedd1f 100644 --- a/lib/integrations/awsoidc/listec2_test.go +++ b/lib/integrations/awsoidc/listec2_test.go @@ -55,10 +55,10 @@ func (m mockListEC2Client) DescribeInstances(ctx context.Context, params *ec2.De stateFilter := false platformFilter := false for _, filter := range params.Filters { - if *filter.Name == "instance-state-name" && len(filter.Values) == 1 && filter.Values[0] == "running" { + if aws.ToString(filter.Name) == "instance-state-name" && len(filter.Values) == 1 && filter.Values[0] == "running" { stateFilter = true } - if *filter.Name == "platform-details" && len(filter.Values) == 1 && filter.Values[0] == "Linux/UNIX" { + if aws.ToString(filter.Name) == "platform-details" && len(filter.Values) == 1 && filter.Values[0] == "Linux/UNIX" { platformFilter = true } }