diff --git a/lib/auth/sessions.go b/lib/auth/sessions.go index 48136cf1630aa..aec4b43bbb173 100644 --- a/lib/auth/sessions.go +++ b/lib/auth/sessions.go @@ -34,7 +34,6 @@ import ( devicepb "github.com/gravitational/teleport/api/gen/proto/go/teleport/devicetrust/v1" mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" "github.com/gravitational/teleport/api/types" - apievents "github.com/gravitational/teleport/api/types/events" "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/api/utils/keys/hardwarekey" "github.com/gravitational/teleport/entitlements" @@ -42,7 +41,6 @@ import ( "github.com/gravitational/teleport/lib/cryptosuites" "github.com/gravitational/teleport/lib/defaults" dtauthz "github.com/gravitational/teleport/lib/devicetrust/authz" - "github.com/gravitational/teleport/lib/events" "github.com/gravitational/teleport/lib/jwt" "github.com/gravitational/teleport/lib/modules" "github.com/gravitational/teleport/lib/services" @@ -557,72 +555,6 @@ func (a *Server) CreateAppSessionFromReq(ctx context.Context, req NewAppSessionR return nil, trace.Wrap(err) } - // Audit fields used for both success and failure - sessionStartEvent := &apievents.AppSessionStart{ - Metadata: apievents.Metadata{ - Type: events.AppSessionStartEvent, - ClusterName: req.ClusterName, - }, - ServerMetadata: apievents.ServerMetadata{ - ServerVersion: teleport.Version, - ServerID: a.ServerID, - ServerNamespace: apidefaults.Namespace, - }, - ConnectionMetadata: apievents.ConnectionMetadata{ - RemoteAddr: req.ClientAddr, - }, - AppMetadata: apievents.AppMetadata{ - AppURI: req.AppURI, - AppPublicAddr: req.PublicAddr, - AppName: req.AppName, - AppTargetPort: uint32(req.AppTargetPort), - }, - } - - // Enforce device trust early via the AccessChecker. - if err = checker.CheckDeviceAccess(services.AccessState{ - DeviceVerified: dtauthz.IsTLSDeviceVerified((*tlsca.DeviceExtensions)(&req.DeviceExtensions)), - EnableDeviceVerification: true, - IsBot: req.BotName != "", - }); err != nil { - userKind := apievents.UserKind_USER_KIND_HUMAN - if req.BotName != "" { - userKind = apievents.UserKind_USER_KIND_BOT - } - - userMetadata := apievents.UserMetadata{ - User: req.User, - BotName: req.BotName, - BotInstanceID: req.BotInstanceID, - UserKind: userKind, - UserRoles: req.Roles, - UserClusterName: req.ClusterName, - UserTraits: req.Traits, - AWSRoleARN: req.AWSRoleARN, - } - - if req.DeviceExtensions.DeviceID != "" { - userMetadata.TrustedDevice = &apievents.DeviceMetadata{ - DeviceId: req.DeviceExtensions.DeviceID, - AssetTag: req.DeviceExtensions.AssetTag, - CredentialId: req.DeviceExtensions.CredentialID, - } - } - errMsg := "requires a trusted device" - - sessionStartEvent.Metadata.SetCode(events.AppSessionStartFailureCode) - sessionStartEvent.UserMetadata = userMetadata - sessionStartEvent.SessionMetadata = apievents.SessionMetadata{ - WithMFA: req.MFAVerified, - } - sessionStartEvent.Error = err.Error() - sessionStartEvent.UserMessage = errMsg - - a.emitter.EmitAuditEvent(a.closeCtx, sessionStartEvent) - // err swallowed/obscured on purpose. - return nil, trace.AccessDenied("%s", errMsg) - } - sessionID := req.SuggestedSessionID if sessionID == "" { // Create services.WebSession for this session. @@ -721,32 +653,6 @@ func (a *Server) CreateAppSessionFromReq(ctx context.Context, req NewAppSessionR return session, nil } - // Extract the identity of the user from the certificate, this will include metadata from any actively assumed access requests. - certificate, err := tlsca.ParseCertificatePEM(session.GetTLSCert()) - if err != nil { - return nil, trace.Wrap(err) - } - identity, err := tlsca.FromSubject(certificate.Subject, certificate.NotAfter) - if err != nil { - return nil, trace.Wrap(err) - } - - userMetadata := identity.GetUserMetadata() - userMetadata.User = session.GetUser() - userMetadata.AWSRoleARN = req.AWSRoleARN - - sessionStartEvent.Metadata.SetCode(events.AppSessionStartCode) - sessionStartEvent.UserMetadata = userMetadata - sessionStartEvent.SessionMetadata = apievents.SessionMetadata{ - SessionID: session.GetName(), - WithMFA: req.MFAVerified, - PrivateKeyPolicy: string(identity.PrivateKeyPolicy), - } - - if err := a.emitter.EmitAuditEvent(a.closeCtx, sessionStartEvent); err != nil { - a.logger.WarnContext(ctx, "Failed to emit app session start event", "error", err) - } - return session, nil } diff --git a/lib/auth/sessions_test.go b/lib/auth/sessions_test.go index eff71f6792922..9ce8e3b318229 100644 --- a/lib/auth/sessions_test.go +++ b/lib/auth/sessions_test.go @@ -34,10 +34,13 @@ import ( devicepb "github.com/gravitational/teleport/api/gen/proto/go/teleport/devicetrust/v1" "github.com/gravitational/teleport/api/types" apievents "github.com/gravitational/teleport/api/types/events" + "github.com/gravitational/teleport/entitlements" "github.com/gravitational/teleport/lib/auth" "github.com/gravitational/teleport/lib/auth/authtest" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/events" + "github.com/gravitational/teleport/lib/modules" + "github.com/gravitational/teleport/lib/modules/modulestest" "github.com/gravitational/teleport/lib/services" "github.com/gravitational/teleport/lib/tlsca" "github.com/gravitational/teleport/lib/utils" @@ -206,6 +209,8 @@ func TestServer_CreateWebSessionFromReq_deviceWebToken(t *testing.T) { } } +// TestCreateAppSession_DeviceTrust validates the core Device Trust enforcement logic. +// It tests every permutation of Device Trust modes and against both human users and bots. func TestCreateAppSession_DeviceTrust(t *testing.T) { t.Parallel() @@ -215,8 +220,10 @@ func TestCreateAppSession_DeviceTrust(t *testing.T) { Clock: fakeClock, Dir: t.TempDir(), }) - require.NoError(t, err) - t.Cleanup(func() { testAuthServer.Close() }) + require.NoError(t, err, "NewAuthServer failed") + t.Cleanup(func() { + assert.NoError(t, testAuthServer.Close(), "testAuthServer.Close() errored") + }) authServer := testAuthServer.AuthServer @@ -281,8 +288,17 @@ func TestCreateAppSession_DeviceTrust(t *testing.T) { suffix := uuid.NewString() username := "user-" + suffix roleName := "role-" + suffix + appName := "app-" + suffix publicAddr := "www-" + suffix + ".example.com" + app, err := types.NewAppV3(types.Metadata{ + Name: appName, + }, types.AppSpecV3{ + URI: "http://example.com", + }) + require.NoError(t, err) + require.NoError(t, authServer.CreateApp(ctx, app)) + role, err := types.NewRole(roleName, types.RoleSpecV6{ Options: types.RoleOptions{ DeviceTrustMode: tt.trustMode, @@ -324,7 +340,7 @@ func TestCreateAppSession_DeviceTrust(t *testing.T) { req := &proto.CreateAppSessionRequest{ Username: username, - AppName: "example-app", + AppName: appName, URI: "http://example.com", ClusterName: clusterName, PublicAddr: publicAddr, @@ -367,7 +383,7 @@ func TestCreateAppSession_DeviceTrust(t *testing.T) { UserTraits: traits, }, AppMetadata: apievents.AppMetadata{ - AppName: "example-app", + AppName: appName, AppURI: "http://example.com", AppPublicAddr: publicAddr, }, @@ -423,3 +439,229 @@ func TestCreateAppSession_DeviceTrust(t *testing.T) { }) } } + +func TestCreateAppSession_UntrustedDevice(t *testing.T) { + t.Parallel() + + testAuthServer, err := authtest.NewAuthServer(authtest.AuthServerConfig{ + Dir: t.TempDir(), + }) + require.NoError(t, err) + t.Cleanup(func() { testAuthServer.Close() }) + + // The first role allows access to all apps and doesn't require a trusted device. + allowAllWithoutDeviceTrust, err := types.NewRole("all-apps-any-device", types.RoleSpecV6{ + Options: types.RoleOptions{DeviceTrustMode: constants.DeviceTrustModeOptional}, + Allow: types.RoleConditions{AppLabels: types.Labels{"*": []string{"*"}}}, + }) + require.NoError(t, err) + _, err = testAuthServer.AuthServer.CreateRole(t.Context(), allowAllWithoutDeviceTrust) + require.NoError(t, err) + + // The second role requires a trusted device, but only applies to apps in prod. + requireDeviceTrustForProd, err := types.NewRole("prod-trusted-device", types.RoleSpecV6{ + Options: types.RoleOptions{DeviceTrustMode: constants.DeviceTrustModeRequired}, + Allow: types.RoleConditions{AppLabels: types.Labels{"env": []string{"prod"}}}, + }) + require.NoError(t, err) + _, err = testAuthServer.AuthServer.CreateRole(t.Context(), requireDeviceTrustForProd) + require.NoError(t, err) + + // Create a user with both roles. + user, err := types.NewUser("bob") + require.NoError(t, err) + user.AddRole(allowAllWithoutDeviceTrust.GetName()) + user.AddRole(requireDeviceTrustForProd.GetName()) + _, err = testAuthServer.AuthServer.CreateUser(t.Context(), user) + require.NoError(t, err) + + devApp, err := types.NewAppV3(types.Metadata{ + Name: "dev-app", + Labels: map[string]string{"env": "dev"}, + }, types.AppSpecV3{ + URI: "http://dev-app.example.com", + }) + require.NoError(t, err) + require.NoError(t, testAuthServer.AuthServer.CreateApp(t.Context(), devApp)) + + prodApp, err := types.NewAppV3(types.Metadata{ + Name: "prod-app", + Labels: map[string]string{"env": "prod"}, + }, types.AppSpecV3{ + URI: "http://prod-app.example.com", + }) + require.NoError(t, err) + require.NoError(t, testAuthServer.AuthServer.CreateApp(t.Context(), prodApp)) + + cName, err := testAuthServer.AuthServer.GetClusterName(t.Context()) + require.NoError(t, err) + clusterName := cName.GetClusterName() + + checker, err := services.NewAccessChecker(&services.AccessInfo{ + Username: user.GetName(), + Roles: user.GetRoles(), + }, clusterName, testAuthServer.AuthServer) + require.NoError(t, err) + + for _, test := range []struct { + app types.Application + assert require.ErrorAssertionFunc + wantErr string + }{ + {app: prodApp, assert: require.Error, wantErr: "requires a trusted device"}, + {app: devApp, assert: require.NoError}, + } { + t.Run(test.app.GetName(), func(t *testing.T) { + req := &proto.CreateAppSessionRequest{ + Username: user.GetName(), + AppName: test.app.GetName(), + URI: test.app.GetURI(), + PublicAddr: test.app.GetPublicAddr(), + ClusterName: clusterName, + } + // simulate a user identity WITHOUT a trusted device + identity := tlsca.Identity{Username: user.GetName()} + + _, err = testAuthServer.AuthServer.CreateAppSession(t.Context(), req, identity, checker) + if test.wantErr == "" { + assert.NoError(t, err, "CreateAppSession errored unexpectedly") + return + } + assert.ErrorContains(t, err, test.wantErr, "CreateAppSession error mismatch") + }) + } +} + +func BenchmarkCreateAppSession(b *testing.B) { + // Enable Enterprise features + modulestest.SetTestModules(b, modulestest.Modules{ + TestBuildType: modules.BuildEnterprise, + TestFeatures: modules.Features{ + Entitlements: map[entitlements.EntitlementKind]modules.EntitlementInfo{ + entitlements.DeviceTrust: {Enabled: true}, + entitlements.App: {Enabled: true}, + }, + }, + }) + + b.StopTimer() + ctx := context.Background() + testAuthServer, err := authtest.NewAuthServer(authtest.AuthServerConfig{ + Dir: b.TempDir(), + }) + if err != nil { + b.Fatalf("NewAuthServer failed: %v", err) + } + defer testAuthServer.Close() + authServer := testAuthServer.AuthServer + + // Set up multiple roles for AccessChecker to evaluate + roleOptional, _ := types.NewRole("optional-role", types.RoleSpecV6{ + Options: types.RoleOptions{DeviceTrustMode: constants.DeviceTrustModeOptional}, + Allow: types.RoleConditions{AppLabels: types.Labels{"*": []string{"*"}}}, + }) + authServer.CreateRole(ctx, roleOptional) + + roleRequired, _ := types.NewRole("required-role", types.RoleSpecV6{ + Options: types.RoleOptions{DeviceTrustMode: constants.DeviceTrustModeRequired}, + Allow: types.RoleConditions{AppLabels: types.Labels{"env": []string{"prod"}}}, + }) + authServer.CreateRole(ctx, roleRequired) + + username := "bench-user" + user, _ := types.NewUser(username) + user.AddRole(roleOptional.GetName()) + user.AddRole(roleRequired.GetName()) + authServer.CreateUser(ctx, user) + + appName := "prod-app" + app, _ := types.NewAppV3(types.Metadata{ + Name: appName, + Labels: map[string]string{"env": "prod"}, + }, types.AppSpecV3{URI: "http://prod.example.com"}) + authServer.CreateApp(ctx, app) + + cName, _ := authServer.GetClusterName(ctx) + clusterName := cName.GetClusterName() + + benchmarks := []struct { + name string + appName string + hasDevice bool + expectDenied bool + }{ + { + name: "Success_App_With_Device", + appName: "prod-app", + hasDevice: true, + expectDenied: false, + }, + { + name: "Fail_App_No_Device", + appName: "prod-app", + hasDevice: false, + expectDenied: true, + }, + { + name: "Pass_Static_App", + appName: "static-app", // should pass early because app isn't found + hasDevice: false, + expectDenied: false, + }, + } + + for _, bm := range benchmarks { + b.Run(bm.name, func(b *testing.B) { + identity := tlsca.Identity{ + Username: username, + Groups: []string{"required-role", "optional-role"}, + } + if bm.hasDevice { + identity.DeviceExtensions = tlsca.DeviceExtensions{ + DeviceID: "macbook-id-123", + AssetTag: "asset-tag-123", + CredentialID: "cred-id-123", + } + } + + checker, err := services.NewAccessChecker(&services.AccessInfo{ + Username: username, + Roles: user.GetRoles(), + }, clusterName, authServer) + if err != nil { + b.Fatalf("NewAccessChecker failed: %v", err) + } + + req := &proto.CreateAppSessionRequest{ + Username: username, + AppName: bm.appName, + ClusterName: clusterName, + } + + _, err = authServer.CreateAppSession(ctx, req, identity, checker) + if bm.expectDenied { + if err == nil || !trace.IsAccessDenied(err) { + b.Fatalf("First call failed: expected access denied, got %v", err) + } + } else { + if err != nil { + b.Fatalf("First call failed: setup is incorrect for %s: %v", bm.name, err) + } + } + + b.ReportAllocs() + b.ResetTimer() + + for i := 0; i < b.N; i++ { + _, err := authServer.CreateAppSession(ctx, req, identity, checker) + if bm.expectDenied { + if err == nil || !trace.IsAccessDenied(err) { + b.Fatal("expected access denied") + } + } else if err != nil { + b.Fatalf("unexpected error: %v", err) + } + } + }) + } +} diff --git a/lib/services/access_checker.go b/lib/services/access_checker.go index 1f5585c7d0cf0..20a4b7f90682d 100644 --- a/lib/services/access_checker.go +++ b/lib/services/access_checker.go @@ -65,10 +65,6 @@ type AccessChecker interface { // CheckAccess checks access to the specified resource. CheckAccess(r AccessCheckable, state AccessState, matchers ...RoleMatcher) error - // CheckDeviceAccess verifies if the current device state satisfies the - // device trust requirements of the user's RoleSet. - CheckDeviceAccess(state AccessState) error - // CheckAccessToRemoteCluster checks access to remote cluster CheckAccessToRemoteCluster(cluster types.RemoteCluster) error diff --git a/lib/services/role.go b/lib/services/role.go index 2d32640f7f669..fabc5ccce36b4 100644 --- a/lib/services/role.go +++ b/lib/services/role.go @@ -2803,36 +2803,6 @@ func (set RoleSet) checkAccess(r AccessCheckable, traits wrappers.Traits, state r.GetKind(), additionalDeniedMessage) } -// CheckDeviceAccess verifies if the device state satisfies the device trust -// requirements of the user's RoleSet. -// -// Only device-related fields on AccessState are considered. -// EnableDeviceVerification is respected; if set to false, this check is a no-op -// and returns nil. -// -// This is used for early authorization checks where a full resource object -// is not yet available, but we need to verify the device before proceeding. -func (set RoleSet) CheckDeviceAccess(state AccessState) error { - if !state.EnableDeviceVerification { - return nil - } - for _, role := range set { - // Note: Unlike RoleSet.checkAccess, VerifyTrustedDeviceMode does not short-circuit - // if the device is already verified. It performs validation across the entire RoleSet. - if err := dtauthz.VerifyTrustedDeviceMode( - role.GetOptions().DeviceTrustMode, - dtauthz.VerifyTrustedDeviceModeParams{ - IsTrustedDevice: state.DeviceVerified, - IsBot: state.IsBot, - AllowEmptyMode: true, - }, - ); err != nil { - return trace.Wrap(err) - } - } - return nil -} - // checkRoleLabelsMatch checks if the [role] matches the labels of [resource] // for [condition]. // It considers both the role labels (_labels) and label expression diff --git a/lib/srv/app/connections_handler.go b/lib/srv/app/connections_handler.go index fd6a91ee239f2..2a72989826c0a 100644 --- a/lib/srv/app/connections_handler.go +++ b/lib/srv/app/connections_handler.go @@ -40,13 +40,14 @@ import ( "github.com/gravitational/teleport" apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/types" - "github.com/gravitational/teleport/api/types/events" + apievents "github.com/gravitational/teleport/api/types/events" apiutils "github.com/gravitational/teleport/api/utils" "github.com/gravitational/teleport/api/utils/clientutils" "github.com/gravitational/teleport/lib/auth/authclient" "github.com/gravitational/teleport/lib/authz" "github.com/gravitational/teleport/lib/cloud/awsconfig" "github.com/gravitational/teleport/lib/defaults" + "github.com/gravitational/teleport/lib/events" "github.com/gravitational/teleport/lib/httplib" "github.com/gravitational/teleport/lib/services" "github.com/gravitational/teleport/lib/srv" @@ -75,7 +76,7 @@ type ConnectionsHandlerConfig struct { DataDir string // Emitter is an event emitter. - Emitter events.Emitter + Emitter apievents.Emitter // Authorizer is used to authorize requests. Authorizer authz.Authorizer @@ -192,6 +193,11 @@ type ConnectionsHandler struct { // This will force the HTTP server to serve an error and close the connection. connAuth map[net.Conn]error + authFailuresMu sync.Mutex + // authFailures maps session IDs to the time of their most recent authentication failure. + // This is used for deduplicating redudant failed audit events + authFailures map[string]time.Time + awsHandler http.Handler azureHandler http.Handler gcpHandler http.Handler @@ -243,6 +249,7 @@ func NewConnectionsHandler(closeContext context.Context, cfg *ConnectionsHandler azureHandler: azureHandler, gcpHandler: gcpHandler, connAuth: make(map[net.Conn]error), + authFailures: make(map[string]time.Time), log: slog.With(teleport.ComponentKey, cfg.ServiceComponent), getAppByPublicAddress: func(ctx context.Context, s string) (types.Application, error) { return nil, trace.NotFound("no applications are being proxied") @@ -606,6 +613,71 @@ func (c *ConnectionsHandler) handleConnection(conn net.Conn) (func(), error) { ctx = authz.ContextWithClientSrcAddr(ctx, conn.RemoteAddr()) authCtx, _, err := c.authorizeContext(ctx) + identity := user.GetIdentity() + sessionID := identity.RouteToApp.SessionID + + uniqueEvent := true + if err != nil { + c.authFailuresMu.Lock() + lastSeen, exists := c.authFailures[sessionID] + + if exists && time.Since(lastSeen) < 2*time.Second { + uniqueEvent = false + } else { + c.authFailures[sessionID] = time.Now() + } + c.authFailuresMu.Unlock() + } + + // If there has not been a recent logged authentication failure for this session, + // emit an audit event for this connection attempt. + if uniqueEvent { + // Audit fields used for both success and failure + remoteAddr, addrErr := authz.ClientSrcAddrFromContext(ctx) + if addrErr != nil { + c.log.WarnContext(c.closeContext, "Failed to extract client source address from context", "error", err) + } + + sessionStartEvent := &apievents.AppSessionStart{ + Metadata: apievents.Metadata{ + Type: events.AppSessionStartEvent, + ClusterName: identity.RouteToApp.ClusterName, + }, + UserMetadata: identity.GetUserMetadata(), + ServerMetadata: apievents.ServerMetadata{ + ServerVersion: teleport.Version, + ServerID: c.cfg.HostID, + ServerNamespace: apidefaults.Namespace, + }, + ConnectionMetadata: apievents.ConnectionMetadata{ + RemoteAddr: remoteAddr.String(), + }, + AppMetadata: apievents.AppMetadata{ + AppURI: app.GetURI(), + AppPublicAddr: app.GetPublicAddr(), + AppName: app.GetName(), + AppTargetPort: uint32(identity.RouteToApp.TargetPort), + }, + SessionMetadata: apievents.SessionMetadata{ + SessionID: identity.RouteToApp.SessionID, + WithMFA: identity.MFAVerified, + }, + } + + if err != nil { + errMsg := "requires a trusted device or session MFA Required" + sessionStartEvent.Metadata.SetCode(events.AppSessionStartFailureCode) + sessionStartEvent.Error = err.Error() + sessionStartEvent.UserMessage = errMsg + } else { + sessionStartEvent.Metadata.SetCode(events.AppSessionStartCode) + } + + if err := c.cfg.Emitter.EmitAuditEvent(c.closeContext, sessionStartEvent); err != nil { + c.log.WarnContext(c.closeContext, "Failed to emit app session start event", "error", err) + } + } + // The behavior here is a little hard to track. To be clear here, if authorization fails // the following will occur: // 1. If the application is a TCP application, error out immediately as expected.