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

Add mfa_weakest_device to UserStatusV2 #46957

Merged
merged 1 commit into from
Oct 2, 2024
Merged

Conversation

tigrato
Copy link
Contributor

@tigrato tigrato commented Sep 30, 2024

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.

@tigrato tigrato force-pushed the tigrato/add-status-mfa-device branch from 33d97a0 to c506141 Compare September 30, 2024 13:18
@tigrato tigrato force-pushed the tigrato/add-status-mfa-device branch from 4dcc8dd to e5642a4 Compare October 2, 2024 09:54
@tigrato tigrato added backport/branch/v14 no-changelog Indicates that a PR does not require a changelog entry backport/branch/v15 backport/branch/v16 labels Oct 2, 2024
@tigrato tigrato marked this pull request as ready for review October 2, 2024 09:55
@github-actions github-actions bot added size/md tctl tctl - Teleport admin tool labels Oct 2, 2024
@tigrato tigrato force-pushed the tigrato/add-status-mfa-device branch from 10b39bb to 15290fe Compare October 2, 2024 10:17
Copy link

github-actions bot commented Oct 2, 2024

🤖 Vercel preview here: https://docs-adv6j0ldi-goteleport.vercel.app/docs/ver/preview

@gravitational gravitational deleted a comment from github-actions bot Oct 2, 2024
Copy link

github-actions bot commented Oct 2, 2024

🤖 Vercel preview here: https://docs-i8l5ofdmr-goteleport.vercel.app/docs/ver/preview

@tigrato tigrato force-pushed the tigrato/add-status-mfa-device branch from 15290fe to a523f08 Compare October 2, 2024 10:36
Copy link

github-actions bot commented Oct 2, 2024

🤖 Vercel preview here: https://docs-gmg46yr41-goteleport.vercel.app/docs/ver/preview

api/proto/teleport/legacy/types/types.proto Outdated Show resolved Hide resolved
lib/services/local/users.go Outdated Show resolved Hide resolved
lib/services/local/users.go Outdated Show resolved Hide resolved
lib/services/local/users.go Outdated Show resolved Hide resolved
lib/services/local/users.go Outdated Show resolved Hide resolved
lib/services/local/users.go Outdated Show resolved Hide resolved
lib/services/local/users.go Outdated Show resolved Hide resolved
api/proto/teleport/legacy/types/types.proto Outdated Show resolved Hide resolved
api/proto/teleport/legacy/types/types.proto Outdated Show resolved Hide resolved
api/proto/teleport/legacy/types/types.proto Show resolved Hide resolved
@@ -352,6 +352,12 @@ func userFromUserItems(name string, items userItems) (*types.UserV2, error) {
return nil, trace.Wrap(err)
}
user.SetLocalAuth(auth)

if auth != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: I would split the new definitions (and more mundane code changes) and new code additions into separate PRs. That would make 2 smaller, more focused PRs, which should make for easier reviews. (It would hopefully touch less files at once too.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(No need to split it now, but consider that for future PRs.)

lib/services/local/resource_test.go Outdated Show resolved Hide resolved
lib/services/local/users.go Show resolved Hide resolved
lib/services/local/users.go Outdated Show resolved Hide resolved
// MFA device is known to be configured using TOTP as the weakest form of MFA.
MFA_STATE_TOTP = 2;
// MFA device is known to be configured using U2F as the weakest form of MFA.
MFA_STATE_U2F = 3;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to distinguish between U2F and WEBAUTHN? Why not call both WEBAUTHN?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to distinguish when a user has a webauthn only configured vs when he has a u2f given that U2F can't be used to passwordless login to teleport.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WEBAUTHN doesn't necessarily mean passkey either, so I think the distinction is moot. All we would detect here are users with relatively old devices / clusters. I would still combine U2F and WEBAUTHN and only re-introduce U2F if we really cared about it specifically.

Copy link

github-actions bot commented Oct 2, 2024

🤖 Vercel preview here: https://docs-9o2pskc35-goteleport.vercel.app/docs/ver/preview

Copy link

github-actions bot commented Oct 2, 2024

🤖 Vercel preview here: https://docs-2ih7nl0rc-goteleport.vercel.app/docs/ver/preview

lib/services/local/resource_test.go Outdated Show resolved Hide resolved
lib/services/local/users_test.go Show resolved Hide resolved
lib/services/local/users_test.go Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Oct 2, 2024

🤖 Vercel preview here: https://docs-mqrqac8bm-goteleport.vercel.app/docs/ver/preview

Copy link
Contributor

@rosstimothy rosstimothy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to update the title/description and commit message to replace mfa_device_state with mfa_weakest_device

lib/services/local/users.go Outdated Show resolved Hide resolved
api/proto/teleport/legacy/types/types.proto Outdated Show resolved Hide resolved
@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from eriktate October 2, 2024 15:20
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.
@tigrato tigrato changed the title Add mfa_device_state to UserStatusV2 Add mfa_weakest_device to UserStatusV2 Oct 2, 2024
@tigrato tigrato force-pushed the tigrato/add-status-mfa-device branch from cd00d9b to 8fd80cd Compare October 2, 2024 15:29
@tigrato tigrato enabled auto-merge October 2, 2024 15:29
Copy link

github-actions bot commented Oct 2, 2024

🤖 Vercel preview here: https://docs-4alswr5g8-goteleport.vercel.app/docs/ver/preview

@tigrato tigrato added this pull request to the merge queue Oct 2, 2024
Merged via the queue into master with commit 65c77a2 Oct 2, 2024
43 checks passed
@tigrato tigrato deleted the tigrato/add-status-mfa-device branch October 2, 2024 16:03
@public-teleport-github-review-bot

@tigrato See the table below for backport results.

Branch Result
branch/v14 Failed
branch/v15 Failed
branch/v16 Failed

tigrato added a commit that referenced this pull request Oct 2, 2024
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.
tigrato added a commit that referenced this pull request Oct 2, 2024
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.
tigrato added a commit that referenced this pull request Oct 2, 2024
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.
github-merge-queue bot pushed a commit that referenced this pull request Oct 2, 2024
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.
github-merge-queue bot pushed a commit that referenced this pull request Oct 2, 2024
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/branch/v14 backport/branch/v15 backport/branch/v16 no-changelog Indicates that a PR does not require a changelog entry size/md tctl tctl - Teleport admin tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants