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

Don't enforce standard k8s and ssh auth mechanisms when joining sessions #11144

Merged
merged 11 commits into from
May 5, 2022

Conversation

xacrimon
Copy link
Contributor

@xacrimon xacrimon commented Mar 15, 2022

We currently enforce standard SSH and K8S auth mechanisms to determine resource access in addition to the Moderated Sessions join check, we shouldn't do this since it prevents configuring the ability to join sessions without the ability to start them.

Notable changes:

  • Use the new SessionTracker system for SSH session discovery on the tsh side, move to phase out the session service and the semi-hacky metadata keys.
  • Get initial terminal size for joining a session from an SSH request rather than fetching a backend key that is constantly updated with the size, this was needed to start phasing out the old metadata system since we don't constantly update the new tracker system with such changing metadata.
  • Introduce a special SSH principal which does not abide by node access RBAC checks, this is restricted to joining sessions only at which point moderated sessions RBAC logic is applied.
  • Remove cluster access checks when joining a kubernetes session.
  • Apply RFD 45 RBAC deny rules to the new moderated sessions metadata system to keep compatibility.

This is well covered by existing tests, relevant tests have been adjusted.

@xacrimon xacrimon requested review from zmb3 and espadolini March 15, 2022 14:47
@xacrimon xacrimon marked this pull request as ready for review March 15, 2022 14:47
@github-actions github-actions bot added kubernetes-access tsh tsh - Teleport's command line tool for logging into nodes running Teleport. labels Mar 15, 2022
@xacrimon xacrimon requested a review from espadolini March 15, 2022 15:53
Copy link
Collaborator

@zmb3 zmb3 left a comment

Choose a reason for hiding this comment

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

Looks okay to me other than the minor suggestions here, but I am not super familiar with this code.

@russjones
Copy link
Contributor

@xacrimon Can you add test coverage to this PR?

@r0mant
Copy link
Collaborator

r0mant commented Mar 23, 2022

@xacrimon Just checking, are we going to close this one and go with a different approach?

@xacrimon
Copy link
Contributor Author

@r0mant We're not, we decided to go with this approach in #11223. I will be updating this PR soon.

@xacrimon
Copy link
Contributor Author

xacrimon commented Apr 12, 2022

@r0mant Can you take another look at this PR please since you left a request-changes status last?

@xacrimon
Copy link
Contributor Author

friendly ping: @r0mant

@xacrimon xacrimon force-pushed the joel/bypass-join branch from d8a21d3 to 7e3c973 Compare May 3, 2022 18:29
@xacrimon xacrimon enabled auto-merge (squash) May 5, 2022 14:40
@xacrimon xacrimon disabled auto-merge May 5, 2022 14:40
@xacrimon xacrimon enabled auto-merge (squash) May 5, 2022 18:33
@xacrimon xacrimon merged commit 652536f into master May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-required kubernetes-access moderated-sessions tsh tsh - Teleport's command line tool for logging into nodes running Teleport.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants