Skip to content

docs: Add role configuration nuance to moderated sessions#32357

Closed
lsgunn-teleport wants to merge 1 commit intomasterfrom
LG/join-session-role-gotcha
Closed

docs: Add role configuration nuance to moderated sessions#32357
lsgunn-teleport wants to merge 1 commit intomasterfrom
LG/join-session-role-gotcha

Conversation

@lsgunn-teleport
Copy link
Copy Markdown
Contributor

@lsgunn-teleport lsgunn-teleport commented Sep 21, 2023

Content change based on https://github.com/gravitational/teleport.e/issues/1359:

Role configuration gotchas
(possibly a note on this page: https://goteleport.com/docs/access-controls/reference/ or https://goteleport.com/docs/access-controls/guides/moderated-sessions/)
join_sessions has a special condition within our RBAC. Although in general deny statements take precedent, when a user is provided the allow for join_sessions the account will be implicitly able to also list session. This is a nuanced exception where the allow will override an explicit denial for listing sessions.

When this list / deny issue is resolved, #32420 must also be updated.

@github-actions
Copy link
Copy Markdown
Contributor

🤖 Vercel preview here: https://docs-13nnkhssa-goteleport.vercel.app/docs/ver/14.x

You should note that users who are assigned a role with a `join_sessions` allow policy are
implicitly allowed to list sessions. In most cases, `deny` statements take precedent.
However, if the `join_sessions` policy is set in a role, the `join_sessions` policy
overrides any explicit deny setting for listing sessions.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This makes it sound like they would be able to list all sessions. Is that actually the case?

My expectation is that a join_sessions policy should only override a deny rule for sessions that you are allowed to join.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jentfoo Can you clarify the intent here? Does join_sessions only allow you to join and list the sessions matching the allow rule?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This makes it sound like they would be able to list all sessions. Is that actually the case?

This is accurate, any user with join_sessions can list sessions, even if list sessions is explicitly denied. This appears to be a one off in our RBAC where deny wont take precedent (at least the only one I know of so far).

We discussed this on the issue here: https://github.com/gravitational/security-findings/issues/26

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah, but the role documented in the linked issue allows you to join all sessions for all roles.

I suspect if your join policy says "you can only join kube sessions for role foo" you might see different behavior.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not sure, I have not tested that case

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ok, let's hold this open until we understand the current behavior. I will try to get to testing it this afternoon.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@zmb3 @jentfoo Any chance someone can take a stab at testing the behavior to clarify? I haven't been able to work out exactly what the environment and roles should look like to test the scenario.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ok, I confirmed the behavior and my expectation in the first comment matches the current behavior.

My expectation is that a join_sessions policy should only override a deny rule for sessions that you are allowed to join.

This is what happens.

So a join_sessions policy doesn't override an explicit deny for all sessions, it only overrides the deny rule for sessions that you have permission to join (which makes sense).

The UI bug is still present, I had to verify this via API.

@lsgunn-teleport lsgunn-teleport temporarily deployed to vercel September 22, 2023 16:40 — with GitHub Actions Inactive
@github-actions
Copy link
Copy Markdown
Contributor

🤖 Vercel preview here: https://docs-bqbdlmjhb-goteleport.vercel.app/docs/ver/14.x

@zmb3
Copy link
Copy Markdown
Collaborator

zmb3 commented Sep 22, 2023

I found this not to be the case.

Here's what I did:

  • user zac has a bunch of permissions (can access anything and join anything)
  • user zac2 has permission to only access nodes labeled foo=bar via role access-foo
  • user zac3 has permission to only join sessions for role access-foo and a wildcard deny rule

Then I started two sessions: one as zac and one as zac2.

What I found:

  • zac can see both active sessions, as expected
  • zac2 can see only his own active session
  • zac3 doesn't see the active sessions option in the UI at all (might be a bug, I think they should be able to see the session they have permission to join)
image

zac3 does however have the ability to join with tsh assuming they know the session ID (which is near impossible to discover without being able to list)

❯ tsh join 11122968-2664-4270-9fd0-bc345a443713 --mode=moderator

~
➜ # this session running as user zac2

tl;dr - it's hard to say if this PR is correct or not because we may have a UI bug. Without being able to see the active sessions page at all we can't tell if the listing would be allowed or not (though I suspect it would).

cc @rudream for the UI portion

@jentfoo
Copy link
Copy Markdown
Contributor

jentfoo commented Sep 22, 2023

@zmb3 to confirm in your testing did you actually verify the restriction at the API level? When I had tested the UI would hide the session list but they were accessible if requested. The would be easiest to see when accessed from a leaf cluster (as described in the linked issue).

For that reason we were tracking it as a UI bug for artificially hiding this ability.

@zmb3
Copy link
Copy Markdown
Collaborator

zmb3 commented Sep 22, 2023

@jentfoo I did not (there is no tsh or tctl command to list active sessions, so the only way to truly verify that would be to write a custom API integration).

However, recall in our discussion on the linked issue that we were seeing different behavior in the root and leaf cluster, and only the leaf cluster was hiding the active sessions page.

In my testing above, everything was done on a single cluster, which suggests that something may have changed since we originally looked at this.

@jentfoo
Copy link
Copy Markdown
Contributor

jentfoo commented Sep 22, 2023

different behavior in the root and leaf cluster, and only the leaf cluster was hiding the active sessions page.

Actually it was reversed, the root would hide the sessions but the leaf would allow them to be shown in the UI.

@zmb3
Copy link
Copy Markdown
Collaborator

zmb3 commented Sep 22, 2023

Ah okay, so maybe nothing has changed then.

In any case, I don't think we can merge this docs update as-is. We either need to fix the issue and then confirm that the behavior described here is accurate or we need to update this to reflect today's behavior.

@lsgunn-teleport
Copy link
Copy Markdown
Contributor Author

@zmb3 @jentfoo This thread is a little hard for me to follow. What are the next steps here?

@zmb3
Copy link
Copy Markdown
Collaborator

zmb3 commented Oct 3, 2023

@lsgunn-teleport I just followed up in #32357 (comment)

The only adjustment needed is to adjust the comment about join_sessions overriding explicit deny rules.

@lsgunn-teleport
Copy link
Copy Markdown
Contributor Author

Closing in favor of #32991

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Oct 4, 2023

🤖 Vercel preview here: https://docs-77ypeb65y-goteleport.vercel.app/docs/ver/14.x

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