From e71c9a56f0a7e03829008e50db55d6d9b227bbd7 Mon Sep 17 00:00:00 2001 From: Luca Di Maio Date: Fri, 10 May 2024 10:46:34 +0200 Subject: [PATCH 1/4] fix(security): make stricter rules for AWS instance policy Signed-off-by: Luca Di Maio --- pkg/aws/aws.go | 54 ++++++++++++++++++++++---------------------------- 1 file changed, 24 insertions(+), 30 deletions(-) diff --git a/pkg/aws/aws.go b/pkg/aws/aws.go index f244aa3..db4018f 100644 --- a/pkg/aws/aws.go +++ b/pkg/aws/aws.go @@ -288,17 +288,31 @@ func CreateDevpodInstanceProfile(ctx context.Context, provider *AwsProvider) (st policyInput := &iam.PutRolePolicyInput{ PolicyDocument: aws.String(`{ - "Version": "2012-10-17", - "Statement": [ - { - "Sid": "VisualEditor0", - "Effect": "Allow", - "Action": [ - "ec2:StopInstances", - ], - "Resource": "*" + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "Describe", + "Action": [ + "ec2:DescribeInstances", + "ec2:DescribeImages" + ], + "Effect": "Allow", + "Resource": "*" + }, + { + "Sid": "Stop", + "Action": [ + "ec2:StopInstances" + ], + "Effect": "Allow", + "Resource": "arn:aws:ec2:*:*:instance/*", + "Condition": { + "StringLike": { + "aws:userid": "*:${ec2:InstanceID}" } - ] + } + } + ] }`), PolicyName: aws.String("devpod-ec2-policy"), RoleName: aws.String("devpod-ec2-role"), @@ -309,26 +323,6 @@ func CreateDevpodInstanceProfile(ctx context.Context, provider *AwsProvider) (st return "", err } - policyInput = &iam.PutRolePolicyInput{ - PolicyDocument: aws.String(`{ - "Version": "2012-10-17", - "Statement": [ - { - "Action": "ec2:*", - "Effect": "Allow", - "Resource": "*" - } - ] -}`), - PolicyName: aws.String("EC2Access"), - RoleName: aws.String("devpod-ec2-role"), - } - - _, err = svc.PutRolePolicy(ctx, policyInput) - if err != nil { - return "", err - } - instanceProfile := &iam.CreateInstanceProfileInput{ InstanceProfileName: aws.String("devpod-ec2-role"), } From 68912d27f387093cb4ba5a08da40fde8e4b30f20 Mon Sep 17 00:00:00 2001 From: Luca Di Maio Date: Fri, 10 May 2024 15:33:51 +0200 Subject: [PATCH 2/4] fix(security): move AMI search to create, allow stricter instance profile permissions Signed-off-by: Luca Di Maio --- pkg/aws/aws.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/pkg/aws/aws.go b/pkg/aws/aws.go index db4018f..dddfcb3 100644 --- a/pkg/aws/aws.go +++ b/pkg/aws/aws.go @@ -41,14 +41,6 @@ func NewProvider(ctx context.Context, logs log.Logger) (*AwsProvider, error) { config.DiskImage = image } - if config.RootDevice == "" { - device, err := GetAMIRootDevice(ctx, cfg, config.DiskImage) - if err != nil { - return nil, err - } - config.RootDevice = device - } - // create provider provider := &AwsProvider{ Config: config, @@ -293,8 +285,7 @@ func CreateDevpodInstanceProfile(ctx context.Context, provider *AwsProvider) (st { "Sid": "Describe", "Action": [ - "ec2:DescribeInstances", - "ec2:DescribeImages" + "ec2:DescribeInstances" ], "Effect": "Allow", "Resource": "*" @@ -638,6 +629,14 @@ func Create( return nil, err } + if providerAws.Config.RootDevice == "" { + device, err := GetAMIRootDevice(ctx, cfg, providerAws.Config.DiskImage) + if err != nil { + return nil, err + } + providerAws.Config.RootDevice = device + } + instance := &ec2.RunInstancesInput{ ImageId: aws.String(providerAws.Config.DiskImage), InstanceType: types.InstanceType(providerAws.Config.MachineType), From 79d87b67e42c411ce03b05ece9ddaed7bc05cae8 Mon Sep 17 00:00:00 2001 From: Luca Di Maio Date: Fri, 10 May 2024 15:45:51 +0200 Subject: [PATCH 3/4] fix(security): detect if we're in EC2 instance, perform actions only if not Signed-off-by: Luca Di Maio --- pkg/aws/aws.go | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/pkg/aws/aws.go b/pkg/aws/aws.go index dddfcb3..37133bc 100644 --- a/pkg/aws/aws.go +++ b/pkg/aws/aws.go @@ -4,6 +4,7 @@ import ( "context" "encoding/base64" "fmt" + "net/http" "regexp" "sort" "strings" @@ -21,6 +22,22 @@ import ( "github.com/pkg/errors" ) +// detect if we're in an ec2 instance +func isEC2Instance() bool { + client := &http.Client{} + req, err := http.NewRequest("GET", "http://instance-data.ec2.internal", nil) + if err != nil { + return false + } + resp, err := client.Do(req) + if err != nil { + return false + } + defer resp.Body.Close() + + return true +} + func NewProvider(ctx context.Context, logs log.Logger) (*AwsProvider, error) { config, err := options.FromEnv(false) if err != nil { @@ -32,7 +49,9 @@ func NewProvider(ctx context.Context, logs log.Logger) (*AwsProvider, error) { return nil, err } - if config.DiskImage == "" { + isEC2 := isEC2Instance() + + if config.DiskImage == "" && !isEC2 { image, err := GetDefaultAMI(ctx, cfg, config.MachineType) if err != nil { return nil, err @@ -41,6 +60,14 @@ func NewProvider(ctx context.Context, logs log.Logger) (*AwsProvider, error) { config.DiskImage = image } + if config.RootDevice == "" && !isEC2 { + device, err := GetAMIRootDevice(ctx, cfg, config.DiskImage) + if err != nil { + return nil, err + } + config.RootDevice = device + } + // create provider provider := &AwsProvider{ Config: config, @@ -629,14 +656,6 @@ func Create( return nil, err } - if providerAws.Config.RootDevice == "" { - device, err := GetAMIRootDevice(ctx, cfg, providerAws.Config.DiskImage) - if err != nil { - return nil, err - } - providerAws.Config.RootDevice = device - } - instance := &ec2.RunInstancesInput{ ImageId: aws.String(providerAws.Config.DiskImage), InstanceType: types.InstanceType(providerAws.Config.MachineType), From 0c9f12c89fb2e1562a0595fb103ecd2aafd14ceb Mon Sep 17 00:00:00 2001 From: Luca Di Maio Date: Fri, 10 May 2024 15:58:25 +0200 Subject: [PATCH 4/4] fix(security): enforce IMDSv2 Signed-off-by: Luca Di Maio --- pkg/aws/aws.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/aws/aws.go b/pkg/aws/aws.go index 37133bc..d10aa71 100644 --- a/pkg/aws/aws.go +++ b/pkg/aws/aws.go @@ -662,6 +662,11 @@ func Create( MinCount: aws.Int32(1), MaxCount: aws.Int32(1), SecurityGroupIds: devpodSG, + MetadataOptions: &types.InstanceMetadataOptionsRequest{ + HttpEndpoint: types.InstanceMetadataEndpointStateEnabled, + HttpTokens: types.HttpTokensStateRequired, + HttpPutResponseHopLimit: aws.Int32(1), + }, BlockDeviceMappings: []types.BlockDeviceMapping{ { DeviceName: aws.String(providerAws.Config.RootDevice),