Skip to content

Azure join method - use subscription ID from attested data#48707

Merged
atburke merged 1 commit intomasterfrom
atburke/azure-discovery-sub-id
Nov 18, 2024
Merged

Azure join method - use subscription ID from attested data#48707
atburke merged 1 commit intomasterfrom
atburke/azure-discovery-sub-id

Conversation

@atburke
Copy link
Copy Markdown
Contributor

@atburke atburke commented Nov 8, 2024

This change updates the Azure join method to lookup VMs based on the subscription ID provided in the VM's attested data, rather than the subscription ID derived from its access token. This allows Azure VMs to join a cluster even if their managed identity is from a different subscription than the VM itself.

Changelog: Allow Azure VMs to join from a different subscription than their managed identity

@bl-nero
Copy link
Copy Markdown
Contributor

bl-nero commented Nov 8, 2024

@atburke I don't feel too confident reviewing this PR, since I don't understand the security implications of this one. Is there an RFD somewhere? Can you get someone who is more familiar with this area? My main concern is that I don't understand what "a different subscription than the VM itself" means, and what kind of attack vectors it opens.

@atburke
Copy link
Copy Markdown
Contributor Author

atburke commented Nov 11, 2024

RFD here
For the Azure join method, an Azure VM will produce two pieces of signed data that we already trust, attested data and an access token for its managed identity. All that's changing is we're using the subscription ID from the attested data instead of the token.

Copy link
Copy Markdown
Contributor

@nklaassen nklaassen left a comment

Choose a reason for hiding this comment

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

I have a bunch of questions here sorry:

  • Is there a related github issue for this change?
  • Is this a bug fix, and if so how did we become aware of the bug?
  • I'm assuming the subscription ID is a similar concept to an AWS account ID?
  • Any chance this will be a breaking change for anyone?

So right now I can make a join token like

  azure:
    allow:
      # Subscription from which nodes can join. Required.
      - azure_subscription: '22222222'

and this should allow VMs running in that azure subscription 22222222 with a managed identity also from 22222222 to join. And I guess with this change, now VMs running in 33333333 could join as long as they have a managed identity from 22222222, did I get that right? If so can we make this clear in the PR description, since the changelog entry will link here?

How does this play with the azure_resource_groups specified in the join token? Does the VM have to be in that group in the subscription of the managed identity or the subscription the VM is running in? I don't fully understand these terms so maybe I'm asking a weird question

I guess this change does somewhat broaden the scope of existing join tokens to now allow joining from VMs in other accounts, so we should be careful with it. Do you think this should come with a docs update at all?

@atburke
Copy link
Copy Markdown
Contributor Author

atburke commented Nov 12, 2024

Is there a related github issue for this change?
Is this a bug fix, and if so how did we become aware of the bug?

No github issue, the bug came from this thread

I'm assuming the subscription ID is a similar concept to an AWS account ID?

Yes.

Any chance this will be a breaking change for anyone?

No. For existing clusters using Azure join successfully, the sub ID of the access token and the attested data are the same, so they won't notice anything different.

Important clarification: this change does NOT affect which VMs can join based on the join token rules. We use the access token to fetch the VM's info from the Azure API and use that result to check against the join token's allow/deny rules; the bug that this fixes was that Teleport was sometimes looking for the VM in the wrong subscription. In your example, the VM in subscription 33333333 would not suddenly be able to join. Same for resource groups, the VM has to be in the allowed groups but the identity doesn't necessarily.

@atburke
Copy link
Copy Markdown
Contributor Author

atburke commented Nov 15, 2024

Friendly ping @nklaassen

@nklaassen nklaassen self-requested a review November 16, 2024 00:05
@atburke atburke added this pull request to the merge queue Nov 18, 2024
@atburke atburke removed this pull request from the merge queue due to a manual request Nov 18, 2024
github-merge-queue Bot pushed a commit that referenced this pull request Nov 18, 2024
* Use attested data subscription id

* Add vmClientGetter
This change updates the Azure join method to look up VMs based on
the subscription ID provided in the VM's attested data, rather
than the subscription ID derived from its access token.
@atburke atburke force-pushed the atburke/azure-discovery-sub-id branch from 17906c4 to fc770c7 Compare November 18, 2024 19:46
@atburke atburke enabled auto-merge November 18, 2024 19:46
@atburke atburke added this pull request to the merge queue Nov 18, 2024
Merged via the queue into master with commit a00f95c Nov 18, 2024
@atburke atburke deleted the atburke/azure-discovery-sub-id branch November 18, 2024 20:21
@public-teleport-github-review-bot
Copy link
Copy Markdown

@atburke See the table below for backport results.

Branch Result
branch/v15 Create PR
branch/v16 Create PR
branch/v17 Create PR

kopiczko pushed a commit that referenced this pull request Nov 19, 2024
This change updates the Azure join method to look up VMs based on
the subscription ID provided in the VM's attested data, rather
than the subscription ID derived from its access token.
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.

4 participants