Skip to content

Commit

Permalink
NIFI-12764 Removed Commons Codec and Lang3 from security-utils (#8380)
Browse files Browse the repository at this point in the history
  • Loading branch information
exceptionfactory committed Feb 9, 2024
1 parent 7dc696e commit 2ea4838
Show file tree
Hide file tree
Showing 10 changed files with 72 additions and 62 deletions.
8 changes: 0 additions & 8 deletions nifi-commons/nifi-security-utils/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,6 @@
<artifactId>nifi-security-cert-builder</artifactId>
<version>2.0.0-SNAPSHOT</version>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
</dependency>
<dependency>
<groupId>commons-codec</groupId>
<artifactId>commons-codec</artifactId>
</dependency>
<dependency>
<groupId>org.bouncycastle</groupId>
<artifactId>bcprov-jdk18on</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,14 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HexFormat;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.TrustManagerFactory;
import javax.security.auth.x500.X500Principal;

import org.apache.commons.codec.binary.Hex;
import org.apache.commons.lang3.StringUtils;
import org.apache.nifi.security.cert.builder.StandardCertificateBuilder;
import org.bouncycastle.jce.provider.BouncyCastleProvider;
import org.slf4j.Logger;
Expand Down Expand Up @@ -171,11 +170,11 @@ public static KeyStore loadSecretKeyStore(final String keystorePath, final char[
public static TlsConfiguration createTlsConfigAndNewKeystoreTruststore(final TlsConfiguration tlsConfiguration, int certDurationDays,
String[] dnsSubjectAlternativeNames) throws IOException, GeneralSecurityException {
final Path keyStorePath;
final String keystorePassword = StringUtils.isNotBlank(tlsConfiguration.getKeystorePassword()) ? tlsConfiguration.getKeystorePassword() : generatePassword();
final String keystorePassword = isNotBlank(tlsConfiguration.getKeystorePassword()) ? tlsConfiguration.getKeystorePassword() : generatePassword();
final KeystoreType keystoreType = tlsConfiguration.getKeystoreType() != null ? tlsConfiguration.getKeystoreType() : KeystoreType.PKCS12;
final String keyPassword = StringUtils.isNotBlank(tlsConfiguration.getKeyPassword()) ? tlsConfiguration.getKeyPassword() : keystorePassword;
final String keyPassword = isNotBlank(tlsConfiguration.getKeyPassword()) ? tlsConfiguration.getKeyPassword() : keystorePassword;
final Path trustStorePath;
final String truststorePassword = StringUtils.isNotBlank(tlsConfiguration.getTruststorePassword()) ? tlsConfiguration.getTruststorePassword() : generatePassword();
final String truststorePassword = isNotBlank(tlsConfiguration.getTruststorePassword()) ? tlsConfiguration.getTruststorePassword() : generatePassword();
final KeystoreType truststoreType = tlsConfiguration.getTruststoreType() != null ? tlsConfiguration.getTruststoreType() : KeystoreType.PKCS12;

// Create temporary Keystore file
Expand Down Expand Up @@ -259,11 +258,11 @@ public static KeyManagerFactory loadKeyManagerFactory(TlsConfiguration tlsConfig
* @throws TlsException if there is a problem initializing or reading from the keystore
*/
public static KeyManagerFactory loadKeyManagerFactory(String keystorePath, String keystorePassword, String keyPassword, String keystoreType) throws TlsException {
if (StringUtils.isEmpty(keystorePassword)) {
if (keystorePassword == null || keystorePassword.isEmpty()) {
throw new IllegalArgumentException("The keystore password cannot be null or empty");
}
final char[] keystorePasswordChars = keystorePassword.toCharArray();
final char[] keyPasswordChars = (StringUtils.isNotEmpty(keyPassword)) ? keyPassword.toCharArray() : keystorePasswordChars;
final char[] keyPasswordChars = isNotBlank(keyPassword) ? keyPassword.toCharArray() : keystorePasswordChars;
KeyStore keyStore = loadKeyStore(keystorePath, keystorePasswordChars, keystoreType);
return getKeyManagerFactoryFromKeyStore(keyStore, keystorePasswordChars, keyPasswordChars);
}
Expand Down Expand Up @@ -331,12 +330,12 @@ public static TrustManagerFactory loadTrustManagerFactory(TlsConfiguration tlsCo
*/
public static TrustManagerFactory loadTrustManagerFactory(String truststorePath, String truststorePassword, String truststoreType) throws TlsException {
// Bouncy Castle PKCS12 type requires a password
if (truststoreType.equalsIgnoreCase(KeystoreType.PKCS12.getType()) && StringUtils.isBlank(truststorePassword)) {
if (truststoreType.equalsIgnoreCase(KeystoreType.PKCS12.getType()) && (truststorePassword == null || truststorePassword.isBlank())) {
throw new IllegalArgumentException("A PKCS12 Truststore Type requires a password");
}

// Legacy truststore passwords can be empty
final char[] truststorePasswordChars = StringUtils.isNotBlank(truststorePassword) ? truststorePassword.toCharArray() : null;
final char[] truststorePasswordChars = isNotBlank(truststorePassword) ? truststorePassword.toCharArray() : null;
KeyStore trustStore = loadTrustStore(truststorePath, truststorePasswordChars, truststoreType);
return getTrustManagerFactoryFromTrustStore(trustStore);
}
Expand Down Expand Up @@ -445,8 +444,8 @@ public static KeystoreType getKeystoreTypeFromExtension(final String keystorePat
KeystoreType keystoreType = KeystoreType.PKCS12;

for (final Map.Entry<KeystoreType, String> keystoreTypeEntry : KEY_STORE_EXTENSIONS.entrySet()) {
final String extension = keystoreTypeEntry.getValue();
if (StringUtils.endsWithIgnoreCase(keystorePath, extension)) {
final String extension = keystoreTypeEntry.getValue().toLowerCase();
if (keystorePath.endsWith(extension)) {
keystoreType = keystoreTypeEntry.getKey();
break;
}
Expand Down Expand Up @@ -574,7 +573,7 @@ private static String getKeystoreExtension(KeystoreType keystoreType) {
private static String generatePassword() {
final byte[] password = new byte[PASSWORD_LENGTH];
new SecureRandom().nextBytes(password);
return Hex.encodeHexString(password);
return HexFormat.of().formatHex(password);
}

private static KeystoreType getKeystoreType(final String keystoreTypeName) {
Expand All @@ -584,4 +583,8 @@ private static KeystoreType getKeystoreType(final String keystoreTypeName) {
.findFirst();
return foundKeystoreType.orElseThrow(() -> new IllegalArgumentException(String.format("Keystore Type [%s] not found", keystoreTypeFilter)));
}

private static boolean isNotBlank(final String string) {
return string != null && !string.isBlank();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
*/
package org.apache.nifi.security.util;

import org.apache.commons.lang3.StringUtils;
import org.apache.nifi.util.NiFiProperties;

import java.io.File;
Expand Down Expand Up @@ -267,7 +266,7 @@ public String getKeyPasswordForLogging() {
*/
@Override
public String getFunctionalKeyPassword() {
return StringUtils.isNotBlank(keyPassword) ? keyPassword : keystorePassword;
return isNotBlank(keyPassword) ? keyPassword : keystorePassword;
}

/**
Expand Down Expand Up @@ -346,7 +345,7 @@ public boolean isAnyKeystorePopulated() {
public boolean isKeystoreValid() {
if (isStoreValid(keystorePath, keystorePassword, keystoreType, StoreType.KEY_STORE)) {
return true;
} else if (StringUtils.isNotBlank(keyPassword) && !keyPassword.equals(keystorePassword)) {
} else if (isNotBlank(keyPassword) && !keyPassword.equals(keystorePassword)) {
return isKeystorePopulated()
&& KeyStoreUtils.isKeyPasswordCorrect(getFileUrl(keystorePath), keystoreType, keystorePassword.toCharArray(),
getFunctionalKeyPassword().toCharArray());
Expand Down Expand Up @@ -464,20 +463,20 @@ public int hashCode() {
}

private static String maskPasswordForLog(String password) {
return StringUtils.isNotBlank(password) ? MASKED_PASSWORD_LOG : NULL_LOG;
return isNotBlank(password) ? MASKED_PASSWORD_LOG : NULL_LOG;
}

private boolean isAnyPopulated(String path, String password, KeystoreType type) {
return StringUtils.isNotBlank(path) || StringUtils.isNotBlank(password) || type != null;
return isNotBlank(path) || isNotBlank(password) || type != null;
}

private boolean isStorePopulated(final String path, final String password, final KeystoreType type, final StoreType storeType) {
boolean populated;

// Legacy trust stores such as JKS can be populated without a password; only check the path and type
populated = StringUtils.isNotBlank(path) && type != null;
populated = isNotBlank(path) && type != null;
if (StoreType.KEY_STORE == storeType) {
populated = populated && StringUtils.isNotBlank(password);
populated = populated && isNotBlank(password);
}

return populated;
Expand All @@ -495,6 +494,10 @@ private URL getFileUrl(final String path) {
}
}

private static boolean isNotBlank(final String string) {
return string != null && !string.isBlank();
}

private enum StoreType {
KEY_STORE,
TRUST_STORE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@

import java.util.Arrays;
import java.util.List;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.builder.ToStringBuilder;
import org.apache.commons.lang3.builder.ToStringStyle;


/**
* Enumeration capturing information about the cryptographic hash algorithms
Expand Down Expand Up @@ -113,12 +109,7 @@ public boolean isBlake2() {

@Override
public String toString() {
final ToStringBuilder builder = new ToStringBuilder(this);
ToStringBuilder.setDefaultStyle(ToStringStyle.SHORT_PREFIX_STYLE);
builder.append("Algorithm Name", name);
builder.append("Digest Length", digestBytesLength + " bytes");
builder.append("Description", description);
return builder.toString();
return "HashAlgorithm[Name=%s,Digest Length=%d bytes,Description=%s".formatted(name, digestBytesLength, description);
}

/**
Expand All @@ -137,7 +128,7 @@ public String buildAllowableValueDescription() {
if (!isStrongAlgorithm()) {
sb.append(" [WARNING -- Cryptographically broken]");
}
if (StringUtils.isNotBlank(description)) {
if (description != null && !description.isBlank()) {
sb.append(" ").append(description);
}
return sb.toString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HexFormat;
import java.util.List;
import org.apache.commons.codec.binary.Hex;
import org.apache.commons.codec.digest.DigestUtils;
import org.apache.nifi.components.AllowableValue;
import org.bouncycastle.crypto.digests.Blake2bDigest;
import org.slf4j.Logger;
Expand Down Expand Up @@ -106,9 +106,9 @@ public static String hashValueStreaming(HashAlgorithm algorithm, InputStream val
}
// The Blake2 algorithms are instantiated differently and rely on BouncyCastle
if (algorithm.isBlake2()) {
return Hex.encodeHexString(blake2HashStreaming(algorithm, value));
return HexFormat.of().formatHex(blake2HashStreaming(algorithm, value));
} else {
return Hex.encodeHexString(traditionalHashStreaming(algorithm, value));
return HexFormat.of().formatHex(traditionalHashStreaming(algorithm, value));
}
}

Expand All @@ -122,7 +122,7 @@ public static String hashValueStreaming(HashAlgorithm algorithm, InputStream val
*/
public static String hashValue(HashAlgorithm algorithm, String value, Charset charset) {
byte[] rawHash = hashValueRaw(algorithm, value, charset);
return Hex.encodeHexString(rawHash);
return HexFormat.of().formatHex(rawHash);
}

/**
Expand Down Expand Up @@ -189,13 +189,28 @@ public static byte[] hashValueRaw(HashAlgorithm algorithm, byte[] value) {
}

private static byte[] traditionalHash(HashAlgorithm algorithm, byte[] value) {
return DigestUtils.getDigest(algorithm.getName()).digest(value);
return getMessageDigest(algorithm).digest(value);
}

private static byte[] traditionalHashStreaming(HashAlgorithm algorithm, InputStream value) throws IOException {
MessageDigest digest = DigestUtils.getDigest(algorithm.getName());
// DigestInputStream digestInputStream = new DigestInputStream(value, digest);
return DigestUtils.digest(digest, value);
final MessageDigest messageDigest = getMessageDigest(algorithm);

final byte[] buffer = new byte[BUFFER_SIZE];
int read = value.read(buffer);
while (read != -1) {
messageDigest.update(buffer, 0 , read);
read = value.read(buffer);
}

return messageDigest.digest();
}

private static MessageDigest getMessageDigest(final HashAlgorithm algorithm) {
try {
return MessageDigest.getInstance(algorithm.getName());
} catch (final NoSuchAlgorithmException e) {
throw new IllegalArgumentException("Message Digest algorithm not found", e);
}
}

private static byte[] blake2Hash(HashAlgorithm algorithm, byte[] value) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

package org.apache.nifi.security.util;

import org.apache.commons.lang3.StringUtils;
import org.apache.nifi.security.cert.builder.StandardCertificateBuilder;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -56,6 +55,7 @@ public class KeyStoreUtilsTest {
private static final String SECRET_KEY_ALGORITHM = "AES";
private static final String KEY_PROTECTION_ALGORITHM = "PBEWithHmacSHA256AndAES_256";
private static final String HYPHEN_SEPARATOR = "-";
private static final String EMPTY = "";

private static KeyPair keyPair;
private static X509Certificate certificate;
Expand All @@ -65,7 +65,7 @@ public class KeyStoreUtilsTest {
public static void generateKeysAndCertificates() throws NoSuchAlgorithmException {
keyPair = KeyPairGenerator.getInstance(KEY_ALGORITHM).generateKeyPair();
certificate = new StandardCertificateBuilder(keyPair, new X500Principal(SUBJECT_DN), Duration.ofDays(DURATION_DAYS)).build();
final byte[] encodedKey = StringUtils.remove(UUID.randomUUID().toString(), HYPHEN_SEPARATOR).getBytes(StandardCharsets.UTF_8);
final byte[] encodedKey = UUID.randomUUID().toString().replaceAll(HYPHEN_SEPARATOR, EMPTY).getBytes(StandardCharsets.UTF_8);
secretKey = new SecretKeySpec(encodedKey, SECRET_KEY_ALGORITHM);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
*/
package org.apache.nifi.security.util;

import org.apache.nifi.util.StringUtils;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

Expand All @@ -29,6 +28,8 @@
import static org.junit.jupiter.api.Assertions.assertNull;

public class SslSocketFactoryTest {
private static final String EMPTY = "";

private static TlsConfiguration tlsConfiguration;

@BeforeAll
Expand All @@ -53,7 +54,7 @@ public void testCreateSslContextEmptyKeyPassword() throws TlsException {
final TlsConfiguration customTlsConfiguration = new StandardTlsConfiguration(
tlsConfiguration.getKeystorePath(),
tlsConfiguration.getKeystorePassword(),
StringUtils.EMPTY,
EMPTY,
tlsConfiguration.getKeystoreType(),
tlsConfiguration.getTruststorePath(),
tlsConfiguration.getTruststorePassword(),
Expand All @@ -68,7 +69,7 @@ public void testCreateSslContextEmptyKeyPassword() throws TlsException {
@Test
public void testCreateSslContextEmptyTrustStorePasswordJks() throws TlsException {
final TlsConfiguration customTlsConfiguration = new TemporaryKeyStoreBuilder()
.trustStorePassword(StringUtils.EMPTY)
.trustStorePassword(EMPTY)
.trustStoreType(KeystoreType.JKS.getType())
.build();
final SSLContext sslContext = SslContextFactory.createSslContext(customTlsConfiguration);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,18 +92,13 @@ void testHashValueShouldDifferOnDifferentEncodings() {
/**
* This test ensures that the service properly handles UTF-16 encoded data to return it without
* the Big Endian Byte Order Mark (BOM). Java treats UTF-16 encoded data without a BOM as Big Endian by default on decoding, but when <em>encoding</em>, it inserts a BE BOM in the data.
*
* Examples:
*
* "apachenifi"
*
* * UTF-8: 0x61 0x70 0x61 0x63 0x68 0x65 0x6E 0x69 0x66 0x69
* * UTF-16: 0xFE 0xFF 0x00 0x61 0x00 0x70 0x00 0x61 0x00 0x63 0x00 0x68 0x00 0x65 0x00 0x6E 0x00 0x69 0x00 0x66 0x00 0x69
* * UTF-16LE: 0x61 0x00 0x70 0x00 0x61 0x00 0x63 0x00 0x68 0x00 0x65 0x00 0x6E 0x00 0x69 0x00 0x66 0x00 0x69 0x00
* * UTF-16BE: 0x00 0x61 0x00 0x70 0x00 0x61 0x00 0x63 0x00 0x68 0x00 0x65 0x00 0x6E 0x00 0x69 0x00 0x66 0x00 0x69
*
* The result of "UTF-16" decoding should have the 0xFE 0xFF stripped on return by encoding in UTF-16BE directly, which will not insert a BOM.
*
* See also: <a href="https://unicode.org/faq/utf_bom.html#bom10">https://unicode.org/faq/utf_bom.html#bom10</a>
*/
@Test
Expand Down Expand Up @@ -191,7 +186,7 @@ void testShouldRejectNullValue() {
}

@Test
void testShouldHashConstantValue() throws Exception {
void testShouldHashConstantValue() {
// Arrange
final List<HashAlgorithm> algorithms = List.of(HashAlgorithm.values());

Expand Down Expand Up @@ -232,7 +227,7 @@ void testShouldHashConstantValue() throws Exception {
}

@Test
void testShouldHashEmptyValue() throws Exception {
void testShouldHashEmptyValue() {
// Arrange
final List<HashAlgorithm> algorithms = List.of(HashAlgorithm.values());
final String EMPTY_VALUE = "";
Expand Down Expand Up @@ -274,7 +269,7 @@ void testShouldHashEmptyValue() throws Exception {
}

@Test
void testShouldBuildHashAlgorithmAllowableValues() throws Exception {
void testShouldBuildHashAlgorithmAllowableValues() {
// Arrange
final List<HashAlgorithm> EXPECTED_ALGORITHMS = List.of(HashAlgorithm.values());

Expand All @@ -298,7 +293,7 @@ void testShouldBuildHashAlgorithmAllowableValues() throws Exception {
}

@Test
void testShouldBuildCharacterSetAllowableValues() throws Exception {
void testShouldBuildCharacterSetAllowableValues() {
// Arrange
final List<Charset> EXPECTED_CHARACTER_SETS = Arrays.asList(
StandardCharsets.US_ASCII,
Expand Down Expand Up @@ -336,7 +331,7 @@ void testShouldBuildCharacterSetAllowableValues() throws Exception {
}

@Test
void testShouldHashValueFromStream() throws Exception {
void testShouldHashValueFromStream() {
// Arrange

// No command-line md2sum tool available
Expand Down

0 comments on commit 2ea4838

Please sign in to comment.