[GitHub] [nifi] alopresto commented on pull request #4263: NIFI-7407 Refactored SSL context generation throughout framework and extensions.

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

alopresto commented on pull request #4263:
URL: https://github.com/apache/nifi/pull/4263#issuecomment-626427812


   High level description of changes:
   * Previously there were `SslContextFactory` implementations in multiple modules. I enhanced the one in `nifi-security-utils` which is now used throughout the project, and removed the implementations in `nifi-framework` and `nifi-socket-utils`. 
   ** Part of this refactoring was removing `public static` methods which created an `SSLContext` object from various combinations of explicit keystore and truststore properties. These were being used in an inconsistent manner. I introduced a container object called `TlsConfiguration` which wraps the state of the configuration and provides internal validation checks. This encapsulates the need to check for different combinations of configuration presence/validity in each use case (components, framework, etc.) and relieves the calling developer of re-implementing this logic every time. 
   ** I also provided static convenience methods like `getX509TrustManager()` and `createSSLSocketFactory()` because in most cases that is what the calling code needs, rather than an intermediate `SSLContext` object they need to further configure. This reduced the need for `Tuple<>` return values throughout the code. 
   ** Duplicate code to transform the various return values and configure the `OkHttpClient` and its `Builder` were refactored to utility methods. 
   * Duplicate enums were refactored. 
   * Unnecessary code dealing with client authentication settings when creating a client connection/socket were removed (these settings would be ignored, as only an SSL/TLS server can decide to enforce/request client authentication). 
   * Some tests were refactored to make mocking easier. 
   * Removed extraneous file loading during `NiFiProperties` construction in many tests. 
   * Enforced modern TLS protocol versions in various internal socket creations. 
   
   The easiest way to test these changes is to configure and deploy a secured cluster (see [Apache NiFi Walkthroughs: Creating and Securing a NiFi Cluster with the TLS Toolkit](https://nifi.apache.org/docs/nifi-docs/html/walkthroughs.html#creating-and-securing-a-nifi-cluster-with-the-tls-toolkit)) and run a flow which handles incoming secured connections such as `ListenHTTP`, `HandleHttpRequest`, etc. 


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