[ https://issues.apache.org/jira/browse/NIFI-3116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16142065#comment-16142065 ]
ASF GitHub Bot commented on NIFI-3116: -------------------------------------- GitHub user alopresto opened a pull request: https://github.com/apache/nifi/pull/2108 NIFI-3116 Remove Jasypt I removed the Jasypt library (still present w/ `test` scope for backwards-compatibility testing). I re-implemented the relevant logic using Java cryptographic primitives. This will make unit testing much easier, reduce our attack surface because we no longer depend on an un-maintained library, and allows for more reasonable security decisions that are not obfuscated by the library. I added unit tests (some ignored as I will build out additional functionality, but this is sufficient for removal of the library), but manual verification is important. To do this: 1. Create a flow with processors that store sensitive values (a sample flow [here](https://gist.github.com/alopresto/28e748358455e93bfc774556ba820b6e) encrypts and then decrypts text -- both components store a key value). 1. Stop NiFi 1. Use the `encrypt-config` tool to migrate the `flow.xml.gz` to use a new `nifi.sensitive.props.key` (and be sure to update the value in `nifi.properties` as well). 1. Note that the flow provided above already uses the `...key` *newpassword*, so enter something different if using that flow 1. This command is operating on a copied flow definition, be sure to point at your actual `flow.xml.gz` ``` ./bin/encrypt-config.sh -n ../../../../../nifi-assembly/target/nifi-1.4.0-SNAPSHOT-bin/nifi-1.4.0-SNAPSHOT/conf/nifi.properties -f ~/Workspace/scratch/encrypt.xml.gz -g ~/Workspace/scratch/encrypt_changed.xml.gz -v -x -s newpassword ``` 1. Verify that the `nifi.properties` file has a new `nifi.sensitive.props.key` value. `more conf/nifi.properties | grep '\''sensitive\|assw\|key\|trust'\''` 1. Start NiFi. 1. Verify that the flow still works. --------- Thank you for submitting a contribution to Apache NiFi. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. - [ ] Has your PR been rebased against the latest commit within the target branch (typically master)? - [ ] Is your initial contribution a single, squashed commit? ### For code changes: - [ ] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? - [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly? - [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly? - [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties? ### For documentation related changes: - [ ] Have you ensured that format looks appropriate for the output in which it is rendered? ### Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. You can merge this pull request into a Git repository by running: $ git pull https://github.com/alopresto/nifi NIFI-3116 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi/pull/2108.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #2108 ---- commit 84c631c005269ad0b9297714f21e813169e7bfb1 Author: Andy LoPresto <alopre...@apache.org> Date: 2017-08-15T19:33:19Z NIFI-3116 Added initial regression test for StringEncryptor to ensure continued functionality during removal of Jasypt. commit ae0c54178fd5d857947d0c06902e3e24a1f2efc0 Author: Andy LoPresto <alopre...@apache.org> Date: 2017-08-16T17:10:15Z NIFI-3116 Added external compatibility regression test for StringEncryptor to ensure continued functionality during removal of Jasypt. Documents custom salt lengths and iteration counts for each encryption method. commit 875d3d434b95bdd71503b9fc1fadee3a06decd64 Author: Andy LoPresto <alopre...@apache.org> Date: 2017-08-16T17:56:04Z NIFI-3116 Cleaned up test. commit 86f0921eee5ed6a5d31a714488278374ae22ac39 Author: Andy LoPresto <alopre...@apache.org> Date: 2017-08-16T18:04:10Z NIFI-3116 Added (ignored) failing tests for keyed encryption (Jasypt does not support keyed encryption). commit 98163625b69a48482203c3ddaeb5291151f444b0 Author: Andy LoPresto <alopre...@apache.org> Date: 2017-08-16T18:51:03Z NIFI-3116 Added failing test for non-final class. Changed StringEncryptor to non-final class and added protected default constructor. commit 5c967d4f0bc6a1fb482dc66427722bd8f92432ff Author: Andy LoPresto <alopre...@apache.org> Date: 2017-08-22T04:39:57Z NIFI-3116 Added failing test for initialization status. Added utility methods in CipherUtility. commit 4bbdb0c3f03afebed50c6456ff7b5a38ec1d64b1 Author: Andy LoPresto <alopre...@apache.org> Date: 2017-08-22T06:30:44Z NIFI-3116 Moved PBE cipher providers (and tests) from nifi-standard-processors to nifi-security-utils module. commit 926d152f2f2127035bd0f91e27267547df83dd5f Author: Andy LoPresto <alopre...@apache.org> Date: 2017-08-23T23:37:41Z NIFI-3116 Implemented PBE and keyed encryption/decryption logic. Added unit tests. commit 5efcd3b6ae03438f04fa20441b9699d5912f74cc Author: Andy LoPresto <alopre...@apache.org> Date: 2017-08-25T19:12:25Z NIFI-3116 Removed Jasypt dependency from production scope (kept in test scope for backward compatibility tests). Fixed checkstyle issues. ---- > Remove Jasypt library > --------------------- > > Key: NIFI-3116 > URL: https://issues.apache.org/jira/browse/NIFI-3116 > Project: Apache NiFi > Issue Type: Improvement > Components: Core Framework > Affects Versions: 1.1.0 > Reporter: Andy LoPresto > Assignee: Andy LoPresto > Labels: encryption, kdf, pbe, security > > The [Jasypt|http://www.jasypt.org/index.html] library is used internally by > NiFi for String encryption operations (specifically password-based encryption > (PBE) in {{EncryptContent}} and sensitive processor property protection). I > feel there are a number of reasons to remove this library from NiFi and > provide centralized symmetric encryption operations using Java cryptographic > primitives (and BouncyCastle features where necessary). > * The library was last updated February 25, 2014. For comparison, > BouncyCastle has been [updated 5 > times|https://www.bouncycastle.org/releasenotes.html] since then > * {{StandardPBEStringEncryptor}}, the high-level class wrapped by NiFi's > {{StringEncryptor}} is final. This makes it, and features relying on it, > difficult to test in isolation > * Jasypt encapsulates many decisions about {{Cipher}} configuration, > specifically salt-generation strategy. This can be a valuable feature for > pluggable libraries, but is less than ideal when dealing with encryption and > key derivation, which are in constant struggle with evolving attacks and > improving hardware. There are hard-coded constants which are not compatible > with better decisions available now (i.e. requiring custom implementations of > the {{SaltGenerator}} interface to provide new derivations). The existence of > these values was opaque to NiFi and led to serious compatibility issues > [NIFI-1259], [NIFI-1257], [NIFI-1242], [NIFI-1463], [NIFI-1465], [NIFI-3024] > * {{StringEncryptor}}, the NiFi class wrapping {{StandardPBEStringEncryptor}} > is also final and does not expose methods to instantiate it with only the > relevant values (i.e. {{algorithm}}, {{provider}}, and {{password}}) but > rather requires an entire {{NiFiProperties}} instance. > * {{StringEncryptor.createEncryptor()}} performs an unnecessary "validation > check" on instantiation, which was one cause of reported issues where a > secure node/cluster blocks on startup on VMs due to lack of entropy in > {{/dev/random}} > * The use of custom salts with PBE means that the internal {{Cipher}} object > must be re-created and initialized and the key re-derived from the password > on every decryption call. Symmetric keyed encryption with a strong KDF (order > of magnitude higher iterations of a stronger algorithm) and unique > initialization vector (IV) values would be substantially more resistant to > brute force attacks and yet more performant at scale. > I have already implemented backwards-compatible code to perform the actions > of symmetric key encryption using keys derived from passwords in both the > {{ConfigEncryptionTool}} and {{OpenSSLPKCS5CipherProvider}} and > {{NiFiLegacyCipherProvider}} classes, which empirical tests confirm are > compatible with the Jasypt output. > Additional research on some underlying/related issues: > * [Why does Java allow AES-256 bit encryption on systems without JCE > unlimited strength policies if using > PBE?|https://security.stackexchange.com/questions/107321/why-does-java-allow-aes-256-bit-encryption-on-systems-without-jce-unlimited-stre] > * [How To Decrypt OpenSSL-encrypted Data In Apache > NiFi|https://community.hortonworks.com/articles/5319/how-to-decrypt-openssl-encrypted-data-in-apache-ni.html] > * [d...@nifi.apache.org "Passwords in > EncryptContent"|https://lists.apache.org/thread.html/b93ced98eff6a77dd0a2a2f0b5785ef42a3b02de2cee5c17607a8c49@%3Cdev.nifi.apache.org%3E] -- This message was sent by Atlassian JIRA (v6.4.14#64029)