RFD 41: Simplified Node Joining for AWS by nklaassen · Pull Request #7292 · gravitational/teleport

nklaassen · 2021-06-14T16:23:27Z

I don't know if this is the right number, but there are 2 pending "RFD 33"s so I'll claim 35 for now.

klizhentas

klizhentas · 2021-06-15T04:00:33Z

+  enabled: yes
+  aws_join:
+    allow:
+    - organization: "o-1111111111"


have you consider to use AWS arn format? It allows flexible patterns

Could do that. We would still need a separate ARN for the organization and the account so I didn't see a major benefit, but it would allow admins to restrict based on the IAM role in the account ARN as well so may be better

ymatsiuk · 2021-06-16T10:16:54Z

+    # Example:
+    allow:
+    - account: "2222222222" # allow any node from this account
+    - organization: "o-1111111111" # allow any node in any account this org


I'm curious about data types of account and organization. Is it a big of a difference in case of []string? I wonder how the config will look like for multiple accounts/organizations?

See the latest revision, multiple accounts can be listed in separate rules (and we no longer plan to support an organization rule)

ymatsiuk · 2021-06-16T10:54:05Z

+(`organizations:DescribeAccount` or `organizations:ListAccounts`) can only be
+called from the organization's management account. At a minimum, this would
+still require creating signed requests on the node and sending them to the auth
+server as the current design does.


This doesn't simplify the flow. We replace the token with request and instead of attaching the policy to read secrets from secret manager we attach another one.
The main idea was to eliminate the need of attachments on node side with shifting all required checks and verifications to the auth node and replace the token with identity document.

as discussed in slack I am going to revamp this design to use the Instance Identity Document method over the IAM method

In the latest revision I describe both the EC2 (Instance Identity Document) method and the IAM method

klizhentas · 2021-06-24T19:00:54Z

whatever heartbeat on AWS is used, identifies the node by instance ID, and includes the signature from the IID. To prevent replay attack, the auth server will first verify the IID signature, getNode(instanceID).

If it's running and it has joined the cluster, the heartbeat should be present, so if the node name == instance id

For the lookup to be efficient, the node name == instance-id

Updated so that the node name will be set to <aws_account_id>-<aws_instance_id> (instance IDs are not necessarily unique across accounts) so auth can efficiently check if the node has already joined.

Don't need to store the IID signature as we will block any second join from the same instance.

Extend token type for auth configuration

phosphore · 2021-06-29T08:41:49Z

+3. Check that the AWS join token matches the AWS account in the IID, and the
+   requested Teleport service role.
+4. Check that this EC2 instance has not already joined the cluster.
+   - The node name will be set to `<aws_account_id>-<aws_instance_id>` so that


For detection/incident response purposes, this should raise an alert or at least be logged in detail.

This would definitely be logged in detail if the same instance attempts to join multiple times, I'm not sure if we currently have any applicable mechanism for raising an alert other than logging @klizhentas ?

phosphore · 2021-06-29T08:48:09Z

+https://github.com/gravitational/teleport/blob/d4247cb150d720be97521347b74bf9c526ae869f/lib/auth/auth.go#L1538-L1563).
+
+The auth server will then:
+1. Use the AWS DSA public certificate to check that the PKCS7 signature for the


Will Teleport trust the region specified in the IID and use that to pick the specific AWS DSA public certificate? Or will this be region-locked via the teleport config/only support the primary AWS DSA? (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/verify-pkcs7.html)

It would be possible to add region-locking to the config, but my current plan is just to trust the region in the IID to select the correct cert, is there a security issue with this?

I'm mainly worried about the potential security risk of having the attacker "pick" the certificate. Such risks would be mainly related to the implementation of the code we'll see later (e.g. path injections/traversal when retrieving the cert from disk or network). On a different note, an operator may need to restrict this (e.g. Govcloud instances, existing IAM policies that deny access to any actions outside the Regions specified for defense in depth), so I would consider the pros/cons of optionally restrict this.

Good point, I've added a configuration option to restrict the regions from which a node can join for the EC2 method (we won't have this info for the IAM method as it isn't included in the sts:GetCallerIdentity response)

phosphore · 2021-06-29T09:07:22Z

+the AWS SDK for Go.
+
+Because the signed request can include arbitrary headers, this allows us to
+issue a challenge (a crypto random string of bytes) that the node must include


The lifetime of this challenge should be limited.

The challenge will only be valid within the original gRPC streaming request, and only one attempt will be allowed. We could put a timeout of something like 1 minute on the complete gRPC request.

I like the timeout idea!

add a 1 minute timeout to the doc

phosphore · 2021-06-29T09:33:13Z

+
+### IAM Method
+
+In place of a join token, nodes will present a signed `sts:GetCallerIdentity`


Ideally, customers should be advised in the docs that this design will work as long as the sts:AssumeRole action is restricted. This should be ensured by them depending on their AWS setups.

Good point, this will definitely be noted. Customers could possibly restrict access for an account to a specific set of IAM roles which cannot be assumed from other accounts, but this does impose a greater configuration burden.

knisbet

I was just curious whether this was considered in the context of Teleport Cloud. It looks like based on the examples this should work with multiple AWS accounts, I just wanted to make sure when we host the auth/proxy servers in teleport cloud and the nodes/db/kube connect using reverse tunnel mode that they'll be able to present tokens that can be validated.

Also, talking to other customers about similar stuff, I don't know if it really applies to this RFD or not, but can be of the labels be enforced/restricted based on the token they use? Some customers might want to ensure that proper RBAC carries through based on the node joining that isn't totally up to the node side configuration.

klizhentas · 2021-08-19T16:00:33Z

@nklaassen ping on updating and merging it, and @pierrebeaucamp please review for compatibility with cloud

smallinsky · 2021-09-08T13:37:08Z

+1. Check that the `pendingTime` of the IID, which is the time the instance was
+   launched, is within a 5 minute TTL.
+   - if the node fails to join the cluster during this window, the user can
+     stop and restart the EC2 instance to reset the `pendingTime` (and
+     effectively create a new IID with a new signature)


Not sure why node joining window is narrowed to only 5 min counting from instance creation time.
Does situation when an instance is started and teleport service is deployed to the node later is not possible ?

That is possible, but not really the intended use-case for this feature where ideally teleport will be installed in the AMI and start up immediately so that the dev can access the instance with Teleport instead of ssh. The 5-minute TTL was introduced to decrease the window where an attacker could steal the Instance Identity Document and join the cluster with a fake/malicious node.

nklaassen requested review from klizhentas, r0mant and russjones as code owners June 14, 2021 16:23

nklaassen requested review from awly, smallinsky and tcsc June 14, 2021 16:24

klizhentas reviewed Jun 15, 2021

View reviewed changes

tcsc reviewed Jun 15, 2021

View reviewed changes

Comment thread rfd/0035-aws-node-join.md Outdated

Comment thread rfd/0035-aws-node-join.md Outdated

ymatsiuk reviewed Jun 16, 2021

View reviewed changes

nklaassen force-pushed the rfd/0035-aws-node-join branch from 366a7d1 to 83b6423 Compare June 24, 2021 17:38

klizhentas reviewed Jun 24, 2021

View reviewed changes

nklaassen added 6 commits June 24, 2021 12:54

add template rfd

6b3f193

first draft

0c7f76f

mention vault and alternative ec2 method

c007787

simplify node configuration

2834d8d

describe EC2 and IAM methods

6e0d1f3

Extend token type for auth configuration

update check for ec2 instance uniqueness

1359bbe

nklaassen force-pushed the rfd/0035-aws-node-join branch from a96dc31 to 1359bbe Compare June 24, 2021 19:55

phosphore reviewed Jun 29, 2021

View reviewed changes

Comment thread rfd/0035-aws-node-join.md

phosphore reviewed Jun 29, 2021

View reviewed changes

Comment thread rfd/0035-aws-node-join.md

nklaassen added 3 commits June 29, 2021 09:56

address comments

ac8ce78

allow admins to restrict the aws region

4c651dc

add timeout for IAM method gRPC request

9329b7f

russjones added the rfd Request for Discussion label Jul 28, 2021

knisbet reviewed Jul 30, 2021

View reviewed changes

klizhentas mentioned this pull request Aug 19, 2021

RFD 64: Bot for certificate renewals (Machine ID) #7986

Merged

update, mention cloud support

9146b04

smallinsky reviewed Sep 8, 2021

View reviewed changes

Merge branch 'master' into rfd/0035-aws-node-join

534059f

smallinsky approved these changes Sep 8, 2021

View reviewed changes

tcsc approved these changes Sep 9, 2021

View reviewed changes

nklaassen mentioned this pull request Sep 14, 2021

Implement Simplified Node Joining #8250

Merged

klizhentas approved these changes Sep 14, 2021

View reviewed changes

nklaassen added 3 commits September 15, 2021 15:44

move file to rfd 41

bb9ffba

renumber rfd to 41

709f78e

Merge branch 'master' into rfd/0035-aws-node-join

c760fe7

nklaassen changed the title ~~RFD 35: Simplified Node Joining for AWS~~ RFD 41: Simplified Node Joining for AWS Sep 15, 2021

nklaassen enabled auto-merge (squash) September 15, 2021 22:46

nklaassen merged commit 3e5480d into master Sep 15, 2021

nklaassen deleted the rfd/0035-aws-node-join branch September 15, 2021 22:49

zmb3 pushed a commit that referenced this pull request Sep 23, 2021

RFD 41: Simplified Node Joining for AWS (#7292)

8d75670


		### IAM Method

		In place of a join token, nodes will present a signed `sts:GetCallerIdentity`

Conversation

nklaassen commented Jun 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

klizhentas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phosphore Jun 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

knisbet left a comment

Choose a reason for hiding this comment

Uh oh!

klizhentas commented Aug 19, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

nklaassen commented Jun 14, 2021 •

edited

Loading

phosphore Jun 29, 2021 •

edited

Loading