NIFI-4237 Added working test for StringEncryptor decryption of sensitive flow values in FlowFromDOMFactory.

NIFI-4237 Cleaned up unused alternate approaches.

NIFI-4237 Added failing unit test for better error message.

NIFI-4237 Added logic to capture unhelpful encryption exception and provide context in message. All tests pass.

This closes #2077

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/serialization/FlowFromDOMFactory.java

@@ -16,9 +16,18 @@
  */
 package org.apache.nifi.controller.serialization;
 
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
 import org.apache.nifi.connectable.Size;
 import org.apache.nifi.controller.ScheduledState;
 import org.apache.nifi.controller.service.ControllerServiceState;
+import org.apache.nifi.encrypt.EncryptionException;
 import org.apache.nifi.encrypt.StringEncryptor;
 import org.apache.nifi.groups.RemoteProcessGroupPortDescriptor;
 import org.apache.nifi.remote.StandardRemoteProcessGroupPortDescriptor;
@@ -39,19 +48,14 @@ import org.apache.nifi.web.api.dto.ProcessorConfigDTO;
 import org.apache.nifi.web.api.dto.ProcessorDTO;
 import org.apache.nifi.web.api.dto.RemoteProcessGroupDTO;
 import org.apache.nifi.web.api.dto.ReportingTaskDTO;
+import org.jasypt.exceptions.EncryptionOperationNotPossibleException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.w3c.dom.Element;
 import org.w3c.dom.NodeList;
 
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.TimeUnit;
-
 public class FlowFromDOMFactory {
+    private static final Logger logger = LoggerFactory.getLogger(FlowFromDOMFactory.class);
 
     public static BundleDTO getBundle(final Element bundleElement) {
         if (bundleElement == null) {
@@ -492,7 +496,14 @@ public class FlowFromDOMFactory {
 
     private static String decrypt(final String value, final StringEncryptor encryptor) {
         if (value != null && value.startsWith(FlowSerializer.ENC_PREFIX) && value.endsWith(FlowSerializer.ENC_SUFFIX)) {
-            return encryptor.decrypt(value.substring(FlowSerializer.ENC_PREFIX.length(), value.length() - FlowSerializer.ENC_SUFFIX.length()));
+            try {
+                return encryptor.decrypt(value.substring(FlowSerializer.ENC_PREFIX.length(), value.length() - FlowSerializer.ENC_SUFFIX.length()));
+            } catch (EncryptionException | EncryptionOperationNotPossibleException e) {
+                final String moreDescriptiveMessage = "There was a problem decrypting a sensitive flow configuration value. " +
+                        "Check that the nifi.sensitive.props.key value in nifi.properties matches the value used to encrypt the flow.xml.gz file";
+                logger.error(moreDescriptiveMessage, e);
+                throw new EncryptionException(moreDescriptiveMessage, e);
+            }
         } else {
             return value;
         }

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/groovy/org/apache/nifi/controller/serialization/FlowFromDOMFactoryTest.groovy

@@ -0,0 +1,150 @@
+/*
+ * 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.controller.serialization
+
+import org.apache.commons.codec.binary.Hex
+import org.apache.nifi.encrypt.EncryptionException
+import org.apache.nifi.encrypt.StringEncryptor
+import org.apache.nifi.properties.StandardNiFiProperties
+import org.apache.nifi.security.kms.CryptoUtils
+import org.apache.nifi.security.util.EncryptionMethod
+import org.apache.nifi.util.NiFiProperties
+import org.bouncycastle.jce.provider.BouncyCastleProvider
+import org.junit.After
+import org.junit.Before
+import org.junit.BeforeClass
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+
+import javax.crypto.Cipher
+import javax.crypto.SecretKey
+import javax.crypto.SecretKeyFactory
+import javax.crypto.spec.PBEKeySpec
+import javax.crypto.spec.PBEParameterSpec
+import java.security.Security
+
+import static groovy.test.GroovyAssert.shouldFail
+
+@RunWith(JUnit4.class)
+class FlowFromDOMFactoryTest {
+    private static final Logger logger = LoggerFactory.getLogger(FlowFromDOMFactoryTest.class)
+
+    private static final String DEFAULT_PASSWORD = "nififtw!"
+    private static final byte[] DEFAULT_SALT = new byte[8]
+    private static final int DEFAULT_ITERATION_COUNT = 0
+
+    private static final String ALGO = NiFiProperties.NF_SENSITIVE_PROPS_ALGORITHM
+    private static final String PROVIDER = NiFiProperties.NF_SENSITIVE_PROPS_PROVIDER
+    private static final String KEY = NiFiProperties.NF_SENSITIVE_PROPS_KEY
+
+    @BeforeClass
+    static void setUpOnce() throws Exception {
+        Security.addProvider(new BouncyCastleProvider())
+
+        logger.metaClass.methodMissing = { String name, args ->
+            logger.info("[${name?.toUpperCase()}] ${(args as List).join(" ")}")
+        }
+    }
+
+    @Before
+    void setUp() throws Exception {
+
+    }
+
+    @After
+    void tearDown() throws Exception {
+
+    }
+
+    @Test
+    void testShouldDecryptSensitiveFlowValue() throws Exception {
+        // Arrange
+        final String plaintext = "This is a plaintext message."
+
+        // Encrypt the value
+
+        // Hard-coded 0x00 * 16
+        byte[] salt = new byte[16]
+        Cipher cipher = generateCipher(true, DEFAULT_PASSWORD, salt)
+
+        byte[] cipherBytes = cipher.doFinal(plaintext.bytes)
+        byte[] saltAndCipherBytes = CryptoUtils.concatByteArrays(salt, cipherBytes)
+        String cipherTextHex = Hex.encodeHexString(saltAndCipherBytes)
+        String wrappedCipherText = "enc{${cipherTextHex}}"
+        logger.info("Cipher text: ${wrappedCipherText}")
+
+        final Map MOCK_PROPERTIES = [(ALGO): EncryptionMethod.MD5_128AES.algorithm, (PROVIDER): EncryptionMethod.MD5_128AES.provider, (KEY): DEFAULT_PASSWORD]
+        NiFiProperties mockProperties = new StandardNiFiProperties(new Properties(MOCK_PROPERTIES))
+        StringEncryptor flowEncryptor = StringEncryptor.createEncryptor(mockProperties)
+
+        // Act
+        String recovered = FlowFromDOMFactory.decrypt(wrappedCipherText, flowEncryptor)
+        logger.info("Recovered: ${recovered}")
+
+        // Assert
+        assert plaintext == recovered
+    }
+
+    @Test
+    void testShouldProvideBetterErrorMessageOnDecryptionFailure() throws Exception {
+        // Arrange
+        final String plaintext = "This is a plaintext message."
+
+        // Encrypt the value
+
+        // Hard-coded 0x00 * 16
+        byte[] salt = new byte[16]
+        Cipher cipher = generateCipher(true, DEFAULT_PASSWORD, salt)
+
+        byte[] cipherBytes = cipher.doFinal(plaintext.bytes)
+        byte[] saltAndCipherBytes = CryptoUtils.concatByteArrays(salt, cipherBytes)
+        String cipherTextHex = Hex.encodeHexString(saltAndCipherBytes)
+        String wrappedCipherText = "enc{${cipherTextHex}}"
+        logger.info("Cipher text: ${wrappedCipherText}")
+
+        // Change the password in "nifi.properties" so it doesn't match the "flow"
+        final Map MOCK_PROPERTIES = [(ALGO): EncryptionMethod.MD5_128AES.algorithm, (PROVIDER): EncryptionMethod.MD5_128AES.provider, (KEY): DEFAULT_PASSWORD.reverse()]
+        NiFiProperties mockProperties = new StandardNiFiProperties(new Properties(MOCK_PROPERTIES))
+        StringEncryptor flowEncryptor = StringEncryptor.createEncryptor(mockProperties)
+
+        // Act
+        def msg = shouldFail(EncryptionException) {
+            String recovered = FlowFromDOMFactory.decrypt(wrappedCipherText, flowEncryptor)
+            logger.info("Recovered: ${recovered}")
+        }
+        logger.expected(msg)
+
+        // Assert
+        assert msg.message =~ "Check that the ${KEY} value in nifi.properties matches the value used to encrypt the flow.xml.gz file"
+    }
+
+    private
+    static Cipher generateCipher(boolean encryptMode, String password = DEFAULT_PASSWORD, byte[] salt = DEFAULT_SALT, int iterationCount = DEFAULT_ITERATION_COUNT) {
+        // Initialize secret key from password
+        final PBEKeySpec pbeKeySpec = new PBEKeySpec(password.toCharArray())
+        final SecretKeyFactory factory = SecretKeyFactory.getInstance(EncryptionMethod.MD5_128AES.algorithm, EncryptionMethod.MD5_128AES.provider)
+        SecretKey tempKey = factory.generateSecret(pbeKeySpec)
+
+        final PBEParameterSpec parameterSpec = new PBEParameterSpec(salt, iterationCount)
+        Cipher cipher = Cipher.getInstance(EncryptionMethod.MD5_128AES.algorithm, EncryptionMethod.MD5_128AES.provider)
+        cipher.init((encryptMode ? Cipher.ENCRYPT_MODE : Cipher.DECRYPT_MODE) as int, tempKey, parameterSpec)
+        cipher
+    }
+}