[GitHub] [nifi] exceptionfactory commented on a change in pull request #4737: NIFI-8096 Deprecated ClientAuth references in SSLContextService

2021-01-06 Thread GitBox


exceptionfactory commented on a change in pull request #4737:
URL: https://github.com/apache/nifi/pull/4737#discussion_r552900595



##
File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ListenHTTP.java
##
@@ -193,7 +193,7 @@
 .description("Client Authentication policy for TLS connections. 
Required when SSL Context Service configured.")
 .required(false)
 .allowableValues(ClientAuth.values())
-.defaultValue(ClientAuth.REQUIRED.name())

Review comment:
   Previous behavior inferred requiring a client certificate when the 
`SSLContextService` was configured with trust store properties, falling back to 
wanting a client certificate in the absence of trust store properties.  Making 
`REQUIRED` the default value going forward sounds like the best approach.  
Perhaps putting a comment in future release notes indicating this change in 
behavior would be worthwhile for users of `ListenHTTP` who do not require 
clients to provide certificates.





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:
us...@infra.apache.org




[GitHub] [nifi] exceptionfactory commented on a change in pull request #4737: NIFI-8096 Deprecated ClientAuth references in SSLContextService

2021-01-06 Thread GitBox


exceptionfactory commented on a change in pull request #4737:
URL: https://github.com/apache/nifi/pull/4737#discussion_r552897870



##
File path: 
nifi-nar-bundles/nifi-standard-services/nifi-ssl-context-service-api/src/main/java/org/apache/nifi/ssl/SSLContextService.java
##
@@ -56,26 +56,36 @@
 }
 
 /**
- * Returns a configured {@link SSLContext} from the populated 
configuration values. This method is preferred
- * over the overloaded method which accepts the deprecated {@link 
ClientAuth} enum.
+ * Create and initialize {@link SSLContext} using configured properties. 
This method is preferred over deprecated
+ * create methods due to not requiring a client authentication policy.
+ *
+ * @return {@link SSLContext} initialized using configured properties
+ */
+SSLContext createContext();
+
+/**
+ * Returns a configured {@link SSLContext} from the populated 
configuration values. This method is deprecated
+ * due to {@link org.apache.nifi.security.util.ClientAuth} not being 
applicable or used when initializing the
+ * {@link SSLContext}
  *
  * @param clientAuth the desired level of client authentication
  * @return the configured SSLContext
  * @throws ProcessException if there is a problem configuring the context
  */
-SSLContext createSSLContext(final org.apache.nifi.security.util.ClientAuth 
clientAuth) throws ProcessException;
+@Deprecated
+SSLContext createSSLContext(org.apache.nifi.security.util.ClientAuth 
clientAuth) throws ProcessException;
 
 /**
  * Returns a configured {@link SSLContext} from the populated 
configuration values. This method is deprecated

Review comment:
   Added





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:
us...@infra.apache.org




[GitHub] [nifi] exceptionfactory commented on a change in pull request #4737: NIFI-8096 Deprecated ClientAuth references in SSLContextService

2021-01-06 Thread GitBox


exceptionfactory commented on a change in pull request #4737:
URL: https://github.com/apache/nifi/pull/4737#discussion_r552897747



##
File path: 
nifi-nar-bundles/nifi-standard-services/nifi-ssl-context-service-api/src/main/java/org/apache/nifi/ssl/SSLContextService.java
##
@@ -56,26 +56,36 @@
 }
 
 /**
- * Returns a configured {@link SSLContext} from the populated 
configuration values. This method is preferred
- * over the overloaded method which accepts the deprecated {@link 
ClientAuth} enum.
+ * Create and initialize {@link SSLContext} using configured properties. 
This method is preferred over deprecated
+ * create methods due to not requiring a client authentication policy.
+ *
+ * @return {@link SSLContext} initialized using configured properties
+ */
+SSLContext createContext();
+
+/**
+ * Returns a configured {@link SSLContext} from the populated 
configuration values. This method is deprecated

Review comment:
   Added





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:
us...@infra.apache.org




[GitHub] [nifi] exceptionfactory commented on a change in pull request #4737: NIFI-8096 Deprecated ClientAuth references in SSLContextService

2021-01-06 Thread GitBox


exceptionfactory commented on a change in pull request #4737:
URL: https://github.com/apache/nifi/pull/4737#discussion_r552897685



##
File path: 
nifi-nar-bundles/nifi-standard-services/nifi-ssl-context-bundle/nifi-ssl-context-service/src/main/java/org/apache/nifi/ssl/StandardSSLContextService.java
##
@@ -238,41 +238,55 @@ public TlsConfiguration createTlsConfiguration() {
 }
 
 /**
- * Returns a configured {@link SSLContext} from the populated 
configuration values. This method is preferred
- * over the overloaded method which accepts the deprecated {@link 
ClientAuth} enum.
+ * Create and initialize {@link SSLContext} using configured properties. 
This method is preferred over deprecated
+ * methods due to not requiring a client authentication policy. Invokes 
createTlsConfiguration() to prepare
+ * properties for processing.
+ *
+ * @return {@link SSLContext} initialized using configured properties
+ */
+@Override
+public SSLContext createContext() {
+final TlsConfiguration tlsConfiguration = createTlsConfiguration();
+if (!tlsConfiguration.isTruststorePopulated()) {
+getLogger().warn("Trust Store properties not found: using platform 
default Certificate Authorities");
+}
+
+try {
+final TrustManager[] trustManagers = 
SslContextFactory.getTrustManagers(tlsConfiguration);
+return SslContextFactory.createSslContext(tlsConfiguration, 
trustManagers);
+} catch (final TlsException e) {
+getLogger().error("Unable to create SSLContext: {}", new 
String[]{e.getLocalizedMessage()});
+throw new ProcessException("Unable to create SSLContext", e);
+}
+}
+
+/**
+ * Returns a configured {@link SSLContext} from the populated 
configuration values. This method is deprecated
+ * due to the Client Authentication policy not being applicable when 
initializing the SSLContext
  *
  * @param clientAuth the desired level of client authentication
  * @return the configured SSLContext
  * @throws ProcessException if there is a problem configuring the context
  */
+@Deprecated
 @Override
 public SSLContext createSSLContext(final 
org.apache.nifi.security.util.ClientAuth clientAuth) throws ProcessException {
-try {
-final TlsConfiguration tlsConfiguration = createTlsConfiguration();
-if (!tlsConfiguration.isTruststorePopulated()) {
-getLogger().warn("Trust Store properties not found: using 
platform default Certificate Authorities");
-}
-final TrustManager[] trustManagers = 
SslContextFactory.getTrustManagers(tlsConfiguration);
-return SslContextFactory.createSslContext(tlsConfiguration, 
trustManagers, clientAuth);
-} catch (TlsException e) {
-getLogger().error("Encountered an error creating the SSL context 
from the SSL context service: {}", new String[]{e.getLocalizedMessage()});
-throw new ProcessException("Error creating SSL context", e);
-}
+return createContext();
 }
 
 /**
  * Returns a configured {@link SSLContext} from the populated 
configuration values. This method is deprecated

Review comment:
   Added





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:
us...@infra.apache.org