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

Create rule S5344: Passwords should not be stored in plain-text or with a fast hashing algorithm (Part 2) #9287

Merged
merged 3 commits into from
May 22, 2024

Conversation

gregory-paidis-sonarsource
Copy link
Contributor

Part of #8998

Whatever is checked, is implemented in this PR:

  • PasswordHasherOptions.IterationCount < 100K (Core)
  • PasswordHasherOptions.CompatibilityMode == IdentityV2 (Core)
  • KeyDerivation.Pbkdf2.iterationCount < 100K (Core)
  • Rfc2898DeriveBytes.Pbkdf2.iterations < 100K (Core)
  • PasswordHasher instantiated (FRM)
  • Rfc2898DeriveBytes.iterations < 100K (cross-platform)
  • Rfc2898DeriveBytes.hashAlgorithm does not exist (cross-platform)
  • (OpenBsdCrypt/BCrypt).Generate.cost < 12 (BouncyCastle)
  • PbeParametersGenerator.Init.iterationCount < 100K (BouncyCastle)
  • SCrypt.Generate N < 2^12, r < 8, or dklen < 32 (BouncyCastle)

@github-actions github-actions bot moved this from In progress to Review in progress in Best Kanban May 16, 2024
@gregory-paidis-sonarsource gregory-paidis-sonarsource changed the title Create rule S5344: Part 2 Create rule S5344: Passwords should not be stored in plain-text or with a fast hashing algorithm (Part 2) May 16, 2024
@gregory-paidis-sonarsource
Copy link
Contributor Author

@costin-zaharia-sonarsource all of the code smells are magic numbers.
I think we should mark them as accepted because:

  • They are explained by the context, they are either argument positions or thresholds
  • Magic is cool

Let me know if you agree and I will take care of them.

@gregory-paidis-sonarsource gregory-paidis-sonarsource force-pushed the greg/implement-S5344 branch 2 times, most recently from 4e8d69a to ccd5c9a Compare May 16, 2024 11:51
Base automatically changed from greg/implement-S5344 to master May 16, 2024 15:47
@gregory-paidis-sonarsource
Copy link
Contributor Author

@costin-zaharia-sonarsource Second commit is review from previous PR.

@gregory-paidis-sonarsource gregory-paidis-sonarsource marked this pull request as ready for review May 17, 2024 08:55
@gregory-paidis-sonarsource gregory-paidis-sonarsource added the Sprint: MMF-3716 crypto rules https://sonarsource.atlassian.net/browse/MMF-3716 label May 17, 2024
Comment on lines +108 to +109
x.IterationCount = 1; // Noncompliant {{Use at least 100,000 iterations here.}}
// ^^^^^^^^^^^^^^^^

Choose a reason for hiding this comment

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

I think a compliant example is missing with setting the IterationCount property to a safe value.

Copy link
Member

Choose a reason for hiding this comment

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

LGTM! 🎉

Please see that there a few minor comments to handle before merging.

@github-actions github-actions bot moved this from Review in progress to Review approved in Best Kanban May 21, 2024
Copy link

sonarcloud bot commented May 22, 2024

Quality Gate Passed Quality Gate passed for 'Sonar .NET Java Plugin'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link

sonarcloud bot commented May 22, 2024

@gregory-paidis-sonarsource gregory-paidis-sonarsource merged commit ac0b6dd into master May 22, 2024
26 checks passed
Best Kanban automation moved this from Review approved to Validate Peach May 22, 2024
@gregory-paidis-sonarsource gregory-paidis-sonarsource deleted the greg/implement-S5344-part-2 branch May 22, 2024 12:18
@gregory-paidis-sonarsource gregory-paidis-sonarsource moved this from Validate Peach to Done in Best Kanban May 24, 2024
@zsolt-kolbay-sonarsource zsolt-kolbay-sonarsource added this to the 9.26 milestone May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Sprint: MMF-3716 crypto rules https://sonarsource.atlassian.net/browse/MMF-3716
Projects
Best Kanban
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants