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.

Sign up for GitHub

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

Fix desktop session playback RBAC #10570

Merged
merged 4 commits into from
Feb 28, 2022
Merged

Fix desktop session playback RBAC #10570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8c193d2
Fix desktop session playback RBAC
zmb3 Feb 24, 2022
f8aa5a7
Merge branch 'master' into zmb3/fix-desktop-session-rbac
zmb3 Feb 25, 2022
f3e6bbe
Merge branch 'master' into zmb3/fix-desktop-session-rbac
zmb3 Feb 25, 2022
8b11af2
Merge branch 'master' into zmb3/fix-desktop-session-rbac
zmb3 Feb 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 13 additions & 14 deletions lib/auth/auth_with_roles.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func (a *ServerWithRoles) actionForKindSession(namespace, verb string, sid sessi
return trace.Wrap(a.actionWithExtendedContext(namespace, types.KindSession, verb, extendContext))
}

// actionForKindSSHSession is a special checker that grants access to active
// actionForKindSSHSession is a special checker that grants access to active SSH
// sessions. It can allow access to a specific session based on the `where`
// section of the user's access rule for kind `ssh_session`.
func (a *ServerWithRoles) actionForKindSSHSession(namespace, verb string, sid session.ID) error {
Expand Down Expand Up @@ -2494,23 +2494,22 @@ func (a *ServerWithRoles) GetSessionEvents(namespace string, sid session.ID, aft
return a.alog.GetSessionEvents(namespace, sid, afterN, includePrintEvents)
}

func (a *ServerWithRoles) findSessionEndEvent(namespace string, sid session.ID) (*apievents.SessionEnd, error) {
sessionEvents, err := a.alog.GetSessionEvents(namespace, sid, 0, false)
func (a *ServerWithRoles) findSessionEndEvent(namespace string, sid session.ID) (apievents.AuditEvent, error) {
sessionEvents, _, err := a.alog.SearchSessionEvents(time.Time{}, a.authServer.clock.Now().UTC(),
defaults.EventsIterationLimit, types.EventOrderAscending, "",
&types.WhereExpr{Equals: types.WhereExpr2{
L: &types.WhereExpr{Field: events.SessionEventID},
R: &types.WhereExpr{Literal: sid.String()},
}},
)
if err != nil {
return nil, trace.Wrap(err)
}
for _, ef := range sessionEvents {
if ef.GetType() == events.SessionEndEvent {
event, err := events.FromEventFields(ef)
if err != nil {
return nil, trace.Wrap(err)
}
if sessionEnd, ok := event.(*apievents.SessionEnd); ok {
return sessionEnd, nil
}
}
if len(sessionEvents) == 1 {
return sessionEvents[0], nil
}
return nil, trace.NotFound("session.end event not found for session ID %q", sid)

return nil, trace.NotFound("session end event not found for session ID %q", sid)
Comment on lines +2508 to +2512
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have a lot of context on this and trying to understand the logic. So if SearchSessionEvents returns exactly 1 event, it has to be a SessionEnd event? What about other types of events? thanks

Copy link
Collaborator Author

@zmb3 zmb3 Feb 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

The prior code used GetEvents - which returned all types of events, and then looked for a session.end (which itself is specific to SSH sessions)

The new code uses SearchSessionEvents instead which searches only for session.end or windows.desktop.session.end events. It also provides a filter for session ID. Since session IDs are unique and a session can only end once, we should get exactly 0 (not found) or 1 (valid end event) results.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation. SearchSessionEvents sounds like it will search for all types and i didn't see a type condition so that's what tripped me up.

}

// GetNamespaces returns a list of namespaces
Expand Down
11 changes: 6 additions & 5 deletions lib/services/parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,9 @@ type Context struct {
// Resource is an optional resource, in case if the rule
// checks access to the resource
Resource types.Resource
// Session is an optional session.end event. These events hold information
// about session recordings.
Session *events.SessionEnd
// Session is an optional session.end or windows.desktop.session.end event.
// These events hold information about session recordings.
Session events.AuditEvent
// SSHSession is an optional (active) SSH session.
SSHSession *session.Session
}
Expand Down Expand Up @@ -236,8 +236,9 @@ func (ctx *Context) GetIdentifier(fields []string) (interface{}, error) {
}
return predicate.GetFieldByTag(resource, teleport.JSON, fields[1:])
case SessionIdentifier:
session := &events.SessionEnd{}
if ctx.Session != nil {
var session events.AuditEvent = &events.SessionEnd{}
switch ctx.Session.(type) {
case *events.SessionEnd, *events.WindowsDesktopSessionEnd:
session = ctx.Session
}
return predicate.GetFieldByTag(session, teleport.JSON, fields[1:])
Expand Down