exceptionfactory commented on a change in pull request #4767:
URL: https://github.com/apache/nifi/pull/4767#discussion_r561195875
##########
File path:
nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/KeyStoreUtils.java
##########
@@ -125,6 +146,63 @@ public static KeyStore loadKeyStore(String keystorePath,
char[] keystorePassword
}
}
+ /**
+ * Creates a temporary default Keystore and Truststore and returns it
wrapped in a TLS configuration.
+ *
+ * @return a {@link org.apache.nifi.security.util.TlsConfiguration}
+ */
+ public static TlsConfiguration createTlsConfigAndNewKeystoreTruststore()
throws IOException, GeneralSecurityException {
+ return createTlsConfigAndNewKeystoreTruststore(new
StandardTlsConfiguration());
+ }
+
+ /**
+ * Creates a temporary Keystore and Truststore and returns it wrapped in a
new TLS configuration with the given values.
+ *
+ * @param tlsConfiguration a {@link
org.apache.nifi.security.util.TlsConfiguration}
+ * @return a {@link org.apache.nifi.security.util.TlsConfiguration}
+ */
+ public static TlsConfiguration
createTlsConfigAndNewKeystoreTruststore(final TlsConfiguration
tlsConfiguration) throws IOException, GeneralSecurityException {
+ final Path keyStorePath;
+ final String keystorePassword =
StringUtils.isNotBlank(tlsConfiguration.getKeystorePassword()) ?
tlsConfiguration.getKeystorePassword() : generatePassword();
+ final String keyPassword =
StringUtils.isNotBlank(tlsConfiguration.getKeyPassword())?
tlsConfiguration.getKeyPassword() : keystorePassword;
+ final KeystoreType keystoreType = tlsConfiguration.getKeystoreType()
!= null ? tlsConfiguration.getKeystoreType() : KeystoreType.PKCS12;
+ final Path trustStorePath;
+ final String truststorePassword =
StringUtils.isNotBlank(tlsConfiguration.getTruststorePassword()) ?
tlsConfiguration.getTruststorePassword() : "";
+ final KeystoreType truststoreType =
tlsConfiguration.getTruststoreType() != null ?
tlsConfiguration.getTruststoreType() : KeystoreType.PKCS12;
+
+ // Create temporary Keystore file
+ try {
+ keyStorePath = generateTempKeystorePath(keystoreType);
+ } catch (IOException e) {
+ logger.error(KEYSTORE_ERROR_MSG);
+ throw new UncheckedIOException(KEYSTORE_ERROR_MSG, e);
+ }
+
+ // Create temporary Truststore file
+ try {
+ trustStorePath = generateTempTruststorePath(truststoreType);
+ } catch (IOException e) {
+ logger.error(TRUSTSTORE_ERROR_MSG);
+ throw new UncheckedIOException(TRUSTSTORE_ERROR_MSG, e);
+ }
+
+ // Create X509 Certificate
+ final X509Certificate clientCert =
createKeyStoreAndGetX509Certificate(KEY_ALIAS, keystorePassword, keyPassword,
keyStorePath.toString(), keystoreType);
+
+ // Create Truststore
+ createTrustStore(clientCert, CERT_ALIAS, truststorePassword,
trustStorePath.toString(), getKeystoreType(truststoreType.toString()));
+
+ return new StandardTlsConfiguration(
+ keyStorePath.toString(),
+ keystorePassword,
+ keyPassword,
+ getKeystoreType(keystoreType.toString()),
+ trustStorePath.toString(),
+ truststorePassword,
+ getKeystoreType(truststoreType.toString()),
Review comment:
Are these calls to `getKeystoreType()` necessary? It looks like the
values are already of the `KeystoreType` enum.
```suggestion
truststoreType,
```
##########
File path:
nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/KeyStoreUtils.java
##########
@@ -125,6 +146,63 @@ public static KeyStore loadKeyStore(String keystorePath,
char[] keystorePassword
}
}
+ /**
+ * Creates a temporary default Keystore and Truststore and returns it
wrapped in a TLS configuration.
+ *
+ * @return a {@link org.apache.nifi.security.util.TlsConfiguration}
+ */
+ public static TlsConfiguration createTlsConfigAndNewKeystoreTruststore()
throws IOException, GeneralSecurityException {
+ return createTlsConfigAndNewKeystoreTruststore(new
StandardTlsConfiguration());
+ }
+
+ /**
+ * Creates a temporary Keystore and Truststore and returns it wrapped in a
new TLS configuration with the given values.
+ *
+ * @param tlsConfiguration a {@link
org.apache.nifi.security.util.TlsConfiguration}
+ * @return a {@link org.apache.nifi.security.util.TlsConfiguration}
+ */
+ public static TlsConfiguration
createTlsConfigAndNewKeystoreTruststore(final TlsConfiguration
tlsConfiguration) throws IOException, GeneralSecurityException {
+ final Path keyStorePath;
+ final String keystorePassword =
StringUtils.isNotBlank(tlsConfiguration.getKeystorePassword()) ?
tlsConfiguration.getKeystorePassword() : generatePassword();
+ final String keyPassword =
StringUtils.isNotBlank(tlsConfiguration.getKeyPassword())?
tlsConfiguration.getKeyPassword() : keystorePassword;
+ final KeystoreType keystoreType = tlsConfiguration.getKeystoreType()
!= null ? tlsConfiguration.getKeystoreType() : KeystoreType.PKCS12;
+ final Path trustStorePath;
+ final String truststorePassword =
StringUtils.isNotBlank(tlsConfiguration.getTruststorePassword()) ?
tlsConfiguration.getTruststorePassword() : "";
+ final KeystoreType truststoreType =
tlsConfiguration.getTruststoreType() != null ?
tlsConfiguration.getTruststoreType() : KeystoreType.PKCS12;
+
+ // Create temporary Keystore file
+ try {
+ keyStorePath = generateTempKeystorePath(keystoreType);
+ } catch (IOException e) {
+ logger.error(KEYSTORE_ERROR_MSG);
+ throw new UncheckedIOException(KEYSTORE_ERROR_MSG, e);
+ }
+
+ // Create temporary Truststore file
+ try {
+ trustStorePath = generateTempTruststorePath(truststoreType);
+ } catch (IOException e) {
+ logger.error(TRUSTSTORE_ERROR_MSG);
Review comment:
It would be helpful to log the exception as well as the error message
for troubleshooting.
```suggestion
logger.error(TRUSTSTORE_ERROR_MSG, e);
```
##########
File path:
nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/KeyStoreUtils.java
##########
@@ -366,4 +443,144 @@ public static String
sslServerSocketToString(SSLServerSocket sslServerSocket) {
.append("useClientMode", sslServerSocket.getUseClientMode())
.toString();
}
+
+ /**
+ * Loads the Keystore and returns a X509 Certificate with the given values.
+ *
+ * @param alias the certificate alias
+ * @param keyStorePassword the keystore password
+ * @param keyPassword the key password
+ * @param keyStorePath the keystore path
+ * @param keyStoreType the keystore type
+ * @return a {@link X509Certificate}
+ */
+ private static X509Certificate createKeyStoreAndGetX509Certificate(
+ final String alias, final String keyStorePassword, final String
keyPassword, final String keyStorePath,
+ final KeystoreType keyStoreType) throws IOException,
KeyStoreException, NoSuchAlgorithmException, CertificateException {
+
+ try (final FileOutputStream outputStream = new
FileOutputStream(keyStorePath)) {
+ final KeyPair keyPair =
KeyPairGenerator.getInstance("RSA").generateKeyPair();
+
+ final X509Certificate selfSignedCert =
CertificateUtils.generateSelfSignedX509Certificate(
+ keyPair, CERT_DN, "SHA256withRSA", 365
Review comment:
Recommend declaring static variables for the signature algorithm
`SHA256withRSA` and the validity length days `365`.
##########
File path:
nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/KeyStoreUtils.java
##########
@@ -366,4 +443,144 @@ public static String
sslServerSocketToString(SSLServerSocket sslServerSocket) {
.append("useClientMode", sslServerSocket.getUseClientMode())
.toString();
}
+
+ /**
+ * Loads the Keystore and returns a X509 Certificate with the given values.
+ *
+ * @param alias the certificate alias
+ * @param keyStorePassword the keystore password
+ * @param keyPassword the key password
+ * @param keyStorePath the keystore path
+ * @param keyStoreType the keystore type
+ * @return a {@link X509Certificate}
+ */
+ private static X509Certificate createKeyStoreAndGetX509Certificate(
+ final String alias, final String keyStorePassword, final String
keyPassword, final String keyStorePath,
+ final KeystoreType keyStoreType) throws IOException,
KeyStoreException, NoSuchAlgorithmException, CertificateException {
+
+ try (final FileOutputStream outputStream = new
FileOutputStream(keyStorePath)) {
+ final KeyPair keyPair =
KeyPairGenerator.getInstance("RSA").generateKeyPair();
Review comment:
Recommend declaring `RSA` as a private static variable named something
like `KEY_ALGORITHM`.
##########
File path:
nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/KeyStoreUtils.java
##########
@@ -245,7 +322,7 @@ public static TrustManagerFactory
loadTrustManagerFactory(TlsConfiguration tlsCo
*/
public static TrustManagerFactory loadTrustManagerFactory(String
truststorePath, String truststorePassword, String truststoreType) throws
TlsException {
// Legacy truststore passwords can be empty
- final char[] truststorePasswordChars =
StringUtils.isNotBlank(truststorePassword) ? truststorePassword.toCharArray() :
null;
+ final char[] truststorePasswordChars =
StringUtils.isNotBlank(truststorePassword) ? truststorePassword.toCharArray() :
"".toCharArray();
Review comment:
Is there are reason for changing the default to an empty array as
opposed to `null`?
##########
File path:
nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/KeyStoreUtils.java
##########
@@ -366,4 +443,144 @@ public static String
sslServerSocketToString(SSLServerSocket sslServerSocket) {
.append("useClientMode", sslServerSocket.getUseClientMode())
.toString();
}
+
+ /**
+ * Loads the Keystore and returns a X509 Certificate with the given values.
+ *
+ * @param alias the certificate alias
+ * @param keyStorePassword the keystore password
+ * @param keyPassword the key password
+ * @param keyStorePath the keystore path
+ * @param keyStoreType the keystore type
+ * @return a {@link X509Certificate}
+ */
+ private static X509Certificate createKeyStoreAndGetX509Certificate(
+ final String alias, final String keyStorePassword, final String
keyPassword, final String keyStorePath,
+ final KeystoreType keyStoreType) throws IOException,
KeyStoreException, NoSuchAlgorithmException, CertificateException {
+
+ try (final FileOutputStream outputStream = new
FileOutputStream(keyStorePath)) {
+ final KeyPair keyPair =
KeyPairGenerator.getInstance("RSA").generateKeyPair();
+
+ final X509Certificate selfSignedCert =
CertificateUtils.generateSelfSignedX509Certificate(
+ keyPair, CERT_DN, "SHA256withRSA", 365
+ );
+
+ final KeyStore keyStore = loadEmptyKeyStore(keyStoreType);
+ keyStore.setKeyEntry(alias, keyPair.getPrivate(),
keyPassword.toCharArray(), new Certificate[]{selfSignedCert});
+ keyStore.store(outputStream, keyStorePassword.toCharArray());
+
+ return selfSignedCert;
+ }
+ }
+
+ /**
+ * Loads the Truststore with the given values.
+ *
+ * @param cert the certificate
+ * @param alias the certificate alias
+ * @param password the truststore password
+ * @param path the truststore path
+ * @param truststoreType the truststore type
+ */
+ private static void createTrustStore(final X509Certificate cert,
+ final String alias, final String
password, final String path, final KeystoreType truststoreType)
+ throws KeyStoreException, NoSuchAlgorithmException,
CertificateException {
+
+ try (final FileOutputStream outputStream = new FileOutputStream(path))
{
+ final KeyStore trustStore = loadEmptyKeyStore(truststoreType);
+ trustStore.setCertificateEntry(alias, cert);
+ trustStore.store(outputStream, password.toCharArray());
+ } catch (IOException e) {
+ throw new UncheckedIOException(TRUSTSTORE_ERROR_MSG, e);
+ }
+ }
+
+ /**
+ * Generates a temporary keystore file and returns the path.
+ *
+ * @param keystoreType the Keystore type
+ * @return a Path
+ */
+ private static Path generateTempKeystorePath(KeystoreType keystoreType)
throws IOException {
+ return Files.createTempFile("test-keystore-",
getKeystoreExtension(keystoreType));
+ }
+
+ /**
+ * Generates a temporary truststore file and returns the path.
+ *
+ * @param truststoreType the Truststore type
+ * @return a Path
+ */
+ private static Path generateTempTruststorePath(KeystoreType
truststoreType) throws IOException {
+ return Files.createTempFile("test-truststore-",
getKeystoreExtension(truststoreType));
+ }
+
+ /**
+ * Loads and returns an empty Keystore backed by the appropriate provider.
+ *
+ * @param keyStoreType the keystore type
+ * @return an empty keystore
+ * @throws KeyStoreException if a keystore of the given type cannot be
instantiated
+ */
+ private static KeyStore loadEmptyKeyStore(KeystoreType keyStoreType)
throws KeyStoreException, CertificateException, NoSuchAlgorithmException {
+ final KeyStore keyStore;
+ try {
+ keyStore = KeyStore.getInstance(
+
Objects.requireNonNull(getKeystoreType(keyStoreType.toString()))
+ .toString());
+ keyStore.load(null, null);
+ return keyStore;
+ } catch (IOException e) {
+ logger.error("Encountered an error loading keystore: {}",
e.getLocalizedMessage());
+ throw new UncheckedIOException("Error loading keystore", e);
+ }
+ }
+
+ /**
+ * Returns the Keystore type in the correct format given the Keystore type.
+ *
+ * @param keystoreType the keystore type as a String
+ * @return the keystore type
+ */
+ private static KeystoreType getKeystoreType(String keystoreType) {
+ // if true
+ if (KeystoreType.isValidKeystoreType(keystoreType)) {
+ return KeystoreType.valueOf(keystoreType.toUpperCase());
+ } else {
+ logger.debug("Invalid Keystore type: \'" + keystoreType + "\'. The
given Keystore must be of type JKS, PKCS12, or BCFKS.");
Review comment:
Recommend using placeholder variables instead of string concatenation
for the `keystoreType`:
```suggestion
logger.debug("Invalid Keystore Type [{}]: Supported Types {}",
keystoreType, Arrays.asList(KeystoreType.values());
```
##########
File path:
nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/KeyStoreUtils.java
##########
@@ -366,4 +443,144 @@ public static String
sslServerSocketToString(SSLServerSocket sslServerSocket) {
.append("useClientMode", sslServerSocket.getUseClientMode())
.toString();
}
+
+ /**
+ * Loads the Keystore and returns a X509 Certificate with the given values.
+ *
+ * @param alias the certificate alias
+ * @param keyStorePassword the keystore password
+ * @param keyPassword the key password
+ * @param keyStorePath the keystore path
+ * @param keyStoreType the keystore type
+ * @return a {@link X509Certificate}
+ */
+ private static X509Certificate createKeyStoreAndGetX509Certificate(
+ final String alias, final String keyStorePassword, final String
keyPassword, final String keyStorePath,
+ final KeystoreType keyStoreType) throws IOException,
KeyStoreException, NoSuchAlgorithmException, CertificateException {
+
+ try (final FileOutputStream outputStream = new
FileOutputStream(keyStorePath)) {
+ final KeyPair keyPair =
KeyPairGenerator.getInstance("RSA").generateKeyPair();
+
+ final X509Certificate selfSignedCert =
CertificateUtils.generateSelfSignedX509Certificate(
+ keyPair, CERT_DN, "SHA256withRSA", 365
+ );
+
+ final KeyStore keyStore = loadEmptyKeyStore(keyStoreType);
+ keyStore.setKeyEntry(alias, keyPair.getPrivate(),
keyPassword.toCharArray(), new Certificate[]{selfSignedCert});
+ keyStore.store(outputStream, keyStorePassword.toCharArray());
+
+ return selfSignedCert;
+ }
+ }
+
+ /**
+ * Loads the Truststore with the given values.
+ *
+ * @param cert the certificate
+ * @param alias the certificate alias
+ * @param password the truststore password
+ * @param path the truststore path
+ * @param truststoreType the truststore type
+ */
+ private static void createTrustStore(final X509Certificate cert,
+ final String alias, final String
password, final String path, final KeystoreType truststoreType)
+ throws KeyStoreException, NoSuchAlgorithmException,
CertificateException {
+
+ try (final FileOutputStream outputStream = new FileOutputStream(path))
{
+ final KeyStore trustStore = loadEmptyKeyStore(truststoreType);
+ trustStore.setCertificateEntry(alias, cert);
+ trustStore.store(outputStream, password.toCharArray());
+ } catch (IOException e) {
+ throw new UncheckedIOException(TRUSTSTORE_ERROR_MSG, e);
+ }
+ }
+
+ /**
+ * Generates a temporary keystore file and returns the path.
+ *
+ * @param keystoreType the Keystore type
+ * @return a Path
+ */
+ private static Path generateTempKeystorePath(KeystoreType keystoreType)
throws IOException {
+ return Files.createTempFile("test-keystore-",
getKeystoreExtension(keystoreType));
+ }
+
+ /**
+ * Generates a temporary truststore file and returns the path.
+ *
+ * @param truststoreType the Truststore type
+ * @return a Path
+ */
+ private static Path generateTempTruststorePath(KeystoreType
truststoreType) throws IOException {
+ return Files.createTempFile("test-truststore-",
getKeystoreExtension(truststoreType));
+ }
+
+ /**
+ * Loads and returns an empty Keystore backed by the appropriate provider.
+ *
+ * @param keyStoreType the keystore type
+ * @return an empty keystore
+ * @throws KeyStoreException if a keystore of the given type cannot be
instantiated
+ */
+ private static KeyStore loadEmptyKeyStore(KeystoreType keyStoreType)
throws KeyStoreException, CertificateException, NoSuchAlgorithmException {
+ final KeyStore keyStore;
+ try {
+ keyStore = KeyStore.getInstance(
+
Objects.requireNonNull(getKeystoreType(keyStoreType.toString()))
+ .toString());
+ keyStore.load(null, null);
+ return keyStore;
+ } catch (IOException e) {
+ logger.error("Encountered an error loading keystore: {}",
e.getLocalizedMessage());
+ throw new UncheckedIOException("Error loading keystore", e);
+ }
+ }
+
+ /**
+ * Returns the Keystore type in the correct format given the Keystore type.
+ *
+ * @param keystoreType the keystore type as a String
+ * @return the keystore type
+ */
+ private static KeystoreType getKeystoreType(String keystoreType) {
+ // if true
+ if (KeystoreType.isValidKeystoreType(keystoreType)) {
+ return KeystoreType.valueOf(keystoreType.toUpperCase());
+ } else {
+ logger.debug("Invalid Keystore type: \'" + keystoreType + "\'. The
given Keystore must be of type JKS, PKCS12, or BCFKS.");
+ throw new IllegalArgumentException("The given Keystore type is not
valid: " + keystoreType);
+ }
+ }
+
+ /**
+ * Returns the Keystore extension given the Keystore type.
+ *
+ * @param keystoreType the keystore type
+ * @return the keystore extension
+ */
+ private static String getKeystoreExtension(KeystoreType keystoreType) {
+ if
(KeystoreType.PKCS12.toString().equalsIgnoreCase(keystoreType.toString())) {
+ return PKCS12_EXT;
+ } else if
(KeystoreType.JKS.toString().equalsIgnoreCase(keystoreType.toString())){
+ return JKS_EXT;
+ } else if
(KeystoreType.BCFKS.toString().equalsIgnoreCase(keystoreType.toString())) {
+ return BCFKS_EXT;
+ } else {
+ logger.debug("There is no Keystore file extension for the given
Keystore Type: " + keystoreType +
+ ". The Keystore must be of type JKS, PKCS12, or BCFKS.");
Review comment:
Recommend adjusting the message similar to the suggestion for the
`getKeystoreType` method so that changes to the `KeystoreType` enum will be
reflected automatically.
##########
File path:
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestInvokeHttpTwoWaySSL.java
##########
@@ -51,4 +59,27 @@ public static void beforeClass() throws Exception {
url = server.getSecureUrl();
}
+ @AfterClass
+ public static void afterClass() throws Exception {
+ if(server != null) {
Review comment:
Recommend checking the spacing here:
```suggestion
if (server != null) {
```
##########
File path:
nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/KeyStoreUtils.java
##########
@@ -366,4 +443,144 @@ public static String
sslServerSocketToString(SSLServerSocket sslServerSocket) {
.append("useClientMode", sslServerSocket.getUseClientMode())
.toString();
}
+
+ /**
+ * Loads the Keystore and returns a X509 Certificate with the given values.
+ *
+ * @param alias the certificate alias
+ * @param keyStorePassword the keystore password
+ * @param keyPassword the key password
+ * @param keyStorePath the keystore path
+ * @param keyStoreType the keystore type
+ * @return a {@link X509Certificate}
+ */
+ private static X509Certificate createKeyStoreAndGetX509Certificate(
+ final String alias, final String keyStorePassword, final String
keyPassword, final String keyStorePath,
+ final KeystoreType keyStoreType) throws IOException,
KeyStoreException, NoSuchAlgorithmException, CertificateException {
+
+ try (final FileOutputStream outputStream = new
FileOutputStream(keyStorePath)) {
+ final KeyPair keyPair =
KeyPairGenerator.getInstance("RSA").generateKeyPair();
+
+ final X509Certificate selfSignedCert =
CertificateUtils.generateSelfSignedX509Certificate(
+ keyPair, CERT_DN, "SHA256withRSA", 365
+ );
+
+ final KeyStore keyStore = loadEmptyKeyStore(keyStoreType);
+ keyStore.setKeyEntry(alias, keyPair.getPrivate(),
keyPassword.toCharArray(), new Certificate[]{selfSignedCert});
+ keyStore.store(outputStream, keyStorePassword.toCharArray());
+
+ return selfSignedCert;
+ }
+ }
+
+ /**
+ * Loads the Truststore with the given values.
+ *
+ * @param cert the certificate
+ * @param alias the certificate alias
+ * @param password the truststore password
+ * @param path the truststore path
+ * @param truststoreType the truststore type
+ */
+ private static void createTrustStore(final X509Certificate cert,
+ final String alias, final String
password, final String path, final KeystoreType truststoreType)
+ throws KeyStoreException, NoSuchAlgorithmException,
CertificateException {
+
+ try (final FileOutputStream outputStream = new FileOutputStream(path))
{
+ final KeyStore trustStore = loadEmptyKeyStore(truststoreType);
+ trustStore.setCertificateEntry(alias, cert);
+ trustStore.store(outputStream, password.toCharArray());
+ } catch (IOException e) {
+ throw new UncheckedIOException(TRUSTSTORE_ERROR_MSG, e);
+ }
+ }
+
+ /**
+ * Generates a temporary keystore file and returns the path.
+ *
+ * @param keystoreType the Keystore type
+ * @return a Path
+ */
+ private static Path generateTempKeystorePath(KeystoreType keystoreType)
throws IOException {
+ return Files.createTempFile("test-keystore-",
getKeystoreExtension(keystoreType));
+ }
+
+ /**
+ * Generates a temporary truststore file and returns the path.
+ *
+ * @param truststoreType the Truststore type
+ * @return a Path
+ */
+ private static Path generateTempTruststorePath(KeystoreType
truststoreType) throws IOException {
+ return Files.createTempFile("test-truststore-",
getKeystoreExtension(truststoreType));
+ }
+
+ /**
+ * Loads and returns an empty Keystore backed by the appropriate provider.
+ *
+ * @param keyStoreType the keystore type
+ * @return an empty keystore
+ * @throws KeyStoreException if a keystore of the given type cannot be
instantiated
+ */
+ private static KeyStore loadEmptyKeyStore(KeystoreType keyStoreType)
throws KeyStoreException, CertificateException, NoSuchAlgorithmException {
+ final KeyStore keyStore;
+ try {
+ keyStore = KeyStore.getInstance(
+
Objects.requireNonNull(getKeystoreType(keyStoreType.toString()))
+ .toString());
+ keyStore.load(null, null);
+ return keyStore;
+ } catch (IOException e) {
+ logger.error("Encountered an error loading keystore: {}",
e.getLocalizedMessage());
+ throw new UncheckedIOException("Error loading keystore", e);
+ }
+ }
+
+ /**
+ * Returns the Keystore type in the correct format given the Keystore type.
+ *
+ * @param keystoreType the keystore type as a String
+ * @return the keystore type
+ */
+ private static KeystoreType getKeystoreType(String keystoreType) {
+ // if true
+ if (KeystoreType.isValidKeystoreType(keystoreType)) {
+ return KeystoreType.valueOf(keystoreType.toUpperCase());
+ } else {
+ logger.debug("Invalid Keystore type: \'" + keystoreType + "\'. The
given Keystore must be of type JKS, PKCS12, or BCFKS.");
+ throw new IllegalArgumentException("The given Keystore type is not
valid: " + keystoreType);
+ }
+ }
+
+ /**
+ * Returns the Keystore extension given the Keystore type.
+ *
+ * @param keystoreType the keystore type
+ * @return the keystore extension
+ */
+ private static String getKeystoreExtension(KeystoreType keystoreType) {
+ if
(KeystoreType.PKCS12.toString().equalsIgnoreCase(keystoreType.toString())) {
+ return PKCS12_EXT;
+ } else if
(KeystoreType.JKS.toString().equalsIgnoreCase(keystoreType.toString())){
+ return JKS_EXT;
+ } else if
(KeystoreType.BCFKS.toString().equalsIgnoreCase(keystoreType.toString())) {
+ return BCFKS_EXT;
Review comment:
Refactoring this method to use a preset `Map<KeystoreType, String>`
would simplify the implementation. The Map could be populated in a static
initialization block.
##########
File path:
nifi-commons/nifi-security-utils/src/test/groovy/org/apache/nifi/security/util/KeyStoreUtilsGroovyTest.groovy
##########
@@ -141,4 +173,181 @@ class KeyStoreUtilsGroovyTest extends GroovyTestCase {
FileOutputStream fos = new
FileOutputStream("/Users/alopresto/Workspace/nifi/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/resources/truststore.no-password.jks")
truststore.store(fos, "".chars)
}
+
+ @Test
+ void testShouldValidateTempKeystorePath() {
+ // Act
+ Path testKeystorePath =
KeyStoreUtils.generateTempKeystorePath(JKS_STORE_TYPE)
+ deletePath(testKeystorePath)
+
+ // Assert
+ logger.info("Keystore path: ${testKeystorePath.toString()}")
+ assert testKeystorePath
+ }
+
+ @Test
+ void testShouldValidateTempTruststorePath() {
+ // Act
+ Path truststorePath =
KeyStoreUtils.generateTempTruststorePath(JKS_STORE_TYPE)
+ deletePath(truststorePath)
+
+ // Assert
+ logger.info("Truststore path: ${truststorePath.toString()}")
+ assert truststorePath
+ }
+
+ @Test
+ void testShouldValidateTlsConfigAndNewKeystoreTruststoreWithParams() {
+ // Arrange
+ TlsConfiguration tlsConfigParam = new StandardTlsConfiguration(null,
TEST_KEYSTORE_PASSWORD, TEST_KEY_PASSWORD, JKS_STORE_TYPE, null,
TEST_TRUSTSTORE_PASSWORD, JKS_STORE_TYPE)
+
+ // Act
+ final TlsConfiguration tlsConfig =
KeyStoreUtils.createTlsConfigAndNewKeystoreTruststore(tlsConfigParam)
+ deleteKeystoreTruststore(tlsConfig)
+
+ // Assert
+ assert tlsConfig.getKeystorePath()
+ assert tlsConfig.getTruststorePath()
+ assert tlsConfig.getKeystoreType() == KeystoreType.JKS
+ assert tlsConfig.getTruststoreType() == KeystoreType.JKS
+ assert tlsConfig.getKeystorePassword() == TEST_KEYSTORE_PASSWORD
+ }
+
+ @Test
+ void testShouldValidateTlsConfigAndNewKeystoreTruststoreWithoutParams() {
+ // Act
+ TlsConfiguration tlsConfig =
KeyStoreUtils.createTlsConfigAndNewKeystoreTruststore()
+ deleteKeystoreTruststore(tlsConfig)
+
+ // Assert
+ assert tlsConfig.getKeystorePath()
+ assert tlsConfig.getKeyPassword() == tlsConfig.getKeystorePassword()
+ assert tlsConfig.getKeystoreType() == KeystoreType.PKCS12
+ assert tlsConfig.getTruststoreType() == KeystoreType.PKCS12
+ }
+
+ @Test
+ void testShouldValidateTlsConfigWithoutKeyPasswordParam() {
+ // Arrange
+ TlsConfiguration tlsConfigParam = new StandardTlsConfiguration(null,
TEST_KEYSTORE_PASSWORD, null, JKS_STORE_TYPE, null, TEST_TRUSTSTORE_PASSWORD,
JKS_STORE_TYPE)
+
+ // Act
+ final TlsConfiguration tlsConfig =
KeyStoreUtils.createTlsConfigAndNewKeystoreTruststore(tlsConfigParam)
+ deleteKeystoreTruststore(tlsConfig)
+
+ // Assert
+ assert tlsConfig.getKeyPassword() == tlsConfig.getKeystorePassword()
+ }
+
+ @Test
+ void testShouldReturnX509CertWithNewKeystore() {
+ // Arrange
+ Path keystorePath =
KeyStoreUtils.generateTempKeystorePath(JKS_STORE_TYPE)
+
+ // Act
+ X509Certificate x509Cert =
KeyStoreUtils.createKeyStoreAndGetX509Certificate(KEY_ALIAS,
TEST_KEYSTORE_PASSWORD, TEST_KEYSTORE_PASSWORD, keystorePath.toString(),
JKS_STORE_TYPE)
+
+ boolean isKeystoreValid =
KeyStoreUtils.isStoreValid(keystorePath.toUri().toURL(), JKS_STORE_TYPE,
TEST_KEYSTORE_PASSWORD.toCharArray())
+ deletePath(keystorePath)
+
+ // Assert
+ final String certDN = x509Cert.getIssuerDN().toString()
+ logger.info("Certificate DN: ${certDN}")
+ assert certDN == "CN=localhost,OU=NiFi,O=Apache"
+ assert isKeystoreValid
+ }
+
+ @Test
+ void testShouldValidateGetKeystoreType() {
+ // Arrange
+ List<String> jks = ["jks", "Jks",]
+ List<String> pkcs12 = ["pkcs12", "Pkcs12"]
+ List<String> bcfks = ["bcfks", "Bcfks"]
Review comment:
Refactoring this test to loop through the values of KeystoreType, then
changing the case of the name would simplify the test and allow it to
automatically pick up new KeystoreType values when added.
##########
File path:
nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/KeyStoreUtils.java
##########
@@ -366,4 +443,144 @@ public static String
sslServerSocketToString(SSLServerSocket sslServerSocket) {
.append("useClientMode", sslServerSocket.getUseClientMode())
.toString();
}
+
+ /**
+ * Loads the Keystore and returns a X509 Certificate with the given values.
+ *
+ * @param alias the certificate alias
+ * @param keyStorePassword the keystore password
+ * @param keyPassword the key password
+ * @param keyStorePath the keystore path
+ * @param keyStoreType the keystore type
+ * @return a {@link X509Certificate}
+ */
+ private static X509Certificate createKeyStoreAndGetX509Certificate(
+ final String alias, final String keyStorePassword, final String
keyPassword, final String keyStorePath,
+ final KeystoreType keyStoreType) throws IOException,
KeyStoreException, NoSuchAlgorithmException, CertificateException {
+
+ try (final FileOutputStream outputStream = new
FileOutputStream(keyStorePath)) {
+ final KeyPair keyPair =
KeyPairGenerator.getInstance("RSA").generateKeyPair();
+
+ final X509Certificate selfSignedCert =
CertificateUtils.generateSelfSignedX509Certificate(
+ keyPair, CERT_DN, "SHA256withRSA", 365
+ );
+
+ final KeyStore keyStore = loadEmptyKeyStore(keyStoreType);
+ keyStore.setKeyEntry(alias, keyPair.getPrivate(),
keyPassword.toCharArray(), new Certificate[]{selfSignedCert});
+ keyStore.store(outputStream, keyStorePassword.toCharArray());
+
+ return selfSignedCert;
+ }
+ }
+
+ /**
+ * Loads the Truststore with the given values.
+ *
+ * @param cert the certificate
+ * @param alias the certificate alias
+ * @param password the truststore password
+ * @param path the truststore path
+ * @param truststoreType the truststore type
+ */
+ private static void createTrustStore(final X509Certificate cert,
+ final String alias, final String
password, final String path, final KeystoreType truststoreType)
+ throws KeyStoreException, NoSuchAlgorithmException,
CertificateException {
+
+ try (final FileOutputStream outputStream = new FileOutputStream(path))
{
+ final KeyStore trustStore = loadEmptyKeyStore(truststoreType);
+ trustStore.setCertificateEntry(alias, cert);
+ trustStore.store(outputStream, password.toCharArray());
+ } catch (IOException e) {
+ throw new UncheckedIOException(TRUSTSTORE_ERROR_MSG, e);
+ }
+ }
+
+ /**
+ * Generates a temporary keystore file and returns the path.
+ *
+ * @param keystoreType the Keystore type
+ * @return a Path
+ */
+ private static Path generateTempKeystorePath(KeystoreType keystoreType)
throws IOException {
+ return Files.createTempFile("test-keystore-",
getKeystoreExtension(keystoreType));
+ }
+
+ /**
+ * Generates a temporary truststore file and returns the path.
+ *
+ * @param truststoreType the Truststore type
+ * @return a Path
+ */
+ private static Path generateTempTruststorePath(KeystoreType
truststoreType) throws IOException {
+ return Files.createTempFile("test-truststore-",
getKeystoreExtension(truststoreType));
Review comment:
Recommend declaring static variables for the file name prefixes.
##########
File path:
nifi-commons/nifi-security-utils/src/test/groovy/org/apache/nifi/security/util/KeyStoreUtilsGroovyTest.groovy
##########
@@ -29,17 +31,22 @@ import org.slf4j.LoggerFactory
import javax.net.ssl.HttpsURLConnection
import javax.net.ssl.SSLSocket
import javax.net.ssl.SSLSocketFactory
+import java.nio.file.Files
+import java.nio.file.Path
+import java.nio.file.Paths
import java.security.KeyStore
import java.security.cert.Certificate
+import java.security.cert.X509Certificate
@RunWith(JUnit4.class)
class KeyStoreUtilsGroovyTest extends GroovyTestCase {
private static final Logger logger =
LoggerFactory.getLogger(KeyStoreUtilsGroovyTest.class)
- private static final File KEYSTORE_FILE = new
File("src/test/resources/keystore.jks")
- private static final String KEYSTORE_PASSWORD = "passwordpassword"
- private static final String KEY_PASSWORD = "keypassword"
- private static final KeystoreType KEYSTORE_TYPE = KeystoreType.JKS
+ private static final String TEST_KEYSTORE_PASSWORD =
KeyStoreUtils.generatePassword()
+ private static final String TEST_KEY_PASSWORD =
KeyStoreUtils.generatePassword()
+ private static final String TEST_TRUSTSTORE_PASSWORD =
KeyStoreUtils.generatePassword()
+ private static final KeystoreType JKS_STORE_TYPE = KeystoreType.JKS
Review comment:
Perhaps renaming this variable to something like `DEFAULT_STORE_TYPE`
would clarify the reason for declaring it here as opposed to just using the
enum value where necessary.
##########
File path:
nifi-commons/nifi-security-utils/src/test/groovy/org/apache/nifi/security/util/KeyStoreUtilsGroovyTest.groovy
##########
@@ -58,12 +65,22 @@ class KeyStoreUtilsGroovyTest extends GroovyTestCase {
}
+ @AfterClass
+ static void tearDownOnce() {
+
Review comment:
This method is empty and could be removed.
##########
File path:
nifi-commons/nifi-security-utils/src/test/groovy/org/apache/nifi/security/util/KeyStoreUtilsGroovyTest.groovy
##########
@@ -141,4 +173,181 @@ class KeyStoreUtilsGroovyTest extends GroovyTestCase {
FileOutputStream fos = new
FileOutputStream("/Users/alopresto/Workspace/nifi/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/resources/truststore.no-password.jks")
truststore.store(fos, "".chars)
}
+
+ @Test
+ void testShouldValidateTempKeystorePath() {
+ // Act
+ Path testKeystorePath =
KeyStoreUtils.generateTempKeystorePath(JKS_STORE_TYPE)
+ deletePath(testKeystorePath)
+
+ // Assert
+ logger.info("Keystore path: ${testKeystorePath.toString()}")
+ assert testKeystorePath
+ }
+
+ @Test
+ void testShouldValidateTempTruststorePath() {
+ // Act
+ Path truststorePath =
KeyStoreUtils.generateTempTruststorePath(JKS_STORE_TYPE)
+ deletePath(truststorePath)
+
+ // Assert
+ logger.info("Truststore path: ${truststorePath.toString()}")
+ assert truststorePath
+ }
+
+ @Test
+ void testShouldValidateTlsConfigAndNewKeystoreTruststoreWithParams() {
+ // Arrange
+ TlsConfiguration tlsConfigParam = new StandardTlsConfiguration(null,
TEST_KEYSTORE_PASSWORD, TEST_KEY_PASSWORD, JKS_STORE_TYPE, null,
TEST_TRUSTSTORE_PASSWORD, JKS_STORE_TYPE)
+
+ // Act
+ final TlsConfiguration tlsConfig =
KeyStoreUtils.createTlsConfigAndNewKeystoreTruststore(tlsConfigParam)
+ deleteKeystoreTruststore(tlsConfig)
+
+ // Assert
+ assert tlsConfig.getKeystorePath()
+ assert tlsConfig.getTruststorePath()
+ assert tlsConfig.getKeystoreType() == KeystoreType.JKS
+ assert tlsConfig.getTruststoreType() == KeystoreType.JKS
+ assert tlsConfig.getKeystorePassword() == TEST_KEYSTORE_PASSWORD
+ }
+
+ @Test
+ void testShouldValidateTlsConfigAndNewKeystoreTruststoreWithoutParams() {
+ // Act
+ TlsConfiguration tlsConfig =
KeyStoreUtils.createTlsConfigAndNewKeystoreTruststore()
+ deleteKeystoreTruststore(tlsConfig)
+
+ // Assert
+ assert tlsConfig.getKeystorePath()
+ assert tlsConfig.getKeyPassword() == tlsConfig.getKeystorePassword()
+ assert tlsConfig.getKeystoreType() == KeystoreType.PKCS12
+ assert tlsConfig.getTruststoreType() == KeystoreType.PKCS12
+ }
+
+ @Test
+ void testShouldValidateTlsConfigWithoutKeyPasswordParam() {
+ // Arrange
+ TlsConfiguration tlsConfigParam = new StandardTlsConfiguration(null,
TEST_KEYSTORE_PASSWORD, null, JKS_STORE_TYPE, null, TEST_TRUSTSTORE_PASSWORD,
JKS_STORE_TYPE)
+
+ // Act
+ final TlsConfiguration tlsConfig =
KeyStoreUtils.createTlsConfigAndNewKeystoreTruststore(tlsConfigParam)
+ deleteKeystoreTruststore(tlsConfig)
+
+ // Assert
+ assert tlsConfig.getKeyPassword() == tlsConfig.getKeystorePassword()
+ }
+
+ @Test
+ void testShouldReturnX509CertWithNewKeystore() {
+ // Arrange
+ Path keystorePath =
KeyStoreUtils.generateTempKeystorePath(JKS_STORE_TYPE)
+
+ // Act
+ X509Certificate x509Cert =
KeyStoreUtils.createKeyStoreAndGetX509Certificate(KEY_ALIAS,
TEST_KEYSTORE_PASSWORD, TEST_KEYSTORE_PASSWORD, keystorePath.toString(),
JKS_STORE_TYPE)
+
+ boolean isKeystoreValid =
KeyStoreUtils.isStoreValid(keystorePath.toUri().toURL(), JKS_STORE_TYPE,
TEST_KEYSTORE_PASSWORD.toCharArray())
+ deletePath(keystorePath)
+
+ // Assert
+ final String certDN = x509Cert.getIssuerDN().toString()
+ logger.info("Certificate DN: ${certDN}")
+ assert certDN == "CN=localhost,OU=NiFi,O=Apache"
Review comment:
This ties the test to a particular private value in the class. Perhaps
changing the check to assert that the string is not empty, or at least starts
with `CN=` would be more flexible.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
