[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-15 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user asfgit closed the pull request at:

https://github.com/apache/calcite-avatica/pull/61


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-14 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user karanmehta93 commented on the issue:

https://github.com/apache/calcite-avatica/pull/57
  
https://github.com/apache/calcite-avatica/pull/61
Done. Small change only. All tests pass locally.


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-14 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

GitHub user karanmehta93 opened a pull request:

https://github.com/apache/calcite-avatica/pull/61

CALCITE-2285 Support client cert keystore for Avatica Client (Addendu…

…m, Remove unused code)
@joshelser Quick review please!

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/karanmehta93/calcite-avatica master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/calcite-avatica/pull/61.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #61


commit a49e0a3d7843522726487ca9535d085a98d0e937
Author: Karan Mehta 
Date:   2018-06-15T04:00:12Z

CALCITE-2285 Support client cert keystore for Avatica Client (Addendum, 
Remove unused code)




> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-14 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user joshelser commented on the issue:

https://github.com/apache/calcite-avatica/pull/57
  
@karanmehta93 feel free to throw up a new PR. I'll just reopen the Jira 
issue for now :)


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-14 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user karanmehta93 commented on the issue:

https://github.com/apache/calcite-avatica/pull/57
  
@joshelser I just realized, I forgot to clean up the code for 
`configureHttpsSocket` variable after separating the registry builder stuff in 
`configureSocketFactories()` for http/https. Should I put up an addendum?


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-14 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user asfgit closed the pull request at:

https://github.com/apache/calcite-avatica/pull/57


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-14 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user risdenk commented on the issue:

https://github.com/apache/calcite-avatica/pull/57
  
Tested this change with upgrading bouncycastle to 1.59 (CALCITE-2361) 
locally on JDK 11 build 17 and all works good. +1 to merge from me.


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-14 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user joshelser commented on the issue:

https://github.com/apache/calcite-avatica/pull/57
  
Thanks, Kevin! I missed that


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-14 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user risdenk commented on the issue:

https://github.com/apache/calcite-avatica/pull/57
  
Looks like the JDK 11 failure is
```
[ERROR] error reading 
/root/.m2/repository/org/bouncycastle/bcprov-jdk15on/1.55/bcprov-jdk15on-1.55.jar;
 invalid manifest format
```

This was brought up on the Calcite dev mailing list and could be a JDK bug 
with JDK 11 build 17. Looks like build 18 might fix this.

Copying below for reference:

> Agree about this might be a JDK bug.
> https://bugs.openjdk.java.net/browse/JDK-8200530
> and followed up by 
> https://bugs.openjdk.java.net/browse/JDK-8204494
> I checked the Netty Manifest file and it uses /r/n.
> This should be fixed in build 18 and was broken in build 17.


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-13 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user F21 commented on the issue:

https://github.com/apache/calcite-avatica/pull/57
  
@joshelser I will let you merge as the JDK 11 tests are failing on travis.


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-13 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user joshelser commented on the issue:

https://github.com/apache/calcite-avatica/pull/57
  
I think this is ready. Will try to swing by and merge if @F21 doesn't beat 
me to it.


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-13 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user joshelser commented on the issue:

https://github.com/apache/calcite-avatica/pull/57
  
One minor change in the test, otherwise looks good.
Seems like we have some JDK11 issues when building -- don't need to fix 
that right now :)


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-13 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user joshelser commented on a diff in the pull request:

https://github.com/apache/calcite-avatica/pull/57#discussion_r195190242
  
--- Diff: 
core/src/test/java/org/apache/calcite/avatica/remote/AvaticaCommonsHttpClientImplSocketFactoryTest.java
 ---
@@ -74,25 +60,51 @@
   }
 
   @Test public void testTrustStoreLoadedInFactory() throws Exception {
+configureHttpsClient();
 client.setTrustStore(storeFile, password);
 assertTrue("Https socket should be configured"
 + " with truststore", client.configureHttpsSocket);
+verifyFactoryInstance(client, HTTP_REGISTRY, null);
 verifyFactoryInstance(client, HTTPS_REGISTRY, 
SSLConnectionSocketFactory.class);
 verify(client, times(1)).configureSocketFactories();
 verify(client, times(1)).loadTrustStore(any(SSLContextBuilder.class));
 verify(client, times(0)).loadKeyStore(any(SSLContextBuilder.class));
   }
 
   @Test public void testKeyStoreLoadedInFactory() throws Exception {
+configureHttpsClient();
 client.setKeyStore(storeFile, password, password);
 assertTrue("Https socket should be configured"
 + " with keystore", client.configureHttpsSocket);
+verifyFactoryInstance(client, HTTP_REGISTRY, null);
 verifyFactoryInstance(client, HTTPS_REGISTRY, 
SSLConnectionSocketFactory.class);
 verify(client, times(1)).configureSocketFactories();
 verify(client, times(0)).loadTrustStore(any(SSLContextBuilder.class));
 verify(client, times(1)).loadKeyStore(any(SSLContextBuilder.class));
   }
 
+  private void configureHttpClient() throws Exception {
+url = new URL("http://fake_url.com";);
+configureClient();
+  }
+
+  private void configureHttpsClient() throws Exception {
+url = new URL("https://fake_url.com";);
+configureClient();
+  }
+
+  private void configureClient() throws Exception {
+client = spy(new AvaticaCommonsHttpClientImpl(url));
+// storeFile can be used as either Keystore/Truststore
+storeFile = mock(File.class);
+when(storeFile.exists()).thenReturn(true);
+when(storeFile.isFile()).thenReturn(true);
+password = new String("");
--- End diff --

Just `""` :)


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-13 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user karanmehta93 commented on a diff in the pull request:

https://github.com/apache/calcite-avatica/pull/57#discussion_r195162268
  
--- Diff: 
core/src/main/java/org/apache/calcite/avatica/remote/AvaticaCommonsHttpClientImpl.java
 ---
@@ -127,11 +121,54 @@ private void initializeClient() {
 final String maxCnxnsPerRoute = 
System.getProperty(MAX_POOLED_CONNECTION_PER_ROUTE_KEY,
 MAX_POOLED_CONNECTION_PER_ROUTE_DEFAULT);
 pool.setDefaultMaxPerRoute(Integer.parseInt(maxCnxnsPerRoute));
+  }
 
-this.authCache = new BasicAuthCache();
+  protected Registry configureSocketFactories() {
+RegistryBuilder registryBuilder = 
RegistryBuilder.create();
+configureHttpRegistry(registryBuilder);
+configureHttpsRegistry(registryBuilder);
+return registryBuilder.build();
+  }
 
-// A single thread-safe HttpClient, pooling connections via the 
ConnectionManager
-this.client = HttpClients.custom().setConnectionManager(pool).build();
+  protected void 
configureHttpsRegistry(RegistryBuilder 
registryBuilder) {
+if (!configureHttpsSocket) {
--- End diff --

As FYI, I ended up using `host.getSchemeName()` where host is a `HttpPost` 
object. The value is populated by `url.getProtocol()` only.


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-12 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user karanmehta93 commented on the issue:

https://github.com/apache/calcite-avatica/pull/57
  
Updated the PR. All the tests pass locally.


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-12 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user joshelser commented on a diff in the pull request:

https://github.com/apache/calcite-avatica/pull/57#discussion_r194907083
  
--- Diff: 
core/src/main/java/org/apache/calcite/avatica/remote/AvaticaCommonsHttpClientImpl.java
 ---
@@ -127,11 +121,54 @@ private void initializeClient() {
 final String maxCnxnsPerRoute = 
System.getProperty(MAX_POOLED_CONNECTION_PER_ROUTE_KEY,
 MAX_POOLED_CONNECTION_PER_ROUTE_DEFAULT);
 pool.setDefaultMaxPerRoute(Integer.parseInt(maxCnxnsPerRoute));
+  }
 
-this.authCache = new BasicAuthCache();
+  protected Registry configureSocketFactories() {
+RegistryBuilder registryBuilder = 
RegistryBuilder.create();
+configureHttpRegistry(registryBuilder);
+configureHttpsRegistry(registryBuilder);
+return registryBuilder.build();
+  }
 
-// A single thread-safe HttpClient, pooling connections via the 
ConnectionManager
-this.client = HttpClients.custom().setConnectionManager(pool).build();
+  protected void 
configureHttpsRegistry(RegistryBuilder 
registryBuilder) {
+if (!configureHttpsSocket) {
--- End diff --

👍 


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-12 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user karanmehta93 commented on a diff in the pull request:

https://github.com/apache/calcite-avatica/pull/57#discussion_r194906872
  
--- Diff: 
core/src/main/java/org/apache/calcite/avatica/remote/AvaticaCommonsHttpClientImpl.java
 ---
@@ -127,11 +121,54 @@ private void initializeClient() {
 final String maxCnxnsPerRoute = 
System.getProperty(MAX_POOLED_CONNECTION_PER_ROUTE_KEY,
 MAX_POOLED_CONNECTION_PER_ROUTE_DEFAULT);
 pool.setDefaultMaxPerRoute(Integer.parseInt(maxCnxnsPerRoute));
+  }
 
-this.authCache = new BasicAuthCache();
+  protected Registry configureSocketFactories() {
+RegistryBuilder registryBuilder = 
RegistryBuilder.create();
+configureHttpRegistry(registryBuilder);
+configureHttpsRegistry(registryBuilder);
+return registryBuilder.build();
+  }
 
-// A single thread-safe HttpClient, pooling connections via the 
ConnectionManager
-this.client = HttpClients.custom().setConnectionManager(pool).build();
+  protected void 
configureHttpsRegistry(RegistryBuilder 
registryBuilder) {
+if (!configureHttpsSocket) {
--- End diff --

Actually I will use `url.getProtocol()` method to handle it for me. Seems 
better option.


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-12 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user karanmehta93 commented on a diff in the pull request:

https://github.com/apache/calcite-avatica/pull/57#discussion_r194906396
  
--- Diff: 
core/src/main/java/org/apache/calcite/avatica/remote/AvaticaCommonsHttpClientImpl.java
 ---
@@ -127,11 +121,54 @@ private void initializeClient() {
 final String maxCnxnsPerRoute = 
System.getProperty(MAX_POOLED_CONNECTION_PER_ROUTE_KEY,
 MAX_POOLED_CONNECTION_PER_ROUTE_DEFAULT);
 pool.setDefaultMaxPerRoute(Integer.parseInt(maxCnxnsPerRoute));
+  }
 
-this.authCache = new BasicAuthCache();
+  protected Registry configureSocketFactories() {
+RegistryBuilder registryBuilder = 
RegistryBuilder.create();
+configureHttpRegistry(registryBuilder);
+configureHttpsRegistry(registryBuilder);
+return registryBuilder.build();
+  }
 
-// A single thread-safe HttpClient, pooling connections via the 
ConnectionManager
-this.client = HttpClients.custom().setConnectionManager(pool).build();
+  protected void 
configureHttpsRegistry(RegistryBuilder 
registryBuilder) {
+if (!configureHttpsSocket) {
--- End diff --

I looked through the code. That's correct. The `HttpHost` required by 
`CloseableHttpClient` is created in the constructor itself. `HttpHost` is the 
one that uses the URL provided during the initialization. So there's a tight 
coupling here and there will never be a case where both of them will be 
together. I will update the code to configure either of these factories 
depending on URL prefix as `http` vs `https`. What do you think?


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-12 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user joshelser commented on a diff in the pull request:

https://github.com/apache/calcite-avatica/pull/57#discussion_r194901281
  
--- Diff: 
core/src/main/java/org/apache/calcite/avatica/remote/AvaticaCommonsHttpClientImpl.java
 ---
@@ -127,11 +121,54 @@ private void initializeClient() {
 final String maxCnxnsPerRoute = 
System.getProperty(MAX_POOLED_CONNECTION_PER_ROUTE_KEY,
 MAX_POOLED_CONNECTION_PER_ROUTE_DEFAULT);
 pool.setDefaultMaxPerRoute(Integer.parseInt(maxCnxnsPerRoute));
+  }
 
-this.authCache = new BasicAuthCache();
+  protected Registry configureSocketFactories() {
+RegistryBuilder registryBuilder = 
RegistryBuilder.create();
+configureHttpRegistry(registryBuilder);
+configureHttpsRegistry(registryBuilder);
+return registryBuilder.build();
+  }
 
-// A single thread-safe HttpClient, pooling connections via the 
ConnectionManager
-this.client = HttpClients.custom().setConnectionManager(pool).build();
+  protected void 
configureHttpsRegistry(RegistryBuilder 
registryBuilder) {
+if (!configureHttpsSocket) {
--- End diff --

I think there's a strong coupling of HttpClient to a specific avatica 
server instance ( going off of memory). If that's the case, then there's no 
problem in making the determination as to what kind of http client we need to 
set up.


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-12 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user karanmehta93 commented on a diff in the pull request:

https://github.com/apache/calcite-avatica/pull/57#discussion_r194900551
  
--- Diff: 
core/src/main/java/org/apache/calcite/avatica/remote/AvaticaCommonsHttpClientImpl.java
 ---
@@ -127,11 +121,54 @@ private void initializeClient() {
 final String maxCnxnsPerRoute = 
System.getProperty(MAX_POOLED_CONNECTION_PER_ROUTE_KEY,
 MAX_POOLED_CONNECTION_PER_ROUTE_DEFAULT);
 pool.setDefaultMaxPerRoute(Integer.parseInt(maxCnxnsPerRoute));
+  }
 
-this.authCache = new BasicAuthCache();
+  protected Registry configureSocketFactories() {
+RegistryBuilder registryBuilder = 
RegistryBuilder.create();
+configureHttpRegistry(registryBuilder);
+configureHttpsRegistry(registryBuilder);
+return registryBuilder.build();
+  }
 
-// A single thread-safe HttpClient, pooling connections via the 
ConnectionManager
-this.client = HttpClients.custom().setConnectionManager(pool).build();
+  protected void 
configureHttpsRegistry(RegistryBuilder 
registryBuilder) {
+if (!configureHttpsSocket) {
--- End diff --

I agree, but sometimes various services can have varied requirements. The 
best and simple case is to have either http or https, however I didn't want to 
change the original implementation, so I added both of them here as well.
What do you think?


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-12 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user karanmehta93 commented on a diff in the pull request:

https://github.com/apache/calcite-avatica/pull/57#discussion_r194900053
  
--- Diff: 
core/src/main/java/org/apache/calcite/avatica/remote/AvaticaCommonsHttpClientImpl.java
 ---
@@ -95,29 +103,15 @@ public AvaticaCommonsHttpClientImpl(URL url) {
   }
 
   private void initializeClient() {
-SSLConnectionSocketFactory sslFactory = null;
-if (null != truststore && null != truststorePassword) {
-  try {
-SSLContext sslcontext = SSLContexts.custom().loadTrustMaterial(
-truststore, truststorePassword.toCharArray()).build();
-
-final HostnameVerifier verifier = 
getHostnameVerifier(hostnameVerification);
-
-sslFactory = new SSLConnectionSocketFactory(sslcontext, verifier);
-  } catch (Exception e) {
-throw new RuntimeException(e);
-  }
-} else {
-  LOG.debug("Not configuring HTTPS because of missing 
truststore/password");
-}
+socketFactoryRegistry = this.configureSocketFactories();
+configureConnectionPool(socketFactoryRegistry);
+this.authCache = new BasicAuthCache();
+// A single thread-safe HttpClient, pooling connections via the 
ConnectionManager
+this.client = HttpClients.custom().setConnectionManager(pool).build();
+  }
 
-RegistryBuilder registryBuilder = 
RegistryBuilder.create();
-registryBuilder.register("http", 
PlainConnectionSocketFactory.getSocketFactory());
-// Only register the SSL factory when provided
-if (null != sslFactory) {
-  registryBuilder.register("https", sslFactory);
-}
-pool = new PoolingHttpClientConnectionManager(registryBuilder.build());
+  protected void configureConnectionPool(Registry 
configureSocketFactory) {
+pool = new PoolingHttpClientConnectionManager(configureSocketFactory);
--- End diff --

Oh yes, updated it locally but forgot to push it. Will do.


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-12 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user joshelser commented on a diff in the pull request:

https://github.com/apache/calcite-avatica/pull/57#discussion_r194899085
  
--- Diff: 
core/src/main/java/org/apache/calcite/avatica/remote/AvaticaCommonsHttpClientImpl.java
 ---
@@ -127,11 +121,54 @@ private void initializeClient() {
 final String maxCnxnsPerRoute = 
System.getProperty(MAX_POOLED_CONNECTION_PER_ROUTE_KEY,
 MAX_POOLED_CONNECTION_PER_ROUTE_DEFAULT);
 pool.setDefaultMaxPerRoute(Integer.parseInt(maxCnxnsPerRoute));
+  }
 
-this.authCache = new BasicAuthCache();
+  protected Registry configureSocketFactories() {
+RegistryBuilder registryBuilder = 
RegistryBuilder.create();
+configureHttpRegistry(registryBuilder);
+configureHttpsRegistry(registryBuilder);
+return registryBuilder.build();
+  }
 
-// A single thread-safe HttpClient, pooling connections via the 
ConnectionManager
-this.client = HttpClients.custom().setConnectionManager(pool).build();
+  protected void 
configureHttpsRegistry(RegistryBuilder 
registryBuilder) {
+if (!configureHttpsSocket) {
--- End diff --

Actually, if we look at the URL, then may push this check up to 
`configureSocketFactories()` and call only one of `configureHttpRegsitry` and 
`configureHttpsRegistry`?


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-12 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user joshelser commented on a diff in the pull request:

https://github.com/apache/calcite-avatica/pull/57#discussion_r194898896
  
--- Diff: 
core/src/main/java/org/apache/calcite/avatica/remote/AvaticaCommonsHttpClientImpl.java
 ---
@@ -95,29 +103,15 @@ public AvaticaCommonsHttpClientImpl(URL url) {
   }
 
   private void initializeClient() {
-SSLConnectionSocketFactory sslFactory = null;
-if (null != truststore && null != truststorePassword) {
-  try {
-SSLContext sslcontext = SSLContexts.custom().loadTrustMaterial(
-truststore, truststorePassword.toCharArray()).build();
-
-final HostnameVerifier verifier = 
getHostnameVerifier(hostnameVerification);
-
-sslFactory = new SSLConnectionSocketFactory(sslcontext, verifier);
-  } catch (Exception e) {
-throw new RuntimeException(e);
-  }
-} else {
-  LOG.debug("Not configuring HTTPS because of missing 
truststore/password");
-}
+socketFactoryRegistry = this.configureSocketFactories();
+configureConnectionPool(socketFactoryRegistry);
+this.authCache = new BasicAuthCache();
+// A single thread-safe HttpClient, pooling connections via the 
ConnectionManager
+this.client = HttpClients.custom().setConnectionManager(pool).build();
+  }
 
-RegistryBuilder registryBuilder = 
RegistryBuilder.create();
-registryBuilder.register("http", 
PlainConnectionSocketFactory.getSocketFactory());
-// Only register the SSL factory when provided
-if (null != sslFactory) {
-  registryBuilder.register("https", sslFactory);
-}
-pool = new PoolingHttpClientConnectionManager(registryBuilder.build());
+  protected void configureConnectionPool(Registry 
configureSocketFactory) {
+pool = new PoolingHttpClientConnectionManager(configureSocketFactory);
--- End diff --

meant to be `connectionSocketFactory`?


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-12 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user joshelser commented on a diff in the pull request:

https://github.com/apache/calcite-avatica/pull/57#discussion_r194898638
  
--- Diff: 
core/src/main/java/org/apache/calcite/avatica/remote/AvaticaCommonsHttpClientImpl.java
 ---
@@ -127,11 +121,54 @@ private void initializeClient() {
 final String maxCnxnsPerRoute = 
System.getProperty(MAX_POOLED_CONNECTION_PER_ROUTE_KEY,
 MAX_POOLED_CONNECTION_PER_ROUTE_DEFAULT);
 pool.setDefaultMaxPerRoute(Integer.parseInt(maxCnxnsPerRoute));
+  }
 
-this.authCache = new BasicAuthCache();
+  protected Registry configureSocketFactories() {
+RegistryBuilder registryBuilder = 
RegistryBuilder.create();
+configureHttpRegistry(registryBuilder);
+configureHttpsRegistry(registryBuilder);
+return registryBuilder.build();
+  }
 
-// A single thread-safe HttpClient, pooling connections via the 
ConnectionManager
-this.client = HttpClients.custom().setConnectionManager(pool).build();
+  protected void 
configureHttpsRegistry(RegistryBuilder 
registryBuilder) {
+if (!configureHttpsSocket) {
--- End diff --

What about looking at the scheme of the URL instead of using "one of" the 
TLS configuration options?


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-11 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user F21 commented on the issue:

https://github.com/apache/calcite-avatica/pull/57
  
ping @joshelser 


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-11 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user karanmehta93 commented on the issue:

https://github.com/apache/calcite-avatica/pull/57
  
@F21 @aaraujo Added tests for the new code. Please review.


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
> Fix For: avatica-1.12.0
>
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-08 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user F21 commented on the issue:

https://github.com/apache/calcite-avatica/pull/57
  
Thanks for contributing, @karanmehta93! I'd love to get this into 
avatica-1.12 if possible.


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-08 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user karanmehta93 commented on the issue:

https://github.com/apache/calcite-avatica/pull/57
  
@aaraujo 
I did manual testing here. I will update the PR with tests. There are no 
tests covering the truststore code path, so I will have to figure out a way of 
adding tests there.


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-08 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

Github user aaraujo commented on the issue:

https://github.com/apache/calcite-avatica/pull/57
  
Lgtm, but can you add tests for the new config paths @karanmehta93?


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-06-08 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on CALCITE-2285:
-

GitHub user karanmehta93 opened a pull request:

https://github.com/apache/calcite-avatica/pull/57

CALCITE-2285 Support client cert keystore for Avatica Client

@joshelser @aaraujo 
Please review.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/karanmehta93/calcite-avatica master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/calcite-avatica/pull/57.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #57


commit 6e5d496ed1c9ca6574c3a93785a181d7addc5d56
Author: Karan Mehta 
Date:   2018-06-08T17:39:03Z

CALCITE-2285 Support client cert keystore for Avatica Client




> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-05-31 Thread Karan Mehta (JIRA)


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

Karan Mehta commented on CALCITE-2285:
--

Jetty offers client/server classes which allow dynamic reloading of 
{{SslContextFactory}} when ever new certificates are loaded, especially for 
short lived certificates. Avatica Client depends on Apache HttpClient lib, 
which doesn't offer that feature. Long running Java clients can potentially run 
into issues with this. 

Any thoughts/ideas? [~alexaraujo] [~risdenk]
I am currently looking into other potential ideas and will post soon.

> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-05-03 Thread Karan Mehta (JIRA)

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

Karan Mehta commented on CALCITE-2285:
--

{quote}
I'm familiar with two-way SSL/mutual authn, but I'm getting the impression that 
MTLS goes farther than that? Is this something that is generally implemented by 
all HTTP servers
{quote}
First level check for MTLS includes whether the client cert if verified by one 
of the trusted roots present in Server truststore or if no truststore is 
provided then it checks if both the server and client side certs have same 
roots.
Customized logic can be added to filter/authenticate users based on other 
fields present in the cert. For example, we can use CN field of the cert to 
whitelist certain hosts (since CN field usually contains hostname) or store 
client information in OU field and whitelist certain clients based on it.

{quote}
 Is this something that is generally implemented by all HTTP servers?
{quote}
Not necessarily, it just extra security measure. Generally used only in 
internal environments, for example, company servers, where all clients are 
well-known in advance.


> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-05-02 Thread Josh Elser (JIRA)

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

Josh Elser commented on CALCITE-2285:
-

Cool stuff, Karan.

I'm familiar with two-way SSL/mutual authn, but I'm getting the impression that 
MTLS goes farther than that? Is this something that is generally implemented by 
all HTTP servers?

> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Karan Mehta
>Priority: Major
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-04-28 Thread Karan Mehta (JIRA)

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

Karan Mehta commented on CALCITE-2285:
--

{quote}Tried to assign to you but doesn't look like you can. In any case you 
can put together a PR
{quote}
Thank you! Will work on it and put a PR.
{quote}Out of curiosity would the change in CALCITE-2284 help here?
{quote}
Yes. At this point, Avatica server doesn't doesn't allow filtering based on 
client-certs. We plan to add our own internal customizer for doing that. The 
client needs to send X509 certs, this PR will enhance the client to allow it. 

 

> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Priority: Major
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-04-27 Thread Kevin Risden (JIRA)

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

Kevin Risden commented on CALCITE-2285:
---

[~karanmehta93] - Tried to assign to you but doesn't look like you can. In any 
case you can put together a PR. 
[https://calcite.apache.org/avatica/develop/avatica.html#contributing]

Out of curiosity would the change in CALCITE-2284 help here?

> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Priority: Major
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (CALCITE-2285) Support client cert keystore for Avatica Client

2018-04-26 Thread Karan Mehta (JIRA)

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

Karan Mehta commented on CALCITE-2285:
--

[~julianhyde] I would like to work on this. Can you assign it to me?

> Support client cert keystore for Avatica Client
> ---
>
> Key: CALCITE-2285
> URL: https://issues.apache.org/jira/browse/CALCITE-2285
> Project: Calcite
>  Issue Type: Improvement
>  Components: avatica
>Reporter: Karan Mehta
>Assignee: Julian Hyde
>Priority: Major
>
> Currently Avatica only supports adding trust-store in {{SSLContext}} in all 
> {{AvaticaHttpClient}} implementations. If keystore support it added, MTLS 
> connections can be established as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)