Skip to content

Always verify the old password when changing it#38203

Merged
bl-nero merged 2 commits intomasterfrom
bl-nero/fix-change-password
Feb 15, 2024
Merged

Always verify the old password when changing it#38203
bl-nero merged 2 commits intomasterfrom
bl-nero/fix-change-password

Conversation

@bl-nero
Copy link
Copy Markdown
Contributor

@bl-nero bl-nero commented Feb 14, 2024

Fixes https://github.com/gravitational/teleport-private/issues/1369

Approved for fixing directly on OSS by @jentfoo.

Note that the logic here will be further amended by the upcoming implementation of RFD 0159; this change is a trimmed down subset of it that will need to be privately backported to all of the supported branches.

Tested locally; the following cases were covered:

Changing password with MFA, through the exploit mentioned in the attached issue.
Changing password without MFA (success, wrong password).
Changing password with an authenticator app (success, wrong old password, wrong app token).
Changing password with an MFA device (success, wrong password).
Changing password with an passwordless device (success, wrong password).
The behavior was verified both by looking at success/error messages, as well as an attempt to sign in using the new password.

Changelog: Fixed an issue where it was possible to skip providing old password when setting a new one.

@bl-nero bl-nero requested a review from jentfoo February 14, 2024 12:18
@github-actions github-actions Bot requested review from greedy52 and kimlisa February 14, 2024 12:18
Comment thread lib/auth/methods.go
@greedy52 greedy52 requested a review from codingllama February 14, 2024 13:47
Comment thread lib/auth/password_test.go Outdated
Copy link
Copy Markdown
Contributor

@codingllama codingllama left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix!

Comment thread lib/auth/password_test.go Outdated
Comment thread lib/auth/password_test.go Outdated
Comment thread lib/auth/password_test.go Outdated
Comment thread lib/auth/password_test.go Outdated
@codingllama
Copy link
Copy Markdown
Contributor

Added backport tags to active versions.

@bl-nero bl-nero enabled auto-merge February 15, 2024 11:12
@bl-nero bl-nero added this pull request to the merge queue Feb 15, 2024
Merged via the queue into master with commit 8d3e48a Feb 15, 2024
@bl-nero bl-nero deleted the bl-nero/fix-change-password branch February 15, 2024 13:43
@public-teleport-github-review-bot
Copy link
Copy Markdown

@bl-nero See the table below for backport results.

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

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants