You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2021/03/01 15:53:10 UTC
[GitHub] [nifi] exceptionfactory opened a new pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
exceptionfactory opened a new pull request #4857:
URL: https://github.com/apache/nifi/pull/4857
#### Description of PR
NIFI-8230 Removes the default internal Sensitive Properties Key which provided fallback support when the value of `nifi.sensitive.props.key` was empty. NiFi used the default internal value on new installations where the administrator had not configured a sensitive properties key.
To maintain ease of use, this PR includes an update to `NiFiPropertiesLoader.get()` that checks for a null or blank sensitive properties key and generates a random key when the flow configuration file is not found. The random key generation process uses `java.util.SecureRandom` to generate an array of 24 bytes, which is then encoded using Base64 without padding to produce a 32 character string. Using Base64 without padding avoids introducing unsupported property characters while also providing a reasonable range of potential random characters.
This approach supports generating a random key for installations of NiFi while avoiding random generation when upgrading. Existing flow configurations may already contain sensitive properties encrypted with the default internal key. For upgrades of existing installations that do not have a sensitive properties key, NiFi logs and error and throws an exception referencing the Administration Guide section on _Migrating a Flow with Sensitive Properties_. To support migration, the `ConfigEncryptionTool` retains the default internal value for reading existing flows.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [X] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [X] 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.
- [X] Has your PR been rebased against the latest commit within the target branch (typically `main`)?
- [X] Is your initial contribution a single, squashed commit? _Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not `squash` or use `--force` when pushing to allow for clean monitoring of changes._
### For code changes:
- [X] Have you ensured that the full suite of tests is executed via `mvn -Pcontrib-check clean install` at the root `nifi` folder?
- [X] Have you written or updated unit tests to verify your changes?
- [X] Have you verified that the full build is successful on JDK 8?
- [X] Have you verified that the full build is successful on JDK 11?
- [ ] 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 GitHub Actions CI for build issues and submit an update to your PR as soon as possible.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on pull request #4857:
URL: https://github.com/apache/nifi/pull/4857#issuecomment-831933666
Thanks @markap14, and thanks to @bbende, @CefBoud, and @thenatog for the feedback along the way!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on pull request #4857:
URL: https://github.com/apache/nifi/pull/4857#issuecomment-812720407
> @exceptionfactory I gave this a try. Loaded up a flow created on `main` and then tried to startup. It failed almost immediately, telling me that i had to set the sensitive props key, which is good.
>
> So then I tried to run the command update the key:
>
> ```
> nifi-1.14.0-SNAPSHOT $ bin/nifi.sh set-sensitive-properties-key "the quick brown fox jumped over the lazy dog"
>
> Java home: /Library/Java/JavaVirtualMachines/jdk1.8.0_281.jdk/Contents/Home
> NiFi home: /Users/mpayne/devel/nifi/nifi-assembly/target/nifi-1.14.0-SNAPSHOT-bin/nifi-1.14.0-SNAPSHOT
>
> Bootstrap Config File: /Users/mpayne/devel/nifi/nifi-assembly/target/nifi-1.14.0-SNAPSHOT-bin/nifi-1.14.0-SNAPSHOT/conf/bootstrap.conf
>
> Unexpected number of arguments [9]
> Usage: SetSensitivePropertiesKey <sensitivePropertiesKey>
> ```
>
> So it appears there's an issue of some sort in the way that the arguments are passed in the `nifi.sh` script to the Java program, as it appears that even though "the quick brown fox jumped over the lazy dog" was quoted, it was passed to the Java program as 9 separate arguments.
Thanks for the feedback @markap14! I pushed an update to `nifi.sh` that passes quoted arguments to `SetSensitivePropertiesKey` to handle quoted values as a single argument.
I also adjusted the minimum required key length to 12, aligning with the existing length requirement of 12 in `StandardPropertySecretKeyProvider`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a change in pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4857:
URL: https://github.com/apache/nifi/pull/4857#discussion_r591798831
##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/main/java/org/apache/nifi/properties/NiFiPropertiesLoader.java
##########
@@ -241,9 +241,54 @@ public NiFiProperties load(String path) {
*/
public NiFiProperties get() {
if (instance == null) {
- instance = loadDefault();
+ instance = getDefaultProperties();
}
return instance;
}
+
+ private NiFiProperties getDefaultProperties() {
+ NiFiProperties defaultProperties = loadDefault();
+ if (isKeyGenerationRequired(defaultProperties)) {
+ final File flowConfiguration = defaultProperties.getFlowConfigurationFile();
+ if (flowConfiguration.exists()) {
+ logger.error("Flow Configuration [{}] Found: Migration Required for blank Sensitive Properties Key [{}]", flowConfiguration, NiFiProperties.SENSITIVE_PROPS_KEY);
+ final String message = String.format("Sensitive Properties Key [%s] not found: %s", NiFiProperties.SENSITIVE_PROPS_KEY, MIGRATION_INSTRUCTIONS);
+ throw new SensitivePropertyProtectionException(message);
+ }
+ setSensitivePropertiesKey();
Review comment:
Thanks for the feedback @bbende! Calling `NiFiProperties.isClustered()` is a simple enough check in addition to checking whether a Flow Configuration already exists. Setting a sensitive properties key should be required for a clustered configuration, and that could be called out separately prior to checking for an existing Flow Configuration.
I will add the check and associated exception scenario.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markap14 commented on pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
Posted by GitBox <gi...@apache.org>.
markap14 commented on pull request #4857:
URL: https://github.com/apache/nifi/pull/4857#issuecomment-831916358
All looks good to me at this point @exceptionfactory. Thanks! I rebased against `main`, resolved the merge conflict, and merged to `main`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on pull request #4857:
URL: https://github.com/apache/nifi/pull/4857#issuecomment-830492715
@markap14 Do you have any additional feedback, or do the latest changes look good? Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on pull request #4857:
URL: https://github.com/apache/nifi/pull/4857#issuecomment-794608324
> Sorry just came back to this. I tested it out and found that it works as described. The issue I foresee is that we will likely have people asking questions on the email distro when they try to use their old configurations when this is put in place.
>
> Currently the error message directs the user as follows:
> _"Sensitive Properties Key [nifi.sensitive.props.key] not found: See Administration Guide section [Migrating a Flow with Sensitive Properties]"_
>
> I followed the steps given in that section and was able to update my configuration and get the flow working with a new password. I also provided empty configuration and a key was generated for me, and I verified the new key was then used to encrypt sensitive values.
>
> As far as backwards compatibility, this could be a hurdle for users who are upgrading. I wouldn't be surprised if the majority of users haven't used the encrypt-config.sh tool at all before. We might need to get some others to chime in to decide if this is a reasonable change to make for a minor version. That being said, I do think it's an important change to be making.
>
> Whilst I'm also raising a question about backwards compatibility, I'm at the same time wondering if we can go a step further and change the default sensitive properties algorithm if nothing in the flow is already encrypted and the algorithm property is empty (though I believe it's already populated by default)? Is it possible we could start using PBEWITHSHA256AND256BITAES-CBC-BC? I'll be honest, I'm not actually sure of how significant of a security benefit this might be or if it will have a negative performance impact, but removing the use of MD5 sounds like a good idea to me. Also, not sure if this algorithm is available in all regions so that would be something to check. The encrypt-config.sh tool appears to allow specifying an old and new flow encryption algorithm. Just a thought.
Thanks for the testing and detailed feedback @thenatog!
There are certainly tradeoffs when it comes to to backwards compatibility, but running with an empty sensitive properties key is a serious security risk, so it is important to move in this direction. Perhaps work could be done to make the encrypt-config.sh toolkit easier to use in this case, but that could be considered in a separate task if necessary. It would be worth noting in migration guidance that using the toolkit is necessary.
Regarding the default algorithm, we should move away from all of the existing PBE values. NIFI-8246 addresses this issue explicitly and I am planning to submit a PR for that issue as soon as this PR is merged. The internal default sensitive properties key does not meet the length requirements of the secure key derivation functions, but once this is merged, changing the default sensitive properties algorithm should be straightforward.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] bbende commented on a change in pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
Posted by GitBox <gi...@apache.org>.
bbende commented on a change in pull request #4857:
URL: https://github.com/apache/nifi/pull/4857#discussion_r591783286
##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/main/java/org/apache/nifi/properties/NiFiPropertiesLoader.java
##########
@@ -241,9 +241,54 @@ public NiFiProperties load(String path) {
*/
public NiFiProperties get() {
if (instance == null) {
- instance = loadDefault();
+ instance = getDefaultProperties();
}
return instance;
}
+
+ private NiFiProperties getDefaultProperties() {
+ NiFiProperties defaultProperties = loadDefault();
+ if (isKeyGenerationRequired(defaultProperties)) {
+ final File flowConfiguration = defaultProperties.getFlowConfigurationFile();
+ if (flowConfiguration.exists()) {
+ logger.error("Flow Configuration [{}] Found: Migration Required for blank Sensitive Properties Key [{}]", flowConfiguration, NiFiProperties.SENSITIVE_PROPS_KEY);
+ final String message = String.format("Sensitive Properties Key [%s] not found: %s", NiFiProperties.SENSITIVE_PROPS_KEY, MIGRATION_INSTRUCTIONS);
+ throw new SensitivePropertyProtectionException(message);
+ }
+ setSensitivePropertiesKey();
Review comment:
If we are setting up a cluster, I assume that this call is going to generate a different random key on each node, which I think would mean that when a node receives a fingerprint or flow from the cluster coordinator, there would be an issue since they don't have the same key.
Maybe it makes sense to also check if one of the clustering properties is set, and make it part of the error scenario above so that in a cluster, the user is required to enter a sensitive props key?
@markap14 do you have any thoughts?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a change in pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4857:
URL: https://github.com/apache/nifi/pull/4857#discussion_r584965498
##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/main/java/org/apache/nifi/properties/NiFiPropertiesLoader.java
##########
@@ -241,9 +239,53 @@ public NiFiProperties load(String path) {
*/
public NiFiProperties get() {
if (instance == null) {
- instance = loadDefault();
+ instance = setSensitivePropertiesKey(loadDefault());
Review comment:
Thanks for the suggestion, will refactor to externalize the conditional for better readability.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markap14 commented on pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
Posted by GitBox <gi...@apache.org>.
markap14 commented on pull request #4857:
URL: https://github.com/apache/nifi/pull/4857#issuecomment-812597084
@exceptionfactory I gave this a try. Loaded up a flow created on `main` and then tried to startup. It failed almost immediately, telling me that i had to set the sensitive props key, which is good.
So then I tried to run the command update the key:
```
nifi-1.14.0-SNAPSHOT $ bin/nifi.sh set-sensitive-properties-key "the quick brown fox jumped over the lazy dog"
Java home: /Library/Java/JavaVirtualMachines/jdk1.8.0_281.jdk/Contents/Home
NiFi home: /Users/mpayne/devel/nifi/nifi-assembly/target/nifi-1.14.0-SNAPSHOT-bin/nifi-1.14.0-SNAPSHOT
Bootstrap Config File: /Users/mpayne/devel/nifi/nifi-assembly/target/nifi-1.14.0-SNAPSHOT-bin/nifi-1.14.0-SNAPSHOT/conf/bootstrap.conf
Unexpected number of arguments [9]
Usage: SetSensitivePropertiesKey <sensitivePropertiesKey>
```
So it appears there's an issue of some sort in the way that the arguments are passed in the `nifi.sh` script to the Java program, as it appears that even though "the quick brown fox jumped over the lazy dog" was quoted, it was passed to the Java program as 9 separate arguments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] thenatog commented on pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
Posted by GitBox <gi...@apache.org>.
thenatog commented on pull request #4857:
URL: https://github.com/apache/nifi/pull/4857#issuecomment-794599050
Sorry just came back to this. I tested it out and found that it works as described. The issue I foresee is that we will likely have people asking questions on the email distro when they try to use their old configurations when this is put in place.
Currently the error message directs the user as follows:
_"Sensitive Properties Key [nifi.sensitive.props.key] not found: See Administration Guide section [Migrating a Flow with Sensitive Properties]"_
I followed the steps given in that section and was able to update my configuration and get the flow working with a new password. I also provided empty configuration and a key was generated for me, and I verified the new key was then used to encrypt sensitive values.
As far as backwards compatibility, this could be a hurdle for users who are upgrading. I wouldn't be surprised if the majority of users haven't used the encrypt-config.sh tool at all before. We might need to get some others to chime in to decide if this is a reasonable change to make for a minor version. That being said, I do think it's an important change to be making.
Whilst I'm also raising a question about backwards compatibility, I'm at the same time wondering if we can go a step further and change the default sensitive properties algorithm if nothing in the flow is already encrypted and the algorithm property is empty (though I believe it's already populated by default)? Is it possible we could start using PBEWITHSHA256AND256BITAES-CBC-BC? I'll be honest, I'm not actually sure of how significant of a security benefit this might be or if it will have a negative performance impact, but removing the use of MD5 sounds like a good idea to me. Also, not sure if this algorithm is available in all regions so that would be something to check. The encrypt-config.sh tool appears to allow specifying an old and new flow encryption algorithm. Just a thought.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] CefBoud commented on a change in pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
Posted by GitBox <gi...@apache.org>.
CefBoud commented on a change in pull request #4857:
URL: https://github.com/apache/nifi/pull/4857#discussion_r584942067
##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/main/java/org/apache/nifi/properties/NiFiPropertiesLoader.java
##########
@@ -241,9 +239,53 @@ public NiFiProperties load(String path) {
*/
public NiFiProperties get() {
if (instance == null) {
- instance = loadDefault();
+ instance = setSensitivePropertiesKey(loadDefault());
Review comment:
if `isKeyGenerationRequired` returns false, `setSensitivePropertiesKey` will have no effect. Maybe renaming the method to something like `setSensitivePropertiesKeyIfBlank` or externalizing the `if ` enclosing `setSensitivePropertiesKey`'s body would provide more readability ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] thenatog commented on pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
Posted by GitBox <gi...@apache.org>.
thenatog commented on pull request #4857:
URL: https://github.com/apache/nifi/pull/4857#issuecomment-789090772
I'll test this out
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] CefBoud commented on pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
Posted by GitBox <gi...@apache.org>.
CefBoud commented on pull request #4857:
URL: https://github.com/apache/nifi/pull/4857#issuecomment-789083062
Excellent work, @exceptionfactory. Nicely refactored!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on pull request #4857:
URL: https://github.com/apache/nifi/pull/4857#issuecomment-788186327
> Great job, @exceptionfactory !
Thanks for the helpful feedback @CefBoud! I made changes and pushed an update to address your comments.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] CefBoud commented on a change in pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
Posted by GitBox <gi...@apache.org>.
CefBoud commented on a change in pull request #4857:
URL: https://github.com/apache/nifi/pull/4857#discussion_r584941994
##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/main/java/org/apache/nifi/properties/NiFiPropertiesLoader.java
##########
@@ -241,9 +239,53 @@ public NiFiProperties load(String path) {
*/
public NiFiProperties get() {
if (instance == null) {
- instance = loadDefault();
+ instance = setSensitivePropertiesKey(loadDefault());
}
return instance;
}
+
+ private NiFiProperties setSensitivePropertiesKey(final NiFiProperties niFiProperties) {
+ NiFiProperties processedProperties = niFiProperties;
+
+ if (isKeyGenerationRequired(niFiProperties)) {
+ final File flowConfigurationFlow = niFiProperties.getFlowConfigurationFile();
+ if (flowConfigurationFlow.exists()) {
+ logger.error("Flow Configuration File [{}] Found: Migration Required for blank Sensitive Properties Key [{}]", NiFiProperties.SENSITIVE_PROPS_KEY);
+ final String message = String.format("Sensitive Properties Key [%s] not found: %s", NiFiProperties.SENSITIVE_PROPS_KEY, MIGRATION_INSTRUCTIONS);
+ throw new SensitivePropertyProtectionException(message);
+ }
+
+ logger.warn("Generating Random Sensitive Properties Key [{}]", NiFiProperties.SENSITIVE_PROPS_KEY);
+ final SecureRandom secureRandom = new SecureRandom();
+ final byte[] sensitivePropertiesKeyBinary = new byte[SENSITIVE_PROPERTIES_KEY_LENGTH];
+ secureRandom.nextBytes(sensitivePropertiesKeyBinary);
+ final String sensitivePropertiesKey = KEY_ENCODER.encodeToString(sensitivePropertiesKeyBinary);
+ try {
+ final File niFiPropertiesFile = new File(CryptoUtils.getDefaultFilePath());
Review comment:
`getDefaultFilePath `can be called up to three times in the case of generation. First inside `get`, then 2 times inside `setSensitivePropertiesKey`. It prints a logging message that says "_Determined default nifi.properties path to be ..._" that may appear 3 times. How about using a variable to store the default path ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a change in pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4857:
URL: https://github.com/apache/nifi/pull/4857#discussion_r584964533
##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/main/java/org/apache/nifi/properties/NiFiPropertiesLoader.java
##########
@@ -241,9 +239,53 @@ public NiFiProperties load(String path) {
*/
public NiFiProperties get() {
if (instance == null) {
- instance = loadDefault();
+ instance = setSensitivePropertiesKey(loadDefault());
}
return instance;
}
+
+ private NiFiProperties setSensitivePropertiesKey(final NiFiProperties niFiProperties) {
+ NiFiProperties processedProperties = niFiProperties;
+
+ if (isKeyGenerationRequired(niFiProperties)) {
+ final File flowConfigurationFlow = niFiProperties.getFlowConfigurationFile();
+ if (flowConfigurationFlow.exists()) {
+ logger.error("Flow Configuration File [{}] Found: Migration Required for blank Sensitive Properties Key [{}]", NiFiProperties.SENSITIVE_PROPS_KEY);
+ final String message = String.format("Sensitive Properties Key [%s] not found: %s", NiFiProperties.SENSITIVE_PROPS_KEY, MIGRATION_INSTRUCTIONS);
+ throw new SensitivePropertyProtectionException(message);
+ }
+
+ logger.warn("Generating Random Sensitive Properties Key [{}]", NiFiProperties.SENSITIVE_PROPS_KEY);
+ final SecureRandom secureRandom = new SecureRandom();
+ final byte[] sensitivePropertiesKeyBinary = new byte[SENSITIVE_PROPERTIES_KEY_LENGTH];
+ secureRandom.nextBytes(sensitivePropertiesKeyBinary);
+ final String sensitivePropertiesKey = KEY_ENCODER.encodeToString(sensitivePropertiesKeyBinary);
+ try {
+ final File niFiPropertiesFile = new File(CryptoUtils.getDefaultFilePath());
Review comment:
That's a good point, will change to an instance variable to avoid multiple calls to `CryptoUtils.getDefaultFilePath()`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on pull request #4857:
URL: https://github.com/apache/nifi/pull/4857#issuecomment-805814048
Just to summarize the latest changes, after some additional discussion with @markap14, he suggested that implementing a simplified command for setting the Sensitive Properties Key to make the migration path easier for current users with blank keys.
The most recent commit includes an update to `nifi.sh` to support the following:
`nifi.sh set-sensitive-properties-key <sensitivePropertiesKey>`
The script passes the new key to the command class, which reads the current key from `nifi.properties` and falls back to the internal default value if the existing value is blank.
This approach required refactoring the PropertyEncryptor classes to a separate module named `nifi-property-encryptor`. The new command class is located under `nifi-flow-encryptor`. The existing `ConfigEncryptionTool` continues to work, but contained a lot of the same code, so this change provided an opportunity to refactor `ConfigEncryptionTool` to leverage the `PropertyEncryptor` interface. This simplifies `ConfigEncryptionTool` and also removes code that diverged from the standard property encryption approach.
With these changes, the Admin Guide now includes a new section describing the `set-sensitive-properties-key` command.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markap14 closed pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
Posted by GitBox <gi...@apache.org>.
markap14 closed pull request #4857:
URL: https://github.com/apache/nifi/pull/4857
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] CefBoud commented on pull request #4857: NIFI-8230 Removed default Sensitive Properties Key
Posted by GitBox <gi...@apache.org>.
CefBoud commented on pull request #4857:
URL: https://github.com/apache/nifi/pull/4857#issuecomment-788161643
Great job, @exceptionfactory !
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org