NIFI-1257 Resolved legacy compatibility issue with NiFi legacy KDF sa…

…lt length dependent on cipher block size.

Replaced screenshot for NiFiLegacy salt encoding.
Added description of legacy salt length determination in admin guide.
Added logic for NiFiLegacyCipherProvider to generate and validate salts of the length determined by the cipher block size.
Changed EncryptContent to default to Bcrypt KDF.

Signed-off-by: Aldrin Piri <aldrin@apache.org>

nifi-docs/src/main/asciidoc/administration-guide.adoc

@@ -383,7 +383,7 @@ Currently, KDFs are ingested by `CipherProvider` implementations and return a fu
 Here are the KDFs currently supported by NiFi (primarily in the `EncryptContent` processor for password-based encryption (PBE)) and relevant notes:
 
 * NiFi Legacy KDF
-** The original KDF used by NiFi for internal key derivation for PBE, this is 1000 iterations of the MD5 digest over the concatenation of the password and 16 bytes of random salt. 
+** The original KDF used by NiFi for internal key derivation for PBE, this is 1000 iterations of the MD5 digest over the concatenation of the password and 8 or 16 bytes of random salt (the salt length depends on the selected cipher block size).
 ** This KDF is *deprecated as of NiFi 0.5.0* and should only be used for backwards compatibility to decrypt data that was previously encrypted by a legacy version of NiFi.
 * OpenSSL PKCS#5 v1.5 EVP_BytesToKey
 ** This KDF was added in v0.4.0.
@@ -405,7 +405,7 @@ Here are the KDFs currently supported by NiFi (primarily in the `EncryptContent`
 *** `s0` - the version of the format. NiFi currently uses `s0` for all salts generated internally.
 *** `e0101` - the cost parameters. This is actually a hexadecimal encoding of `N`, `r`, `p` using shifts. This can be formed/parsed using `Scrypt#encodeParams()` and `Scrypt#parseParameters()`.
 **** Some external libraries encode `N`, `r`, and `p` separately in the form `$400$1$1$`. A utility method is available at `ScryptCipherProvider#translateSalt()` which will convert the external form to the internal form.
-*** `ABCDEFGHIJKLMNOPQRSTUV` - the 11-44 character, Base64-encoded, unpadded, raw salt value. This decodes to a 8-32 byte salt used in the key derivation.
+*** `ABCDEFGHIJKLMNOPQRSTUV` - the 12-44 character, Base64-encoded, unpadded, raw salt value. This decodes to a 8-32 byte salt used in the key derivation.
 * PBKDF2
 ** This KDF was added in v0.5.0.
 ** https://en.wikipedia.org/wiki/PBKDF2[Password-Based Key Derivation Function 2] is an adaptive derivation function which uses an internal pseudorandom function (PRF) and iterates it many times over a password and salt (at least 16 bytes).
@@ -442,7 +442,7 @@ For the existing KDFs, the salt format has not changed.
 NiFi Legacy
 ^^^^^^^^^^^
 
-The first 16 bytes of the input are the salt. On decryption, the salt is read in and combined with the password to derive the encryption key and IV.
+The first 8 or 16 bytes of the input are the salt. The salt length is determined based on the selected algorithm's cipher block length. If the cipher block size cannot be determined (such as with a stream cipher like `RC4`), the default value of 8 bytes is used. On decryption, the salt is read in and combined with the password to derive the encryption key and IV.
 
 image:nifi-legacy-salt.png["NiFi Legacy Salt Encoding"]
 

nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/EncryptContent.java

@@ -89,7 +89,7 @@ public class EncryptContent extends AbstractProcessor {
             .description("Specifies the key derivation function to generate the key from the password (and salt)")
             .required(true)
             .allowableValues(buildKeyDerivationFunctionAllowableValues())
-            .defaultValue(KeyDerivationFunction.NIFI_LEGACY.name())
+            .defaultValue(KeyDerivationFunction.BCRYPT.name())
             .build();
     public static final PropertyDescriptor ENCRYPTION_ALGORITHM = new PropertyDescriptor.Builder()
             .name("Encryption Algorithm")

nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/util/crypto/CipherUtility.java

@@ -303,7 +303,7 @@ public static boolean passwordLengthIsValidForAlgorithmOnLimitedStrengthCrypto(f
             throw new IllegalArgumentException("Cannot evaluate an empty encryption method algorithm");
         }
 
-            return passwordLength <= getMaximumPasswordLengthForAlgorithmOnLimitedStrengthCrypto(encryptionMethod);
+        return passwordLength <= getMaximumPasswordLengthForAlgorithmOnLimitedStrengthCrypto(encryptionMethod);
     }
 
     public static int getMaximumPasswordLengthForAlgorithmOnLimitedStrengthCrypto(EncryptionMethod encryptionMethod) {
@@ -317,4 +317,13 @@ public static int getMaximumPasswordLengthForAlgorithmOnLimitedStrengthCrypto(En
             return -1;
         }
     }
+
+    public static byte[] concatBytes(byte[]... arrays) throws IOException {
+        ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+        for (byte[] bytes : arrays) {
+            outputStream.write(bytes);
+        }
+
+        return outputStream.toByteArray();
+    }
 }

nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/util/crypto/NiFiLegacyCipherProvider.java

@@ -26,6 +26,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.security.SecureRandom;
 
 /**
  * Provides a cipher initialized with the original NiFi key derivation process for password-based encryption (MD5 @ 1000 iterations). This is not a secure
@@ -83,17 +84,55 @@ public Cipher getCipher(EncryptionMethod encryptionMethod, String password, byte
         }
     }
 
+    public byte[] generateSalt(EncryptionMethod encryptionMethod) {
+        byte[] salt = new byte[calculateSaltLength(encryptionMethod)];
+        new SecureRandom().nextBytes(salt);
+        return salt;
+    }
+
+    protected void validateSalt(EncryptionMethod encryptionMethod, byte[] salt) {
+        final int saltLength = calculateSaltLength(encryptionMethod);
+        if (salt.length != saltLength && salt.length != 0) {
+            throw new IllegalArgumentException("Salt must be " + saltLength + " bytes or empty");
+        }
+    }
+
+    private int calculateSaltLength(EncryptionMethod encryptionMethod) {
+        try {
+            Cipher cipher = Cipher.getInstance(encryptionMethod.getAlgorithm(), encryptionMethod.getProvider());
+            return cipher.getBlockSize() > 0 ? cipher.getBlockSize() : getDefaultSaltLength();
+        } catch (Exception e) {
+            logger.warn("Encountered exception determining salt length from encryption method {}", encryptionMethod.getAlgorithm(), e);
+            final int defaultSaltLength = getDefaultSaltLength();
+            logger.warn("Returning default length: {} bytes", defaultSaltLength);
+            return defaultSaltLength;
+        }
+    }
+
     @Override
     public byte[] readSalt(InputStream in) throws IOException, ProcessException {
+        return readSalt(EncryptionMethod.AES_CBC, in);
+    }
+
+    /**
+     * Returns the salt provided as part of the cipher stream, or throws an exception if one cannot be detected.
+     * This method is only implemented by {@link NiFiLegacyCipherProvider} because the legacy salt generation was dependent on the cipher block size.
+     *
+     * @param encryptionMethod the encryption method
+     * @param in the cipher InputStream
+     * @return the salt
+     */
+    public byte[] readSalt(EncryptionMethod encryptionMethod, InputStream in) throws IOException {
         if (in == null) {
             throw new IllegalArgumentException("Cannot read salt from null InputStream");
         }
 
-        // The first 16 bytes of the input stream are the salt
-        if (in.available() < getDefaultSaltLength()) {
+        // The first 8-16 bytes (depending on the cipher blocksize) of the input stream are the salt
+        final int saltLength = calculateSaltLength(encryptionMethod);
+        if (in.available() < saltLength) {
             throw new ProcessException("The cipher stream is too small to contain the salt");
         }
-        byte[] salt = new byte[getDefaultSaltLength()];
+        byte[] salt = new byte[saltLength];
         StreamUtils.fillBuffer(in, salt);
         return salt;
     }

nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/util/crypto/OpenSSLPKCS5CipherProvider.java

@@ -129,10 +129,7 @@ protected Cipher getInitializedCipher(EncryptionMethod encryptionMethod, String
             throw new IllegalArgumentException("Encryption with an empty password is not supported");
         }
 
-        if (salt.length != DEFAULT_SALT_LENGTH && salt.length != 0) {
-            // This does not enforce ASCII encoding, just length
-            throw new IllegalArgumentException("Salt must be 8 bytes US-ASCII encoded or empty");
-        }
+        validateSalt(encryptionMethod, salt);
 
         String algorithm = encryptionMethod.getAlgorithm();
         String provider = encryptionMethod.getProvider();
@@ -148,6 +145,13 @@ protected Cipher getInitializedCipher(EncryptionMethod encryptionMethod, String
         return cipher;
     }
 
+    protected void validateSalt(EncryptionMethod encryptionMethod, byte[] salt) {
+        if (salt.length != DEFAULT_SALT_LENGTH && salt.length != 0) {
+            // This does not enforce ASCII encoding, just length
+            throw new IllegalArgumentException("Salt must be 8 bytes US-ASCII encoded or empty");
+        }
+    }
+
     protected int getIterationCount() {
         return ITERATION_COUNT;
     }

nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/util/crypto/PasswordBasedEncryptor.java

@@ -121,7 +121,12 @@ public void process(final InputStream in, final OutputStream out) throws IOExcep
             // Read salt
             byte[] salt;
             try {
-                salt = cipherProvider.readSalt(in);
+                // NiFi legacy code determined the salt length based on the cipher block size
+                if (cipherProvider instanceof NiFiLegacyCipherProvider) {
+                    salt = ((NiFiLegacyCipherProvider) cipherProvider).readSalt(encryptionMethod, in);
+                } else {
+                    salt = cipherProvider.readSalt(in);
+                }
             } catch (final EOFException e) {
                 throw new ProcessException("Cannot decrypt because file size is smaller than salt size", e);
             }
@@ -158,7 +163,13 @@ public void process(final InputStream in, final OutputStream out) throws IOExcep
             PBECipherProvider cipherProvider = (PBECipherProvider) CipherProviderFactory.getCipherProvider(kdf);
 
             // Generate salt
-            byte[] salt = cipherProvider.generateSalt();
+            byte[] salt;
+            // NiFi legacy code determined the salt length based on the cipher block size
+            if (cipherProvider instanceof NiFiLegacyCipherProvider) {
+                salt = ((NiFiLegacyCipherProvider) cipherProvider).generateSalt(encryptionMethod);
+            } else {
+                salt = cipherProvider.generateSalt();
+            }
 
             // Write to output stream
             cipherProvider.writeSalt(salt, out);

nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/groovy/org/apache/nifi/processors/standard/util/crypto/NiFiLegacyCipherProviderGroovyTest.groovy

@@ -44,6 +44,8 @@ public class NiFiLegacyCipherProviderGroovyTest {
     private static final String PROVIDER_NAME = "BC";
     private static final int ITERATION_COUNT = 1000;
 
+    private static final byte[] SALT_16_BYTES = Hex.decodeHex("aabbccddeeff00112233445566778899".toCharArray());
+
     @BeforeClass
     public static void setUpOnce() throws Exception {
         Security.addProvider(new BouncyCastleProvider());
@@ -85,26 +87,27 @@ public class NiFiLegacyCipherProviderGroovyTest {
         NiFiLegacyCipherProvider cipherProvider = new NiFiLegacyCipherProvider();
 
         final String PASSWORD = "shortPassword";
-        final byte[] SALT = Hex.decodeHex("aabbccddeeff0011".toCharArray());
-
         final String plaintext = "This is a plaintext message.";
 
         // Act
-        for (EncryptionMethod em : limitedStrengthPbeEncryptionMethods) {
-            logger.info("Using algorithm: {}", em.getAlgorithm());
+        for (EncryptionMethod encryptionMethod : limitedStrengthPbeEncryptionMethods) {
+            logger.info("Using algorithm: {}", encryptionMethod.getAlgorithm());
 
-            if (!CipherUtility.passwordLengthIsValidForAlgorithmOnLimitedStrengthCrypto(PASSWORD.length(), em)) {
+            if (!CipherUtility.passwordLengthIsValidForAlgorithmOnLimitedStrengthCrypto(PASSWORD.length(), encryptionMethod)) {
                 logger.warn("This test is skipped because the password length exceeds the undocumented limit BouncyCastle imposes on a JVM with limited strength crypto policies")
                 continue
             }
 
+            byte[] salt = cipherProvider.generateSalt(encryptionMethod)
+            logger.info("Generated salt ${Hex.encodeHexString(salt)} (${salt.length})")
+
             // Initialize a cipher for encryption
-            Cipher cipher = cipherProvider.getCipher(em, PASSWORD, SALT, true);
+            Cipher cipher = cipherProvider.getCipher(encryptionMethod, PASSWORD, salt, true);
 
             byte[] cipherBytes = cipher.doFinal(plaintext.getBytes("UTF-8"));
             logger.info("Cipher text: {} {}", Hex.encodeHexString(cipherBytes), cipherBytes.length);
 
-            cipher = cipherProvider.getCipher(em, PASSWORD, SALT, false);
+            cipher = cipherProvider.getCipher(encryptionMethod, PASSWORD, salt, false);
             byte[] recoveredBytes = cipher.doFinal(cipherBytes);
             String recovered = new String(recoveredBytes, "UTF-8");
 
@@ -122,21 +125,22 @@ public class NiFiLegacyCipherProviderGroovyTest {
         NiFiLegacyCipherProvider cipherProvider = new NiFiLegacyCipherProvider();
 
         final String PASSWORD = "shortPassword";
-        final byte[] SALT = Hex.decodeHex("aabbccddeeff0011".toCharArray());
-
         final String plaintext = "This is a plaintext message.";
 
         // Act
-        for (EncryptionMethod em : pbeEncryptionMethods) {
-            logger.info("Using algorithm: {}", em.getAlgorithm());
+        for (EncryptionMethod encryptionMethod : pbeEncryptionMethods) {
+            logger.info("Using algorithm: {}", encryptionMethod.getAlgorithm());
+
+            byte[] salt = cipherProvider.generateSalt(encryptionMethod)
+            logger.info("Generated salt ${Hex.encodeHexString(salt)} (${salt.length})")
 
             // Initialize a cipher for encryption
-            Cipher cipher = cipherProvider.getCipher(em, PASSWORD, SALT, true);
+            Cipher cipher = cipherProvider.getCipher(encryptionMethod, PASSWORD, salt, true);
 
             byte[] cipherBytes = cipher.doFinal(plaintext.getBytes("UTF-8"));
             logger.info("Cipher text: {} {}", Hex.encodeHexString(cipherBytes), cipherBytes.length);
 
-            cipher = cipherProvider.getCipher(em, PASSWORD, SALT, false);
+            cipher = cipherProvider.getCipher(encryptionMethod, PASSWORD, salt, false);
             byte[] recoveredBytes = cipher.doFinal(cipherBytes);
             String recovered = new String(recoveredBytes, "UTF-8");
 
@@ -150,27 +154,28 @@ public class NiFiLegacyCipherProviderGroovyTest {
         // Arrange
         NiFiLegacyCipherProvider cipherProvider = new NiFiLegacyCipherProvider();
 
-        final String PASSWORD = "shortPassword";
-        final byte[] SALT = Hex.decodeHex("0011223344556677".toCharArray());
-
+        final String PASSWORD = "short";
         final String plaintext = "This is a plaintext message.";
 
         // Act
-        for (EncryptionMethod em : limitedStrengthPbeEncryptionMethods) {
-            logger.info("Using algorithm: {}", em.getAlgorithm());
+        for (EncryptionMethod encryptionMethod : limitedStrengthPbeEncryptionMethods) {
+            logger.info("Using algorithm: {}", encryptionMethod.getAlgorithm());
 
-            if (!CipherUtility.passwordLengthIsValidForAlgorithmOnLimitedStrengthCrypto(PASSWORD.length(), em)) {
+            if (!CipherUtility.passwordLengthIsValidForAlgorithmOnLimitedStrengthCrypto(PASSWORD.length(), encryptionMethod)) {
                 logger.warn("This test is skipped because the password length exceeds the undocumented limit BouncyCastle imposes on a JVM with limited strength crypto policies")
                 continue
             }
 
+            byte[] salt = cipherProvider.generateSalt(encryptionMethod)
+            logger.info("Generated salt ${Hex.encodeHexString(salt)} (${salt.length})")
+
             // Initialize a legacy cipher for encryption
-            Cipher legacyCipher = getLegacyCipher(PASSWORD, SALT, em.getAlgorithm());
+            Cipher legacyCipher = getLegacyCipher(PASSWORD, salt, encryptionMethod.getAlgorithm());
 
             byte[] cipherBytes = legacyCipher.doFinal(plaintext.getBytes("UTF-8"));
             logger.info("Cipher text: {} {}", Hex.encodeHexString(cipherBytes), cipherBytes.length);
 
-            Cipher providedCipher = cipherProvider.getCipher(em, PASSWORD, SALT, false);
+            Cipher providedCipher = cipherProvider.getCipher(encryptionMethod, PASSWORD, salt, false);
             byte[] recoveredBytes = providedCipher.doFinal(cipherBytes);
             String recovered = new String(recoveredBytes, "UTF-8");
 
@@ -184,7 +189,7 @@ public class NiFiLegacyCipherProviderGroovyTest {
         // Arrange
         NiFiLegacyCipherProvider cipherProvider = new NiFiLegacyCipherProvider();
 
-        final String PASSWORD = "shortPassword";
+        final String PASSWORD = "short";
         final byte[] SALT = new byte[0];
 
         final String plaintext = "This is a plaintext message.";
@@ -219,7 +224,7 @@ public class NiFiLegacyCipherProviderGroovyTest {
         NiFiLegacyCipherProvider cipherProvider = new NiFiLegacyCipherProvider();
 
         final String PASSWORD = "shortPassword";
-        final byte[] SALT = Hex.decodeHex("aabbccddeeff0011".toCharArray());
+        final byte[] SALT = SALT_16_BYTES
 
         final String plaintext = "This is a plaintext message.";
 
@@ -250,6 +255,7 @@ public class NiFiLegacyCipherProviderGroovyTest {
      * from the password using a long digest result at the time of key length checking.
      * @throws IOException
      */
+    @Ignore("Only needed once to determine max supported password lengths")
     @Test
     public void testShouldDetermineDependenceOnUnlimitedStrengthCrypto() throws IOException {
         def encryptionMethods = EncryptionMethod.values().findAll { it.algorithm.startsWith("PBE") }