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.

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

fix(userdata): remove user_data and switch user_data_base64 to sensitive #1133

Closed

Conversation

haarchri
Copy link
Member

@haarchri haarchri commented Feb 7, 2024

Description of your changes

  • Remove the user_data attribute from aws_instance because it has encoding problems (The user_data attribute on aws_instance has always been defined with a special StateFunc <<EOF ... >>)

  • Instead, use user_data_base64 and mark it as sensitive since it can include sensitive info like keys or API tokens, and it takes input from Kubernetes secrets.

  • Update the example that we can connect use SSM Connect to check the user-data.txt on an EC2 instance.

Fixes #1109

I have:

  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

image

kubectl get managed
NAME                                               READY   SYNCED   EXTERNAL-NAME                       AGE
route.ec2.aws.upbound.io/sample-instance-private   True    True     r-rtb-02c0d8aec6c5cc51b1080289494   7m49s
route.ec2.aws.upbound.io/sample-instance-public    True    True     r-rtb-00486f2b3b2d5de091080289494   7m49s

NAME                                     READY   SYNCED   EXTERNAL-NAME                AGE
eip.ec2.aws.upbound.io/sample-instance   True    True     eipalloc-07d53fc98d0d94ea4   7m49s

NAME                                          READY   SYNCED   EXTERNAL-NAME         AGE
instance.ec2.aws.upbound.io/sample-instance   True    True     i-0e3cbe60e415e8ed2   7m49s

NAME                                                 READY   SYNCED   EXTERNAL-NAME           AGE
internetgateway.ec2.aws.upbound.io/sample-instance   True    True     igw-0bb15566dbaf40181   7m49s

NAME                                    READY   SYNCED   EXTERNAL-NAME           AGE
natgateway.ec2.aws.upbound.io/example   True    True     nat-080c050eaec2489a1   7m49s

NAME                                                               READY   SYNCED   EXTERNAL-NAME                AGE
routetableassociation.ec2.aws.upbound.io/sample-instance-private   True    True     rtbassoc-0711b53b0c4434edc   7m49s
routetableassociation.ec2.aws.upbound.io/sample-instance-public    True    True     rtbassoc-06a8de24d103e8c85   7m49s

NAME                                                    READY   SYNCED   EXTERNAL-NAME           AGE
routetable.ec2.aws.upbound.io/sample-instance-private   True    True     rtb-02c0d8aec6c5cc51b   7m49s
routetable.ec2.aws.upbound.io/sample-instance-public    True    True     rtb-00486f2b3b2d5de09   7m49s

NAME                                                                   READY   SYNCED   EXTERNAL-NAME           AGE
securitygroupegressrule.ec2.aws.upbound.io/ec2-instance-443-outbound   True    True     sgr-0ba17697a5e2f9952   7m49s

NAME                                                                   READY   SYNCED   EXTERNAL-NAME           AGE
securitygroupingressrule.ec2.aws.upbound.io/ec2-endpoint-443-inbound   True    True     sgr-06e952c213434ed4d   7m49s

NAME                                            READY   SYNCED   EXTERNAL-NAME          AGE
securitygroup.ec2.aws.upbound.io/ec2-endpoint   True    True     sg-07e2062a45b6dfc49   7m49s
securitygroup.ec2.aws.upbound.io/ec2-instance   True    True     sg-0f1636ed5da2b8634   7m49s

NAME                                                READY   SYNCED   EXTERNAL-NAME              AGE
subnet.ec2.aws.upbound.io/sample-instance-private   True    True     subnet-0e0e6764869309358   7m49s
subnet.ec2.aws.upbound.io/sample-instance-public    True    True     subnet-0f1922dcfeba4e573   7m49s

NAME                                         READY   SYNCED   EXTERNAL-NAME            AGE
vpcendpoint.ec2.aws.upbound.io/ec2messages   True    True     vpce-0a0a0cfb52e9843aa   7m49s
vpcendpoint.ec2.aws.upbound.io/ssm           True    True     vpce-08ab6ba10b373c7bc   7m49s
vpcendpoint.ec2.aws.upbound.io/ssmmessages   True    True     vpce-0b81871da9d1d12f6   7m49s

NAME                                                                                READY   SYNCED   EXTERNAL-NAME                        AGE
vpcendpointsecuritygroupassociation.ec2.aws.upbound.io/sg-ec2messages-vpcendpoint   True    True     a-vpce-0a0a0cfb52e9843aa4026951556   7m49s
vpcendpointsecuritygroupassociation.ec2.aws.upbound.io/sg-ssm-vpcendpoint           True    True     a-vpce-08ab6ba10b373c7bc4026951556   7m49s
vpcendpointsecuritygroupassociation.ec2.aws.upbound.io/sg-ssmmessages-vpcendpoint   True    True     a-vpce-0b81871da9d1d12f64026951556   7m49s

NAME                                     READY   SYNCED   EXTERNAL-NAME           AGE
vpc.ec2.aws.upbound.io/sample-instance   True    True     vpc-013e3ee886a92717a   7m49s

NAME                                                 READY   SYNCED   EXTERNAL-NAME     AGE
instanceprofile.iam.aws.upbound.io/sample-instance   True    True     sample-instance   7m49s

NAME                                      READY   SYNCED   EXTERNAL-NAME     AGE
role.iam.aws.upbound.io/sample-instance   True    True     sample-instance   7m49s

@haarchri
Copy link
Member Author

haarchri commented Feb 7, 2024

/test-examples="examples/ec2/v1beta1/instance.yaml"

@dhaines-quera
Copy link

I'd rather see this fixed than removed.

@@ -54,6 +54,8 @@ func Configure(p *config.Provider) {
},
}
config.MoveToStatus(r.TerraformResource, "security_groups")
r.TerraformResource.Schema["user_data_base64"].Sensitive = true
config.MoveToStatus(r.TerraformResource, "user_data")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this leak secrets that are in the user data?

@sergenyalcin
Copy link
Collaborator

With #1188, we make the user_data field working again. As a result of the detailed investigation, the related field started working correctly with the fix of a bug we found in Upjet. The PR mentioned above consumes the related upjet fix.

Another topic that this PR is about is marking the user_data_base64 field as sensitive, as stated in the description. The reason here is that sensitive data may be present in the field. However, at this point, we would like to mention two main points:

  • The first is that AWS itself says not to put sensitive data in the user data. This shows that having sensitive data in the field is not the best practice and is not recommended. Let me quote from the document I shared:

Although you can only access instance metadata and user data from within the instance itself, the data is not protected by authentication or cryptographic methods. Anyone who has direct access to the instance, and potentially any software running on the instance, can view its metadata. Therefore, you should not store sensitive data, such as passwords or long-lived encryption keys, as user data.

  • The second is that the Terraform provider also does not mark the field as sensitive in its schema.

You can also find many discussions about this issue on the web. Generally, users say that if sensitive data is to be put in user data, it should be done through AWS Secret Manager, referring to the relevant document. As a result, since it is not recommended or best practice for user data to contain sensitive information, we think marking the relevant field as sensitive is not the proper behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Garbled user data on EC2 instance
4 participants