[
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
Date: 2017-08-16T17:56:04Z
NIFI-3116 Cleaned up test.
commit 86f0921eee5ed6a5d31a714488278374ae22ac39
Author: Andy LoPresto <[email protected]>
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 <[email protected]>
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 <[email protected]>
Date: 2017-08-22T04:39:57Z
NIFI-3116 Added failing test for initialization status.
Added utility methods in CipherUtility.
commit 4bbdb0c3f03afebed50c6456ff7b5a38ec1d64b1
Author: Andy LoPresto <[email protected]>
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 <[email protected]>
Date: 2017-08-23T23:37:41Z
NIFI-3116 Implemented PBE and keyed encryption/decryption logic.
Added unit tests.
commit 5efcd3b6ae03438f04fa20441b9699d5912f74cc
Author: Andy LoPresto <[email protected]>
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]
> * [[email protected] "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)
