Skip to content

Commit

Permalink
Don't enforce standard k8s and ssh auth mechanisms when joining sessi…
Browse files Browse the repository at this point in the history
…ons (#11144)
  • Loading branch information
xacrimon authored May 5, 2022
1 parent c89e4c9 commit 652536f
Show file tree
Hide file tree
Showing 22 changed files with 239 additions and 118 deletions.
6 changes: 3 additions & 3 deletions api/types/session_tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ type SessionTracker interface {
// GetAddress returns the address of the session target.
GetAddress() string

// GetClustername returns the name of the cluster.
GetClustername() string
// GetClusterName returns the name of the cluster.
GetClusterName() string

// GetLogin returns the target machine username used for this session.
GetLogin() string
Expand Down Expand Up @@ -246,7 +246,7 @@ func (s *SessionTrackerV1) GetAddress() string {
}

// GetClustername returns the name of the cluster the session is running in.
func (s *SessionTrackerV1) GetClustername() string {
func (s *SessionTrackerV1) GetClusterName() string {
return s.Spec.ClusterName
}

Expand Down
7 changes: 7 additions & 0 deletions constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,9 @@ const (
// ForceTerminateRequest is an SSH request to forcefully terminate a session.
ForceTerminateRequest = "x-teleport-force-terminate"

// TerminalSizeRequest is a request for the terminal size of the session.
TerminalSizeRequest = "x-teleport-terminal-size"

// MFAPresenceRequest is an SSH request to notify clients that MFA presence is required for a session.
MFAPresenceRequest = "x-teleport-mfa-presence"

Expand All @@ -635,6 +638,10 @@ const (
// EnvSSHSessionDisplayParticipantRequirements is set to true or false to indicate if participant
// requirement information should be printed.
EnvSSHSessionDisplayParticipantRequirements = "TELEPORT_SESSION_PARTICIPANT_REQUIREMENTS"

// SSHSessionJoinPrincipal is the SSH principal used when joining sessions.
// This starts with a hyphen so it isn't a valid unix login.
SSHSessionJoinPrincipal = "-teleport-internal-join"
)

const (
Expand Down
13 changes: 12 additions & 1 deletion integration/kube_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1526,8 +1526,19 @@ func testKubeJoin(t *testing.T, suite *KubeSuite) {
},
})
require.NoError(t, err)
joinRole, err := types.NewRoleV3("participant", types.RoleSpecV5{
Allow: types.RoleConditions{
JoinSessions: []*types.SessionJoinPolicy{{
Name: "foo",
Roles: []string{"kubemaster"},
Kinds: []string{string(types.KubernetesSessionKind)},
Modes: []string{string(types.SessionPeerMode)},
}},
},
})
require.NoError(t, err)
teleport.AddUserWithRole(hostUsername, role)
teleport.AddUserWithRole(participantUsername, role)
teleport.AddUserWithRole(participantUsername, joinRole)

err = teleport.CreateEx(t, nil, tconf)
require.NoError(t, err)
Expand Down
4 changes: 4 additions & 0 deletions lib/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -1043,6 +1043,10 @@ func (a *Server) generateUserCert(req certRequest) (*proto.Certs, error) {
return nil, trace.Wrap(err)
}

// Add the special join-only principal used for joining sessions.
// All users have access to this and join RBAC rules are checked after the connection is established.
allowedLogins = append(allowedLogins, "-teleport-internal-join")

params := services.UserCertParams{
CASigner: caSigner,
CASigningAlg: sshutils.GetSigningAlgName(userCA),
Expand Down
14 changes: 7 additions & 7 deletions lib/auth/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ func TestAuthenticateSSHUser(t *testing.T) {
gotSSHCert, err := sshutils.ParseCertificate(resp.Cert)
require.NoError(t, err)
require.Equal(t, gotSSHCert.Key, inSSHPub)
require.Equal(t, gotSSHCert.ValidPrincipals, []string{user})
require.Equal(t, gotSSHCert.ValidPrincipals, []string{user, teleport.SSHSessionJoinPrincipal})
// Verify the public key and Subject in TLS cert.
inCryptoPub := inSSHPub.(ssh.CryptoPublicKey).CryptoPublicKey()
gotTLSCert, err := tlsca.ParseCertificatePEM(resp.TLSCert)
Expand All @@ -290,7 +290,7 @@ func TestAuthenticateSSHUser(t *testing.T) {
wantID := tlsca.Identity{
Username: user,
Groups: []string{role.GetName()},
Principals: []string{user},
Principals: []string{user, teleport.SSHSessionJoinPrincipal},
KubernetesUsers: []string{user},
KubernetesGroups: []string{"system:masters"},
Expires: gotTLSCert.NotAfter,
Expand Down Expand Up @@ -319,7 +319,7 @@ func TestAuthenticateSSHUser(t *testing.T) {
wantID = tlsca.Identity{
Username: user,
Groups: []string{role.GetName()},
Principals: []string{user},
Principals: []string{user, teleport.SSHSessionJoinPrincipal},
KubernetesUsers: []string{user},
KubernetesGroups: []string{"system:masters"},
// It's OK to use a non-existent kube cluster for leaf teleport
Expand Down Expand Up @@ -363,7 +363,7 @@ func TestAuthenticateSSHUser(t *testing.T) {
wantID = tlsca.Identity{
Username: user,
Groups: []string{role.GetName()},
Principals: []string{user},
Principals: []string{user, teleport.SSHSessionJoinPrincipal},
KubernetesUsers: []string{user},
KubernetesGroups: []string{"system:masters"},
KubernetesCluster: "root-kube-cluster",
Expand Down Expand Up @@ -396,7 +396,7 @@ func TestAuthenticateSSHUser(t *testing.T) {
wantID = tlsca.Identity{
Username: user,
Groups: []string{role.GetName()},
Principals: []string{user},
Principals: []string{user, teleport.SSHSessionJoinPrincipal},
KubernetesUsers: []string{user},
KubernetesGroups: []string{"system:masters"},
KubernetesCluster: "root-kube-cluster",
Expand Down Expand Up @@ -438,7 +438,7 @@ func TestAuthenticateSSHUser(t *testing.T) {
wantID = tlsca.Identity{
Username: user,
Groups: []string{role.GetName()},
Principals: []string{user},
Principals: []string{user, teleport.SSHSessionJoinPrincipal},
KubernetesUsers: []string{user},
KubernetesGroups: []string{"system:masters"},
KubernetesCluster: "root-kube-cluster",
Expand Down Expand Up @@ -471,7 +471,7 @@ func TestAuthenticateSSHUser(t *testing.T) {
wantID = tlsca.Identity{
Username: user,
Groups: []string{role.GetName()},
Principals: []string{user},
Principals: []string{user, teleport.SSHSessionJoinPrincipal},
KubernetesUsers: []string{user},
KubernetesGroups: []string{"system:masters"},
KubernetesCluster: "root-kube-cluster",
Expand Down
40 changes: 36 additions & 4 deletions lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,19 +266,51 @@ func (a *ServerWithRoles) GetActiveSessionTrackers(ctx context.Context) ([]types

var filteredSessions []types.SessionTracker

for _, session := range sessions {
evaluator := NewSessionAccessEvaluator(session.GetHostPolicySets(), session.GetSessionKind())
for _, sess := range sessions {
evaluator := NewSessionAccessEvaluator(sess.GetHostPolicySets(), sess.GetSessionKind())
joinerRoles, err := a.authServer.GetRoles(ctx)
if err != nil {
return nil, trace.Wrap(err)
}

modes, err := evaluator.CanJoin(SessionAccessContext{Roles: joinerRoles})
if err == nil || len(modes) > 0 {
filteredSessions = append(filteredSessions, session)
// Apply RFD 45 RBAC rules to the session if it's SSH.
// This is a bit of a hack. It converts to the old legacy format
// which we don't have all data for, luckily the fields we don't have aren't made available
// to the RBAC filter anyway.
if sess.GetKind() == types.KindSSHSession {
ruleCtx := &services.Context{User: a.context.User}
ruleCtx.SSHSession = &session.Session{
ID: session.ID(sess.GetSessionID()),
Namespace: apidefaults.Namespace,
Login: sess.GetLogin(),
Created: sess.GetCreated(),
LastActive: a.authServer.GetClock().Now(),
ServerID: sess.GetAddress(),
ServerAddr: sess.GetAddress(),
ServerHostname: sess.GetHostname(),
ClusterName: sess.GetClusterName(),
}

for _, participant := range sess.GetParticipants() {
// We only need to fill in User here since other fields get discarded anyway.
ruleCtx.SSHSession.Parties = append(ruleCtx.SSHSession.Parties, session.Party{
User: participant.User,
})
}

// Skip past it if there's a deny rule in place blocking access.
if err := a.context.Checker.CheckAccessToRule(ruleCtx, apidefaults.Namespace, types.KindSSHSession, types.VerbList, true /* silent */); err != nil {
continue
}
}

filteredSessions = append(filteredSessions, sess)
} else {
log.Warnf("Session %v is not allowed to join: %v", sess.GetSessionID(), err)
}
}

return filteredSessions, nil
}

Expand Down
3 changes: 2 additions & 1 deletion lib/auth/auth_with_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,8 @@ func TestGenerateUserCertsWithRoleRequest(t *testing.T) {
require.NoError(t, err)

if len(tt.expectPrincipals) > 0 {
require.ElementsMatch(t, tt.expectPrincipals, userCert.ValidPrincipals, "principals must match")
expectPrincipals := append(tt.expectPrincipals, teleport.SSHSessionJoinPrincipal)
require.ElementsMatch(t, expectPrincipals, userCert.ValidPrincipals, "principals must match")
}

if tt.expectRoles != nil {
Expand Down
59 changes: 11 additions & 48 deletions lib/auth/session_access.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,16 +171,21 @@ func (e *SessionAccessEvaluator) matchesKind(allow []string) bool {
return false
}

func HasV5Role(roles []types.Role) bool {
for _, role := range roles {
if role.GetVersion() == types.V5 {
return true
}
}

return false
}

// CanJoin returns the modes a user has access to join a session with.
// If the list is empty, the user doesn't have access to join the session at all.
func (e *SessionAccessEvaluator) CanJoin(user SessionAccessContext) ([]types.SessionParticipantMode, error) {
supported, err := e.supportsSessionAccessControls()
if err != nil {
return nil, trace.Wrap(err)
}

// If we don't support session access controls, return the default mode set that was supported prior to Moderated Sessions.
if !supported {
if !HasV5Role(user.Roles) {
return preAccessControlsModes(e.kind), nil
}

Expand Down Expand Up @@ -217,16 +222,6 @@ type PolicyOptions struct {
TerminateOnLeave bool
}

func (e *SessionAccessEvaluator) hasPolicies() bool {
for _, policySet := range e.policySets {
if len(policySet.RequireSessionJoin) > 0 {
return true
}
}

return false
}

// Generate a pretty-printed string of precise requirements for session start suitable for user display.
func (e *SessionAccessEvaluator) PrettyRequirementsList() string {
s := new(strings.Builder)
Expand Down Expand Up @@ -262,16 +257,6 @@ func (e *SessionAccessEvaluator) extractApplicablePolicies(set *types.SessionTra

// FulfilledFor checks if a given session may run with a list of participants.
func (e *SessionAccessEvaluator) FulfilledFor(participants []SessionAccessContext) (bool, PolicyOptions, error) {
supported, err := e.supportsSessionAccessControls()
if err != nil {
return false, PolicyOptions{}, trace.Wrap(err)
}

// If advanced access controls are supported or no require policies are defined, we allow by default.
if !e.hasPolicies() || !supported {
return true, PolicyOptions{TerminateOnLeave: true}, nil
}

options := PolicyOptions{TerminateOnLeave: true}

// Check every policy set to check if it's fulfilled.
Expand Down Expand Up @@ -341,28 +326,6 @@ policySetLoop:
return true, options, nil
}

// supportsSessionAccessControls checks if moderated sessions-style access controls can be applied to the session.
// If a set only has v4 or earlier roles, we don't want to apply the access checks to SSH sessions.
//
// This only applies to SSH sessions since they previously had no access control for joining sessions.
// We don't need this fallback behaviour for multiparty kubernetes since it's a new feature.
func (e *SessionAccessEvaluator) supportsSessionAccessControls() (bool, error) {
if e.kind == types.SSHSessionKind {
for _, policySet := range e.policySets {
switch policySet.Version {
case types.V1, types.V2, types.V3, types.V4:
return false, nil
case types.V5:
return true, nil
default:
return false, trace.BadParameter("unsupported role version: %v", policySet.Version)
}
}
}

return true, nil
}

func preAccessControlsModes(kind types.SessionKind) []types.SessionParticipantMode {
switch kind {
case types.SSHSessionKind:
Expand Down
6 changes: 3 additions & 3 deletions lib/auth/tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1432,10 +1432,10 @@ func (s *TLSSuite) TestAccessRequest(c *check.C) {
c.Assert(certRequests(userCerts.TLS), check.HasLen, 0)

// verify that cert for user with no static logins is generated with
// exactly one login and that it is an invalid unix login (indicated
// exactly two logins and that the one that isn't a join principal is an invalid unix login (indicated
// by preceding dash (-).
logins := certLogins(userCerts.SSH)
c.Assert(len(logins), check.Equals, 1)
c.Assert(len(logins), check.Equals, 2)
c.Assert(rune(logins[0][0]), check.Equals, '-')

// attempt to apply request in PENDING state (should fail)
Expand All @@ -1462,7 +1462,7 @@ func (s *TLSSuite) TestAccessRequest(c *check.C) {
// verify that dynamically applied role granted a login,
// which is is valid and has replaced the dummy login.
logins = certLogins(userCerts.SSH)
c.Assert(len(logins), check.Equals, 1)
c.Assert(len(logins), check.Equals, 2)
c.Assert(rune(logins[0][0]), check.Not(check.Equals), '-')

elevatedCert, err := tls.X509KeyPair(userCerts.TLS, priv)
Expand Down
46 changes: 10 additions & 36 deletions lib/client/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -1562,51 +1562,25 @@ func (tc *TeleportClient) Join(ctx context.Context, mode types.SessionParticipan
return trace.Wrap(err)
}

// find the session ID on the site:
sessions, err := site.GetSessions(namespace)
var session types.SessionTracker
sessions, err := site.GetActiveSessionTrackers(ctx)
if err != nil {
return trace.Wrap(err)
}
var session *session.Session
for _, s := range sessions {
if s.ID == sessionID {
session = &s
break

for _, sessionIter := range sessions {
if sessionIter.GetSessionID() == string(sessionID) {
session = sessionIter
}
}
if session == nil {
return trace.NotFound(notFoundErrorMessage)
}

// pick the 1st party of the session and use his server ID to connect to
if len(session.Parties) == 0 {
if session == nil {
return trace.NotFound(notFoundErrorMessage)
}
serverID := session.Parties[0].ServerID

// find a server address by its ID
nodes, err := site.GetNodes(ctx, namespace)
if err != nil {
return trace.Wrap(err)
}
var node types.Server
for _, n := range nodes {
if n.GetName() == serverID {
node = n
break
}
}
if node == nil {
return trace.NotFound(notFoundErrorMessage)
}
target := node.GetAddr()
if target == "" {
// address is empty, try dialing by UUID instead
target = fmt.Sprintf("%s:0", serverID)
}
// connect to server:
nc, err := proxyClient.ConnectToNode(ctx, NodeAddr{
Addr: target,
Addr: session.GetAddress() + ":0",
Namespace: tc.Namespace,
Cluster: tc.SiteName,
}, tc.Config.HostLogin, false)
Expand All @@ -1625,7 +1599,7 @@ func (tc *TeleportClient) Join(ctx context.Context, mode types.SessionParticipan
if mode == types.SessionModeratorMode {
beforeStart = func(out io.Writer) {
nc.OnMFA = func() {
runPresenceTask(presenceCtx, out, site, tc, string(session.ID))
runPresenceTask(presenceCtx, out, site, tc, session.GetSessionID())
}
}
}
Expand Down Expand Up @@ -2136,7 +2110,7 @@ func (tc *TeleportClient) runCommand(ctx context.Context, nodeClient *NodeClient

// runShell starts an interactive SSH session/shell.
// sessionID : when empty, creates a new shell. otherwise it tries to join the existing session.
func (tc *TeleportClient) runShell(ctx context.Context, nodeClient *NodeClient, mode types.SessionParticipantMode, sessToJoin *session.Session, beforeStart func(io.Writer)) error {
func (tc *TeleportClient) runShell(ctx context.Context, nodeClient *NodeClient, mode types.SessionParticipantMode, sessToJoin types.SessionTracker, beforeStart func(io.Writer)) error {
env := make(map[string]string)
env[teleport.EnvSSHJoinMode] = string(mode)
env[teleport.EnvSSHSessionReason] = tc.Config.Reason
Expand Down
Loading

0 comments on commit 652536f

Please sign in to comment.