[GitHub] [nifi] CefBoud commented on a change in pull request #4857: NIFI-8230 Removed default Sensitive Properties Key

[GitBox <gi...@apache.org>]

CefBoud commented on a change in pull request #4857:
URL: https://github.com/apache/nifi/pull/4857#discussion_r584941994



##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/main/java/org/apache/nifi/properties/NiFiPropertiesLoader.java
##########
@@ -241,9 +239,53 @@ public NiFiProperties load(String path) {
      */
     public NiFiProperties get() {
         if (instance == null) {
-            instance = loadDefault();
+            instance = setSensitivePropertiesKey(loadDefault());
         }
 
         return instance;
     }
+
+    private NiFiProperties setSensitivePropertiesKey(final NiFiProperties niFiProperties) {
+        NiFiProperties processedProperties = niFiProperties;
+
+        if (isKeyGenerationRequired(niFiProperties)) {
+            final File flowConfigurationFlow = niFiProperties.getFlowConfigurationFile();
+            if (flowConfigurationFlow.exists()) {
+                logger.error("Flow Configuration File [{}] Found: Migration Required for blank Sensitive Properties Key [{}]", NiFiProperties.SENSITIVE_PROPS_KEY);
+                final String message = String.format("Sensitive Properties Key [%s] not found: %s", NiFiProperties.SENSITIVE_PROPS_KEY, MIGRATION_INSTRUCTIONS);
+                throw new SensitivePropertyProtectionException(message);
+            }
+
+            logger.warn("Generating Random Sensitive Properties Key [{}]", NiFiProperties.SENSITIVE_PROPS_KEY);
+            final SecureRandom secureRandom = new SecureRandom();
+            final byte[] sensitivePropertiesKeyBinary = new byte[SENSITIVE_PROPERTIES_KEY_LENGTH];
+            secureRandom.nextBytes(sensitivePropertiesKeyBinary);
+            final String sensitivePropertiesKey = KEY_ENCODER.encodeToString(sensitivePropertiesKeyBinary);
+            try {
+                final File niFiPropertiesFile = new File(CryptoUtils.getDefaultFilePath());

Review comment:
       `getDefaultFilePath `can be called up to three times in the case of generation. First inside `get`, then  2 times inside `setSensitivePropertiesKey`. It prints a logging message that says "_Determined default nifi.properties path to be ..._" that may appear 3 times. How about using a variable to store the default path ? 




----------------------------------------------------------------
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:
users@infra.apache.org