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.

Sign up for GitHub

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

Jump to bottom

fix: Forbid SSO users from logging in using passwordless #41062

Merged
merged 4 commits into from
Apr 30, 2024
Merged

fix: Forbid SSO users from logging in using passwordless #41062

merged 4 commits into from
Apr 30, 2024

Conversation

codingllama
Copy link
Contributor

It is possible for an SSO user to register a passkey and skip SSO authentication by doing a local/passwordless login. This PR fixes that by checking the user "type" on passwordless logins.

Note that, for passwordless, the user is only confirmed after the challenge is solved, so the error has to happen in the "finish" step of the ceremony. Similarly the Web UI can't do much to identify the user or forbid passwordless before that.

Changelog: Fix user SSO bypass by performing a local passwordless login

@github-actions github-actions bot added size/md tctl tctl - Teleport admin tool labels Apr 30, 2024
@codingllama
Copy link
Contributor Author

I've snuck in some mock authenticator improvements as 4841324 and 389c6c7. The PR is relatively small and I'm backporting this all the way, so I didn't see the need to split them. Let me know if you disagree, of course.

@codingllama
Copy link
Contributor Author

FYI @espadolini @rosstimothy @webvictim @zmb3

codingllama
codingllama commented Apr 30, 2024
View reviewed changes
lib/auth/helpers_mfa.go
d.Key.AllowResidentKey = true
d.Key.SetUV = true
d.Key.SetPasswordless()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This here was missing a flag, plus at some point I added SetPasswordless() but probably missed this line.

@codingllama
Copy link
Contributor Author

image

zmb3
zmb3 approved these changes Apr 30, 2024
View reviewed changes
lib/auth/auth_login_test.go Outdated Show resolved Hide resolved
@codingllama
Copy link
Contributor Author

Thanks for the quick reviews! I'm trying to land a few #41023 backports before I merge this one, to save myself some trouble. I should be able to proceed here soon.

@codingllama codingllama added this pull request to the merge queue Apr 30, 2024
Merged via the queue into master with commit 72780dc Apr 30, 2024
41 checks passed
@codingllama codingllama deleted the codingllama/passwordless-sso branch April 30, 2024 19:24
@public-teleport-github-review-bot
Copy link

@codingllama See the table below for backport results.

Branch Result
branch/v13 Failed
branch/v14 Failed
branch/v15 Create PR

@codingllama codingllama mentioned this pull request Apr 30, 2024
Merged
codingllama added a commit that referenced this pull request Apr 30, 2024
@codingllama
fix: Forbid SSO users from logging in using passwordless (#41062)
f06c308 
* Save the WebAuthn UserHandle in the mock Key

* Simplify passwordless tests

* fix: Forbid SSO users from logging in using passwordless

* Rename tests to TestPasswordlessProhibitedForSSO
@codingllama codingllama mentioned this pull request Apr 30, 2024
Merged
codingllama added a commit that referenced this pull request Apr 30, 2024
@codingllama
fix: Forbid SSO users from logging in using passwordless (#41062)
644e926 
* Save the WebAuthn UserHandle in the mock Key

* Simplify passwordless tests

* fix: Forbid SSO users from logging in using passwordless

* Rename tests to TestPasswordlessProhibitedForSSO
@codingllama codingllama mentioned this pull request Apr 30, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Apr 30, 2024
@codingllama
fix: Forbid SSO users from logging in using passwordless (#41062) (#4…
e4e5f46 
…1071)

* Save the WebAuthn UserHandle in the mock Key

* Simplify passwordless tests

* fix: Forbid SSO users from logging in using passwordless

* Rename tests to TestPasswordlessProhibitedForSSO
github-merge-queue bot pushed a commit that referenced this pull request Apr 30, 2024
@codingllama
fix: Forbid SSO users from logging in using passwordless (#41062) (#4…
13d0980 
…1072)

* Save the WebAuthn UserHandle in the mock Key

* Simplify passwordless tests

* fix: Forbid SSO users from logging in using passwordless

* Rename tests to TestPasswordlessProhibitedForSSO
@gregnetau
Copy link

How is an SSO user supposed to auth in tsh with MFA with these change now? This has broken our authentication/DB access in production.

@codingllama
Copy link
Contributor Author

Hey @gregnetau, this change targets only passwordless logins, other kinds of MFA are unaffected. This includes MFA checks with a resident credential.

If you are having problems, please raise an issue or a support ticket with details of your problem and repro steps and we'll take a look at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rosstimothy rosstimothy rosstimothy approved these changes

@zmb3 zmb3 zmb3 approved these changes

Assignees
No one assigned
Labels
backport/branch/v13 backport/branch/v14 backport/branch/v15 size/md tctl tctl - Teleport admin tool
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@codingllama @gregnetau @zmb3 @rosstimothy