[jira] [Commented] (NIFI-8057) Remove truststore check from SslContextFactory.createSslContext()

2020-12-03 Thread Peter Turcsanyi (Jira)


[ 
https://issues.apache.org/jira/browse/NIFI-8057?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17243400#comment-17243400
 ] 

Peter Turcsanyi commented on NIFI-8057:
---

[~exceptionfactory] It is absolutely fine with me and tanks for the proposed 
fix for the GRPC processors. Created a ticket for it: NIFI-8067
I believe this jira can be closed now and the generic solution for the other 
processors will be implemented in separate one.

> Remove truststore check from SslContextFactory.createSslContext()
> -
>
> Key: NIFI-8057
> URL: https://issues.apache.org/jira/browse/NIFI-8057
> Project: Apache NiFi
>  Issue Type: Bug
>Affects Versions: 1.12.0, 1.12.1
>Reporter: Peter Turcsanyi
>Priority: Major
>
> NIFI-7407 introduced a check in {{SslContextFactory.createSslContext()}}: if 
> KS is configured, then TS must be configured too 
> ([https://github.com/apache/nifi/blob/857eeca3c7d4b275fd698430594e7fae4864feff/nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/SslContextFactory.java#L79])
> This constraint is too strict for server-style processors (like ListenGRPC) 
> where only a KS is needed for 1-way SSL (and the presence of TS turns on 
> 2-way SSL).
> The check should be removed/relieved.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (NIFI-8057) Remove truststore check from SslContextFactory.createSslContext()

2020-12-03 Thread David Handermann (Jira)


[ 
https://issues.apache.org/jira/browse/NIFI-8057?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17243349#comment-17243349
 ] 

David Handermann commented on NIFI-8057:


[~turcsanyip] What do you think about making the proposed changes to ListenGRPC 
and addressing other processors separately as described?

> Remove truststore check from SslContextFactory.createSslContext()
> -
>
> Key: NIFI-8057
> URL: https://issues.apache.org/jira/browse/NIFI-8057
> Project: Apache NiFi
>  Issue Type: Bug
>Affects Versions: 1.12.0, 1.12.1
>Reporter: Peter Turcsanyi
>Priority: Major
>
> NIFI-7407 introduced a check in {{SslContextFactory.createSslContext()}}: if 
> KS is configured, then TS must be configured too 
> ([https://github.com/apache/nifi/blob/857eeca3c7d4b275fd698430594e7fae4864feff/nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/SslContextFactory.java#L79])
> This constraint is too strict for server-style processors (like ListenGRPC) 
> where only a KS is needed for 1-way SSL (and the presence of TS turns on 
> 2-way SSL).
> The check should be removed/relieved.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (NIFI-8057) Remove truststore check from SslContextFactory.createSslContext()

2020-12-03 Thread Joe Witt (Jira)


[ 
https://issues.apache.org/jira/browse/NIFI-8057?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17243318#comment-17243318
 ] 

Joe Witt commented on NIFI-8057:


I think that is totally up to you if you're doing the work.  

> Remove truststore check from SslContextFactory.createSslContext()
> -
>
> Key: NIFI-8057
> URL: https://issues.apache.org/jira/browse/NIFI-8057
> Project: Apache NiFi
>  Issue Type: Bug
>Affects Versions: 1.12.0, 1.12.1
>Reporter: Peter Turcsanyi
>Priority: Major
>
> NIFI-7407 introduced a check in {{SslContextFactory.createSslContext()}}: if 
> KS is configured, then TS must be configured too 
> ([https://github.com/apache/nifi/blob/857eeca3c7d4b275fd698430594e7fae4864feff/nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/SslContextFactory.java#L79])
> This constraint is too strict for server-style processors (like ListenGRPC) 
> where only a KS is needed for 1-way SSL (and the presence of TS turns on 
> 2-way SSL).
> The check should be removed/relieved.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (NIFI-8057) Remove truststore check from SslContextFactory.createSslContext()

2020-12-03 Thread David Handermann (Jira)


[ 
https://issues.apache.org/jira/browse/NIFI-8057?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17243317#comment-17243317
 ] 

David Handermann commented on NIFI-8057:


That makes sense.  For the particular issue with the ListenGRPC Processor, the 
previous behavior can be restored by refactoring to remove the call to 
createSslContext() and removing the unnecessary references to 
SSLContext.getProvider(), without changing the behavior of 
SslContextFactory.createSslContext().

[~joewitt] Do you recommend addressing issues with other Processors under this 
issue, or creating a new issue for each Processor impacted?

> Remove truststore check from SslContextFactory.createSslContext()
> -
>
> Key: NIFI-8057
> URL: https://issues.apache.org/jira/browse/NIFI-8057
> Project: Apache NiFi
>  Issue Type: Bug
>Affects Versions: 1.12.0, 1.12.1
>Reporter: Peter Turcsanyi
>Priority: Major
>
> NIFI-7407 introduced a check in {{SslContextFactory.createSslContext()}}: if 
> KS is configured, then TS must be configured too 
> ([https://github.com/apache/nifi/blob/857eeca3c7d4b275fd698430594e7fae4864feff/nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/SslContextFactory.java#L79])
> This constraint is too strict for server-style processors (like ListenGRPC) 
> where only a KS is needed for 1-way SSL (and the presence of TS turns on 
> 2-way SSL).
> The check should be removed/relieved.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (NIFI-8057) Remove truststore check from SslContextFactory.createSslContext()

2020-12-03 Thread Joe Witt (Jira)


[ 
https://issues.apache.org/jira/browse/NIFI-8057?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17243311#comment-17243311
 ] 

Joe Witt commented on NIFI-8057:


Fair point we already created disruption.  But lets still try to minimize it as 
best we can.  Thanks

> Remove truststore check from SslContextFactory.createSslContext()
> -
>
> Key: NIFI-8057
> URL: https://issues.apache.org/jira/browse/NIFI-8057
> Project: Apache NiFi
>  Issue Type: Bug
>Affects Versions: 1.12.0, 1.12.1
>Reporter: Peter Turcsanyi
>Priority: Major
>
> NIFI-7407 introduced a check in {{SslContextFactory.createSslContext()}}: if 
> KS is configured, then TS must be configured too 
> ([https://github.com/apache/nifi/blob/857eeca3c7d4b275fd698430594e7fae4864feff/nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/SslContextFactory.java#L79])
> This constraint is too strict for server-style processors (like ListenGRPC) 
> where only a KS is needed for 1-way SSL (and the presence of TS turns on 
> 2-way SSL).
> The check should be removed/relieved.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (NIFI-8057) Remove truststore check from SslContextFactory.createSslContext()

2020-12-03 Thread David Handermann (Jira)


[ 
https://issues.apache.org/jira/browse/NIFI-8057?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17243308#comment-17243308
 ] 

David Handermann commented on NIFI-8057:


Reviewing the release history, it appears that this change was released in 
version 1.12.0, so anyone upgrading from previous versions would already be 
impacted.

Reviewing ListenGRPC more closely, it appears that the createSslContext() call 
is not necessary, since the GRPC server depends on the Netty SslContextBuilder, 
which does not use the javax.net.ssl.SSLContext.  For this particular issue, 
ListenGRPC could be refactored to support the behavior from 1.11 and previous 
versions, which would still involve the implied one-way or two-way TLS handling 
based on whether trust store properties are configured.

Other processors would need to be evaluated separately, but it seems best to 
preserve the checks for empty trust store properties introduced in 1.12.0.

As far as maintaining backward compatibility in other processors, one option 
would be to review where createSslContext() is being called, determine whether 
that behavior exists now, and introduce an additional method that would 
explicitly load the JVM default trust store.  The component could log a warning 
indicating what is happening.  Introducing explicit loading of the default 
trust store at a higher level introduces more code, but it would preserve the 
sanity checking in the NiFi SslContextFactory.

> Remove truststore check from SslContextFactory.createSslContext()
> -
>
> Key: NIFI-8057
> URL: https://issues.apache.org/jira/browse/NIFI-8057
> Project: Apache NiFi
>  Issue Type: Bug
>Affects Versions: 1.12.0, 1.12.1
>Reporter: Peter Turcsanyi
>Priority: Major
>
> NIFI-7407 introduced a check in {{SslContextFactory.createSslContext()}}: if 
> KS is configured, then TS must be configured too 
> ([https://github.com/apache/nifi/blob/857eeca3c7d4b275fd698430594e7fae4864feff/nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/SslContextFactory.java#L79])
> This constraint is too strict for server-style processors (like ListenGRPC) 
> where only a KS is needed for 1-way SSL (and the presence of TS turns on 
> 2-way SSL).
> The check should be removed/relieved.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (NIFI-8057) Remove truststore check from SslContextFactory.createSslContext()

2020-12-03 Thread Joe Witt (Jira)


[ 
https://issues.apache.org/jira/browse/NIFI-8057?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17243274#comment-17243274
 ] 

Joe Witt commented on NIFI-8057:


Any thoughts on how to do that in a backward compatible way?  Such that old 
flows would keep working as they did?

> Remove truststore check from SslContextFactory.createSslContext()
> -
>
> Key: NIFI-8057
> URL: https://issues.apache.org/jira/browse/NIFI-8057
> Project: Apache NiFi
>  Issue Type: Bug
>Affects Versions: 1.12.1
>Reporter: Peter Turcsanyi
>Priority: Major
>
> NIFI-7407 introduced a check in {{SslContextFactory.createSslContext()}}: if 
> KS is configured, then TS must be configured too 
> ([https://github.com/apache/nifi/blob/857eeca3c7d4b275fd698430594e7fae4864feff/nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/SslContextFactory.java#L79])
> This constraint is too strict for server-style processors (like ListenGRPC) 
> where only a KS is needed for 1-way SSL (and the presence of TS turns on 
> 2-way SSL).
> The check should be removed/relieved.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (NIFI-8057) Remove truststore check from SslContextFactory.createSslContext()

2020-12-03 Thread David Handermann (Jira)


[ 
https://issues.apache.org/jira/browse/NIFI-8057?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17243228#comment-17243228
 ] 

David Handermann commented on NIFI-8057:


Removing the null check for trust store properties seems like it would obscure 
the behavior of SSLContext references.  Although some processors, such as 
ListenHTTP, use the lack of trust store properties to infer one-way TLS 
communication, it would be much better to make this behavior explicit through 
the addition of Client Authentication properties on relevant processors.

The fundamental issue is that when trust store properties are not provided, 
null is passed to SSLContext.init() as the argument for Trust Managers.  Under 
the hood, the JVM attempts to load the system default trust store, which 
includes standard public certificate authorities.  These public certificate 
authorities are not referenced for one-way TLS, since the client is not 
presenting a certificate, but they would be referenced in two-way TLS.  In that 
scenario, if someone did not pass trust store properties as part of the TLS 
configuration, but attempted to enable two-way TLS, client certificate 
validation would fail unless the client certificate was signed by a public 
certificate authority.  There is at least one other open issue for supporting 
SSLContextService configuration using the JVM default trust store when 
communicating with public HTTPS services, but using empty values to imply the 
system default trust store does not seem like the best approach.

With that background, it seems better to address this issue by adding Client 
Authentication properties to ListenGRPC and any other applicable listening 
processors.

> Remove truststore check from SslContextFactory.createSslContext()
> -
>
> Key: NIFI-8057
> URL: https://issues.apache.org/jira/browse/NIFI-8057
> Project: Apache NiFi
>  Issue Type: Bug
>Affects Versions: 1.12.1
>Reporter: Peter Turcsanyi
>Priority: Major
>
> NIFI-7407 introduced a check in {{SslContextFactory.createSslContext()}}: if 
> KS is configured, then TS must be configured too 
> ([https://github.com/apache/nifi/blob/857eeca3c7d4b275fd698430594e7fae4864feff/nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/SslContextFactory.java#L79])
> This constraint is too strict for server-style processors (like ListenGRPC) 
> where only a KS is needed for 1-way SSL (and the presence of TS turns on 
> 2-way SSL).
> The check should be removed/relieved.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (NIFI-8057) Remove truststore check from SslContextFactory.createSslContext()

2020-12-01 Thread Peter Turcsanyi (Jira)


[ 
https://issues.apache.org/jira/browse/NIFI-8057?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17241877#comment-17241877
 ] 

Peter Turcsanyi commented on NIFI-8057:
---

[~alopresto] do you have any concerns about it?

> Remove truststore check from SslContextFactory.createSslContext()
> -
>
> Key: NIFI-8057
> URL: https://issues.apache.org/jira/browse/NIFI-8057
> Project: Apache NiFi
>  Issue Type: Bug
>Affects Versions: 1.12.1
>Reporter: Peter Turcsanyi
>Priority: Major
>
> NIFI-7407 introduced a check in {{SslContextFactory.createSslContext()}}: if 
> KS is configured, then TS must be configured too 
> ([https://github.com/apache/nifi/blob/857eeca3c7d4b275fd698430594e7fae4864feff/nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/SslContextFactory.java#L79])
> This constraint is too strict for server-style processors (like ListenGRPC) 
> where only a KS is needed for 1-way SSL (and the presence of TS turns on 
> 2-way SSL).
> The check should be removed/relieved.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)