Skip to content

Nifi 1242#140

Closed
alopresto wants to merge 3 commits intoapache:masterfrom
alopresto:NIFI-1242
Closed

Nifi 1242#140
alopresto wants to merge 3 commits intoapache:masterfrom
alopresto:NIFI-1242

Conversation

@alopresto
Copy link
Contributor

NIFI-1242:

Created KeyDerivationFunction enum and provided as a property on EncryptContent processor to allow backward compatibility with legacy NiFi KDF (MD5 @ 1000 iterations) and OpenSSL EVP_BytesToKey (custom KDF with MD5 PKCS#5 v1.5).

Added test resources.

plain.txt: This is a plaintext message.

0s @ 12:20:32 $ openssl enc -aes-256-cbc -e -in plain.txt -out salted_raw.enc -k thisIsABadPassword -p
salt=31DC301A6C7B8A0B
key=CB878A6E167A5B530B8F2BD175E6359E3092AFF7C83274A22A5B421D79E599AC
iv =0C614A72FC06B454B84E035B3FA8F877
0s @ 12:20:44 $ xxd salted_raw.enc
0000000: 5361 6c74 6564 5f5f 31dc 301a 6c7b 8a0b  Salted__1.0.l{..
0000010: 616b c65d f767 504d c085 ba7a c517 d0cb  ak.].gPM...z....
0000020: 7832 211e f573 b6f1 ded2 8f59 88e8 088f  x2!..s.....Y....

0s @ 20:14:00 $ openssl enc -aes-256-cbc -e -in plain.txt -out unsalted_raw.enc -k thisIsABadPassword -p -nosalt
key=711E85689CE7AFF6F410AEA43ABC5446842F685B84879B2E00F977C22B9E9A7D
iv =0C90ABF8ECE84B92BAA2CD448EC760F0
0s @ 20:14:17 $ xxd unsalted_raw.enc
0000000: 70cd 2984 fdbb 0e7c c01b 7206 88b1 6b50  p.)....|..r...kP
0000010: 5eeb e4f3 4036 773b 00ce dd8e 85d8 f90a  ^...@6w;........
Added KeyDerivationFunction enum.
Added kdf property in EncryptContent processor and provided to PasswordBasedEncryptor.
Added logic in PasswordBasedEncryptor to handle variable KDF.
Added unit tests for EncryptContent processor.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test failed in build:
[pool-2-thread-1] ERROR org.apache.nifi.processors.standard.EncryptContent - EncryptContent[id=3720f844-b303-4315-9b69-2007297eb6b8] Cannot decrypt FlowFile[0,salted_raw.enc,48B] - : org.apache.nifi.processor.exception.ProcessException: java.security.InvalidKeyException: Illegal key size

@alopresto
Copy link
Contributor Author

After debugging, this is related to the JCE Unlimited Strength Cryptographic Policies that must be installed to allow the JRE to use key lengths above export limits. AES is restricted to 128 bit (16 bytes) by default without these policy files.

However, in examining the code, we believe we have found a larger issue in Java's handling of the key when using password-based encryption. In a normal scenario, the key is generated or derived and then used to initialize the cipher as follows:

    try {
                MessageDigest sha1 = MessageDigest.getInstance("SHA1");
                String key = Hex.encodeHexString(sha1.digest("thisIsABadPassword".getBytes())).substring(0, 32);
                String iv = Hex.encodeHexString(sha1.digest("thisIsABadIv".getBytes())).substring(0, 32);

                SecretKey secretKey = new SecretKeySpec(Hex.decodeHex(key.toCharArray()), "AES");
                IvParameterSpec ivParameterSpec = new IvParameterSpec(Hex.decodeHex(iv.toCharArray()));

                cipher = Cipher.getInstance("AES/CBC/PKCS7Padding");
                cipher.init(Cipher.ENCRYPT_MODE, secretKey, ivParameterSpec);

                String message = "This is a plaintext message.";

                byte[] cipherBytes = cipher.doFinal(message.getBytes());

                cipher.init(Cipher.DECRYPT_MODE, secretKey, ivParameterSpec);

                byte[] recoveredBytes = cipher.doFinal(cipherBytes);

                System.out.println("Recovered message: " + new String(recoveredBytes));

            } catch (Exception e) {
                fail(e.getMessage());
            }

During cipher.init(), the system checks Cipher.getMaxAllowedKeyLength("AES") for the key limit in bits. As 32 hex == 16 bytes == 128 bits, this will run on a vanilla JRE installation. However, using the password-based encryption PBEParameterSpec is slightly different.

    try {
        String password = "thisIsABadPassword";
        String salt = "SALTSALT";

        PBEKeySpec keySpec = new PBEKeySpec(password);
SecretKeyFactory keyFactory = SecretKeyFactory.getInstance("PBEWITHMD5AND256BITAES-CBC-OPENSSL", "BC");
        SecretKey key = keyFactory.generateSecret(keySpec);

        PBEParameterSpec saltSpec = new PBEParameterSpec(salt.getBytes("US-ASCII"), 0);

        Cipher cipher = Cipher.getInstance("PBEWITHMD5AND256BITAES-CBC-OPENSSL", "BC");
        cipher.init(Cipher.ENCRYPT_MODE, key, saltSpec);

        String message = "This is a plaintext message.";

        byte[] cipherBytes = cipher.doFinal(message.getBytes());

        cipher.init(Cipher.DECRYPT_MODE, key, saltSpec);

        byte[] recoveredBytes = cipher.doFinal(cipherBytes);

        System.out.println("Recovered message: " + new String(recoveredBytes));

    } catch (Exception e) {
        fail(e.getMessage());
    }

The actual key is not derived until during cipher.init(), so at the time the key length check is done, it is actually checking the length of the raw password. This means that a vanilla JRE installation can use 256-bit AES to encrypt or decrypt a file provided the password is <= 16 bytes. The tests above fail because the provided password is 18 bytes. However, the algorithm used is still AES-256-CBC, which means the derived key is 32 bytes. This has been verified on a vanilla JRE installation decrypting OpenSSL AES-256-CBC encrypted files.

I will look at @apiri 's patch on NIFI-1242 but this is a serious issue and should be well-documented. The usual test of "is available on a vanilla JRE because the key length is <= 128 bits (for AES)" is no longer sufficient because the supplied password length now affects the determination.

Added logic and test resources to debug JCE unlimited strength cryptography policy issues.
Merged patch from Aldrin Piri for processor property validation.
Excluded test resources from build.
@asfgit asfgit closed this in f83e6d3 Dec 5, 2015
mattyb149 pushed a commit to mattyb149/nifi that referenced this pull request Dec 9, 2020
…sistent with NiFi docs

This closes apache#140.

Signed-off-by: Aldrin Piri <aldrin@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants