diff --git a/integration/appaccess/appaccess_test.go b/integration/appaccess/appaccess_test.go index 3e996b0f37787..f6d9124c90812 100644 --- a/integration/appaccess/appaccess_test.go +++ b/integration/appaccess/appaccess_test.go @@ -588,6 +588,7 @@ func testAuditEvents(p *Pack, t *testing.T) { AppPublicAddr: p.rootAppPublicAddr, AppName: p.rootAppName, }, + PublicAddr: p.rootAppPublicAddr, } return len(cmp.Diff( expectedEvent, diff --git a/lib/auth/sessions.go b/lib/auth/sessions.go index 0320300acc42e..c6ef72374f7a2 100644 --- a/lib/auth/sessions.go +++ b/lib/auth/sessions.go @@ -427,6 +427,8 @@ type NewAppSessionRequest struct { AppURI string // AppTargetPort signifies that the session is made to a specific port of a multi-port TCP app. AppTargetPort int + // Identity is the identity of the user. + Identity tlsca.Identity // ClientAddr is a client (user's) address. ClientAddr string @@ -496,8 +498,6 @@ func (a *Server) CreateAppSession(ctx context.Context, req *proto.CreateAppSessi MFAVerified: verifiedMFADeviceID, AppName: req.AppName, AppURI: req.URI, - BotName: identity.BotName, - BotInstanceID: identity.BotInstanceID, DeviceExtensions: DeviceExtensions(identity.DeviceExtensions), }) if err != nil { @@ -543,69 +543,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, - 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) - } - // Create certificate for this session. priv, err := cryptosuites.GenerateKey(ctx, cryptosuites.GetCurrentSuiteFromAuthPreference(a), cryptosuites.UserTLS) if err != nil { @@ -704,16 +641,36 @@ func (a *Server) CreateAppSessionFromReq(ctx context.Context, req NewAppSessionR 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) + err = a.emitter.EmitAuditEvent(a.closeCtx, &apievents.AppSessionStart{ + Metadata: apievents.Metadata{ + Type: events.AppSessionStartEvent, + Code: events.AppSessionStartCode, + ClusterName: req.ClusterName, + }, + ServerMetadata: apievents.ServerMetadata{ + ServerVersion: teleport.Version, + ServerID: a.ServerID, + ServerNamespace: apidefaults.Namespace, + }, + SessionMetadata: apievents.SessionMetadata{ + SessionID: session.GetName(), + WithMFA: req.MFAVerified, + PrivateKeyPolicy: string(req.Identity.PrivateKeyPolicy), + }, + UserMetadata: userMetadata, + ConnectionMetadata: apievents.ConnectionMetadata{ + RemoteAddr: req.ClientAddr, + }, + PublicAddr: req.PublicAddr, + AppMetadata: apievents.AppMetadata{ + AppURI: req.AppURI, + AppPublicAddr: req.PublicAddr, + AppName: req.AppName, + AppTargetPort: uint32(req.AppTargetPort), + }, + }) + if err != nil { + log.WithError(err).Warn("Failed to emit app session start event") } return session, nil diff --git a/lib/auth/sessions_test.go b/lib/auth/sessions_test.go index 1ae94a321967e..9d77d4d1fcaec 100644 --- a/lib/auth/sessions_test.go +++ b/lib/auth/sessions_test.go @@ -22,23 +22,13 @@ import ( "time" "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" - "github.com/google/uuid" - "github.com/gravitational/trace" - "github.com/jonboulle/clockwork" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/gravitational/teleport/api/client/proto" - "github.com/gravitational/teleport/api/constants" 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/lib/auth" "github.com/gravitational/teleport/lib/auth/authtest" - "github.com/gravitational/teleport/lib/events" - "github.com/gravitational/teleport/lib/services" - "github.com/gravitational/teleport/lib/tlsca" ) func TestServer_CreateWebSessionFromReq_deviceWebToken(t *testing.T) { @@ -95,218 +85,3 @@ func TestServer_CreateWebSessionFromReq_deviceWebToken(t *testing.T) { } }) } - -func TestCreateAppSession_DeviceTrust(t *testing.T) { - t.Parallel() - - fakeClock := clockwork.NewFakeClock() - - testAuthServer, err := authtest.NewAuthServer(authtest.AuthServerConfig{ - Clock: fakeClock, - Dir: t.TempDir(), - }) - require.NoError(t, err) - t.Cleanup(func() { testAuthServer.Close() }) - - authServer := testAuthServer.AuthServer - - tests := []struct { - name string - trustMode string - botName string - deviceExtensions auth.DeviceExtensions - wantErr string - }{ - { - name: "Access Denied - Trusted Device Required but Missing", - trustMode: constants.DeviceTrustModeRequired, - deviceExtensions: auth.DeviceExtensions{}, - wantErr: "requires a trusted device", - }, - { - name: "Success - Trust Optional and Device Missing", - trustMode: constants.DeviceTrustModeOptional, - deviceExtensions: auth.DeviceExtensions{}, - }, - { - name: "Success - Trust Mode Not Set", - trustMode: "", - deviceExtensions: auth.DeviceExtensions{}, - }, - { - name: "Success - Trusted Device Required and Provided", - trustMode: constants.DeviceTrustModeRequired, - deviceExtensions: auth.DeviceExtensions{ - DeviceID: "macbook-id-123", - AssetTag: "asset-tag-123", - CredentialID: "cred-id-123", - }, - }, - { - name: "Success - Bot Access with RequiredForHumans and Missing Device", - trustMode: constants.DeviceTrustModeRequiredForHumans, - botName: "example-bot", - deviceExtensions: auth.DeviceExtensions{}, - }, - { - name: "Access Denied - Bot Access with Device Trust Required", - trustMode: constants.DeviceTrustModeRequired, - botName: "example-bot", - deviceExtensions: auth.DeviceExtensions{}, - wantErr: "requires a trusted device", - }, - { - name: "Access Denied - Human Access with RequiredForHumans but Missing Device", - trustMode: constants.DeviceTrustModeRequiredForHumans, - botName: "", - deviceExtensions: auth.DeviceExtensions{}, - wantErr: "requires a trusted device", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - ctx := t.Context() - suffix := uuid.NewString() - username := "user-" + suffix - roleName := "role-" + suffix - publicAddr := "www-" + suffix + ".example.com" - - role, err := types.NewRole(roleName, types.RoleSpecV6{ - Options: types.RoleOptions{ - DeviceTrustMode: tt.trustMode, - }, - Allow: types.RoleConditions{ - AppLabels: types.Labels{"*": []string{"*"}}, - }, - }) - require.NoError(t, err) - _, err = authServer.CreateRole(ctx, role) - require.NoError(t, err) - - traits := map[string][]string{ - "groups": {"admins", "devs"}, - "email": {"alice@example.com"}, - } - user, err := types.NewUser(username) - require.NoError(t, err) - user.SetTraits(traits) - user.AddRole(roleName) - _, err = authServer.CreateUser(ctx, user) - require.NoError(t, err) - - identity := tlsca.Identity{ - Username: username, - BotName: tt.botName, - DeviceExtensions: tlsca.DeviceExtensions(tt.deviceExtensions), - } - - cName, err := authServer.GetClusterName() - require.NoError(t, err) - clusterName := cName.GetClusterName() - - checker, err := services.NewAccessChecker(&services.AccessInfo{ - Username: username, - Roles: user.GetRoles(), - }, clusterName, authServer) - require.NoError(t, err) - - req := &proto.CreateAppSessionRequest{ - Username: username, - AppName: "example-app", - URI: "http://example.com", - ClusterName: clusterName, - PublicAddr: publicAddr, - } - - startTime := fakeClock.Now() - _, err = authServer.CreateAppSession(ctx, req, identity, checker) - endTime := fakeClock.Now() - - if tt.wantErr != "" { - require.ErrorContains(t, err, tt.wantErr) - assert.True(t, trace.IsAccessDenied(err), "Expected AccessDenied, got %v", err) - } else { - require.NoError(t, err) - } - - logEntries, _, err := testAuthServer.AuditLog.SearchEvents(ctx, events.SearchEventsRequest{ - From: startTime.Add(-time.Second), - To: endTime.Add(time.Second), - Limit: 100, // arbitrary, enough to allow for events from concurrent tests - }) - require.NoError(t, err) - - expectedKind := apievents.UserKind_USER_KIND_HUMAN - if tt.botName != "" { - expectedKind = apievents.UserKind_USER_KIND_BOT - } - - expectedEvent := &apievents.AppSessionStart{ - Metadata: apievents.Metadata{ - Type: events.AppSessionStartEvent, - ClusterName: clusterName, - }, - UserMetadata: apievents.UserMetadata{ - User: username, - BotName: tt.botName, - UserKind: expectedKind, - }, - AppMetadata: apievents.AppMetadata{ - AppName: "example-app", - AppURI: "http://example.com", - AppPublicAddr: publicAddr, - }, - } - - if tt.wantErr == "" { - expectedEvent.Metadata.Code = events.AppSessionStartCode - expectedEvent.SessionMetadata.PrivateKeyPolicy = "none" - } else { - expectedEvent.Metadata.Code = events.AppSessionStartFailureCode - expectedEvent.UserMessage = "requires a trusted device" - expectedEvent.Error = "access to resource requires a trusted device" - } - - if tt.deviceExtensions.DeviceID != "" { - expectedEvent.UserMetadata.TrustedDevice = &apievents.DeviceMetadata{ - DeviceId: tt.deviceExtensions.DeviceID, - AssetTag: tt.deviceExtensions.AssetTag, - CredentialId: tt.deviceExtensions.CredentialID, - } - } - - var eventFound bool - for _, event := range logEntries { - appStart, ok := event.(*apievents.AppSessionStart) - if !ok || appStart.UserMetadata.User != username { - continue - } - - eventFound = true - - diff := cmp.Diff(expectedEvent, appStart, - cmpopts.IgnoreUnexported( - apievents.AppSessionStart{}, - apievents.Metadata{}, - apievents.UserMetadata{}, - apievents.AppMetadata{}, - apievents.SessionMetadata{}, - apievents.ConnectionMetadata{}, - apievents.ServerMetadata{}, - apievents.DeviceMetadata{}, - ), - cmpopts.IgnoreFields(apievents.Metadata{}, "ID", "Time", "Index"), - cmpopts.IgnoreFields(apievents.ServerMetadata{}, "ServerID", "ServerVersion", "ServerNamespace"), - cmpopts.IgnoreFields(apievents.ConnectionMetadata{}, "RemoteAddr", "Protocol"), - cmpopts.IgnoreFields(apievents.SessionMetadata{}, "SessionID"), - cmpopts.IgnoreFields(apievents.AppSessionStart{}, "PublicAddr"), // deprecated field - ) - - require.Empty(t, diff, "Audit event mismatch for case: %s\n%s", tt.name, diff) - } - require.True(t, eventFound, "Expected AppSessionStart event was not found in audit log") - }) - } -} diff --git a/lib/services/access_checker.go b/lib/services/access_checker.go index 0cdaf4cdb021a..269a0af90989a 100644 --- a/lib/services/access_checker.go +++ b/lib/services/access_checker.go @@ -59,10 +59,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 92f802bc47c7d..1cc7a6a0ba746 100644 --- a/lib/services/role.go +++ b/lib/services/role.go @@ -2736,36 +2736,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