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

[JENKINS-71971][JEP-237] FIPS-140 compliant version of HudsonPrivateSecurityRealm #8393

Merged

Conversation

divyasivasamy
Copy link
Contributor

@divyasivasamy divyasivasamy commented Aug 18, 2023

JENKINS-71971 Allow the use of HudsonPrivateSecurityRealm with a FIPS compliant password algorithm.

Prior to this change the internal security realm used bcrypt (which uses the blowfish algorithm) to hash passwords. Blowfish is not a FIPS compliant algorithm and as this was used in the initial install it meant a clean install setup via the wizard could never be FIPS-140 compliant.
This addresses that by Implementing an alternative (enabled by a SystemProperty that uses PBKDF2withHmacSHA512.

see also JEP-237

Testing done

To validate the FIPS Mode and create hash of the user password and store it.
to validate a given password during the authentication process against stored hash of the password

Proposed changelog entries

  • Developer: Optionally support a FIPS-140 compliant algorithm in the Jenkins' own user database.

Proposed upgrade guidelines

N/A

@divyasivasamy divyasivasamy changed the title Hudson private security realm implementation Password hashing ,FIPS compliant algorithm implementation -Hudson private security realm Aug 18, 2023
@github-actions github-actions bot added the unresolved-merge-conflict There is a merge conflict with the target branch. label Aug 18, 2023
@github-actions
Copy link
Contributor

Please take a moment and address the merge conflicts of your pull request. Thanks!

@timja timja requested a review from a team August 19, 2023 16:34
@timja timja added the needs-security-review Awaiting review by a security team member label Aug 19, 2023
@github-actions github-actions bot removed the unresolved-merge-conflict There is a merge conflict with the target branch. label Aug 22, 2023
@divyasivasamy divyasivasamy requested a review from jtnord August 29, 2023 10:15
@jtnord jtnord added security-approved @jenkinsci/core-security-review reviewed this PR for security issues and removed needs-security-review Awaiting review by a security team member labels Sep 22, 2023
@jtnord
Copy link
Member

jtnord commented Sep 22, 2023

adding security-approved as both @daniel-beck, @Kevin-CB and @yaroslavafenkin have reviewed and left no security related issues (and myself as a cert member 😱 ).

Copy link
Member

@daniel-beck daniel-beck left a comment

Choose a reason for hiding this comment

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

I see no major problems with this PR.

@daniel-beck
Copy link
Member

Spotless failed the build.

IMO Spotless means applying PR suggestions has a very small chance of being successful, so should just not be done.

@github-actions
Copy link
Contributor

Please take a moment and address the merge conflicts of your pull request. Thanks!

@github-actions github-actions bot added the unresolved-merge-conflict There is a merge conflict with the target branch. label Sep 27, 2023
@github-actions github-actions bot removed the unresolved-merge-conflict There is a merge conflict with the target branch. label Sep 27, 2023
Co-authored-by: Daniel Beck <[email protected]>
@jtnord
Copy link
Member

jtnord commented Oct 4, 2023

failure on windows appears to be the known flake that @jglick and @Vlatombe are tackling in #8534 but is too old to contain the diagnostics from jenkinsci/jenkins-test-harness#657

@jtnord jtnord added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Oct 4, 2023
@olamy olamy added the fips label Oct 5, 2023
@jtnord jtnord merged commit 32834c5 into jenkinsci:master Oct 5, 2023
16 checks passed
@welcome
Copy link

welcome bot commented Oct 5, 2023

Congratulations on getting your very first Jenkins core pull request merged 🎉🥳

This is a fantastic achievement, and we're thrilled to have you as part of our community! Thank you for your valuable input, and we look forward to seeing more of your contributions in the future!

We would like to invite you to join the community chats and forums to meet other Jenkins contributors 😊
Don't forget to check out the participation page to learn more about how to contribute to Jenkins.


@NotMyFault NotMyFault added the rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted label Oct 5, 2023
@MarkEWaite
Copy link
Contributor

Congratulations on the merged pull request @divyasivasamy. Much appreciated!

In future pull requests, it would be best if you retained the formatting from the pull request template so that the automatic changelog generation process works as expected. I've inserted the headings into the pull request description and will confirm that the automatic changelog generation accepts the pull request, but it would be nice in future pull requests if automatic changelog maintenance could do the work instead of a person needing to do the work.

@MarkEWaite MarkEWaite added the developer Changes which impact plugin developers label Oct 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer Changes which impact plugin developers fips ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted security-approved @jenkinsci/core-security-review reviewed this PR for security issues squash-merge-me Unclean or useless commit history, should be merged only with squash-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants