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

RFD - Oracle join method #49480

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

RFD - Oracle join method #49480

wants to merge 1 commit into from

Conversation

atburke
Copy link
Contributor

@atburke atburke commented Nov 26, 2024

This change adds an RFD for the Oracle Cloud join method.

Part of #41705.

@atburke atburke added rfd Request for Discussion no-changelog Indicates that a PR does not require a changelog entry labels Nov 26, 2024
Comment on lines 87 to 105
compartments: ["my_compartment", "ocid1.compartment.oc1...<unique_ID>"]
# Regions to allow instances to join from. Both full names ("us-phoenix-1")
# and abbreviations ("phx") are allowed. If regions is empty or
# a wildcard, instances can join from any region.
regions: ["phx", "us-ashburn-1"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This structure would mean that you have to allow joining from any region from any compartment.

Do we ever foresee a desire to further restrict this? Maybe something like:

allow:
- regions: [ phx ]
  compartments: [ compartment1, compartment2 ]
- regions: [ syd ]
  compartments: [ compartment3 ]

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What you're suggesting is what I intended, I'm not sure what the difference is between that and what I wrote (other than that I forgot to mention that tenancy is required).

rfd/0192-oci-join.md Outdated Show resolved Hide resolved
rfd/0192-oci-join.md Outdated Show resolved Hide resolved
rfd/0192-oci-join.md Outdated Show resolved Hide resolved
rfd/0192-oci-join.md Outdated Show resolved Hide resolved
rfd/0192-oci-join.md Outdated Show resolved Hide resolved
@ravicious ravicious removed their request for review November 27, 2024 13:36
rfd/0192-oci-join.md Outdated Show resolved Hide resolved
rfd/0192-oci-join.md Outdated Show resolved Hide resolved
rfd/0192-oci-join.md Outdated Show resolved Hide resolved
rfd/0192-oci-join.md Outdated Show resolved Hide resolved
rfd/0192-oci-join.md Outdated Show resolved Hide resolved
rfd/0192-oci-join.md Outdated Show resolved Hide resolved
rfd/0192-oci-join.md Outdated Show resolved Hide resolved
rfd/0192-oci-join.md Outdated Show resolved Hide resolved
rfd/0192-oci-join.md Outdated Show resolved Hide resolved
rfd/0192-oci-join.md Outdated Show resolved Hide resolved
@atburke
Copy link
Contributor Author

atburke commented Dec 10, 2024

I made a small mistake with instance principal permissions; they only have permission to list compartments by default, not get a compartment. If we stick will no extra permissions, all this means is that we can use compartment names in the Teleport provision token, only OCIDs. I think I prefer this solution since it's less work to set up, but if we do want to keep compartment names, configuring those permissions isn't a huge hassle either.

rfd/0192-oci-join.md Outdated Show resolved Hide resolved
@nklaassen
Copy link
Contributor

I had a look at Vault's oci auth method https://developer.hashicorp.com/vault/docs/auth/oci

It looks like what they do is:

  1. The client signs a request to their own vault URL with their oracle credentials, this is <vault_server>/v1/auth/login/role
  2. On the vault server, they include the signed headers from this request in an actual request to the vault auth endpoint auth.<region>.oraclecloud.com/v1/authentication/authenticateClient (ref), this returns the client's principal and tenant ID
  3. From there they send another request to get the client's group membership

The downside is this requires the vault server to have oci credentials and the following policy:

allow dynamic-group VaultDynamicGroup to {AUTHENTICATION_INSPECT} in tenancy
allow dynamic-group VaultDynamicGroup to {GROUP_MEMBERSHIP_INSPECT} in tenancy

I think it's possible we could do a version of this, where instead of our auth server signing the authenticateClient request, the node would have to pre-sign THAT request and send it to auth, to send to oracle.

The downside is this would unfortunately require every joining instance to have that AUTHENTICATION_INSPECT policy. The upside is we wouldn't be relying on getting the caller's tenant ID from the undocumented and unverified JWT. I'm not sure if this is viable for us but I wanted to share my findings

Copy link

@viktor-doyensec-teleport viktor-doyensec-teleport left a comment

Choose a reason for hiding this comment

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

Hey team! I just submitted our review of the RFD.

Overall, the "Security" section sufficiently outlines potential security-related issues with the feature. As a recap:

Replay Attacks

Adding a custom header with the challenge that Teleport generates will ensure the legetimacy of the request as well as ensuring that any replys after the token is accepted will be rejected.

JWT Validation

Because Teleport is not able to validate the signature of the JWT, the application cannot be certaing that the claims have not been tempered. To this end, only use values/claims that you can independently verify. All other values should not be considered safe and used without additional validation and verification.

Server-Side Request Forgery (SSRF)

To enrol an Oracle Cloud resource, Teleport will have to perform an HTTP request to Oracle's API to retrieve a list of available resources. This request is in partial control to the request. Proper precautions need to be taked to ensure that SSRF attacks are properly mitigated. This entails:

  1. removing the ability for user-controlled URLs - Users should not have control over the destination of the request. This value should be statically defined in the application.
  2. strict input validation - Any values that are user-controlled and will be used to build the final request need to undergo strict input validation. Validation should be performed based on the expected parameter type (e.g. UUID) or a pre-defined allowlist of expected values. Any deviations from the expected values should result in an error
  3. HTTP redirects - To further protect the application from SSRF attacks, disable any unneeded HTTP redirects. This ensures that the target URL defined by Teleport is the final destination of the request

Let me known if you have any comments or questions.

rfd/0192-oci-join.md Show resolved Hide resolved
rfd/0192-oci-join.md Outdated Show resolved Hide resolved
rfd/0192-oci-join.md Outdated Show resolved Hide resolved
@atburke
Copy link
Contributor Author

atburke commented Dec 20, 2024

Friendly ping @nklaassen @strideynet

rfd/0192-oci-join.md Outdated Show resolved Hide resolved

- Create a [dynamic group](https://docs.oracle.com/en-us/iaas/Content/Identity/Tasks/managingdynamicgroups.htm)
that matches all the instances that will join Teleport.
- Create the following policy: `Allow dynamic-group <dynamic-group-name> to inspect authentication in tenancy`
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you been able to find much about what exactly this policy allows and if it seems acceptable to allow this for all nodes that need to join teleport? If it only allows you call the authenticateClient endpoint it seems okay to me, all it allows is for the node to get information about itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't found anything that explicitly states what it allows, but these docs on the authentication_oci plugin for Oracle Heatwave (1, 2) suggest to me that it's only authenticateClient.

rfd/0192-oci-join.md Outdated Show resolved Hide resolved
@atburke
Copy link
Contributor Author

atburke commented Jan 4, 2025

Friendly ping @strideynet

}

message RegisterUsingOracleMethodResponse {
string challenge = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Perhaps a oneof is appropriate here in the request/responses?

@atburke atburke force-pushed the rfd/0192-oci-join branch 2 times, most recently from 3be29bf to 678e592 Compare January 6, 2025 19:08
@atburke atburke force-pushed the rfd/0192-oci-join branch from 678e592 to fad4a4d Compare January 6, 2025 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Indicates that a PR does not require a changelog entry rfd Request for Discussion size/md
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants