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

[v9] Rollup bugfix backport #11890

Merged
merged 3 commits into from
Apr 13, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
42 changes: 39 additions & 3 deletions lib/services/local/sessiontracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ import (
"github.com/gravitational/teleport/lib/modules"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/utils"

"github.com/gravitational/trace"
"github.com/sirupsen/logrus"
)

const (
Expand Down Expand Up @@ -121,14 +123,48 @@ func (s *sessionTracker) GetActiveSessionTrackers(ctx context.Context) ([]types.
return nil, trace.Wrap(err)
}

sessions := make([]types.SessionTracker, len(result.Items))
for i, item := range result.Items {
sessions := make([]types.SessionTracker, 0, len(result.Items))

// We don't overallocate expired since cleaning up sessions here should be rare.
var noExpiry []backend.Item
now := s.bk.Clock().Now()
for _, item := range result.Items {
session, err := unmarshalSession(item.Value)
if err != nil {
return nil, trace.Wrap(err)
}

sessions[i] = session
// NOTE: This is the session expiry timestamp, not the backend timestamp stored in `item.Expires`.
exp := session.GetExpires()
if session.Expiry().After(exp) {
exp = session.Expiry()
}

after := exp.After(now)

switch {
case after:
// Keep any items that aren't expired.
sessions = append(sessions, session)
case !after && item.Expires.IsZero():
// Clear item if expiry is not set on the backend.
// We currently don't set the expiry here but we will when #11551 is merged.
noExpiry = append(noExpiry, item)
default:
// If we take this branch, the expiry is set and the backend is responsible for cleaning up the item.
}
}

if len(noExpiry) > 0 {
go func() {
for _, item := range noExpiry {
if err := s.bk.Delete(ctx, item.Key); err != nil {
if !trace.IsNotFound(err) {
logrus.WithError(err).Error("Failed to remove stale session tracker")
}
}
}
}()
}

return sessions, nil
Expand Down
52 changes: 52 additions & 0 deletions lib/services/local/sessiontracker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package local
import (
"context"
"testing"
"time"

"github.com/google/uuid"
"github.com/gravitational/teleport/api/client/proto"
Expand Down Expand Up @@ -47,6 +48,7 @@ func TestSessionTrackerStorage(t *testing.T) {
User: "eve",
Mode: string(types.SessionPeerMode),
},
Expires: time.Now().UTC().Add(24 * time.Hour),
})
require.NoError(t, err)

Expand Down Expand Up @@ -87,3 +89,53 @@ func TestSessionTrackerStorage(t *testing.T) {
require.Error(t, err)
require.Nil(t, session)
}

func TestSessionTrackerImplicitExpiry(t *testing.T) {
ctx := context.Background()
bk, err := memory.New(memory.Config{})
require.NoError(t, err)

id := uuid.New().String()
id2 := uuid.New().String()
srv, err := NewSessionTrackerService(bk)
require.NoError(t, err)

req1 := proto.CreateSessionTrackerRequest{
Namespace: defaults.Namespace,
ID: id,
Type: types.KindSSHSession,
Hostname: "hostname",
ClusterName: "cluster",
Login: "foo",
Initiator: &types.Participant{
ID: uuid.New().String(),
User: "eve",
Mode: string(types.SessionPeerMode),
},
Expires: time.Now().UTC().Add(time.Second),
}

req2 := req1
req2.ID = id2
req2.Expires = time.Now().UTC().Add(24 * time.Hour)

_, err = srv.CreateSessionTracker(ctx, &req1)
require.NoError(t, err)

_, err = srv.CreateSessionTracker(ctx, &req2)
require.NoError(t, err)

require.Eventually(t, func() bool {
sessions, err := srv.GetActiveSessionTrackers(ctx)
require.NoError(t, err)

// Verify that we only get one session and that it's `id2` since we expect that
// `id` is filtered out due to it's expiry.
if len(sessions) == 1 {
require.Equal(t, sessions[0].GetSessionID(), id2)
return true
}

return false
}, time.Minute, time.Second)
}
7 changes: 6 additions & 1 deletion lib/srv/sess.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
rsession "github.com/gravitational/teleport/lib/session"
"github.com/gravitational/teleport/lib/sshutils"
"github.com/gravitational/teleport/lib/utils"
"github.com/gravitational/trace/trail"

"github.com/gravitational/trace"
"github.com/prometheus/client_golang/prometheus"
Expand Down Expand Up @@ -703,6 +704,10 @@ func newSession(id rsession.ID, r *SessionRegistry, ctx *ServerContext) (*sessio

err = sess.trackerCreate(ctx.Identity.TeleportUser, policySets)
if err != nil {
if trace.IsNotImplemented(err) {
return nil, trace.NotImplemented("Attempted to use Moderated Sessions with an Auth Server below the minimum version of 9.0.0.")
}

return nil, trace.Wrap(err)
}

Expand Down Expand Up @@ -1745,7 +1750,7 @@ func (s *session) trackerCreate(teleportUser string, policySet []*types.SessionT

_, err := s.registry.auth.CreateSessionTracker(s.serverCtx, req)
if err != nil {
return trace.Wrap(err)
return trail.FromGRPC(err)
}

// Start go routine to push back session expiration while session is still active.
Expand Down