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 15 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
13 changes: 13 additions & 0 deletions core/src/main/java/hudson/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -1952,4 +1952,17 @@ public static String getHexOfSHA256DigestOf(byte[] input) throws IOException {
byte[] payloadDigest = Util.getSHA256DigestOf(input);
return (payloadDigest != null) ? Util.toHexString(payloadDigest) : null;
}

/**
* Setting this flag to true enables FIPS mode
*/
@SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL")
@Restricted(NoExternalUse.class)
public static boolean FIPS_MODE = SystemProperties.getBoolean(Util.class.getName() + ".FIPS_MODE");
jtnord marked this conversation as resolved.
Show resolved Hide resolved

public static boolean isFipsMode() {
jtnord marked this conversation as resolved.
Show resolved Hide resolved
LOGGER.info("FIPS mode : " + FIPS_MODE);
divyasivasamy marked this conversation as resolved.
Show resolved Hide resolved
return FIPS_MODE;
}

}
162 changes: 147 additions & 15 deletions core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

package hudson.security;

import static java.util.logging.Level.WARNING;
import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED;

import com.thoughtworks.xstream.converters.UnmarshallingContext;
Expand All @@ -48,8 +49,12 @@
import hudson.util.XStream2;
import java.io.IOException;
import java.lang.reflect.Constructor;
import java.math.BigInteger;
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 +65,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 Down Expand Up @@ -885,9 +892,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 {
jtnord marked this conversation as resolved.
Show resolved Hide resolved
// 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 +918,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,49 +933,172 @@ public boolean isHashValid(String hash) {
}
}

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

public static final int HASH_LENGTH = 8;
Copy link
Member

Choose a reason for hiding this comment

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

is this really a length of a hash or just the number of bits in a byte?

public static final int ZERO = 0;
public static final int HEX_TWO = 2;
public static final int RADIX_LENGTH = 16;
public static final String STRING_SEPARATION = ":";
public static final int KEY_LENGTH = 512;
public static final String PBKDF2_ALGORITHM = "PBKDF2WithHmacSHA512";

@SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "Accessible via System Groovy Scripts")
@Restricted(NoExternalUse.class)
private static int MAXIMUM_PBKDF2_LOG_ROUND = SystemProperties.getInteger(HudsonPrivateSecurityRealm.class.getName() + ".maximumPBKDF2LogRound", 1000);
jtnord marked this conversation as resolved.
Show resolved Hide resolved

private static final Pattern PBKDF2_PATTERN = Pattern.compile("^\\$PBKDF2\\$HMACSHA512\\:([0-9]{4})\\:[a-zA-Z0-9=]+\\$[a-zA-Z0-9=]+");

@Override
public String encode(CharSequence rawPassword) {
return generatePasswordHashWithPBKDF2(rawPassword);
}

@Override
public boolean matches(CharSequence rawPassword, String encodedPassword) {
return validatePassword(rawPassword.toString(), encodedPassword);
}

private static String generatePasswordHashWithPBKDF2(CharSequence password) {
byte[] salt = generateSalt();
PBEKeySpec spec = new PBEKeySpec(password.toString().toCharArray(), salt, 1000, KEY_LENGTH);
byte[] hash = generateSecretKey(spec);
return PBKDF2 + "$HMACSHA512:" + 1000 + STRING_SEPARATION + toHex(salt) + "$" + toHex(hash);
}

private static byte[] generateSecretKey(PBEKeySpec spec) {
SecretKeyFactory secretKeyFactory = null;
byte[] hash = new byte[0];
try {
secretKeyFactory = SecretKeyFactory.getInstance(PBKDF2_ALGORITHM);
hash = secretKeyFactory.generateSecret(spec).getEncoded();
} catch (NoSuchAlgorithmException e) {
LOGGER.log(WARNING, "No such algorithm found for encode", e);
} catch (InvalidKeySpecException e) {
LOGGER.log(WARNING, "Invalid key spec exception", e);
}
divyasivasamy marked this conversation as resolved.
Show resolved Hide resolved
return hash;
}

private static String toHex(byte[] secretKey) {
divyasivasamy marked this conversation as resolved.
Show resolved Hide resolved

BigInteger bi = new BigInteger(1, secretKey);
String saltHashValue = bi.toString(RADIX_LENGTH);

int paddingLength = (secretKey.length * HEX_TWO) - saltHashValue.length();
if (paddingLength > ZERO) {
return String.format("%0" + paddingLength + "d", ZERO) + saltHashValue;
} else {
return saltHashValue;
}
}

@SuppressFBWarnings(value = {"DMI_RANDOM_USED_ONLY_ONCE", "PREDICTABLE_RANDOM"}, justification = "https://github.com/spotbugs/spotbugs/issues/1539 and doesn't need to be secure, we're just not hardcoding a 'wrong' password")
jtnord marked this conversation as resolved.
Show resolved Hide resolved
private static byte[] generateSalt() {
SecureRandom random = new SecureRandom();
divyasivasamy marked this conversation as resolved.
Show resolved Hide resolved
byte[] salt = new byte[16];
random.nextBytes(salt);
jtnord marked this conversation as resolved.
Show resolved Hide resolved
return salt;
}

@Override
public boolean isHashValid(String hash) {
Matcher matcher = PBKDF2_PATTERN.matcher(hash);
if (matcher.matches()) {
String logNumOfRound = matcher.group(1);
// no number format exception due to the expression
int logNumOfRoundInt = Integer.parseInt(logNumOfRound);
if (logNumOfRoundInt > 0 && logNumOfRoundInt <= MAXIMUM_PBKDF2_LOG_ROUND) {
return true;
}
}
return false;
}

private static boolean validatePassword(String originalPassword, String storedPassword) {
String[] parts = storedPassword.split("[:$]");
int iterations = Integer.parseInt(parts[3]);

byte[] salt = fromHex(parts[4]);
byte[] hash = fromHex(parts[5]);

PBEKeySpec spec = new PBEKeySpec(originalPassword.toCharArray(),
salt, iterations, hash.length * HASH_LENGTH);
Copy link
Member

Choose a reason for hiding this comment

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

see comment on HASH_LENGH - is this really a hash_lenght - or is it really the number of bits in a byte, so we are calculating the bit lenght of the hash based on the length of the byte[]?


byte[] generatedHashValue = generateSecretKey(spec);

int diff = hash.length ^ generatedHashValue.length;
for (int i = ZERO; i < hash.length && i < generatedHashValue.length; i++)
{
diff |= hash[i] ^ generatedHashValue[i];
}
divyasivasamy marked this conversation as resolved.
Show resolved Hide resolved
return diff == 0;
}

private static byte[] fromHex(String storedPassword) {
divyasivasamy marked this conversation as resolved.
Show resolved Hide resolved
byte[] bytes = new byte[storedPassword.length() / HEX_TWO];
for (int i = ZERO; i < bytes.length; i++)
{
bytes[i] = (byte) Integer.parseInt(storedPassword.substring(HEX_TWO * i, HEX_TWO * i + HEX_TWO), RADIX_LENGTH);
}
return bytes;
}

}

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


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

/**
* Magic header used to detect if a password is hashed.
*/

public static String getPasswordHeader() {
Copy link
Member

Choose a reason for hiding this comment

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

does this need to be public?

return Util.isFipsMode() ? PBKDF2 : JBCRYPT;
}
divyasivasamy marked this conversation as resolved.
Show resolved Hide resolved


// 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
}
}

/**
* Returns true if the supplied password starts with a prefix indicating it is already hashed.
* Returns true if the supplied password starts with a prefix indicating i
* t is already hashed.
divyasivasamy marked this conversation as resolved.
Show resolved Hide resolved
*/
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
}

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

public static final MultiPasswordEncoder PASSWORD_ENCODER = new MultiPasswordEncoder();
Expand Down Expand Up @@ -1032,5 +1163,6 @@ public void destroy() {
}
};

private static final Logger LOGGER = Logger.getLogger(HudsonPrivateSecurityRealm.class.getName());
static final Logger LOGGER = Logger.getLogger(HudsonPrivateSecurityRealm.class.getName());
divyasivasamy marked this conversation as resolved.
Show resolved Hide resolved

}
7 changes: 7 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,7 @@
package hudson.security;
jtnord marked this conversation as resolved.
Show resolved Hide resolved

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

public interface PasswordHashEncoder extends PasswordEncoder {
divyasivasamy marked this conversation as resolved.
Show resolved Hide resolved
boolean isHashValid(String hash);
}
Loading