exceptionfactory commented on a change in pull request #4986: URL: https://github.com/apache/nifi/pull/4986#discussion_r612867117
########## File path: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/util/SecureNiFiConfigUtil.java ########## @@ -0,0 +1,191 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.bootstrap.util; + +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; +import org.apache.nifi.security.util.KeyStoreUtils; +import org.apache.nifi.security.util.StandardTlsConfiguration; +import org.apache.nifi.security.util.TlsConfiguration; +import org.apache.nifi.util.NiFiProperties; +import org.bouncycastle.util.IPAddress; +import org.slf4j.Logger; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.security.GeneralSecurityException; +import java.util.HashSet; +import java.util.List; +import java.util.Properties; +import java.util.Set; +import java.util.stream.Collectors; + +public class SecureNiFiConfigUtil { + + private static final int CERT_DURATION_DAYS = 60; + public static final String LOCALHOST_IP = "127.0.0.1"; + private static final String LOCALHOST_NAME = "localhost"; + + private SecureNiFiConfigUtil() { + + } + + /** + * If HTTPS is enabled (nifi.web.https.port is set), but the keystore file specified in nifi.security.keystore + * does not exist, this will generate a key pair and self-signed certificate, generate the associated keystore + * and truststore and write them to disk under the configured filepaths, generate a secure random keystore password + * and truststore password, and write these to the nifi.properties file. + * @param nifiPropertiesFilename The filename of the nifi.properties file + * @param cmdLogger The bootstrap logger + * @throws IOException can be thrown when writing keystores to disk + * @throws RuntimeException indicates a security exception while generating keystores + */ + public static void configureSecureNiFiProperties(String nifiPropertiesFilename, Logger cmdLogger) throws IOException, RuntimeException { + final File propertiesFile = new File(nifiPropertiesFilename); + final Properties nifiProperties = loadProperties(propertiesFile); + + if (!nifiProperties.getProperty(NiFiProperties.WEB_HTTPS_PORT, "").isEmpty()) { + String keystorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE, ""); + String truststorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE, ""); + if (!keystorePath.isEmpty() && !truststorePath.isEmpty()) { + File keystore = new File(keystorePath); + File truststore = new File(truststorePath); + if (!keystore.exists() && !truststore.exists()) { Review comment: Is it possible to refactor these nested conditionals into more discrete methods? One approach might be wrapping the `File` objects in `Optional` and having separate methods that evaluate the presence of the keystore and truststore properties before returning the objects to this method for processing. Separating out the evaluation would also provide the opportunity for logging as currently implemented. ########## File path: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/util/SecureNiFiConfigUtil.java ########## @@ -0,0 +1,191 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.bootstrap.util; + +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; +import org.apache.nifi.security.util.KeyStoreUtils; +import org.apache.nifi.security.util.StandardTlsConfiguration; +import org.apache.nifi.security.util.TlsConfiguration; +import org.apache.nifi.util.NiFiProperties; +import org.bouncycastle.util.IPAddress; +import org.slf4j.Logger; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.security.GeneralSecurityException; +import java.util.HashSet; +import java.util.List; +import java.util.Properties; +import java.util.Set; +import java.util.stream.Collectors; + +public class SecureNiFiConfigUtil { + + private static final int CERT_DURATION_DAYS = 60; + public static final String LOCALHOST_IP = "127.0.0.1"; + private static final String LOCALHOST_NAME = "localhost"; + + private SecureNiFiConfigUtil() { + + } + + /** + * If HTTPS is enabled (nifi.web.https.port is set), but the keystore file specified in nifi.security.keystore + * does not exist, this will generate a key pair and self-signed certificate, generate the associated keystore + * and truststore and write them to disk under the configured filepaths, generate a secure random keystore password + * and truststore password, and write these to the nifi.properties file. + * @param nifiPropertiesFilename The filename of the nifi.properties file + * @param cmdLogger The bootstrap logger + * @throws IOException can be thrown when writing keystores to disk + * @throws RuntimeException indicates a security exception while generating keystores + */ + public static void configureSecureNiFiProperties(String nifiPropertiesFilename, Logger cmdLogger) throws IOException, RuntimeException { + final File propertiesFile = new File(nifiPropertiesFilename); + final Properties nifiProperties = loadProperties(propertiesFile); + + if (!nifiProperties.getProperty(NiFiProperties.WEB_HTTPS_PORT, "").isEmpty()) { + String keystorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE, ""); + String truststorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE, ""); + if (!keystorePath.isEmpty() && !truststorePath.isEmpty()) { + File keystore = new File(keystorePath); + File truststore = new File(truststorePath); + if (!keystore.exists() && !truststore.exists()) { + TlsConfiguration tlsConfiguration = null; + cmdLogger.info("Generating default self-signed certificates, keystore and truststore for secure Apache NiFi configuration. " + Review comment: Recommend shortening this log message or breaking it up into multiple messages. Logging the expiration is probably more intuitive: ```suggestion cmdLogger.info("Generating Self-Signed Certificate: Expires on {}", LocalDate.now().plus(CERT_DURATION_DAYS)); ``` ########## File path: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/util/SecureNiFiConfigUtil.java ########## @@ -0,0 +1,191 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.bootstrap.util; + +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; +import org.apache.nifi.security.util.KeyStoreUtils; +import org.apache.nifi.security.util.StandardTlsConfiguration; +import org.apache.nifi.security.util.TlsConfiguration; +import org.apache.nifi.util.NiFiProperties; +import org.bouncycastle.util.IPAddress; +import org.slf4j.Logger; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.security.GeneralSecurityException; +import java.util.HashSet; +import java.util.List; +import java.util.Properties; +import java.util.Set; +import java.util.stream.Collectors; + +public class SecureNiFiConfigUtil { + + private static final int CERT_DURATION_DAYS = 60; + public static final String LOCALHOST_IP = "127.0.0.1"; + private static final String LOCALHOST_NAME = "localhost"; + + private SecureNiFiConfigUtil() { + + } + + /** + * If HTTPS is enabled (nifi.web.https.port is set), but the keystore file specified in nifi.security.keystore + * does not exist, this will generate a key pair and self-signed certificate, generate the associated keystore + * and truststore and write them to disk under the configured filepaths, generate a secure random keystore password + * and truststore password, and write these to the nifi.properties file. + * @param nifiPropertiesFilename The filename of the nifi.properties file + * @param cmdLogger The bootstrap logger + * @throws IOException can be thrown when writing keystores to disk + * @throws RuntimeException indicates a security exception while generating keystores + */ + public static void configureSecureNiFiProperties(String nifiPropertiesFilename, Logger cmdLogger) throws IOException, RuntimeException { + final File propertiesFile = new File(nifiPropertiesFilename); + final Properties nifiProperties = loadProperties(propertiesFile); + + if (!nifiProperties.getProperty(NiFiProperties.WEB_HTTPS_PORT, "").isEmpty()) { + String keystorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE, ""); + String truststorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE, ""); + if (!keystorePath.isEmpty() && !truststorePath.isEmpty()) { + File keystore = new File(keystorePath); + File truststore = new File(truststorePath); + if (!keystore.exists() && !truststore.exists()) { + TlsConfiguration tlsConfiguration = null; + cmdLogger.info("Generating default self-signed certificates, keystore and truststore for secure Apache NiFi configuration. " + + "This certificate will expire in {} days, and it is recommended to acquire your own certificates in order to properly secure Apache NiFi.", CERT_DURATION_DAYS); + try { + Pair<String[], String[]> subjectAlternativeNames = getSubjectAlternativeNames(nifiProperties); + tlsConfiguration = KeyStoreUtils.createTlsConfigAndNewKeystoreTruststore(StandardTlsConfiguration + .fromNiFiProperties(nifiProperties), CERT_DURATION_DAYS, subjectAlternativeNames.getLeft(), + subjectAlternativeNames.getRight()); + } catch (IOException e) { + cmdLogger.error("Encountered an I/O exception while generating secure Apache NiFi configuration.", e); + throw e; + } catch (GeneralSecurityException e) { + cmdLogger.error("Encountered a security exception while generating secure Apache NiFi configuration.", e); + throw new RuntimeException(e); + } + + // Move over the new stores from temp dir + Files.move(new File(tlsConfiguration.getKeystorePath()).toPath(), new File(keystorePath).toPath(), + StandardCopyOption.REPLACE_EXISTING); + Files.move(new File(tlsConfiguration.getTruststorePath()).toPath(), new File(truststorePath).toPath(), + StandardCopyOption.REPLACE_EXISTING); + + updateProperties(propertiesFile, tlsConfiguration); + + cmdLogger.info("Successfully generated {} and {}.", keystorePath, truststorePath); Review comment: This message may not be necessary as an information log since the properties are already specified. What do you think about changing this to debug or removing? ########## File path: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/util/SecureNiFiConfigUtil.java ########## @@ -0,0 +1,191 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.bootstrap.util; + +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; +import org.apache.nifi.security.util.KeyStoreUtils; +import org.apache.nifi.security.util.StandardTlsConfiguration; +import org.apache.nifi.security.util.TlsConfiguration; +import org.apache.nifi.util.NiFiProperties; +import org.bouncycastle.util.IPAddress; +import org.slf4j.Logger; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.security.GeneralSecurityException; +import java.util.HashSet; +import java.util.List; +import java.util.Properties; +import java.util.Set; +import java.util.stream.Collectors; + +public class SecureNiFiConfigUtil { + + private static final int CERT_DURATION_DAYS = 60; + public static final String LOCALHOST_IP = "127.0.0.1"; + private static final String LOCALHOST_NAME = "localhost"; + + private SecureNiFiConfigUtil() { + + } + + /** + * If HTTPS is enabled (nifi.web.https.port is set), but the keystore file specified in nifi.security.keystore + * does not exist, this will generate a key pair and self-signed certificate, generate the associated keystore + * and truststore and write them to disk under the configured filepaths, generate a secure random keystore password + * and truststore password, and write these to the nifi.properties file. + * @param nifiPropertiesFilename The filename of the nifi.properties file + * @param cmdLogger The bootstrap logger + * @throws IOException can be thrown when writing keystores to disk + * @throws RuntimeException indicates a security exception while generating keystores + */ + public static void configureSecureNiFiProperties(String nifiPropertiesFilename, Logger cmdLogger) throws IOException, RuntimeException { + final File propertiesFile = new File(nifiPropertiesFilename); + final Properties nifiProperties = loadProperties(propertiesFile); + + if (!nifiProperties.getProperty(NiFiProperties.WEB_HTTPS_PORT, "").isEmpty()) { + String keystorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE, ""); + String truststorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE, ""); + if (!keystorePath.isEmpty() && !truststorePath.isEmpty()) { + File keystore = new File(keystorePath); + File truststore = new File(truststorePath); + if (!keystore.exists() && !truststore.exists()) { + TlsConfiguration tlsConfiguration = null; + cmdLogger.info("Generating default self-signed certificates, keystore and truststore for secure Apache NiFi configuration. " + + "This certificate will expire in {} days, and it is recommended to acquire your own certificates in order to properly secure Apache NiFi.", CERT_DURATION_DAYS); + try { + Pair<String[], String[]> subjectAlternativeNames = getSubjectAlternativeNames(nifiProperties); + tlsConfiguration = KeyStoreUtils.createTlsConfigAndNewKeystoreTruststore(StandardTlsConfiguration + .fromNiFiProperties(nifiProperties), CERT_DURATION_DAYS, subjectAlternativeNames.getLeft(), + subjectAlternativeNames.getRight()); + } catch (IOException e) { + cmdLogger.error("Encountered an I/O exception while generating secure Apache NiFi configuration.", e); Review comment: Changing the message wording to be more specific would be helpful: ```suggestion cmdLogger.error("Self-Signed Certificate Generation Failed", e); ``` ########## File path: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/util/SecureNiFiConfigUtil.java ########## @@ -0,0 +1,191 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.bootstrap.util; + +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; +import org.apache.nifi.security.util.KeyStoreUtils; +import org.apache.nifi.security.util.StandardTlsConfiguration; +import org.apache.nifi.security.util.TlsConfiguration; +import org.apache.nifi.util.NiFiProperties; +import org.bouncycastle.util.IPAddress; +import org.slf4j.Logger; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.security.GeneralSecurityException; +import java.util.HashSet; +import java.util.List; +import java.util.Properties; +import java.util.Set; +import java.util.stream.Collectors; + +public class SecureNiFiConfigUtil { + + private static final int CERT_DURATION_DAYS = 60; + public static final String LOCALHOST_IP = "127.0.0.1"; + private static final String LOCALHOST_NAME = "localhost"; + + private SecureNiFiConfigUtil() { + + } + + /** + * If HTTPS is enabled (nifi.web.https.port is set), but the keystore file specified in nifi.security.keystore + * does not exist, this will generate a key pair and self-signed certificate, generate the associated keystore + * and truststore and write them to disk under the configured filepaths, generate a secure random keystore password + * and truststore password, and write these to the nifi.properties file. + * @param nifiPropertiesFilename The filename of the nifi.properties file + * @param cmdLogger The bootstrap logger + * @throws IOException can be thrown when writing keystores to disk + * @throws RuntimeException indicates a security exception while generating keystores + */ + public static void configureSecureNiFiProperties(String nifiPropertiesFilename, Logger cmdLogger) throws IOException, RuntimeException { + final File propertiesFile = new File(nifiPropertiesFilename); + final Properties nifiProperties = loadProperties(propertiesFile); + + if (!nifiProperties.getProperty(NiFiProperties.WEB_HTTPS_PORT, "").isEmpty()) { + String keystorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE, ""); + String truststorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE, ""); + if (!keystorePath.isEmpty() && !truststorePath.isEmpty()) { + File keystore = new File(keystorePath); + File truststore = new File(truststorePath); + if (!keystore.exists() && !truststore.exists()) { + TlsConfiguration tlsConfiguration = null; + cmdLogger.info("Generating default self-signed certificates, keystore and truststore for secure Apache NiFi configuration. " + + "This certificate will expire in {} days, and it is recommended to acquire your own certificates in order to properly secure Apache NiFi.", CERT_DURATION_DAYS); + try { + Pair<String[], String[]> subjectAlternativeNames = getSubjectAlternativeNames(nifiProperties); + tlsConfiguration = KeyStoreUtils.createTlsConfigAndNewKeystoreTruststore(StandardTlsConfiguration + .fromNiFiProperties(nifiProperties), CERT_DURATION_DAYS, subjectAlternativeNames.getLeft(), + subjectAlternativeNames.getRight()); + } catch (IOException e) { + cmdLogger.error("Encountered an I/O exception while generating secure Apache NiFi configuration.", e); + throw e; + } catch (GeneralSecurityException e) { + cmdLogger.error("Encountered a security exception while generating secure Apache NiFi configuration.", e); + throw new RuntimeException(e); + } + + // Move over the new stores from temp dir + Files.move(new File(tlsConfiguration.getKeystorePath()).toPath(), new File(keystorePath).toPath(), + StandardCopyOption.REPLACE_EXISTING); + Files.move(new File(tlsConfiguration.getTruststorePath()).toPath(), new File(truststorePath).toPath(), + StandardCopyOption.REPLACE_EXISTING); + + updateProperties(propertiesFile, tlsConfiguration); + + cmdLogger.info("Successfully generated {} and {}.", keystorePath, truststorePath); + } else if (!keystore.exists() && truststore.exists()) { + cmdLogger.error("Tried to generate keystore {} for secure Apache NiFi configuration, but truststore file {} already exists. Aborting.", + keystorePath, truststorePath); + throw new RuntimeException("Will not generate keystore and truststore separately."); + } else if (keystore.exists() && !truststore.exists()) { + cmdLogger.error("Tried to generate truststore {} for secure Apache NiFi configuration, but keystore file {} already exists. Aborting.", + truststorePath, keystorePath); + throw new RuntimeException("Will not generate keystore and truststore separately."); + } else { + cmdLogger.info("Existing keystore and truststore detected: skipping Apache Nifi certificate generation."); + } + } else { + cmdLogger.warn("HTTPS is configured, but keystore and truststore are not specified. This will result in an invalid configuration."); + } + } else { + cmdLogger.info("No HTTPS configuration detected: skipping Apache Nifi certificate generation."); + } + } + + /** + * Attempts to add some reasonable guesses at desired SAN values that can be added to the generated + * certificate. + * @param nifiProperties The nifi.properties + * @return A Pair with IP SANs on the left and DNS SANs on the right + */ + private static Pair<String[], String[]> getSubjectAlternativeNames(Properties nifiProperties) { + Set<String> ipSubjectAlternativeNames = new HashSet<>(); + Set<String> dnsSubjectAlternativeNames = new HashSet<>(); + + try { + ipSubjectAlternativeNames.add(InetAddress.getLocalHost().getHostAddress()); + dnsSubjectAlternativeNames.add(InetAddress.getLocalHost().getHostName()); + } catch (UnknownHostException e) { + // This was just a courtesy, so we'll skip adding it + } + addSubjectAlternativeName(nifiProperties, NiFiProperties.REMOTE_INPUT_HOST, ipSubjectAlternativeNames, dnsSubjectAlternativeNames); + addSubjectAlternativeName(nifiProperties, NiFiProperties.WEB_HTTPS_HOST, ipSubjectAlternativeNames, dnsSubjectAlternativeNames); + addSubjectAlternativeName(nifiProperties, NiFiProperties.WEB_PROXY_HOST, ipSubjectAlternativeNames, dnsSubjectAlternativeNames); + addSubjectAlternativeName(nifiProperties, NiFiProperties.LOAD_BALANCE_ADDRESS, ipSubjectAlternativeNames, dnsSubjectAlternativeNames); + + // Not necessary to add as a SAN + ipSubjectAlternativeNames.remove(LOCALHOST_IP); + dnsSubjectAlternativeNames.remove(LOCALHOST_NAME); + + return new ImmutablePair<>(ipSubjectAlternativeNames.toArray(new String[ipSubjectAlternativeNames.size()]), + dnsSubjectAlternativeNames.toArray(new String[dnsSubjectAlternativeNames.size()])); + } + + private static void addSubjectAlternativeName(Properties nifiProperties, String propertyName, Set<String> ipSubjectAlternativeNames, + Set<String> dnsSubjectAlternativeNames) { + String hostValue = nifiProperties.getProperty(propertyName, ""); + if (!hostValue.isEmpty()) { + if (IPAddress.isValid(hostValue)) { + ipSubjectAlternativeNames.add(hostValue); + } else { + dnsSubjectAlternativeNames.add(hostValue); + } + } + } + + private static void updateProperties(final File propertiesFile, final TlsConfiguration tlsConfiguration) throws IOException { + final Path propertiesFilePath = propertiesFile.toPath(); + final List<String> lines = Files.readAllLines(propertiesFilePath); + final List<String> updatedLines = lines.stream().map(line -> { + if (line.startsWith(NiFiProperties.SECURITY_KEYSTORE_PASSWD)) { + return String.format("%s=%s", NiFiProperties.SECURITY_KEYSTORE_PASSWD, tlsConfiguration.getKeystorePassword()); Review comment: Recommend declaring `%s=%s` as a private static variable and reusing it across the lines, or adding a simple `getPropertyLine()` method to wrap the String formatting. ########## File path: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/util/SecureNiFiConfigUtil.java ########## @@ -0,0 +1,191 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.bootstrap.util; + +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; +import org.apache.nifi.security.util.KeyStoreUtils; +import org.apache.nifi.security.util.StandardTlsConfiguration; +import org.apache.nifi.security.util.TlsConfiguration; +import org.apache.nifi.util.NiFiProperties; +import org.bouncycastle.util.IPAddress; +import org.slf4j.Logger; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.security.GeneralSecurityException; +import java.util.HashSet; +import java.util.List; +import java.util.Properties; +import java.util.Set; +import java.util.stream.Collectors; + +public class SecureNiFiConfigUtil { + + private static final int CERT_DURATION_DAYS = 60; + public static final String LOCALHOST_IP = "127.0.0.1"; + private static final String LOCALHOST_NAME = "localhost"; + + private SecureNiFiConfigUtil() { + + } + + /** + * If HTTPS is enabled (nifi.web.https.port is set), but the keystore file specified in nifi.security.keystore + * does not exist, this will generate a key pair and self-signed certificate, generate the associated keystore + * and truststore and write them to disk under the configured filepaths, generate a secure random keystore password + * and truststore password, and write these to the nifi.properties file. + * @param nifiPropertiesFilename The filename of the nifi.properties file + * @param cmdLogger The bootstrap logger + * @throws IOException can be thrown when writing keystores to disk + * @throws RuntimeException indicates a security exception while generating keystores + */ + public static void configureSecureNiFiProperties(String nifiPropertiesFilename, Logger cmdLogger) throws IOException, RuntimeException { + final File propertiesFile = new File(nifiPropertiesFilename); + final Properties nifiProperties = loadProperties(propertiesFile); + + if (!nifiProperties.getProperty(NiFiProperties.WEB_HTTPS_PORT, "").isEmpty()) { + String keystorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE, ""); + String truststorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE, ""); + if (!keystorePath.isEmpty() && !truststorePath.isEmpty()) { + File keystore = new File(keystorePath); + File truststore = new File(truststorePath); + if (!keystore.exists() && !truststore.exists()) { + TlsConfiguration tlsConfiguration = null; + cmdLogger.info("Generating default self-signed certificates, keystore and truststore for secure Apache NiFi configuration. " + + "This certificate will expire in {} days, and it is recommended to acquire your own certificates in order to properly secure Apache NiFi.", CERT_DURATION_DAYS); + try { + Pair<String[], String[]> subjectAlternativeNames = getSubjectAlternativeNames(nifiProperties); + tlsConfiguration = KeyStoreUtils.createTlsConfigAndNewKeystoreTruststore(StandardTlsConfiguration + .fromNiFiProperties(nifiProperties), CERT_DURATION_DAYS, subjectAlternativeNames.getLeft(), + subjectAlternativeNames.getRight()); + } catch (IOException e) { + cmdLogger.error("Encountered an I/O exception while generating secure Apache NiFi configuration.", e); + throw e; + } catch (GeneralSecurityException e) { + cmdLogger.error("Encountered a security exception while generating secure Apache NiFi configuration.", e); + throw new RuntimeException(e); + } + + // Move over the new stores from temp dir + Files.move(new File(tlsConfiguration.getKeystorePath()).toPath(), new File(keystorePath).toPath(), + StandardCopyOption.REPLACE_EXISTING); + Files.move(new File(tlsConfiguration.getTruststorePath()).toPath(), new File(truststorePath).toPath(), + StandardCopyOption.REPLACE_EXISTING); + + updateProperties(propertiesFile, tlsConfiguration); + + cmdLogger.info("Successfully generated {} and {}.", keystorePath, truststorePath); + } else if (!keystore.exists() && truststore.exists()) { + cmdLogger.error("Tried to generate keystore {} for secure Apache NiFi configuration, but truststore file {} already exists. Aborting.", + keystorePath, truststorePath); + throw new RuntimeException("Will not generate keystore and truststore separately."); + } else if (keystore.exists() && !truststore.exists()) { + cmdLogger.error("Tried to generate truststore {} for secure Apache NiFi configuration, but keystore file {} already exists. Aborting.", + truststorePath, keystorePath); + throw new RuntimeException("Will not generate keystore and truststore separately."); + } else { + cmdLogger.info("Existing keystore and truststore detected: skipping Apache Nifi certificate generation."); + } + } else { + cmdLogger.warn("HTTPS is configured, but keystore and truststore are not specified. This will result in an invalid configuration."); + } + } else { + cmdLogger.info("No HTTPS configuration detected: skipping Apache Nifi certificate generation."); + } + } + + /** + * Attempts to add some reasonable guesses at desired SAN values that can be added to the generated + * certificate. + * @param nifiProperties The nifi.properties + * @return A Pair with IP SANs on the left and DNS SANs on the right + */ + private static Pair<String[], String[]> getSubjectAlternativeNames(Properties nifiProperties) { + Set<String> ipSubjectAlternativeNames = new HashSet<>(); + Set<String> dnsSubjectAlternativeNames = new HashSet<>(); + + try { + ipSubjectAlternativeNames.add(InetAddress.getLocalHost().getHostAddress()); + dnsSubjectAlternativeNames.add(InetAddress.getLocalHost().getHostName()); + } catch (UnknownHostException e) { + // This was just a courtesy, so we'll skip adding it Review comment: In contrast to some other comments, adding a debug log message here seems more useful than the comment with an empty catch block. ########## File path: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/util/SecureNiFiConfigUtil.java ########## @@ -0,0 +1,191 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.bootstrap.util; + +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; +import org.apache.nifi.security.util.KeyStoreUtils; +import org.apache.nifi.security.util.StandardTlsConfiguration; +import org.apache.nifi.security.util.TlsConfiguration; +import org.apache.nifi.util.NiFiProperties; +import org.bouncycastle.util.IPAddress; +import org.slf4j.Logger; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.security.GeneralSecurityException; +import java.util.HashSet; +import java.util.List; +import java.util.Properties; +import java.util.Set; +import java.util.stream.Collectors; + +public class SecureNiFiConfigUtil { + + private static final int CERT_DURATION_DAYS = 60; + public static final String LOCALHOST_IP = "127.0.0.1"; + private static final String LOCALHOST_NAME = "localhost"; + + private SecureNiFiConfigUtil() { + + } + + /** + * If HTTPS is enabled (nifi.web.https.port is set), but the keystore file specified in nifi.security.keystore + * does not exist, this will generate a key pair and self-signed certificate, generate the associated keystore + * and truststore and write them to disk under the configured filepaths, generate a secure random keystore password + * and truststore password, and write these to the nifi.properties file. + * @param nifiPropertiesFilename The filename of the nifi.properties file + * @param cmdLogger The bootstrap logger + * @throws IOException can be thrown when writing keystores to disk + * @throws RuntimeException indicates a security exception while generating keystores + */ + public static void configureSecureNiFiProperties(String nifiPropertiesFilename, Logger cmdLogger) throws IOException, RuntimeException { + final File propertiesFile = new File(nifiPropertiesFilename); + final Properties nifiProperties = loadProperties(propertiesFile); + + if (!nifiProperties.getProperty(NiFiProperties.WEB_HTTPS_PORT, "").isEmpty()) { + String keystorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE, ""); + String truststorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE, ""); + if (!keystorePath.isEmpty() && !truststorePath.isEmpty()) { + File keystore = new File(keystorePath); + File truststore = new File(truststorePath); + if (!keystore.exists() && !truststore.exists()) { + TlsConfiguration tlsConfiguration = null; + cmdLogger.info("Generating default self-signed certificates, keystore and truststore for secure Apache NiFi configuration. " + + "This certificate will expire in {} days, and it is recommended to acquire your own certificates in order to properly secure Apache NiFi.", CERT_DURATION_DAYS); + try { + Pair<String[], String[]> subjectAlternativeNames = getSubjectAlternativeNames(nifiProperties); + tlsConfiguration = KeyStoreUtils.createTlsConfigAndNewKeystoreTruststore(StandardTlsConfiguration + .fromNiFiProperties(nifiProperties), CERT_DURATION_DAYS, subjectAlternativeNames.getLeft(), + subjectAlternativeNames.getRight()); + } catch (IOException e) { + cmdLogger.error("Encountered an I/O exception while generating secure Apache NiFi configuration.", e); + throw e; + } catch (GeneralSecurityException e) { + cmdLogger.error("Encountered a security exception while generating secure Apache NiFi configuration.", e); + throw new RuntimeException(e); + } + + // Move over the new stores from temp dir + Files.move(new File(tlsConfiguration.getKeystorePath()).toPath(), new File(keystorePath).toPath(), Review comment: Is it possible to use `Paths.get()` instead of `new File().toPath()`? There may be tradeoffs to this approach depending on relative file path handling, but it may also be worth creating a private method called `getPath(String filePath)` to streamline creation of `Path` objects. ########## File path: nifi-bootstrap/src/test/java/org/apache/nifi/bootstrap/util/TestSecureNiFiConfigUtil.java ########## @@ -0,0 +1,208 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.bootstrap.util; + +import org.apache.nifi.properties.NiFiPropertiesLoader; +import org.apache.nifi.security.util.KeyStoreUtils; +import org.apache.nifi.security.util.TlsException; +import org.apache.nifi.util.NiFiProperties; +import org.bouncycastle.asn1.x509.GeneralName; +import org.bouncycastle.util.IPAddress; +import org.junit.After; +import org.junit.Assert; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.xml.bind.DatatypeConverter; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; +import java.security.InvalidKeyException; +import java.security.Key; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; +import java.security.SignatureException; +import java.security.UnrecoverableKeyException; +import java.security.cert.Certificate; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +public class TestSecureNiFiConfigUtil { + private Logger logger = LoggerFactory.getLogger("org.apache.nifi.bootstrap.util.TestSecureNiFiConfigUtil"); + + private static final boolean EXPECT_STORES_TO_EXIST = true; + + private String nifiPropertiesFile = "src/test/resources/nifi.properties"; + private String keystorePath = "src/test/resources/keystore.p12"; + private String truststorePath = "src/test/resources/truststore.p12"; Review comment: Rather than checking in binary files, recommend using the `KeyStoreUtils` class to generate keystore and truststore temporary files that can be used for testing. This will require writing the values to a temporary `nifi.properties` file for testing, but it avoids additional binary files in the repository, as well as associated hard-coded passwords and expiring certificates. ########## File path: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/util/SecureNiFiConfigUtil.java ########## @@ -0,0 +1,191 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.bootstrap.util; + +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; +import org.apache.nifi.security.util.KeyStoreUtils; +import org.apache.nifi.security.util.StandardTlsConfiguration; +import org.apache.nifi.security.util.TlsConfiguration; +import org.apache.nifi.util.NiFiProperties; +import org.bouncycastle.util.IPAddress; +import org.slf4j.Logger; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.security.GeneralSecurityException; +import java.util.HashSet; +import java.util.List; +import java.util.Properties; +import java.util.Set; +import java.util.stream.Collectors; + +public class SecureNiFiConfigUtil { + + private static final int CERT_DURATION_DAYS = 60; + public static final String LOCALHOST_IP = "127.0.0.1"; + private static final String LOCALHOST_NAME = "localhost"; + + private SecureNiFiConfigUtil() { + + } + + /** + * If HTTPS is enabled (nifi.web.https.port is set), but the keystore file specified in nifi.security.keystore + * does not exist, this will generate a key pair and self-signed certificate, generate the associated keystore + * and truststore and write them to disk under the configured filepaths, generate a secure random keystore password + * and truststore password, and write these to the nifi.properties file. + * @param nifiPropertiesFilename The filename of the nifi.properties file + * @param cmdLogger The bootstrap logger + * @throws IOException can be thrown when writing keystores to disk + * @throws RuntimeException indicates a security exception while generating keystores + */ + public static void configureSecureNiFiProperties(String nifiPropertiesFilename, Logger cmdLogger) throws IOException, RuntimeException { + final File propertiesFile = new File(nifiPropertiesFilename); + final Properties nifiProperties = loadProperties(propertiesFile); + + if (!nifiProperties.getProperty(NiFiProperties.WEB_HTTPS_PORT, "").isEmpty()) { + String keystorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE, ""); + String truststorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE, ""); + if (!keystorePath.isEmpty() && !truststorePath.isEmpty()) { + File keystore = new File(keystorePath); + File truststore = new File(truststorePath); + if (!keystore.exists() && !truststore.exists()) { + TlsConfiguration tlsConfiguration = null; + cmdLogger.info("Generating default self-signed certificates, keystore and truststore for secure Apache NiFi configuration. " + + "This certificate will expire in {} days, and it is recommended to acquire your own certificates in order to properly secure Apache NiFi.", CERT_DURATION_DAYS); + try { + Pair<String[], String[]> subjectAlternativeNames = getSubjectAlternativeNames(nifiProperties); + tlsConfiguration = KeyStoreUtils.createTlsConfigAndNewKeystoreTruststore(StandardTlsConfiguration + .fromNiFiProperties(nifiProperties), CERT_DURATION_DAYS, subjectAlternativeNames.getLeft(), + subjectAlternativeNames.getRight()); + } catch (IOException e) { + cmdLogger.error("Encountered an I/O exception while generating secure Apache NiFi configuration.", e); + throw e; + } catch (GeneralSecurityException e) { + cmdLogger.error("Encountered a security exception while generating secure Apache NiFi configuration.", e); + throw new RuntimeException(e); + } + + // Move over the new stores from temp dir + Files.move(new File(tlsConfiguration.getKeystorePath()).toPath(), new File(keystorePath).toPath(), + StandardCopyOption.REPLACE_EXISTING); + Files.move(new File(tlsConfiguration.getTruststorePath()).toPath(), new File(truststorePath).toPath(), + StandardCopyOption.REPLACE_EXISTING); + + updateProperties(propertiesFile, tlsConfiguration); + + cmdLogger.info("Successfully generated {} and {}.", keystorePath, truststorePath); + } else if (!keystore.exists() && truststore.exists()) { + cmdLogger.error("Tried to generate keystore {} for secure Apache NiFi configuration, but truststore file {} already exists. Aborting.", + keystorePath, truststorePath); + throw new RuntimeException("Will not generate keystore and truststore separately."); + } else if (keystore.exists() && !truststore.exists()) { + cmdLogger.error("Tried to generate truststore {} for secure Apache NiFi configuration, but keystore file {} already exists. Aborting.", + truststorePath, keystorePath); + throw new RuntimeException("Will not generate keystore and truststore separately."); Review comment: Throwing an exception in these cases does not seem necessary. It should result in a configuration error later during initialization, so doesn't seem to be necessary to handle these cases in this class. ########## File path: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/util/SecureNiFiConfigUtil.java ########## @@ -0,0 +1,191 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.bootstrap.util; + +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; +import org.apache.nifi.security.util.KeyStoreUtils; +import org.apache.nifi.security.util.StandardTlsConfiguration; +import org.apache.nifi.security.util.TlsConfiguration; +import org.apache.nifi.util.NiFiProperties; +import org.bouncycastle.util.IPAddress; +import org.slf4j.Logger; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.security.GeneralSecurityException; +import java.util.HashSet; +import java.util.List; +import java.util.Properties; +import java.util.Set; +import java.util.stream.Collectors; + +public class SecureNiFiConfigUtil { + + private static final int CERT_DURATION_DAYS = 60; + public static final String LOCALHOST_IP = "127.0.0.1"; + private static final String LOCALHOST_NAME = "localhost"; + + private SecureNiFiConfigUtil() { + + } + + /** + * If HTTPS is enabled (nifi.web.https.port is set), but the keystore file specified in nifi.security.keystore + * does not exist, this will generate a key pair and self-signed certificate, generate the associated keystore + * and truststore and write them to disk under the configured filepaths, generate a secure random keystore password + * and truststore password, and write these to the nifi.properties file. + * @param nifiPropertiesFilename The filename of the nifi.properties file + * @param cmdLogger The bootstrap logger + * @throws IOException can be thrown when writing keystores to disk + * @throws RuntimeException indicates a security exception while generating keystores + */ + public static void configureSecureNiFiProperties(String nifiPropertiesFilename, Logger cmdLogger) throws IOException, RuntimeException { + final File propertiesFile = new File(nifiPropertiesFilename); + final Properties nifiProperties = loadProperties(propertiesFile); + + if (!nifiProperties.getProperty(NiFiProperties.WEB_HTTPS_PORT, "").isEmpty()) { + String keystorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE, ""); + String truststorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE, ""); + if (!keystorePath.isEmpty() && !truststorePath.isEmpty()) { + File keystore = new File(keystorePath); + File truststore = new File(truststorePath); + if (!keystore.exists() && !truststore.exists()) { + TlsConfiguration tlsConfiguration = null; + cmdLogger.info("Generating default self-signed certificates, keystore and truststore for secure Apache NiFi configuration. " + + "This certificate will expire in {} days, and it is recommended to acquire your own certificates in order to properly secure Apache NiFi.", CERT_DURATION_DAYS); + try { + Pair<String[], String[]> subjectAlternativeNames = getSubjectAlternativeNames(nifiProperties); + tlsConfiguration = KeyStoreUtils.createTlsConfigAndNewKeystoreTruststore(StandardTlsConfiguration + .fromNiFiProperties(nifiProperties), CERT_DURATION_DAYS, subjectAlternativeNames.getLeft(), + subjectAlternativeNames.getRight()); + } catch (IOException e) { + cmdLogger.error("Encountered an I/O exception while generating secure Apache NiFi configuration.", e); + throw e; + } catch (GeneralSecurityException e) { + cmdLogger.error("Encountered a security exception while generating secure Apache NiFi configuration.", e); + throw new RuntimeException(e); + } + + // Move over the new stores from temp dir + Files.move(new File(tlsConfiguration.getKeystorePath()).toPath(), new File(keystorePath).toPath(), + StandardCopyOption.REPLACE_EXISTING); + Files.move(new File(tlsConfiguration.getTruststorePath()).toPath(), new File(truststorePath).toPath(), + StandardCopyOption.REPLACE_EXISTING); + + updateProperties(propertiesFile, tlsConfiguration); + + cmdLogger.info("Successfully generated {} and {}.", keystorePath, truststorePath); + } else if (!keystore.exists() && truststore.exists()) { + cmdLogger.error("Tried to generate keystore {} for secure Apache NiFi configuration, but truststore file {} already exists. Aborting.", + keystorePath, truststorePath); + throw new RuntimeException("Will not generate keystore and truststore separately."); + } else if (keystore.exists() && !truststore.exists()) { + cmdLogger.error("Tried to generate truststore {} for secure Apache NiFi configuration, but keystore file {} already exists. Aborting.", + truststorePath, keystorePath); + throw new RuntimeException("Will not generate keystore and truststore separately."); + } else { + cmdLogger.info("Existing keystore and truststore detected: skipping Apache Nifi certificate generation."); Review comment: Although this might be useful as a debug log, it may also be cleaner to remove the log message completely, as this is would be the normal behavior for NiFi configured with a standard keystore and truststore. ########## File path: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/util/SecureNiFiConfigUtil.java ########## @@ -0,0 +1,191 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.bootstrap.util; + +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; +import org.apache.nifi.security.util.KeyStoreUtils; +import org.apache.nifi.security.util.StandardTlsConfiguration; +import org.apache.nifi.security.util.TlsConfiguration; +import org.apache.nifi.util.NiFiProperties; +import org.bouncycastle.util.IPAddress; +import org.slf4j.Logger; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.security.GeneralSecurityException; +import java.util.HashSet; +import java.util.List; +import java.util.Properties; +import java.util.Set; +import java.util.stream.Collectors; + +public class SecureNiFiConfigUtil { + + private static final int CERT_DURATION_DAYS = 60; + public static final String LOCALHOST_IP = "127.0.0.1"; + private static final String LOCALHOST_NAME = "localhost"; + + private SecureNiFiConfigUtil() { + + } + + /** + * If HTTPS is enabled (nifi.web.https.port is set), but the keystore file specified in nifi.security.keystore + * does not exist, this will generate a key pair and self-signed certificate, generate the associated keystore + * and truststore and write them to disk under the configured filepaths, generate a secure random keystore password + * and truststore password, and write these to the nifi.properties file. + * @param nifiPropertiesFilename The filename of the nifi.properties file + * @param cmdLogger The bootstrap logger + * @throws IOException can be thrown when writing keystores to disk + * @throws RuntimeException indicates a security exception while generating keystores + */ + public static void configureSecureNiFiProperties(String nifiPropertiesFilename, Logger cmdLogger) throws IOException, RuntimeException { + final File propertiesFile = new File(nifiPropertiesFilename); + final Properties nifiProperties = loadProperties(propertiesFile); + + if (!nifiProperties.getProperty(NiFiProperties.WEB_HTTPS_PORT, "").isEmpty()) { + String keystorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE, ""); + String truststorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE, ""); + if (!keystorePath.isEmpty() && !truststorePath.isEmpty()) { + File keystore = new File(keystorePath); + File truststore = new File(truststorePath); + if (!keystore.exists() && !truststore.exists()) { + TlsConfiguration tlsConfiguration = null; + cmdLogger.info("Generating default self-signed certificates, keystore and truststore for secure Apache NiFi configuration. " + + "This certificate will expire in {} days, and it is recommended to acquire your own certificates in order to properly secure Apache NiFi.", CERT_DURATION_DAYS); + try { + Pair<String[], String[]> subjectAlternativeNames = getSubjectAlternativeNames(nifiProperties); + tlsConfiguration = KeyStoreUtils.createTlsConfigAndNewKeystoreTruststore(StandardTlsConfiguration + .fromNiFiProperties(nifiProperties), CERT_DURATION_DAYS, subjectAlternativeNames.getLeft(), + subjectAlternativeNames.getRight()); + } catch (IOException e) { + cmdLogger.error("Encountered an I/O exception while generating secure Apache NiFi configuration.", e); + throw e; + } catch (GeneralSecurityException e) { + cmdLogger.error("Encountered a security exception while generating secure Apache NiFi configuration.", e); + throw new RuntimeException(e); + } + + // Move over the new stores from temp dir + Files.move(new File(tlsConfiguration.getKeystorePath()).toPath(), new File(keystorePath).toPath(), + StandardCopyOption.REPLACE_EXISTING); + Files.move(new File(tlsConfiguration.getTruststorePath()).toPath(), new File(truststorePath).toPath(), + StandardCopyOption.REPLACE_EXISTING); + + updateProperties(propertiesFile, tlsConfiguration); + + cmdLogger.info("Successfully generated {} and {}.", keystorePath, truststorePath); + } else if (!keystore.exists() && truststore.exists()) { + cmdLogger.error("Tried to generate keystore {} for secure Apache NiFi configuration, but truststore file {} already exists. Aborting.", + keystorePath, truststorePath); + throw new RuntimeException("Will not generate keystore and truststore separately."); + } else if (keystore.exists() && !truststore.exists()) { + cmdLogger.error("Tried to generate truststore {} for secure Apache NiFi configuration, but keystore file {} already exists. Aborting.", + truststorePath, keystorePath); + throw new RuntimeException("Will not generate keystore and truststore separately."); + } else { + cmdLogger.info("Existing keystore and truststore detected: skipping Apache Nifi certificate generation."); + } + } else { + cmdLogger.warn("HTTPS is configured, but keystore and truststore are not specified. This will result in an invalid configuration."); Review comment: This may be useful information, but it is probably better handled in other existing initialization methods. ########## File path: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/util/SecureNiFiConfigUtil.java ########## @@ -0,0 +1,191 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.bootstrap.util; + +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; +import org.apache.nifi.security.util.KeyStoreUtils; +import org.apache.nifi.security.util.StandardTlsConfiguration; +import org.apache.nifi.security.util.TlsConfiguration; +import org.apache.nifi.util.NiFiProperties; +import org.bouncycastle.util.IPAddress; +import org.slf4j.Logger; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.security.GeneralSecurityException; +import java.util.HashSet; +import java.util.List; +import java.util.Properties; +import java.util.Set; +import java.util.stream.Collectors; + +public class SecureNiFiConfigUtil { + + private static final int CERT_DURATION_DAYS = 60; + public static final String LOCALHOST_IP = "127.0.0.1"; + private static final String LOCALHOST_NAME = "localhost"; + + private SecureNiFiConfigUtil() { + + } + + /** + * If HTTPS is enabled (nifi.web.https.port is set), but the keystore file specified in nifi.security.keystore + * does not exist, this will generate a key pair and self-signed certificate, generate the associated keystore + * and truststore and write them to disk under the configured filepaths, generate a secure random keystore password + * and truststore password, and write these to the nifi.properties file. + * @param nifiPropertiesFilename The filename of the nifi.properties file + * @param cmdLogger The bootstrap logger + * @throws IOException can be thrown when writing keystores to disk + * @throws RuntimeException indicates a security exception while generating keystores + */ + public static void configureSecureNiFiProperties(String nifiPropertiesFilename, Logger cmdLogger) throws IOException, RuntimeException { + final File propertiesFile = new File(nifiPropertiesFilename); + final Properties nifiProperties = loadProperties(propertiesFile); + + if (!nifiProperties.getProperty(NiFiProperties.WEB_HTTPS_PORT, "").isEmpty()) { + String keystorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE, ""); + String truststorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE, ""); + if (!keystorePath.isEmpty() && !truststorePath.isEmpty()) { + File keystore = new File(keystorePath); + File truststore = new File(truststorePath); + if (!keystore.exists() && !truststore.exists()) { + TlsConfiguration tlsConfiguration = null; + cmdLogger.info("Generating default self-signed certificates, keystore and truststore for secure Apache NiFi configuration. " + + "This certificate will expire in {} days, and it is recommended to acquire your own certificates in order to properly secure Apache NiFi.", CERT_DURATION_DAYS); + try { + Pair<String[], String[]> subjectAlternativeNames = getSubjectAlternativeNames(nifiProperties); + tlsConfiguration = KeyStoreUtils.createTlsConfigAndNewKeystoreTruststore(StandardTlsConfiguration + .fromNiFiProperties(nifiProperties), CERT_DURATION_DAYS, subjectAlternativeNames.getLeft(), + subjectAlternativeNames.getRight()); + } catch (IOException e) { + cmdLogger.error("Encountered an I/O exception while generating secure Apache NiFi configuration.", e); + throw e; + } catch (GeneralSecurityException e) { + cmdLogger.error("Encountered a security exception while generating secure Apache NiFi configuration.", e); + throw new RuntimeException(e); + } + + // Move over the new stores from temp dir + Files.move(new File(tlsConfiguration.getKeystorePath()).toPath(), new File(keystorePath).toPath(), + StandardCopyOption.REPLACE_EXISTING); + Files.move(new File(tlsConfiguration.getTruststorePath()).toPath(), new File(truststorePath).toPath(), + StandardCopyOption.REPLACE_EXISTING); + + updateProperties(propertiesFile, tlsConfiguration); + + cmdLogger.info("Successfully generated {} and {}.", keystorePath, truststorePath); + } else if (!keystore.exists() && truststore.exists()) { + cmdLogger.error("Tried to generate keystore {} for secure Apache NiFi configuration, but truststore file {} already exists. Aborting.", + keystorePath, truststorePath); + throw new RuntimeException("Will not generate keystore and truststore separately."); + } else if (keystore.exists() && !truststore.exists()) { + cmdLogger.error("Tried to generate truststore {} for secure Apache NiFi configuration, but keystore file {} already exists. Aborting.", + truststorePath, keystorePath); + throw new RuntimeException("Will not generate keystore and truststore separately."); + } else { + cmdLogger.info("Existing keystore and truststore detected: skipping Apache Nifi certificate generation."); + } + } else { + cmdLogger.warn("HTTPS is configured, but keystore and truststore are not specified. This will result in an invalid configuration."); + } + } else { + cmdLogger.info("No HTTPS configuration detected: skipping Apache Nifi certificate generation."); + } + } + + /** + * Attempts to add some reasonable guesses at desired SAN values that can be added to the generated + * certificate. + * @param nifiProperties The nifi.properties + * @return A Pair with IP SANs on the left and DNS SANs on the right + */ + private static Pair<String[], String[]> getSubjectAlternativeNames(Properties nifiProperties) { + Set<String> ipSubjectAlternativeNames = new HashSet<>(); + Set<String> dnsSubjectAlternativeNames = new HashSet<>(); + + try { + ipSubjectAlternativeNames.add(InetAddress.getLocalHost().getHostAddress()); + dnsSubjectAlternativeNames.add(InetAddress.getLocalHost().getHostName()); + } catch (UnknownHostException e) { + // This was just a courtesy, so we'll skip adding it + } + addSubjectAlternativeName(nifiProperties, NiFiProperties.REMOTE_INPUT_HOST, ipSubjectAlternativeNames, dnsSubjectAlternativeNames); + addSubjectAlternativeName(nifiProperties, NiFiProperties.WEB_HTTPS_HOST, ipSubjectAlternativeNames, dnsSubjectAlternativeNames); + addSubjectAlternativeName(nifiProperties, NiFiProperties.WEB_PROXY_HOST, ipSubjectAlternativeNames, dnsSubjectAlternativeNames); + addSubjectAlternativeName(nifiProperties, NiFiProperties.LOAD_BALANCE_ADDRESS, ipSubjectAlternativeNames, dnsSubjectAlternativeNames); + + // Not necessary to add as a SAN + ipSubjectAlternativeNames.remove(LOCALHOST_IP); + dnsSubjectAlternativeNames.remove(LOCALHOST_NAME); + + return new ImmutablePair<>(ipSubjectAlternativeNames.toArray(new String[ipSubjectAlternativeNames.size()]), Review comment: Interesting use of the `Pair` class, but perhaps there are alternatives. Although including IP Address Subject Alternative Names may be useful, in practice, they do not seem to be widely used, and are more brittle in cases of dynamic IP address assignments. To simplify both the return and the implementation, what do you think about changing the approach to include only DNS names? ########## File path: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/util/SecureNiFiConfigUtil.java ########## @@ -0,0 +1,191 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.bootstrap.util; + +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; +import org.apache.nifi.security.util.KeyStoreUtils; +import org.apache.nifi.security.util.StandardTlsConfiguration; +import org.apache.nifi.security.util.TlsConfiguration; +import org.apache.nifi.util.NiFiProperties; +import org.bouncycastle.util.IPAddress; +import org.slf4j.Logger; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.security.GeneralSecurityException; +import java.util.HashSet; +import java.util.List; +import java.util.Properties; +import java.util.Set; +import java.util.stream.Collectors; + +public class SecureNiFiConfigUtil { + + private static final int CERT_DURATION_DAYS = 60; + public static final String LOCALHOST_IP = "127.0.0.1"; + private static final String LOCALHOST_NAME = "localhost"; + + private SecureNiFiConfigUtil() { + + } + + /** + * If HTTPS is enabled (nifi.web.https.port is set), but the keystore file specified in nifi.security.keystore + * does not exist, this will generate a key pair and self-signed certificate, generate the associated keystore + * and truststore and write them to disk under the configured filepaths, generate a secure random keystore password + * and truststore password, and write these to the nifi.properties file. + * @param nifiPropertiesFilename The filename of the nifi.properties file + * @param cmdLogger The bootstrap logger + * @throws IOException can be thrown when writing keystores to disk + * @throws RuntimeException indicates a security exception while generating keystores + */ + public static void configureSecureNiFiProperties(String nifiPropertiesFilename, Logger cmdLogger) throws IOException, RuntimeException { + final File propertiesFile = new File(nifiPropertiesFilename); + final Properties nifiProperties = loadProperties(propertiesFile); + + if (!nifiProperties.getProperty(NiFiProperties.WEB_HTTPS_PORT, "").isEmpty()) { + String keystorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE, ""); + String truststorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE, ""); + if (!keystorePath.isEmpty() && !truststorePath.isEmpty()) { + File keystore = new File(keystorePath); + File truststore = new File(truststorePath); + if (!keystore.exists() && !truststore.exists()) { + TlsConfiguration tlsConfiguration = null; + cmdLogger.info("Generating default self-signed certificates, keystore and truststore for secure Apache NiFi configuration. " + + "This certificate will expire in {} days, and it is recommended to acquire your own certificates in order to properly secure Apache NiFi.", CERT_DURATION_DAYS); + try { + Pair<String[], String[]> subjectAlternativeNames = getSubjectAlternativeNames(nifiProperties); + tlsConfiguration = KeyStoreUtils.createTlsConfigAndNewKeystoreTruststore(StandardTlsConfiguration + .fromNiFiProperties(nifiProperties), CERT_DURATION_DAYS, subjectAlternativeNames.getLeft(), + subjectAlternativeNames.getRight()); + } catch (IOException e) { + cmdLogger.error("Encountered an I/O exception while generating secure Apache NiFi configuration.", e); + throw e; + } catch (GeneralSecurityException e) { + cmdLogger.error("Encountered a security exception while generating secure Apache NiFi configuration.", e); + throw new RuntimeException(e); + } + + // Move over the new stores from temp dir + Files.move(new File(tlsConfiguration.getKeystorePath()).toPath(), new File(keystorePath).toPath(), + StandardCopyOption.REPLACE_EXISTING); + Files.move(new File(tlsConfiguration.getTruststorePath()).toPath(), new File(truststorePath).toPath(), + StandardCopyOption.REPLACE_EXISTING); + + updateProperties(propertiesFile, tlsConfiguration); + + cmdLogger.info("Successfully generated {} and {}.", keystorePath, truststorePath); + } else if (!keystore.exists() && truststore.exists()) { + cmdLogger.error("Tried to generate keystore {} for secure Apache NiFi configuration, but truststore file {} already exists. Aborting.", + keystorePath, truststorePath); + throw new RuntimeException("Will not generate keystore and truststore separately."); + } else if (keystore.exists() && !truststore.exists()) { + cmdLogger.error("Tried to generate truststore {} for secure Apache NiFi configuration, but keystore file {} already exists. Aborting.", + truststorePath, keystorePath); + throw new RuntimeException("Will not generate keystore and truststore separately."); + } else { + cmdLogger.info("Existing keystore and truststore detected: skipping Apache Nifi certificate generation."); + } + } else { + cmdLogger.warn("HTTPS is configured, but keystore and truststore are not specified. This will result in an invalid configuration."); + } + } else { + cmdLogger.info("No HTTPS configuration detected: skipping Apache Nifi certificate generation."); Review comment: As with some other log messages, this is probably better as a debug log, or it may not be necessary. ########## File path: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/util/SecureNiFiConfigUtil.java ########## @@ -0,0 +1,191 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.bootstrap.util; + +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; +import org.apache.nifi.security.util.KeyStoreUtils; +import org.apache.nifi.security.util.StandardTlsConfiguration; +import org.apache.nifi.security.util.TlsConfiguration; +import org.apache.nifi.util.NiFiProperties; +import org.bouncycastle.util.IPAddress; +import org.slf4j.Logger; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.security.GeneralSecurityException; +import java.util.HashSet; +import java.util.List; +import java.util.Properties; +import java.util.Set; +import java.util.stream.Collectors; + +public class SecureNiFiConfigUtil { + + private static final int CERT_DURATION_DAYS = 60; + public static final String LOCALHOST_IP = "127.0.0.1"; + private static final String LOCALHOST_NAME = "localhost"; + + private SecureNiFiConfigUtil() { + + } + + /** + * If HTTPS is enabled (nifi.web.https.port is set), but the keystore file specified in nifi.security.keystore + * does not exist, this will generate a key pair and self-signed certificate, generate the associated keystore + * and truststore and write them to disk under the configured filepaths, generate a secure random keystore password + * and truststore password, and write these to the nifi.properties file. + * @param nifiPropertiesFilename The filename of the nifi.properties file + * @param cmdLogger The bootstrap logger + * @throws IOException can be thrown when writing keystores to disk + * @throws RuntimeException indicates a security exception while generating keystores + */ + public static void configureSecureNiFiProperties(String nifiPropertiesFilename, Logger cmdLogger) throws IOException, RuntimeException { + final File propertiesFile = new File(nifiPropertiesFilename); + final Properties nifiProperties = loadProperties(propertiesFile); + + if (!nifiProperties.getProperty(NiFiProperties.WEB_HTTPS_PORT, "").isEmpty()) { + String keystorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE, ""); + String truststorePath = nifiProperties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE, ""); + if (!keystorePath.isEmpty() && !truststorePath.isEmpty()) { + File keystore = new File(keystorePath); + File truststore = new File(truststorePath); + if (!keystore.exists() && !truststore.exists()) { + TlsConfiguration tlsConfiguration = null; + cmdLogger.info("Generating default self-signed certificates, keystore and truststore for secure Apache NiFi configuration. " + + "This certificate will expire in {} days, and it is recommended to acquire your own certificates in order to properly secure Apache NiFi.", CERT_DURATION_DAYS); + try { + Pair<String[], String[]> subjectAlternativeNames = getSubjectAlternativeNames(nifiProperties); + tlsConfiguration = KeyStoreUtils.createTlsConfigAndNewKeystoreTruststore(StandardTlsConfiguration + .fromNiFiProperties(nifiProperties), CERT_DURATION_DAYS, subjectAlternativeNames.getLeft(), + subjectAlternativeNames.getRight()); + } catch (IOException e) { Review comment: Given that the error messages do not provide different details, recommend combining `IOException` and `GeneralSecurityException` in a multi-catch block: ```suggestion } catch (final IOException|GenerallSecurityException e) { ``` ########## File path: nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/StandardTlsConfiguration.java ########## @@ -203,6 +205,38 @@ public static StandardTlsConfiguration fromNiFiProperties(NiFiProperties niFiPro return tlsConfiguration; } + /** + * Returns a {@link org.apache.nifi.security.util.TlsConfiguration} instantiated from the relevant {@link NiFiProperties} properties. + * + * @param niFiProperties the NiFi properties, as a simple java.util.Properties object + * @return a populated TlsConfiguration container object + */ + public static StandardTlsConfiguration fromNiFiProperties(Properties niFiProperties) { Review comment: Recommend changing the return type to `TlsConfiguration` to promote use the interface instead of the implementation class. ```suggestion public static TlsConfiguration fromNiFiProperties(final Properties niFiProperties) { ``` ########## File path: nifi-bootstrap/src/test/java/org/apache/nifi/bootstrap/util/TestSecureNiFiConfigUtil.java ########## @@ -0,0 +1,208 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.bootstrap.util; + +import org.apache.nifi.properties.NiFiPropertiesLoader; +import org.apache.nifi.security.util.KeyStoreUtils; +import org.apache.nifi.security.util.TlsException; +import org.apache.nifi.util.NiFiProperties; +import org.bouncycastle.asn1.x509.GeneralName; +import org.bouncycastle.util.IPAddress; +import org.junit.After; +import org.junit.Assert; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.xml.bind.DatatypeConverter; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; +import java.security.InvalidKeyException; +import java.security.Key; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; +import java.security.SignatureException; +import java.security.UnrecoverableKeyException; +import java.security.cert.Certificate; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +public class TestSecureNiFiConfigUtil { + private Logger logger = LoggerFactory.getLogger("org.apache.nifi.bootstrap.util.TestSecureNiFiConfigUtil"); + + private static final boolean EXPECT_STORES_TO_EXIST = true; + + private String nifiPropertiesFile = "src/test/resources/nifi.properties"; + private String keystorePath = "src/test/resources/keystore.p12"; + private String truststorePath = "src/test/resources/truststore.p12"; + + private NiFiProperties configureSecureNiFiProperties(String testPropertiesFile) throws IOException { + Files.copy(new File(testPropertiesFile).toPath(), new File(nifiPropertiesFile).toPath(), StandardCopyOption.REPLACE_EXISTING); + SecureNiFiConfigUtil.configureSecureNiFiProperties(nifiPropertiesFile, logger); + + return new NiFiPropertiesLoader().load(nifiPropertiesFile); + } + + private static String getFileHash(String filename) throws NoSuchAlgorithmException, IOException { + MessageDigest md = MessageDigest.getInstance("MD5"); Review comment: Although this is just for testing, the `MD5` algorithm should be avoided. Recommend using SHA-256, using Apache Commons `DigestUtils` or one of the other NiFi utility classes for hashing. ########## File path: nifi-bootstrap/src/test/java/org/apache/nifi/bootstrap/util/TestSecureNiFiConfigUtil.java ########## @@ -0,0 +1,208 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.bootstrap.util; + +import org.apache.nifi.properties.NiFiPropertiesLoader; +import org.apache.nifi.security.util.KeyStoreUtils; +import org.apache.nifi.security.util.TlsException; +import org.apache.nifi.util.NiFiProperties; +import org.bouncycastle.asn1.x509.GeneralName; +import org.bouncycastle.util.IPAddress; +import org.junit.After; +import org.junit.Assert; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.xml.bind.DatatypeConverter; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; +import java.security.InvalidKeyException; +import java.security.Key; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; +import java.security.SignatureException; +import java.security.UnrecoverableKeyException; +import java.security.cert.Certificate; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +public class TestSecureNiFiConfigUtil { + private Logger logger = LoggerFactory.getLogger("org.apache.nifi.bootstrap.util.TestSecureNiFiConfigUtil"); + + private static final boolean EXPECT_STORES_TO_EXIST = true; + + private String nifiPropertiesFile = "src/test/resources/nifi.properties"; + private String keystorePath = "src/test/resources/keystore.p12"; + private String truststorePath = "src/test/resources/truststore.p12"; + + private NiFiProperties configureSecureNiFiProperties(String testPropertiesFile) throws IOException { + Files.copy(new File(testPropertiesFile).toPath(), new File(nifiPropertiesFile).toPath(), StandardCopyOption.REPLACE_EXISTING); + SecureNiFiConfigUtil.configureSecureNiFiProperties(nifiPropertiesFile, logger); + + return new NiFiPropertiesLoader().load(nifiPropertiesFile); + } + + private static String getFileHash(String filename) throws NoSuchAlgorithmException, IOException { + MessageDigest md = MessageDigest.getInstance("MD5"); + md.update(Files.readAllBytes(Paths.get(filename))); + byte[] digest = md.digest(); + return DatatypeConverter.printHexBinary(digest).toUpperCase(); + } + + @After + public void cleanUp() throws IOException { + Files.deleteIfExists(new File(nifiPropertiesFile).toPath()); + Files.deleteIfExists(new File(keystorePath).toPath()); + Files.deleteIfExists(new File(truststorePath).toPath()); + } + + private void runTestWithExpectedSuccess(String testPropertiesFile, List<String> expectedSANs) throws IOException, TlsException, KeyStoreException, + NoSuchAlgorithmException, UnrecoverableKeyException, CertificateException, InvalidKeyException, NoSuchProviderException, SignatureException { + NiFiProperties niFiProperties = this.configureSecureNiFiProperties(testPropertiesFile); + + keystorePath = niFiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE); + Assert.assertEquals("./src/test/resources/keystore.p12", keystorePath); + Assert.assertFalse(niFiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE_PASSWD).isEmpty()); + Assert.assertEquals("PKCS12", niFiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE_TYPE)); + + char[] keyPassword = niFiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE_PASSWD).toCharArray(); + KeyStore keyStore = KeyStoreUtils.loadKeyStore(keystorePath, + niFiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE_PASSWD).toCharArray(), + niFiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE_TYPE)); + String alias = keyStore.aliases().nextElement(); + Assert.assertTrue(keyStore.isKeyEntry(alias)); + Key key = keyStore.getKey(alias, keyPassword); + Assert.assertEquals("PKCS#8", key.getFormat()); Review comment: Is there a reason for checking the key format? ########## File path: nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/StandardTlsConfiguration.java ########## @@ -203,6 +205,38 @@ public static StandardTlsConfiguration fromNiFiProperties(NiFiProperties niFiPro return tlsConfiguration; } + /** + * Returns a {@link org.apache.nifi.security.util.TlsConfiguration} instantiated from the relevant {@link NiFiProperties} properties. + * + * @param niFiProperties the NiFi properties, as a simple java.util.Properties object + * @return a populated TlsConfiguration container object + */ + public static StandardTlsConfiguration fromNiFiProperties(Properties niFiProperties) { + if (niFiProperties == null) { + throw new IllegalArgumentException("The NiFi properties cannot be null"); + } Review comment: This could be replaced with `Objects.requireNonNull()` ########## File path: nifi-bootstrap/src/test/java/org/apache/nifi/bootstrap/util/TestSecureNiFiConfigUtil.java ########## @@ -0,0 +1,208 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.bootstrap.util; + +import org.apache.nifi.properties.NiFiPropertiesLoader; +import org.apache.nifi.security.util.KeyStoreUtils; +import org.apache.nifi.security.util.TlsException; +import org.apache.nifi.util.NiFiProperties; +import org.bouncycastle.asn1.x509.GeneralName; +import org.bouncycastle.util.IPAddress; +import org.junit.After; +import org.junit.Assert; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.xml.bind.DatatypeConverter; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; +import java.security.InvalidKeyException; +import java.security.Key; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; +import java.security.SignatureException; +import java.security.UnrecoverableKeyException; +import java.security.cert.Certificate; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +public class TestSecureNiFiConfigUtil { + private Logger logger = LoggerFactory.getLogger("org.apache.nifi.bootstrap.util.TestSecureNiFiConfigUtil"); + + private static final boolean EXPECT_STORES_TO_EXIST = true; + + private String nifiPropertiesFile = "src/test/resources/nifi.properties"; Review comment: Recommend evaluating Class.getResource() as an alternative the specifying the full `src/test/resources` path here and in other places. ########## File path: nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/StandardTlsConfiguration.java ########## @@ -203,6 +205,38 @@ public static StandardTlsConfiguration fromNiFiProperties(NiFiProperties niFiPro return tlsConfiguration; } + /** + * Returns a {@link org.apache.nifi.security.util.TlsConfiguration} instantiated from the relevant {@link NiFiProperties} properties. + * + * @param niFiProperties the NiFi properties, as a simple java.util.Properties object + * @return a populated TlsConfiguration container object + */ + public static StandardTlsConfiguration fromNiFiProperties(Properties niFiProperties) { + if (niFiProperties == null) { + throw new IllegalArgumentException("The NiFi properties cannot be null"); + } + + String keystorePath = niFiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE); + String keystorePassword = niFiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE_PASSWD); + String keyPassword = niFiProperties.getProperty(NiFiProperties.SECURITY_KEY_PASSWD); + String keystoreType = niFiProperties.getProperty(NiFiProperties.SECURITY_KEYSTORE_TYPE); + String truststorePath = niFiProperties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE); + String truststorePassword = niFiProperties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE_PASSWD); + String truststoreType = niFiProperties.getProperty(NiFiProperties.SECURITY_TRUSTSTORE_TYPE); + String protocol = TLS_PROTOCOL_VERSION; + + final StandardTlsConfiguration tlsConfiguration = new StandardTlsConfiguration(keystorePath, keystorePassword, keyPassword, + keystoreType, truststorePath, truststorePassword, + truststoreType, protocol); + if (logger.isDebugEnabled()) { + logger.debug("Instantiating TlsConfiguration from NiFi properties: {}, {}, {}, {}, {}, {}, {}, {}", + keystorePath, tlsConfiguration.getKeystorePasswordForLogging(), tlsConfiguration.getKeyPasswordForLogging(), keystoreType, + truststorePath, tlsConfiguration.getTruststorePasswordForLogging(), truststoreType, protocol); + } Review comment: Is this debug log necessary? -- 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: [email protected]
