Re: [PR] NIFI-12501 Encrypt MiNiFi bootstrap properties [nifi]

2024-01-22 Thread via GitHub


briansolo1985 commented on PR #8151:
URL: https://github.com/apache/nifi/pull/8151#issuecomment-1903544615

   Thanks for the updates. Looked good to me. Merged.


-- 
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.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] NIFI-12501 Encrypt MiNiFi bootstrap properties [nifi]

2024-01-22 Thread via GitHub


briansolo1985 closed pull request #8151: NIFI-12501 Encrypt MiNiFi bootstrap 
properties
URL: https://github.com/apache/nifi/pull/8151


-- 
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.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] NIFI-12501 Encrypt MiNiFi bootstrap properties [nifi]

2024-01-19 Thread via GitHub


ferencerdei commented on PR #8151:
URL: https://github.com/apache/nifi/pull/8151#issuecomment-1900366458

   Thanks for the review comments, I resolved the requested changes and 
conflicts as well.


-- 
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.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] NIFI-12501 Encrypt MiNiFi bootstrap properties [nifi]

2024-01-05 Thread via GitHub


briansolo1985 commented on code in PR #8151:
URL: https://github.com/apache/nifi/pull/8151#discussion_r1442662592


##
minifi/minifi-bootstrap/src/test/java/org/apache/nifi/minifi/bootstrap/status/reporters/StatusLoggerTest.java:
##
@@ -142,10 +144,7 @@ public void testError() {
 // Exception testing

Review Comment:
   I know it's not your code, but please remove the comment, as it does not 
hold any additional or valuable information



##
minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-properties-loader/src/main/java/org/apache/nifi/minifi/properties/DuplicateDetectingProperties.java:
##
@@ -0,0 +1,54 @@
+/*
+ * 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.minifi.properties;
+
+import java.util.HashSet;
+import java.util.Properties;
+import java.util.Set;
+
+class DuplicateDetectingProperties extends Properties {
+// Only need to retain Properties key. This will help prevent possible 
inadvertent exposure of sensitive Properties value
+private final Set duplicateKeys = new HashSet<>();
+private final Set redundantKeys = new HashSet<>();
+
+public DuplicateDetectingProperties() {
+super();
+}
+
+public Set duplicateKeySet() {
+return duplicateKeys;
+}
+
+public Set redundantKeySet() {
+return redundantKeys;
+}
+
+@Override
+public Object put(Object key, Object value) {
+Object existingValue = super.put(key, value);
+if (existingValue != null) {
+if (existingValue.toString().equals(value.toString())) {
+redundantKeys.add(key.toString());
+return existingValue;

Review Comment:
   I know it's just a copy paste code, but this return is unnecessary. 
Practically we always return with value.



##
minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-properties-loader/src/main/java/org/apache/nifi/minifi/properties/BootstrapProperties.java:
##
@@ -41,4 +50,22 @@ public String getProperty(String key) {
 .orElseGet(() -> super.getProperty(key));
 }
 
+@Override
+public String getProperty(String key, String defaultValue) {

Review Comment:
   With introducing this method the previous one could be refactored to
   ```
   @Override
   public String getProperty(String key) {
   return getProperty(key, null);
   }
   ```



##
minifi/minifi-commons/minifi-commons-api/src/main/java/org/apache/nifi/minifi/commons/api/MiNiFiProperties.java:
##
@@ -63,7 +63,7 @@ public enum MiNiFiProperties {
 NIFI_MINIFI_SECURITY_SSL_PROTOCOL("nifi.minifi.security.ssl.protocol", 
null, false, false, VALID),
 NIFI_MINIFI_FLOW_USE_PARENT_SSL("nifi.minifi.flow.use.parent.ssl", null, 
false, true, VALID),
 NIFI_MINIFI_SENSITIVE_PROPS_KEY("nifi.minifi.sensitive.props.key", null, 
true, false, VALID),
-
NIFI_MINIFI_SENSITIVE_PROPS_ALGORITHM("nifi.minifi.sensitive.props.algorithm", 
null, true, false, VALID),
+
NIFI_MINIFI_SENSITIVE_PROPS_ALGORITHM("nifi.minifi.sensitive.props.algorithm", 
null, false, false, VALID),

Review Comment:
   Nice catch!



##
minifi/minifi-bootstrap/src/test/java/org/apache/nifi/minifi/bootstrap/service/GracefulShutdownParameterProviderTest.java:
##
@@ -46,9 +49,11 @@ class GracefulShutdownParameterProviderTest {
 @NullSource
 @ValueSource(strings = {"notAnInteger", "-1"})
 void 
testGetBootstrapPropertiesShouldReturnDefaultShutdownPropertyValue(String 
shutdownProperty) throws IOException {
-Properties properties = new Properties();
+BootstrapProperties properties = mock(BootstrapProperties.class);
 if (shutdownProperty != null) {
-properties.setProperty(GRACEFUL_SHUTDOWN_PROP, shutdownProperty);

Review Comment:
   We could eliminate the if with 
Optional.ofNullable(shutdownProperty).orElse(DEFAULT_GRACEFUL_SHUTDOWN_VALUE)



##
minifi/minifi-bootstrap/src/test/java/org/apache/nifi/minifi/bootstrap/configuration/ingestors/FileChangeIngestorTest.java:
##
@@ -60,15 +63,15 @@ public void setUp() {
 notifierSpy = Mockito.spy(new FileChangeIngestor());
 

Re: [PR] NIFI-12501 Encrypt MiNiFi bootstrap properties [nifi]

2023-12-13 Thread via GitHub


exceptionfactory commented on code in PR #8151:
URL: https://github.com/apache/nifi/pull/8151#discussion_r1425531380


##
minifi/minifi-toolkit/minifi-toolkit-encrypt-config/src/main/java/org/apache/nifi/minifi/toolkit/config/command/MiNiFiEncryptConfig.java:
##
@@ -0,0 +1,146 @@
+/*
+ * 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.minifi.toolkit.config.command;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
+import java.security.SecureRandom;
+import java.util.HashSet;
+import java.util.Optional;
+import java.util.Set;
+import org.apache.commons.codec.binary.Hex;
+import org.apache.nifi.minifi.properties.BootstrapProperties;
+import org.apache.nifi.minifi.properties.BootstrapPropertiesLoader;
+import org.apache.nifi.minifi.properties.ProtectedBootstrapProperties;
+import 
org.apache.nifi.minifi.toolkit.config.transformer.ApplicationPropertiesFileTransformer;
+import 
org.apache.nifi.minifi.toolkit.config.transformer.BootstrapConfigurationFileTransformer;
+import org.apache.nifi.minifi.toolkit.config.transformer.FileTransformer;
+import org.apache.nifi.properties.AesGcmSensitivePropertyProvider;
+import org.apache.nifi.properties.SensitivePropertyProvider;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import picocli.CommandLine.Command;
+import picocli.CommandLine.Option;
+
+/**
+ * Shared Encrypt Configuration for NiFi and NiFi Registry
+ */
+@Command(
+name = "encrypt-config",
+sortOptions = false,
+mixinStandardHelpOptions = true,
+usageHelpWidth = 160,
+separator = " ",
+version = {
+"Java ${java.version} (${java.vendor} ${java.vm.name} 
${java.vm.version})"
+},
+descriptionHeading = "Description: ",
+description = {
+"encrypt-config supports protection of sensitive values in 
Apache MiNiFi"
+}
+)
+public class MiNiFiEncryptConfig implements Runnable{
+
+static final String BOOTSTRAP_ROOT_KEY_PROPERTY = 
"minifi.bootstrap.sensitive.key";
+
+private static final String WORKING_FILE_NAME_FORMAT = "%s.%d.working";
+private static final int KEY_LENGTH = 32;
+
+@Option(
+names = {"-b", "--bootstrapConf"},
+description = "Path to file containing Bootstrap Configuration 
[bootstrap.conf] for optional root key and property protection scheme settings"
+)
+Path bootstrapConfPath;
+
+@Option(
+names = {"-B", "--outputBootstrapConf"},
+description = "Path to output file for Bootstrap Configuration 
[bootstrap.conf] with root key configured"
+)
+Path outputBootstrapConf;
+
+protected final Logger logger = LoggerFactory.getLogger(getClass());
+
+@Override
+public void run() {
+processBootstrapConf();
+}
+
+/**
+ * Process bootstrap.conf writing new Root Key to specified Root Key 
Property when bootstrap.conf is specified
+ *
+ */
+protected void processBootstrapConf() {
+BootstrapProperties unprotectedProperties = 
BootstrapPropertiesLoader.load(bootstrapConfPath.toFile());

Review Comment:
   Thanks for confirming, that's great!



-- 
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.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] NIFI-12501 Encrypt MiNiFi bootstrap properties [nifi]

2023-12-13 Thread via GitHub


exceptionfactory commented on code in PR #8151:
URL: https://github.com/apache/nifi/pull/8151#discussion_r1425529797


##
minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-runtime/src/main/java/org/apache/nifi/minifi/MiNiFi.java:
##
@@ -216,12 +221,43 @@ public void run() {
  * @param args things which are ignored
  */
 public static void main(String[] args) {
-logger.info("Launching MiNiFi...");
+LOGGER.info("Launching MiNiFi...");
 try {
-NiFiProperties niFiProperties = 
MultiSourceMinifiProperties.getInstance();
-new MiNiFi(niFiProperties);
+NiFiProperties properties = getValidatedMiNifiProperties(args);
+new MiNiFi(properties);
 } catch (final Throwable t) {
-logger.error("Failure to launch MiNiFi due to " + t, t);
+LOGGER.error("Failure to launch MiNiFi due to " + t, t);
+}
+}
+
+protected static NiFiProperties getValidatedMiNifiProperties(String[] 
args) {
+NiFiProperties properties = initializeProperties(args, 
createBootstrapClassLoader());
+properties.validate();
+return properties;
+}
+
+private static NiFiProperties initializeProperties(final String[] args, 
final ClassLoader boostrapLoader) {
+ClassLoader contextClassLoader = 
Thread.currentThread().getContextClassLoader();
+String key = getFormattedKey(args);
+
+Thread.currentThread().setContextClassLoader(boostrapLoader);
+
+try {
+Class propsLoaderClass = 
Class.forName("org.apache.nifi.minifi.properties.MiNiFiPropertiesLoader", true, 
boostrapLoader);
+Method withKeyMethod = propsLoaderClass.getMethod("withKey", 
String.class);
+Object loaderInstance = withKeyMethod.invoke(null, key);

Review Comment:
   Thanks for confirming, that's good to know.



-- 
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.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] NIFI-12501 Encrypt MiNiFi bootstrap properties [nifi]

2023-12-13 Thread via GitHub


exceptionfactory commented on code in PR #8151:
URL: https://github.com/apache/nifi/pull/8151#discussion_r1425528372


##
minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-properties-loader/src/main/java/org/apache/nifi/minifi/properties/ProtectedMiNiFiProperties.java:
##
@@ -0,0 +1,241 @@
+/*
+ * 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.minifi.properties;
+
+import static java.util.Arrays.asList;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.nifi.minifi.commons.api.MiNiFiProperties;
+import org.apache.nifi.properties.ApplicationPropertiesProtector;
+import org.apache.nifi.properties.ProtectedProperties;
+import org.apache.nifi.properties.SensitivePropertyProtectionException;
+import org.apache.nifi.properties.SensitivePropertyProtector;
+import org.apache.nifi.properties.SensitivePropertyProvider;
+import org.apache.nifi.util.NiFiProperties;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Decorator class for intermediate phase when {@link MiNiFiPropertiesLoader} 
loads the
+ * raw properties file and performs unprotection activities before returning a 
clean
+ * implementation of {@link NiFiProperties}.
+ * This encapsulates the sensitive property access logic from external 
consumers
+ * of {@code NiFiProperties}.
+ */
+public class ProtectedMiNiFiProperties extends NiFiProperties implements 
ProtectedProperties,
+SensitivePropertyProtector {
+private static final Logger logger = 
LoggerFactory.getLogger(ProtectedMiNiFiProperties.class);
+public static final String ADDITIONAL_SENSITIVE_PROPERTIES_KEY = 
"nifi.sensitive.props.additional.keys";
+public static final List DEFAULT_SENSITIVE_PROPERTIES = new 
ArrayList<>(asList(
+SECURITY_KEY_PASSWD,
+SECURITY_KEYSTORE_PASSWD,
+SECURITY_TRUSTSTORE_PASSWD,
+SENSITIVE_PROPS_KEY,
+REPOSITORY_ENCRYPTION_KEY_PROVIDER_KEYSTORE_PASSWORD,
+SECURITY_USER_OIDC_CLIENT_SECRET

Review Comment:
   The OIDC and Repository Encryption Key properties are not applicable, so at 
least those should be removed.



-- 
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.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] NIFI-12501 Encrypt MiNiFi bootstrap properties [nifi]

2023-12-13 Thread via GitHub


exceptionfactory commented on code in PR #8151:
URL: https://github.com/apache/nifi/pull/8151#discussion_r1425522467


##
minifi/minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/service/MiNiFiExecCommandProvider.java:
##
@@ -159,15 +183,58 @@ private String buildClassPath(File confDir, File libDir) {
 .collect(joining(File.pathSeparator));
 }
 
-private List getJavaAdditionalArgs(Properties props) {
-return props.entrySet()
+private List getJavaAdditionalArgs(BootstrapProperties props) {
+return props.getPropertyKeys()
 .stream()
-.filter(entry -> ((String) 
entry.getKey()).startsWith(JAVA_ARG_KEY_PREFIX))
-.map(entry -> (String) entry.getValue())
+.filter(entry -> entry.startsWith(JAVA_ARG_KEY_PREFIX))
+.map(props::getProperty)
 .toList();
 }
 
 private String systemProperty(String key, Object value) {
 return String.format(SYSTEM_PROPERTY_TEMPLATE, key, value);
 }
+
+private boolean isSensitiveKeyPresent(BootstrapProperties 
bootstrapProperties) {
+return bootstrapProperties.containsKey(MINIFI_BOOTSTRAP_SENSITIVE_KEY) 
&& 
!StringUtils.isBlank(bootstrapProperties.getProperty(MINIFI_BOOTSTRAP_SENSITIVE_KEY));
+}
+
+private Path createSensitiveKeyFile(File confDir) {

Review Comment:
   Thanks for the reply, given that this is a new implementation, although 
based on the existing one, this is a good opportunity to change the approach 
for MiNiFi itself.



-- 
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.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] NIFI-12501 Encrypt MiNiFi bootstrap properties [nifi]

2023-12-13 Thread via GitHub


ferencerdei commented on code in PR #8151:
URL: https://github.com/apache/nifi/pull/8151#discussion_r1424971904


##
minifi/minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/service/MiNiFiExecCommandProvider.java:
##
@@ -159,15 +183,58 @@ private String buildClassPath(File confDir, File libDir) {
 .collect(joining(File.pathSeparator));
 }
 
-private List getJavaAdditionalArgs(Properties props) {
-return props.entrySet()
+private List getJavaAdditionalArgs(BootstrapProperties props) {
+return props.getPropertyKeys()
 .stream()
-.filter(entry -> ((String) 
entry.getKey()).startsWith(JAVA_ARG_KEY_PREFIX))
-.map(entry -> (String) entry.getValue())
+.filter(entry -> entry.startsWith(JAVA_ARG_KEY_PREFIX))
+.map(props::getProperty)
 .toList();
 }
 
 private String systemProperty(String key, Object value) {
 return String.format(SYSTEM_PROPERTY_TEMPLATE, key, value);
 }
+
+private boolean isSensitiveKeyPresent(BootstrapProperties 
bootstrapProperties) {
+return bootstrapProperties.containsKey(MINIFI_BOOTSTRAP_SENSITIVE_KEY) 
&& 
!StringUtils.isBlank(bootstrapProperties.getProperty(MINIFI_BOOTSTRAP_SENSITIVE_KEY));
+}
+
+private Path createSensitiveKeyFile(File confDir) {

Review Comment:
   I wanted to keep to logic as consistent as possible with the NiFi, so we can 
easily switch when we change the bootstrap / application properties file 
structure to the same as in NiFi. But yea, It can be read from bootstrap 
directly (it could be even in NiFi.)



##
minifi/minifi-commons/minifi-commons-utils/src/main/java/org/apache/nifi/minifi/commons/utils/PropertyUtil.java:
##
@@ -48,4 +62,96 @@ private static Optional getPropertyValue(String 
name, Map properti
 private static Stream keyPermutations(String name) {
 return Stream.of(name, name.replace(DOT, UNDERSCORE), 
name.replace(HYPHEN, UNDERSCORE), name.replace(DOT, UNDERSCORE).replace(HYPHEN, 
UNDERSCORE)).distinct();
 }
+
+public static String getFormattedKey(String[] args) {

Review Comment:
   Moved to SensitivePropertyUtils



##
minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-properties-loader/src/main/java/org/apache/nifi/minifi/properties/MiNiFiPropertiesLoader.java:
##
@@ -0,0 +1,168 @@
+/*
+ * 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.minifi.properties;
+
+import java.io.BufferedInputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.InputStream;
+import java.util.Properties;
+import org.apache.nifi.properties.AesGcmSensitivePropertyProvider;
+import org.apache.nifi.properties.SensitivePropertyProvider;
+import org.apache.nifi.util.NiFiBootstrapUtils;
+import org.apache.nifi.util.NiFiProperties;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class MiNiFiPropertiesLoader {
+
+private static final Logger logger = 
LoggerFactory.getLogger(MiNiFiPropertiesLoader.class);
+private static final String DEFAULT_APPLICATION_PROPERTIES_FILE_PATH = 
NiFiBootstrapUtils.getDefaultApplicationPropertiesFilePath();
+
+private NiFiProperties instance;
+private String keyHex;
+
+private MiNiFiPropertiesLoader() {
+}
+
+/**
+ * Returns an instance of the loader configured with the key.
+ * 
+ * 
+ * NOTE: This method is used reflectively by the process which starts 
MiNiFi
+ * so changes to it must be made in conjunction with that mechanism.
+ *
+ * @param keyHex the key used to encrypt any sensitive properties
+ * @return the configured loader
+ */
+public static MiNiFiPropertiesLoader withKey(String keyHex) {
+MiNiFiPropertiesLoader loader = new MiNiFiPropertiesLoader();
+loader.setKeyHex(keyHex);
+return loader;
+}

Review Comment:
   done



##
minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-runtime/pom.xml:
##
@@ -76,5 +77,11 @@ limitations under the License.
 com.fasterxml.jackson.core
 jackson-databind
 
+
+
+   

Re: [PR] NIFI-12501 Encrypt MiNiFi bootstrap properties [nifi]

2023-12-12 Thread via GitHub


exceptionfactory commented on code in PR #8151:
URL: https://github.com/apache/nifi/pull/8151#discussion_r1424788213


##
minifi/minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/service/MiNiFiExecCommandProvider.java:
##
@@ -116,8 +134,14 @@ public List getMiNiFiExecCommand(int listenPort, 
File workingDir) throws
 systemProperty(BOOTSTRAP_LOG_FILE_EXTENSION, 
minifiBootstrapLogFileExtension)
 );
 
-return List.of(javaCommand, javaAdditionalArgs, systemProperties, 
List.of(MINIFI_FULLY_QUALIFIED_CLASS_NAME))
-.stream()
+String sensitiveParameter = null;
+if (isSensitiveKeyPresent(bootstrapProperties)) {
+Path sensitiveKeyFile = createSensitiveKeyFile(confDir);
+writeSensitiveKeyFile(bootstrapProperties, sensitiveKeyFile);
+sensitiveParameter = "-K " + 
sensitiveKeyFile.toFile().getAbsolutePath();

Review Comment:
   As mentioned in writing the key, it seems like it one option would be to 
pass the path to the bootstrap configuration and having the called process read 
the file, instead of creating a new copy of the key in a new file.



##
minifi/minifi-commons/minifi-commons-utils/src/main/java/org/apache/nifi/minifi/commons/utils/PropertyUtil.java:
##
@@ -48,4 +62,96 @@ private static Optional getPropertyValue(String 
name, Map properti
 private static Stream keyPermutations(String name) {
 return Stream.of(name, name.replace(DOT, UNDERSCORE), 
name.replace(HYPHEN, UNDERSCORE), name.replace(DOT, UNDERSCORE).replace(HYPHEN, 
UNDERSCORE)).distinct();
 }
+
+public static String getFormattedKey(String[] args) {
+String key = null;
+Optional keyFileLocation = getKeyFileArg(args);
+if (keyFileLocation.isPresent()) {
+LOGGER.debug("The bootstrap process provided the {} flag", 
KEY_FILE_FLAG);
+key = getKeyFromKeyFileAndPrune(keyFileLocation.get());
+}
+
+// Format the key (check hex validity and remove spaces)
+return getFormattedKey(key);
+}
+
+public static String getFormattedKey(String unformattedKey) {
+String key = formatHexKey(unformattedKey);
+
+if (isNotEmpty(key) && !isHexKeyValid(key)) {
+throw new IllegalArgumentException("The key was not provided in 
valid hex format and of the correct length");
+} else {
+return key;
+}
+}
+
+private static String formatHexKey(String input) {
+return Optional.ofNullable(input)
+.map(String::trim)
+.filter(PropertyUtil::isNotEmpty)
+.map(str -> str.replaceAll("[^0-9a-fA-F]", EMPTY).toLowerCase())
+.orElse(EMPTY);
+}
+
+private static boolean isHexKeyValid(String key) {
+return Optional.ofNullable(key)
+.map(String::trim)
+.filter(PropertyUtil::isNotEmpty)
+.map(__ -> Arrays.asList(128, 196, 256).contains(key.length() * 4) 
&& key.matches("^[0-9a-fA-F]*$"))

Review Comment:
   The `196` option should be removed for simplicity, and as mentioned in the 
Jira issue, this is a good opportunity to remove the `128` option and simplify 
support to only 256 bit keys.



##
minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-properties-loader/src/main/java/org/apache/nifi/minifi/properties/MiNiFiPropertiesLoader.java:
##
@@ -0,0 +1,168 @@
+/*
+ * 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.minifi.properties;
+
+import java.io.BufferedInputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.InputStream;
+import java.util.Properties;
+import org.apache.nifi.properties.AesGcmSensitivePropertyProvider;
+import org.apache.nifi.properties.SensitivePropertyProvider;
+import org.apache.nifi.util.NiFiBootstrapUtils;
+import org.apache.nifi.util.NiFiProperties;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class MiNiFiPropertiesLoader {
+
+private static final Logger logger = 
LoggerFactory.getLogger(MiNiFiPropertiesLoader.class);
+private static final String DEFAULT_APPLICATION_PROPERTIES_FILE_PATH =