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
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
ba9e113
Password Hashing changes in HudsonPrivateSecurityRealm
divyasivasamy Jul 27, 2023
1e640c1
Password Hashing -PBKDF2 Implementation
divyasivasamy Aug 4, 2023
a1b676b
reverted docs
divyasivasamy Aug 4, 2023
2616476
isHashValid Changes
divyasivasamy Aug 10, 2023
195db22
isHashValid Changes
divyasivasamy Aug 14, 2023
c0cc0ae
commented logger
divyasivasamy Aug 17, 2023
2cb8dbb
commented logger
divyasivasamy Aug 17, 2023
43d758b
modified test cases
divyasivasamy Aug 17, 2023
c0c932f
modified test cases
divyasivasamy Aug 18, 2023
033c8aa
modified test cases
divyasivasamy Aug 18, 2023
1cde056
Merge branch 'master' into HudsonPrivateSecurityRealmImplementation
divyasivasamy Aug 22, 2023
20beb48
resolved Util conflicts
divyasivasamy Aug 22, 2023
b8defa4
resolved Util conflicts
divyasivasamy Aug 22, 2023
1164b8b
resolved Util conflicts
divyasivasamy Aug 22, 2023
4b0eabe
modified test cases
divyasivasamy Aug 22, 2023
5bb3227
fixed review comments
divyasivasamy Aug 29, 2023
3b4bafb
fixed review comments
divyasivasamy Aug 29, 2023
4f329b2
fixed review comments
divyasivasamy Aug 29, 2023
f3175cb
fixed review comments
divyasivasamy Sep 6, 2023
4d27f84
fixed review comments
divyasivasamy Sep 6, 2023
769a594
Apply suggestions from code review
jtnord Sep 7, 2023
4877cb1
checkstyle
jtnord Sep 7, 2023
13887f5
introduce specific class for FIPS
jtnord Sep 7, 2023
4fcc6b0
Merge remote-tracking branch 'origin/master' into HudsonPrivateSecuri…
jtnord Sep 7, 2023
cfafad8
misc code cleanups
jtnord Sep 7, 2023
83e6d32
revert changes to HudsonPrivateSecurityRealmTest
jtnord Sep 7, 2023
d6853a0
code fixups and moving tests
jtnord Sep 8, 2023
46a1c5a
code fixups and moving tests
jtnord Sep 8, 2023
1b41044
make methods and fields private, and add message to exception
jtnord Sep 8, 2023
0d73df1
SecureRandom s -> SecureRandoms
jtnord Sep 8, 2023
0f7acfd
fixup IT after increasing the number of hasing rounds
jtnord Sep 8, 2023
67c7a24
move SecureRandom to double-checked-locking as SecureRandom itself is…
jtnord Sep 11, 2023
1cf01dd
make FIPS_COMPLIANCE_MODE final
jtnord Sep 12, 2023
5c60709
Add (c) header
jtnord Sep 22, 2023
e8fb999
keep the checkstyle overlord happy
jtnord Sep 22, 2023
f37bcc3
update exception message depending on the specific error issue
jtnord Sep 22, 2023
15e6c88
Leave comment about commented out tests
jtnord Sep 26, 2023
d76ff17
remove extra space in comment
jtnord Sep 26, 2023
3fbbf1d
remove trailing line ends to keep spotless happy
jtnord Sep 26, 2023
24d63a7
Merge branch 'master' into HudsonPrivateSecurityRealmImplementation
daniel-beck Sep 27, 2023
9946596
warn when password hashed with incorrect algorithm
jtnord Oct 3, 2023
526118a
fix typos
jtnord Oct 4, 2023
5fc2cd8
Merge branch 'master' into HudsonPrivateSecurityRealmImplementation
jtnord Oct 4, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import java.util.List;
import java.util.Random;
import java.util.Set;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand Down Expand Up @@ -539,10 +540,10 @@
public User createAccountWithHashedPassword(String userName, String hashedPassword) throws IOException {
if (!PASSWORD_ENCODER.isPasswordHashed(hashedPassword)) {
jtnord marked this conversation as resolved.
Show resolved Hide resolved
final String message;
if (hashedPassword == null) {

Check warning on line 543 in core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 543 is only partially covered, one branch is missing
message = "The hashed password cannot be null";

Check warning on line 544 in core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 544 is not covered by tests
} else if (hashedPassword.startsWith(getPasswordHeader())) {

Check warning on line 545 in core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 545 is only partially covered, one branch is missing
message = "The hashed password was hashed with the correct algorithm, but the format was not correct";

Check warning on line 546 in core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 546 is not covered by tests
} else {
message = "The hashed password was hashed with an incorrect algorithm. Jenkins is expecting " + getPasswordHeader();
}
Expand Down Expand Up @@ -992,7 +993,7 @@
// lazy initialisation so that we do not block startup due to entropy
if (random == null) {
synchronized (this) {
if (random == null) {

Check warning on line 996 in core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 996 is only partially covered, one branch is missing
random = new SecureRandom();
}
}
Expand Down Expand Up @@ -1078,7 +1079,17 @@
if (password == null) {
return false;
}
return password.startsWith(getPasswordHeader()) && PASSWORD_HASH_ENCODER.isHashValid(password.substring(getPasswordHeader().length()));
if (password.startsWith(getPasswordHeader())) {
return PASSWORD_HASH_ENCODER.isHashValid(password.substring(getPasswordHeader().length()));
}
if (password.startsWith(FIPS140.useCompliantAlgorithms() ? JBCRYPT : PBKDF2)) {
// switch the header to see if this is using a different encryption
LOGGER.log(Level.WARNING, "A password appears to be stored (or is attempting to be stored) that was created with a different"
+ " hashing/encryption algorithm, check the FIPS-140 state of the system has not changed inadvertently");
} else {
LOGGER.log(Level.FINE, "A password appears to be stored (or is attempting to be stored) that is not hashed/encrypted.");
}
return false;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,15 @@
package hudson.security;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.Assert.assertThrows;

import hudson.model.User;
import hudson.security.HudsonPrivateSecurityRealm.Details;
import java.util.logging.Level;
import org.hamcrest.Matcher;
import org.htmlunit.FailingHttpStatusCodeException;
import org.junit.ClassRule;
import org.junit.Rule;
Expand All @@ -39,15 +43,22 @@
import org.jvnet.hudson.test.For;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.JenkinsRule.WebClient;
import org.jvnet.hudson.test.LoggerRule;


@For(HudsonPrivateSecurityRealm.class)
public class HudsonPrivateSecurityRealmFIPSTest {

// the jbcrypt encoded for of "a" without the quotes
private static final String JBCRYPT_ENCODED_PASSWORD = "#jbcrypt:$2a$06$m0CrhHm10qJ3lXRY.5zDGO3rS2KdeeWLuGmsfGlMfOxih58VYVfxe";

@ClassRule
// do not use the FIPS140 class here as that initializes the field before we set the property!
public static TestRule flagRule = FlagRule.systemProperty("jenkins.security.FIPS140.COMPLIANCE", "true");

@Rule
public LoggerRule lr = new LoggerRule().record(HudsonPrivateSecurityRealm.class, Level.WARNING).capture(5);

@Rule
public JenkinsRule j = new JenkinsRule();

Expand Down Expand Up @@ -83,5 +94,40 @@ public void userCreationWithHashedPasswords() throws Exception {
wc.login("user_hashed", "password");

assertThrows(FailingHttpStatusCodeException.class, () -> wc.login("user_hashed", "password2"));
assertThat(lr, not(hasIncorrectHashingLogEntry()));
}

@Test
public void userLoginAfterEnablingFIPS() throws Exception {
HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(false, false, null);
j.jenkins.setSecurityRealm(securityRealm);

User u1 = securityRealm.createAccount("user", "a");
u1.setFullName("A User");
// overwrite the password property using an password created using an incorrect algorithm
u1.addProperty(Details.fromHashedPassword(JBCRYPT_ENCODED_PASSWORD));

u1.save();
assertThat(u1.getProperty(Details.class).getPassword(), is(JBCRYPT_ENCODED_PASSWORD));

try (WebClient wc = j.createWebClient()) {
assertThrows(FailingHttpStatusCodeException.class, () -> wc.login("user", "a"));
}
assertThat(lr, hasIncorrectHashingLogEntry());
}

@Test
public void userCreationWithJBCryptPasswords() throws Exception {
HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(false, false, null);

IllegalArgumentException illegalArgumentException = assertThrows(IllegalArgumentException.class,
() -> securityRealm.createAccountWithHashedPassword("user_hashed_incorrect_algorithm", JBCRYPT_ENCODED_PASSWORD));
assertThat(illegalArgumentException.getMessage(),
is("The hashed password was hashed with an incorrect algorithm. Jenkins is expecting $PBKDF2"));
}

private static Matcher<LoggerRule> hasIncorrectHashingLogEntry() {
return LoggerRule.recorded(is(
"A password appears to be stored (or is attempting to be stored) that was created with a different hashing/encryption algorithm, check the FIPS-140 state of the system has not changed inadvertently"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.ExtensionList;
import hudson.model.User;
import hudson.security.HudsonPrivateSecurityRealm.Details;
import hudson.security.pages.SignupPage;
import java.lang.reflect.Field;
import java.net.URL;
Expand All @@ -50,11 +51,14 @@
import java.util.Base64;
import java.util.Collections;
import java.util.List;
import java.util.logging.Level;
import jenkins.security.ApiTokenProperty;
import jenkins.security.SecurityListener;
import jenkins.security.apitoken.ApiTokenPropertyConfiguration;
import jenkins.security.seed.UserSeedProperty;
import org.apache.commons.lang.StringUtils;
import org.hamcrest.Matcher;
import org.htmlunit.FailingHttpStatusCodeException;
import org.htmlunit.HttpMethod;
import org.htmlunit.WebRequest;
import org.htmlunit.html.HtmlForm;
Expand All @@ -70,15 +74,23 @@
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.JenkinsRule.WebClient;
import org.jvnet.hudson.test.LoggerRule;
import org.jvnet.hudson.test.TestExtension;
import org.mindrot.jbcrypt.BCrypt;

@For({UserSeedProperty.class, HudsonPrivateSecurityRealm.class})
public class HudsonPrivateSecurityRealmTest {

// the PBKDF encoded for of "password" without the quotes
jtnord marked this conversation as resolved.
Show resolved Hide resolved
private static final String PBKDF_ENDOCED_PASSWORD =
"$PBKDF2$HMACSHA512:210000:ffbb207b847010af98cdd2b09c79392c$f67c3b985daf60db83a9088bc2439f7b77016d26c1439a9877c4f863c377272283ce346edda4578a5607ea620a4beb662d853b800f373297e6f596af797743a6";

@Rule
public JenkinsRule j = new JenkinsRule();

@Rule
public LoggerRule lr = new LoggerRule().record(HudsonPrivateSecurityRealm.class, Level.WARNING).capture(5);

private SpySecurityListenerImpl spySecurityListener;

@Before
Expand Down Expand Up @@ -511,6 +523,7 @@ public void createAccountSupportsHashedPasswords() throws Exception {

XmlPage w2 = (XmlPage) wc.goTo("whoAmI/api/xml", "application/xml");
assertThat(w2, hasXPath("//name", is("user_hashed")));
assertThat(lr, not(hasIncorrectHashingLogEntry()));
}

@Test
Expand Down Expand Up @@ -717,6 +730,40 @@ public void changingPassword_withSeedDisable_hasNoImpact() throws Exception {
}
}

@Test
public void userLoginAfterDisalbingFIPS() throws Exception {
jtnord marked this conversation as resolved.
Show resolved Hide resolved
HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(false, false, null);
j.jenkins.setSecurityRealm(securityRealm);

User u1 = securityRealm.createAccount("user", "password");
u1.setFullName("A User");
// overwrite the password property using an password created using an incorrect algorithm
u1.addProperty(Details.fromHashedPassword(PBKDF_ENDOCED_PASSWORD));

u1.save();
assertThat(u1.getProperty(Details.class).getPassword(), is(PBKDF_ENDOCED_PASSWORD));

try (WebClient wc = j.createWebClient()) {
assertThrows(FailingHttpStatusCodeException.class, () -> wc.login("user", "password"));
}
assertThat(lr, hasIncorrectHashingLogEntry());
}

@Test
public void userCreationWithPBKDFPasswords() throws Exception {
HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(false, false, null);

IllegalArgumentException illegalArgumentException = assertThrows(IllegalArgumentException.class,
() -> securityRealm.createAccountWithHashedPassword("user_hashed_incorrect_algorithm", PBKDF_ENDOCED_PASSWORD));
assertThat(illegalArgumentException.getMessage(),
is("The hashed password was hashed with an incorrect algorithm. Jenkins is expecting #jbcrypt:"));
}

private static Matcher<LoggerRule> hasIncorrectHashingLogEntry() {
return LoggerRule.recorded(is(
"A password appears to be stored (or is attempting to be stored) that was created with a different hashing/encryption algorithm, check the FIPS-140 state of the system has not changed inadvertently"));
}

private User prepareRealmAndAlice() throws Exception {
j.jenkins.setDisableRememberMe(false);
HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(false, false, null);
Expand Down
Loading