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 36 commits
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
144 changes: 128 additions & 16 deletions core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@
import java.lang.reflect.Constructor;
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.security.spec.InvalidKeySpecException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand All @@ -60,6 +63,8 @@
import java.util.logging.Logger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.PBEKeySpec;
import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
Expand All @@ -70,6 +75,7 @@
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;
import jenkins.model.Jenkins;
import jenkins.security.FIPS140;
import jenkins.security.SecurityListener;
import jenkins.security.seed.UserSeedProperty;
import jenkins.util.SystemProperties;
Expand Down Expand Up @@ -524,14 +530,23 @@ public User createAccount(String userName, String password) throws IOException {
}

/**
* Creates a new user account by registering a JBCrypt Hashed password with the user.
* Creates a new user account by registering a Hashed password with the user.
*
* @param userName The user's name
* @param hashedPassword A hashed password, must begin with {@code #jbcrypt:}
* @param hashedPassword A hashed password, must begin with {@code getPasswordHeader()}
* @see #getPasswordHeader()
*/
public User createAccountWithHashedPassword(String userName, String hashedPassword) throws IOException {
if (!PASSWORD_ENCODER.isPasswordHashed(hashedPassword)) {
jtnord marked this conversation as resolved.
Show resolved Hide resolved
throw new IllegalArgumentException("this method should only be called with a pre-hashed password");
final String message;
if (hashedPassword == null) {
message = "The hashed password cannot be null";
} else if (hashedPassword.startsWith(getPasswordHeader())) {
message = "The hashed password was hashed with the correct algorithm, but the format was not correct";
} else {
message = "The hashed password was hashed with an incorrect algorithm. Jenkins is expecting " + getPasswordHeader();
}
throw new IllegalArgumentException(message);
}
User user = User.getById(userName, true);
user.addProperty(Details.fromHashedPassword(hashedPassword));
Expand Down Expand Up @@ -885,9 +900,9 @@ public Category getCategory() {

// TODO can we instead use BCryptPasswordEncoder from Spring Security, which has its own copy of BCrypt so we could drop the special library?
/**
* {@link PasswordEncoder} that uses jBCrypt.
* {@link PasswordHashEncoder} that uses jBCrypt.
*/
private static class JBCryptEncoder implements PasswordEncoder {
static class JBCryptEncoder implements PasswordHashEncoder {
// in jBCrypt the maximum is 30, which takes ~22h with laptop late-2017
// and for 18, it's "only" 20s
@SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "Accessible via System Groovy Scripts")
Expand All @@ -911,6 +926,7 @@ public boolean matches(CharSequence rawPassword, String encodedPassword) {
* implementation defined in jBCrypt and <a href="https://en.wikipedia.org/wiki/Bcrypt">the Wikipedia page</a>.
*
*/
@Override
public boolean isHashValid(String hash) {
Matcher matcher = BCRYPT_PATTERN.matcher(hash);
if (matcher.matches()) {
Expand All @@ -925,34 +941,131 @@ public boolean isHashValid(String hash) {
}
}

/* package */ static final JBCryptEncoder JBCRYPT_ENCODER = new JBCryptEncoder();
static class PBKDF2PasswordEncoder implements PasswordHashEncoder {

private static final String STRING_SEPARATION = ":";
private static final int KEY_LENGTH_BITS = 512;
private static final int SALT_LENGTH_BYTES = 16;
// https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#pbkdf2
// ~230ms on an Intel i7-10875H CPU (JBCryptEncoder is ~57ms)
private static final int ITTERATIONS = 210_000;
private static final String PBKDF2_ALGORITHM = "PBKDF2WithHmacSHA512";

private volatile SecureRandom random; // defer construction until we need to use it to not delay startup in the case of lack of entropy.

// $PBDKF2 is already checked before we get here.
// $algorithm(HMACSHA512) : rounds : salt_in_hex $ mac_in_hex
private static final Pattern PBKDF2_PATTERN =
Pattern.compile("^\\$HMACSHA512\\:" + ITTERATIONS + "\\:[a-f0-9]{" + (SALT_LENGTH_BYTES * 2) + "}\\$[a-f0-9]{" + ((KEY_LENGTH_BITS / 8) * 2) + "}$");

@Override
public String encode(CharSequence rawPassword) {
try {
return generatePasswordHashWithPBKDF2(rawPassword);
} catch (NoSuchAlgorithmException | InvalidKeySpecException e) {
throw new RuntimeException("Unable to generate password with PBKDF2WithHmacSHA512", e);
}
}

@Override
public boolean matches(CharSequence rawPassword, String encodedPassword) {
try {
return validatePassword(rawPassword.toString(), encodedPassword);
} catch (NoSuchAlgorithmException | InvalidKeySpecException e) {
throw new RuntimeException("Unable to check password with PBKDF2WithHmacSHA512", e);
}
}

private String generatePasswordHashWithPBKDF2(CharSequence password) throws NoSuchAlgorithmException, InvalidKeySpecException {
byte[] salt = generateSalt();
PBEKeySpec spec = new PBEKeySpec(password.toString().toCharArray(), salt, ITTERATIONS, KEY_LENGTH_BITS);
byte[] hash = generateSecretKey(spec);
return "$HMACSHA512:" + ITTERATIONS + STRING_SEPARATION + Util.toHexString(salt) + "$" + Util.toHexString(hash);
}

private static byte[] generateSecretKey(PBEKeySpec spec) throws NoSuchAlgorithmException, InvalidKeySpecException {
SecretKeyFactory secretKeyFactory = SecretKeyFactory.getInstance(PBKDF2_ALGORITHM);
return secretKeyFactory.generateSecret(spec).getEncoded();
}

private SecureRandom secureRandom() {
// lazy initialisation so that we do not block startup due to entropy
if (random == null) {
synchronized (this) {
if (random == null) {
random = new SecureRandom();
}
}
}
daniel-beck marked this conversation as resolved.
Show resolved Hide resolved
return random;
}

private byte[] generateSalt() {
byte[] salt = new byte[SALT_LENGTH_BYTES];
secureRandom().nextBytes(salt);
return salt;
}

@Override
public boolean isHashValid(String hash) {
Matcher matcher = PBKDF2_PATTERN.matcher(hash);
return matcher.matches();
}

private static boolean validatePassword(String password, String storedPassword) throws NoSuchAlgorithmException, InvalidKeySpecException {
String[] parts = storedPassword.split("[:$]");
int iterations = Integer.parseInt(parts[2]);

byte[] salt = Util.fromHexString(parts[3]);
byte[] hash = Util.fromHexString(parts[4]);

PBEKeySpec spec = new PBEKeySpec(password.toCharArray(),
salt, iterations, hash.length * 8 /* bits in a byte */);

byte[] generatedHashValue = generateSecretKey(spec);

return MessageDigest.isEqual(hash, generatedHashValue);
}

}

/* package */ static final PasswordHashEncoder PASSWORD_HASH_ENCODER = FIPS140.useCompliantAlgorithms() ? new PBKDF2PasswordEncoder() : new JBCryptEncoder();


private static final String PBKDF2 = "$PBKDF2";
private static final String JBCRYPT = "#jbcrypt:";

/**
* Magic header used to detect if a password is hashed.
*/
private static String getPasswordHeader() {
return FIPS140.useCompliantAlgorithms() ? PBKDF2 : JBCRYPT;
}


// TODO check if DelegatingPasswordEncoder can be used
/**
* Wraps {@link #JBCRYPT_ENCODER}.
* Wraps {@link #PASSWORD_HASH_ENCODER}.
* There used to be a SHA-256-based encoder but this is long deprecated, and insecure anyway.
*/
/* package */ static class MultiPasswordEncoder implements PasswordEncoder {
/**
* Magic header used to detect if a password is bcrypt hashed.
*/
private static final String JBCRYPT_HEADER = "#jbcrypt:";

/*
CLASSIC encoder outputs "salt:hash" where salt is [a-z]+, so we use unique prefix '#jbcyrpt"
to designate JBCRYPT-format hash.
to designate JBCRYPT-format hash and $PBKDF2 to designate PBKDF2 format hash.

'#' is neither in base64 nor hex, which makes it a good choice.
*/

@Override
public String encode(CharSequence rawPassword) {
return JBCRYPT_HEADER + JBCRYPT_ENCODER.encode(rawPassword);
return getPasswordHeader() + PASSWORD_HASH_ENCODER.encode(rawPassword);
}

@Override
public boolean matches(CharSequence rawPassword, String encPass) {
if (isPasswordHashed(encPass)) {
return JBCRYPT_ENCODER.matches(rawPassword, encPass.substring(JBCRYPT_HEADER.length()));
return PASSWORD_HASH_ENCODER.matches(rawPassword, encPass.substring(getPasswordHeader().length()));
divyasivasamy marked this conversation as resolved.
Show resolved Hide resolved
} else {
return false;
jtnord marked this conversation as resolved.
Show resolved Hide resolved
}
Expand All @@ -965,9 +1078,8 @@ public boolean isPasswordHashed(String password) {
if (password == null) {
return false;
}
return password.startsWith(JBCRYPT_HEADER) && JBCRYPT_ENCODER.isHashValid(password.substring(JBCRYPT_HEADER.length()));
return password.startsWith(getPasswordHeader()) && PASSWORD_HASH_ENCODER.isHashValid(password.substring(getPasswordHeader().length()));
}

divyasivasamy marked this conversation as resolved.
Show resolved Hide resolved
}

public static final MultiPasswordEncoder PASSWORD_ENCODER = new MultiPasswordEncoder();
Expand Down
30 changes: 30 additions & 0 deletions core/src/main/java/hudson/security/PasswordHashEncoder.java
jtnord marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* The MIT License
*
* Copyright (c) 2023, Cloudbees, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

package hudson.security;
jtnord marked this conversation as resolved.
Show resolved Hide resolved

import org.springframework.security.crypto.password.PasswordEncoder;

interface PasswordHashEncoder extends PasswordEncoder {
boolean isHashValid(String hash);
}
60 changes: 60 additions & 0 deletions core/src/main/java/jenkins/security/FIPS140.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* The MIT License
*
* Copyright (c) 2023, Cloudbees, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

package jenkins.security;

import java.util.logging.Level;
import java.util.logging.Logger;
import jenkins.util.SystemProperties;

/**
* Utilities to help code change behaviour when it is desired to run in a FIPS-140 enabled environment.
* The environment (host, JVM and servlet container), must be suitably configured which is outside the scope of the Jenkins project.
* @see <a href="https://csrc.nist.gov/pubs/fips/140-2/upd2/final">FIPS-140-2</a>
* @see <a href="https://github.com/jenkinsci/jep/pull/398/files">JEP-XXXX</a>
daniel-beck marked this conversation as resolved.
Show resolved Hide resolved
* @since TODO
*/
public class FIPS140 {

private static final Logger LOGGER = Logger.getLogger(FIPS140.class.getName());

private static final boolean FIPS_COMPLIANCE_MODE = SystemProperties.getBoolean(FIPS140.class.getName() + ".COMPLIANCE");

static {
if (useCompliantAlgorithms()) {
LOGGER.log(Level.CONFIG, "System has been asked to run in FIPS-140 compliant mode");
}
}

/**
* Provide a hint that the system should strive to be compliant with <a href="https://csrc.nist.gov/pubs/fips/140-2/upd2/final">FIPS-140-2</a>.
* This can be used by code that needs to make choices at runtime whether to disable some optional behaviour that is not compliant with FIPS-140,
* or to switch to a compliant (yet less secure) alternative.
* If this returns {@code true} it does not mean that the instance is compliant, it merely acts as a hint.
* @return {@code true} iff the system should prefer compliance with FIPS-140-2 over compatibility with existing data or alternative non approved algorithms.
*/
public static boolean useCompliantAlgorithms() {
return FIPS_COMPLIANCE_MODE;
}

}
Loading
Loading