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

Increase Password Hash Iterations #23915

Closed
wezell opened this issue Jan 26, 2023 · 4 comments · Fixed by #24035
Closed

Increase Password Hash Iterations #23915

wezell opened this issue Jan 26, 2023 · 4 comments · Fixed by #24035

Comments

@wezell
Copy link
Contributor

wezell commented Jan 26, 2023

Problem Statement

Passwords stored in dotCMS are hashed multiple times using an industry recognized algo PBKDF2-HMAC-SHA256. When we implemented password hashing, 20,000 iterations were enough to slow down a brute force cracking attack. With todays modern processors and GPUs, OWasp recommends at least 600,000 iterations of hashing to protect passwords from modern hardware. See this page for more information:

https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#pbkdf2

Luckily, dotCMS's password impl allows us to up the number of hashings without much drama, and it takes effect the next time a user logs in - dotCMS will automatically re-hash the users password with the correct number of iterations. Note that it is impossible to go back and re-hash user's passwords without the user's interaction - as we need the password in the clear to be able to re-hash it.

Steps to Reproduce

Login

Acceptance Criteria

  1. Set up a new starter.
  2. Take a look at the user's password in the database and see that it is not hashed.
    select password_ from user_ where userid='dotcms.org.1';
  3. Login as that user.
  4. Take a look at the user's password in the database. You will see that it has been updated and is now hashed.
  5. You should also see a log message like :
12:32:08.187  INFO  util.SecurityLogger - class com.dotmarketing.cms.login.factories.LoginFactory : User (dotcms.org.1) password was re-hashed 600000 times -- ip:127.0.0.1,user:null

dotCMS Version

4.x-23.01

Proposed Objective

Security & Privacy

Proposed Priority

Priority 2 - Important

External Links... Slack Conversations, Support Tickets, Figma Designs, etc.

No response

Assumptions & Initiation Needs

No response

Sub-Tasks & Estimates

No response

@mbiuki
Copy link
Contributor

mbiuki commented Jan 30, 2023

Please have me review it before this code is merged. Must ensure the enumeration issue is also fixed, thanks.

jcastro-dotcms added a commit that referenced this issue Feb 3, 2023
* Increasing password hash iterations to 600,000 minimum. Updating the INFO message in the log to reflect the number of iterations.
* Fixing the enumeration issue, as requested by Mehdi.
@jcastro-dotcms jcastro-dotcms linked a pull request Feb 3, 2023 that will close this issue
@erickgonzalez erickgonzalez added the OKR : Customer Support Owned by Scott label Feb 6, 2023
fmontes pushed a commit that referenced this issue Feb 10, 2023
* #23915 : Increase Password Hash Iterations

* Increasing password hash iterations to 600,000 minimum. Updating the INFO message in the log to reflect the number of iterations.
* Fixing the enumeration issue, as requested by Mehdi.

* #23915 : Implementing SonarQube feedback.

* #23915 : Implementing more SonarQube feedback.
@fmontes fmontes reopened this Feb 10, 2023
@fmontes fmontes closed this as completed Feb 10, 2023
@fmontes fmontes reopened this Feb 28, 2023
@jcastro-dotcms
Copy link
Contributor

NOTE TO QA

Here are the steps to verify the code fix:

  1. Start up dotCMS 23.01, and log in.
  2. Shut it down, and change the image to the latest 23.03 in DockerHub.
  3. Start it up and log in again. Right after that, you must see the following entry in the dotcms.log file:
User (dotcms.org.1) password was re-hashed 600000 times

@fmontes
Copy link
Member

fmontes commented Feb 28, 2023

image

@bryanboza
Copy link
Member

Fixed, tested on release-23.03 // Docker // FF

@erickgonzalez erickgonzalez added LTS : Next Ticket that will be added to LTS Next LTS Release and removed LTS: Discuss To be discussed between Support & R&D LTS : Next Ticket that will be added to LTS labels Apr 6, 2023
erickgonzalez added a commit that referenced this issue Apr 6, 2023
@erickgonzalez erickgonzalez added the Release : 23.01.2 Included in LTS patch release 23.01.2 label Apr 11, 2023
erickgonzalez added a commit that referenced this issue May 4, 2023
@AndreyDotcms AndreyDotcms added the Release : 22.03.6 Included in LTS patch release 22.03.6 label May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

8 participants