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:
[email protected]


Reply via email to