Skip to content

Commit

Permalink
Add mfa_weakest_device to UserStatusV2 (#46957) (#47093)
Browse files Browse the repository at this point in the history
This PR introduces the `mfa_weakest_device` value which is used to specify the weakest MFA device for the account.

When a user has no MFA device, it's set to `MFA_DEVICE_KIND_UNSET`.

When a user has at least one TOTP device, it's set to `MFA_DEVICE_KIND_TOTP`.

When a user ONLY has webauthn or U2F devices, it's set to `MFA_DEVICE_KIND_WEBAUTHN`.

This newly introduced field will be utilized by Access Graph to identify insecure patterns that could be potential phishing attack targets, particularly for users without MFA devices or those using TOTP devices.
  • Loading branch information
tigrato authored Oct 2, 2024
1 parent e8c252c commit f14e77b
Show file tree
Hide file tree
Showing 11 changed files with 1,969 additions and 1,594 deletions.
15 changes: 15 additions & 0 deletions api/proto/teleport/legacy/types/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -3398,6 +3398,9 @@ message UserStatusV2 {
// perform any password-related activity since then. See RFD 0159 for
// details. Do NOT use this value for authentication purposes!
PasswordState password_state = 1 [(gogoproto.jsontag) = "password_state,omitempty"];
// mfa_weakest_device reflects what the system knows about the user's weakest MFA device.
// Note that this is a "best effort" property, in that it can be UNSPECIFIED.
MFADeviceKind mfa_weakest_device = 2 [(gogoproto.jsontag) = "mfa_weakest_device,omitempty"];
}

// PasswordState indicates what is known about existence of user's password.
Expand All @@ -3410,6 +3413,18 @@ enum PasswordState {
PASSWORD_STATE_SET = 2;
}

// MFADeviceKind indicates what is known about existence of user's MFA device.
enum MFADeviceKind {
// Unable to tell whether the MFA device has been configured.
MFA_DEVICE_KIND_UNSPECIFIED = 0;
// MFA device is known to be not configured.
MFA_DEVICE_KIND_UNSET = 1;
// MFA device is known to be configured using TOTP as the weakest form of MFA.
MFA_DEVICE_KIND_TOTP = 2;
// MFA device is known to be configured using WebAuthn as the weakest form of MFA.
MFA_DEVICE_KIND_WEBAUTHN = 3;
}

// UserSpecV2 is a specification for V2 user
message UserSpecV2 {
// OIDCIdentities lists associated OpenID Connect identities
Expand Down
3,237 changes: 1,655 additions & 1,582 deletions api/types/types.pb.go

Large diffs are not rendered by default.

12 changes: 12 additions & 0 deletions api/types/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,10 @@ type User interface {
// who were created before this property was introduced and didn't perform any
// password-related activity since then. See RFD 0159 for details.
SetPasswordState(PasswordState)
// SetWeakestDevice sets the MFA state for the user.
SetWeakestDevice(MFADeviceKind)
// GetWeakestDevice gets the MFA state for the user.
GetWeakestDevice() MFADeviceKind
}

// NewUser creates new empty user
Expand Down Expand Up @@ -567,6 +571,14 @@ func (u *UserV2) SetPasswordState(state PasswordState) {
u.Status.PasswordState = state
}

func (u *UserV2) SetWeakestDevice(state MFADeviceKind) {
u.Status.MfaWeakestDevice = state
}

func (u *UserV2) GetWeakestDevice() MFADeviceKind {
return u.Status.MfaWeakestDevice
}

// IsEmpty returns true if there's no info about who created this user
func (c CreatedBy) IsEmpty() bool {
return c.User.Name == ""
Expand Down
56 changes: 51 additions & 5 deletions integrations/terraform/tfschema/types_terraform.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions lib/auth/machineid/machineidv1/machineidv1_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,7 @@ func TestCreateBot(t *testing.T) {
gotUser,
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
cmpopts.IgnoreFields(types.CreatedBy{}, "Time"),
cmpopts.IgnoreFields(types.UserStatusV2{}, "MfaWeakestDevice"),
),
)
}
Expand Down Expand Up @@ -876,6 +877,7 @@ func TestUpdateBot(t *testing.T) {
gotUser,
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
cmpopts.IgnoreFields(types.CreatedBy{}, "Time"),
cmpopts.IgnoreFields(types.UserStatusV2{}, "MfaWeakestDevice"),
),
)
}
Expand Down Expand Up @@ -1304,6 +1306,7 @@ func TestUpsertBot(t *testing.T) {
gotUser,
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
cmpopts.IgnoreFields(types.CreatedBy{}, "Time"),
cmpopts.IgnoreFields(types.UserStatusV2{}, "MfaWeakestDevice"),
),
)
}
Expand Down Expand Up @@ -1973,6 +1976,7 @@ func TestGetBotUsersLegacy(t *testing.T) {
res,
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
cmpopts.IgnoreFields(types.CreatedBy{}, "Time"),
cmpopts.IgnoreFields(types.UserStatusV2{}, "MfaWeakestDevice"),
cmpopts.SortSlices(func(a, b types.User) bool {
return a.GetName() < b.GetName()
}),
Expand Down
2 changes: 1 addition & 1 deletion lib/auth/tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1345,7 +1345,7 @@ func TestGetCurrentUser(t *testing.T) {
Spec: types.UserSpecV2{
Roles: []string{"user:user1"},
},
}, currentUser, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision")))
}, currentUser, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"), cmpopts.IgnoreFields(types.UserStatusV2{}, "MfaWeakestDevice")))
}

func TestGetCurrentUserRoles(t *testing.T) {
Expand Down
6 changes: 6 additions & 0 deletions lib/services/local/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,12 @@ func userFromUserItems(name string, items userItems) (*types.UserV2, error) {
return nil, trace.Wrap(err)
}
user.SetLocalAuth(auth)

if auth != nil {
// when reading with secrets, we can populate the data automatically.
user.SetWeakestDevice(getWeakestMFADeviceKind(auth.MFA))
}

return user, nil
}

Expand Down
1 change: 1 addition & 0 deletions lib/services/local/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ func newUserTestCase(t *testing.T, name string, roles []string, withSecrets bool
if withSecrets {
auth := localAuthSecretsTestCase(t)
user.SetLocalAuth(&auth)
user.SetWeakestDevice(types.MFADeviceKind_MFA_DEVICE_KIND_TOTP)
}
return &user
}
Expand Down
104 changes: 100 additions & 4 deletions lib/services/local/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,12 +327,16 @@ func (s *IdentityService) CreateUser(ctx context.Context, user types.User) (type
// technically possible to create a user along with a password using a direct
// RPC call or `tctl create`, so we need to support this case, too.
auth := user.GetLocalAuth()
if auth != nil && len(auth.PasswordHash) > 0 {
user.SetPasswordState(types.PasswordState_PASSWORD_STATE_SET)
if auth != nil {
if len(auth.PasswordHash) > 0 {
user.SetPasswordState(types.PasswordState_PASSWORD_STATE_SET)
}
} else {
user.SetPasswordState(types.PasswordState_PASSWORD_STATE_UNSET)
}

s.buildAndSetWeakestMFADeviceKind(ctx, user, auth)

value, err := services.MarshalUser(user.WithoutSecrets().(types.User))
if err != nil {
return nil, trace.Wrap(err)
Expand Down Expand Up @@ -371,6 +375,11 @@ func (s *IdentityService) LegacyUpdateUser(ctx context.Context, user types.User)
}

rev := user.GetRevision()

// if the user has no local auth, we need to check if the user
// was previously created and enrolled an MFA device.
s.buildAndSetWeakestMFADeviceKind(ctx, user, user.GetLocalAuth())

value, err := services.MarshalUser(user.WithoutSecrets().(types.User))
if err != nil {
return nil, trace.Wrap(err)
Expand Down Expand Up @@ -402,6 +411,8 @@ func (s *IdentityService) UpdateUser(ctx context.Context, user types.User) (type
return nil, trace.Wrap(err)
}

s.buildAndSetWeakestMFADeviceKind(ctx, user, user.GetLocalAuth())

rev := user.GetRevision()
value, err := services.MarshalUser(user.WithoutSecrets().(types.User))
if err != nil {
Expand Down Expand Up @@ -468,6 +479,9 @@ func (s *IdentityService) UpsertUser(ctx context.Context, user types.User) (type
if err := services.ValidateUser(user); err != nil {
return nil, trace.Wrap(err)
}

s.buildAndSetWeakestMFADeviceKind(ctx, user, user.GetLocalAuth())

rev := user.GetRevision()
value, err := services.MarshalUser(user.WithoutSecrets().(types.User))
if err != nil {
Expand Down Expand Up @@ -538,6 +552,8 @@ func (s *IdentityService) CompareAndSwapUser(ctx context.Context, new, existing
return trace.CompareFailed("user %v did not match expected existing value", new.GetName())
}

s.buildAndSetWeakestMFADeviceKind(ctx, newWithoutSecrets, new.GetLocalAuth())

if item.Value == nil {
v, err := services.MarshalUser(newWithoutSecrets)
if err != nil {
Expand Down Expand Up @@ -624,7 +640,7 @@ func (s *IdentityService) upsertLocalAuthSecrets(ctx context.Context, user strin
}
}
for _, d := range auth.MFA {
if err := s.UpsertMFADevice(ctx, user, d); err != nil {
if err := s.upsertMFADevice(ctx, user, d); err != nil {
return trace.Wrap(err)
}
}
Expand Down Expand Up @@ -1158,6 +1174,15 @@ func globalSessionDataKey(scope, id string) backend.Key {
}

func (s *IdentityService) UpsertMFADevice(ctx context.Context, user string, d *types.MFADevice) error {
if err := s.upsertMFADevice(ctx, user, d); err != nil {
return trace.Wrap(err)
}
if err := s.upsertUserStatusMFADevice(ctx, user); err != nil {
s.log.WithError(err).Warn("Unable to update user status after adding MFA device")
}
return nil
}
func (s *IdentityService) upsertMFADevice(ctx context.Context, user string, d *types.MFADevice) error {
if user == "" {
return trace.BadParameter("missing parameter user")
}
Expand Down Expand Up @@ -1213,6 +1238,71 @@ func (s *IdentityService) UpsertMFADevice(ctx context.Context, user string, d *t
return nil
}

// upsertUserStatusMFADevice updates the user's MFA state based on the devices
// they have.
// It's called after adding or removing an MFA device and ensures the user's
// MFA state is up-to-date and reflects the weakest MFA device they have.
func (s *IdentityService) upsertUserStatusMFADevice(ctx context.Context, user string) error {
devs, err := s.GetMFADevices(ctx, user, false)
if err != nil {
return trace.Wrap(err)
}
mfaState := getWeakestMFADeviceKind(devs)

_, err = s.UpdateAndSwapUser(
ctx,
user,
false, /*withSecrets*/
func(u types.User) (bool, error) {
u.SetWeakestDevice(mfaState)
return true, nil
})

return trace.Wrap(err)
}

// buildAndSetWeakestMFADeviceKind builds the MFA state for a user and sets it on the user if successful.
func (s *IdentityService) buildAndSetWeakestMFADeviceKind(ctx context.Context, user types.User, localAuthSecrets *types.LocalAuthSecrets) {
// upsertingMFA is the list of MFA devices that are being upserted when updating the user.
var upsertingMFA []*types.MFADevice
if localAuthSecrets != nil {
upsertingMFA = localAuthSecrets.MFA
}
state, err := s.buildWeakestMFADeviceKind(ctx, user.GetName(), upsertingMFA...)
if err != nil {
s.log.WithError(err).Warn("Failed to determine weakest mfa device kind for user")
return
}
user.SetWeakestDevice(state)
}

func (s *IdentityService) buildWeakestMFADeviceKind(ctx context.Context, user string, upsertingMFA ...*types.MFADevice) (types.MFADeviceKind, error) {
devs, err := s.GetMFADevices(ctx, user, false)
if err != nil {
return types.MFADeviceKind_MFA_DEVICE_KIND_UNSET, trace.Wrap(err)
}
return getWeakestMFADeviceKind(append(devs, upsertingMFA...)), nil
}

// getWeakestMFADeviceKind returns the weakest MFA state based on the devices the user
// has.
// When a user has no MFA device, it's set to `MFADeviceKind_MFA_DEVICE_KIND_UNSET`.
// When a user has at least one TOTP device, it's set to `MFADeviceKind_MFA_DEVICE_KIND_TOTP`.
// When a user ONLY has webauthn devices, it's set to `MFADeviceKind_MFA_DEVICE_KIND_WEBAUTHN`.
func getWeakestMFADeviceKind(devs []*types.MFADevice) types.MFADeviceKind {
mfaState := types.MFADeviceKind_MFA_DEVICE_KIND_UNSET
for _, d := range devs {
if (d.GetWebauthn() != nil || d.GetU2F() != nil) && mfaState == types.MFADeviceKind_MFA_DEVICE_KIND_UNSET {
mfaState = types.MFADeviceKind_MFA_DEVICE_KIND_WEBAUTHN
}
if d.GetTotp() != nil {
mfaState = types.MFADeviceKind_MFA_DEVICE_KIND_TOTP
break
}
}
return mfaState
}

func getCredentialID(d *types.MFADevice) ([]byte, bool) {
switch d := d.Device.(type) {
case *types.MFADevice_U2F:
Expand All @@ -1232,7 +1322,13 @@ func (s *IdentityService) DeleteMFADevice(ctx context.Context, user, id string)
}

err := s.Delete(ctx, backend.NewKey(webPrefix, usersPrefix, user, mfaDevicePrefix, id))
return trace.Wrap(err)
if err != nil {
return trace.Wrap(err)
}
if err := s.upsertUserStatusMFADevice(ctx, user); err != nil {
s.log.WithError(err).Warn("Unable to update user status after deleting MFA device")
}
return nil
}

func (s *IdentityService) GetMFADevices(ctx context.Context, user string, withSecrets bool) ([]*types.MFADevice, error) {
Expand Down
Loading

0 comments on commit f14e77b

Please sign in to comment.