[GitHub] [nifi] alopresto commented on issue #3352: NIFI-6102 Create a set of processors to wrap and unwrap potentially malicious files so as to make them inert during transit

[GitBox <gi...@apache.org>]

alopresto commented on issue #3352: NIFI-6102 Create a set of processors to wrap and unwrap potentially malicious files so as to make them inert during transit
URL: https://github.com/apache/nifi/pull/3352#issuecomment-470631275
 
 
   Hi David,
   
   I have a few concerns about this PR, and hopefully you can address these points. 
   
   I have only done a cursory review, not a line-by-line examination, but in general, I am seeing:
   * A lot of duplicated code -- I think a number of the supporting classes would need to be refactored
   * Some security concerns -- for example, regular Java `Random` is used when generating an encryption key rather than `SecureRandom`
   * Some unit test concerns -- the `setup()` method of the tests reads in every test resource file from disk to the same fields before every test case is run
   
   My larger concern is that this functionality is not needed within core NiFi -- it appears to be an integration with an external legacy/proprietary system. Casual internet searches for the unique names in the source code don't reveal a commonly-used toolset that NiFi would need compatibility with. Indeed, I am curious as to if this code can even be contributed as Apache License 2.0. 
   
   From my understanding, this is basically encryption with AES/CTR/NoPadding and then optional application of a CRC32 or SHA-256 checksum. Both of these tasks (encryption & SHA-256 hashing) can be accomplished using native processors in NiFi currently. This seems like a situation that would be better handled as:
   
   * From core NiFi -- call the external tool on the command-line using `ExecuteStreamCommand` or write a Groovy/Java script to call the APIs directly using `ExecuteScript`
   * Supporting these processors -- it is perfectly acceptable to independently host these processors in a NAR bundle which individual users can download and use, but I don't think the community is prepared to support and maintain this code at this time. Many other contributors have gone this route when the use cases they had were extremely specialized
   
   We do obviously appreciate the work that went into this contribution, but I am recommending this PR be closed. 

----------------------------------------------------------------
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


With regards,
Apache Git Services