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

NIFI-12764 Removed Commons Codec and Lang3 from security-utils #8380

Merged
merged 1 commit into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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