You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "prmoore77 (via GitHub)" <gi...@apache.org> on 2023/10/25 14:10:21 UTC
[PR] GH-38460: [Arrow Flight JDBC Drvier] - add mTLS support [arrow]
prmoore77 opened a new pull request, #38461:
URL: https://github.com/apache/arrow/pull/38461
<!--
Thanks for opening a pull request!
If this is your first pull request you can find detailed information on how
to contribute here:
* [New Contributor's Guide](https://arrow.apache.org/docs/dev/developers/guide/step_by_step/pr_lifecycle.html#reviews-and-merge-of-the-pull-request)
* [Contributing Overview](https://arrow.apache.org/docs/dev/developers/overview.html)
If this is not a [minor PR](https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose
Opening GitHub issues ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
Then could you also rename the pull request title in the following format?
GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
or
MINOR: [${COMPONENT}] ${SUMMARY}
In the case of PARQUET issues on JIRA the title also supports:
PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
-->
### Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
### What changes are included in this PR?
This PR adds support for mTLS (client certificate verification/authentication) to the Arrow Flight SQL JDBC driver.
### Are these changes tested?
Yes, I've added tests of the new mTLS functionality - and have ensured that the change is backward compatible by verifying all existing tests pass.
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
-->
### Are there any user-facing changes?
Yes - but the end-user documentation for the Arrow Flight SQL JDBC driver has been updated in the PR itself.
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please uncomment the line below and explain which changes are breaking.
-->
<!-- **This PR includes breaking changes to public APIs.** -->
<!--
Please uncomment the line below (and provide explanation) if the changes fix either (a) a security vulnerability, (b) a bug that caused incorrect or invalid data to be produced, or (c) a bug that causes a crash (even when the API contract is upheld). We use this to highlight fixes to issues that may affect users without their knowledge. For this reason, fixing bugs that cause errors don't count, since those are usually obvious.
-->
<!-- **This PR contains a "Critical Fix".** -->
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on PR #38461:
URL: https://github.com/apache/arrow/pull/38461#issuecomment-1795458426
> @prmoore77 heads up, after #38521 a copy constructor was added to ArrowFlightSqlClientHandler.Builder So any new fields that this PR adds will need to be added to that copy constructor.
>
> #38580 also changes ArrowFligthSqlClientHandler to set defaults correctly and adds tests for verifying defaults, so if that goes in first, new fields will be added there too.
Hi @jduo - I've added the new fields to the new copy constructor (from https://github.com/apache/arrow/pull/38521) - per your recommendation. Thanks!
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1382151693
##########
java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/client/utils/ClientAuthenticationUtils.java:
##########
@@ -235,6 +235,57 @@ public static InputStream getCertificateStream(final String keyStorePath,
return getSingleCertificateInputStream(keyStore);
}
+ /**
+ * Generates an {@link InputStream} that contains certificates for path-based
+ * TLS Root Certificates.
+ *
+ * @param tlsRootsCertificatesPath The path of the TLS Root Certificates.
+ * @return a new {code InputStream} containing the certificates.
+ * @throws GeneralSecurityException on error.
+ * @throws IOException on error.
+ */
+ public static InputStream getTlsRootCertificatesStream(final String tlsRootsCertificatesPath)
+ throws GeneralSecurityException, IOException {
+ Preconditions.checkNotNull(tlsRootsCertificatesPath, "TLS Root certificates path cannot be null!");
+
+ return Files
+ .newInputStream(Paths.get(Preconditions.checkNotNull(tlsRootsCertificatesPath)));
Review Comment:
This should now be fixed.
##########
java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTlsRootCertsTest.java:
##########
@@ -0,0 +1,356 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.driver.jdbc;
+
+import static org.junit.Assert.assertNotNull;
+
+import java.net.URLEncoder;
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.Properties;
+
+import org.apache.arrow.driver.jdbc.authentication.UserPasswordAuthentication;
+import org.apache.arrow.driver.jdbc.client.ArrowFlightSqlClientHandler;
+import org.apache.arrow.driver.jdbc.utils.ArrowFlightConnectionConfigImpl.ArrowFlightConnectionProperty;
+import org.apache.arrow.driver.jdbc.utils.FlightSqlTestCertificates;
+import org.apache.arrow.driver.jdbc.utils.MockFlightSqlProducer;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.util.AutoCloseables;
+import org.apache.calcite.avatica.org.apache.http.auth.UsernamePasswordCredentials;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+
+/**
+ * Tests encrypted connections.
+ */
+public class ConnectionTlsRootCertsTest {
+
+ @ClassRule
+ public static final FlightServerTestRule FLIGHT_SERVER_TEST_RULE;
+ private static final String tlsRootCertsPath;
+ private static final String badTlsRootCertsPath;
+ private static final MockFlightSqlProducer PRODUCER = new MockFlightSqlProducer();
+ private static final String userTest = "user1";
+ private static final String passTest = "pass1";
+
+ static {
+ final FlightSqlTestCertificates.CertKeyPair
+ certKey = FlightSqlTestCertificates.exampleTlsCerts().get(0);
+
+ tlsRootCertsPath = certKey.cert.getPath();
+
+ badTlsRootCertsPath = certKey.cert.getPath() + ".bad";
+
+ UserPasswordAuthentication authentication = new UserPasswordAuthentication.Builder()
+ .user(userTest, passTest)
+ .build();
+
+ FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder()
+ .authentication(authentication)
+ .useEncryption(certKey.cert, certKey.key)
+ .producer(PRODUCER)
+ .build();
+ }
+
+ private BufferAllocator allocator;
+
+ @Before
+ public void setUp() throws Exception {
+ allocator = new RootAllocator(Long.MAX_VALUE);
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ allocator.getChildAllocators().forEach(BufferAllocator::close);
+ AutoCloseables.close(allocator);
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetEncryptedClientAuthenticated() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler client =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ assertNotNull(client);
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient providing a bad TLS Root Certs Path. It's expected to
+ * receive the SQLException.
+ *
+ * @throws Exception on error.
+ */
+ @Test(expected = SQLException.class)
Review Comment:
This should now be done.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #38461:
URL: https://github.com/apache/arrow/pull/38461#issuecomment-1795705144
I did a quick test and review. LGTM. Thanks !
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "jduo (via GitHub)" <gi...@apache.org>.
jduo commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1380622748
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -306,28 +329,63 @@ public Builder maxInboundMessageSize(int maxMessageSize) {
return this;
}
+ /**
+ * A small utility function to ensure that InputStream attributes.
+ * are closed if they are not null
+ * @param stream The InputStream to close (if it is not null).
+ */
+ private void closeInputStreamIfNotNull(InputStream stream) throws IOException {
+ if (stream != null) {
+ stream.close();
+ }
+ }
+
/**
* Enable TLS on the server.
* @param certChain The certificate chain to use.
* @param key The private key to use.
*/
public Builder useTls(final File certChain, final File key) throws IOException {
+ closeInputStreamIfNotNull(this.certChain);
+ closeInputStreamIfNotNull(this.key);
Review Comment:
We should set these to null after closing, because there can be an exception opening the new inputs. So this would leave these fields in a bad state (holding closed streams).
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1375557798
##########
java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/client/ArrowFlightSqlClientHandler.java:
##########
@@ -463,8 +466,37 @@ public Builder withSystemTrustStore(final boolean useSystemTrustStore) {
return this;
}
+
+ public Builder withTlsRootCertificates(final String tlsRootCertificatesPath) {
Review Comment:
nit: shall we add documentation?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1381973540
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -306,28 +329,63 @@ public Builder maxInboundMessageSize(int maxMessageSize) {
return this;
}
+ /**
+ * A small utility function to ensure that InputStream attributes.
+ * are closed if they are not null
+ * @param stream The InputStream to close (if it is not null).
+ */
+ private void closeInputStreamIfNotNull(InputStream stream) throws IOException {
+ if (stream != null) {
+ stream.close();
+ }
+ }
+
/**
* Enable TLS on the server.
* @param certChain The certificate chain to use.
* @param key The private key to use.
*/
public Builder useTls(final File certChain, final File key) throws IOException {
+ closeInputStreamIfNotNull(this.certChain);
+ closeInputStreamIfNotNull(this.key);
Review Comment:
No, because that only sets the local - you need to set `this.certChain` etc to null
##########
java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/client/utils/ClientAuthenticationUtils.java:
##########
@@ -235,6 +235,57 @@ public static InputStream getCertificateStream(final String keyStorePath,
return getSingleCertificateInputStream(keyStore);
}
+ /**
+ * Generates an {@link InputStream} that contains certificates for path-based
+ * TLS Root Certificates.
+ *
+ * @param tlsRootsCertificatesPath The path of the TLS Root Certificates.
+ * @return a new {code InputStream} containing the certificates.
+ * @throws GeneralSecurityException on error.
+ * @throws IOException on error.
+ */
+ public static InputStream getTlsRootCertificatesStream(final String tlsRootsCertificatesPath)
+ throws GeneralSecurityException, IOException {
+ Preconditions.checkNotNull(tlsRootsCertificatesPath, "TLS Root certificates path cannot be null!");
+
+ return Files
+ .newInputStream(Paths.get(Preconditions.checkNotNull(tlsRootsCertificatesPath)));
Review Comment:
Yes, please
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1382153486
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -306,28 +329,63 @@ public Builder maxInboundMessageSize(int maxMessageSize) {
return this;
}
+ /**
+ * A small utility function to ensure that InputStream attributes.
+ * are closed if they are not null
+ * @param stream The InputStream to close (if it is not null).
+ */
+ private void closeInputStreamIfNotNull(InputStream stream) throws IOException {
+ if (stream != null) {
+ stream.close();
+ }
+ }
+
/**
* Enable TLS on the server.
* @param certChain The certificate chain to use.
* @param key The private key to use.
*/
public Builder useTls(final File certChain, final File key) throws IOException {
+ closeInputStreamIfNotNull(this.certChain);
+ closeInputStreamIfNotNull(this.key);
Review Comment:
Also - I'm unsure as to Pull Request protocol - should I hit the "Resolve conversation" button when I believe I've addressed the concern, or is that up to the reviewer to do?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1373896544
##########
java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionMutualTlsTest.java:
##########
@@ -0,0 +1,436 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.driver.jdbc;
+
+import static org.junit.Assert.assertNotNull;
+
+import java.io.File;
+import java.net.URLEncoder;
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.Properties;
+
+import org.apache.arrow.driver.jdbc.authentication.UserPasswordAuthentication;
+import org.apache.arrow.driver.jdbc.client.ArrowFlightSqlClientHandler;
+import org.apache.arrow.driver.jdbc.utils.ArrowFlightConnectionConfigImpl.ArrowFlightConnectionProperty;
+import org.apache.arrow.driver.jdbc.utils.FlightSqlTestCertificates;
+import org.apache.arrow.driver.jdbc.utils.MockFlightSqlProducer;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.util.AutoCloseables;
+import org.apache.calcite.avatica.org.apache.http.auth.UsernamePasswordCredentials;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+/**
+ * Tests encrypted connections.
+ */
+public class ConnectionMutualTlsTest {
+
+ @ClassRule
+ public static final FlightServerTestRule FLIGHT_SERVER_TEST_RULE;
+ private static final String tlsRootCertsPath;
+
+ private static final String serverMTlsCACertPath;
+ private static final String clientMTlsCertPath;
+ private static final String badClientMTlsCertPath;
+ private static final String clientMTlsKeyPath;
+ private static final String badClientMTlsKeyPath;
+ private static final MockFlightSqlProducer PRODUCER = new MockFlightSqlProducer();
+ private static final String userTest = "user1";
+ private static final String passTest = "pass1";
+
+ static {
+ final FlightSqlTestCertificates.CertKeyPair
+ certKey = FlightSqlTestCertificates.exampleTlsCerts().get(0);
+
+ tlsRootCertsPath = certKey.cert.getPath();
+
+ final File serverMTlsCACert = FlightSqlTestCertificates.exampleCACert();
+
+ serverMTlsCACertPath = serverMTlsCACert.getPath();
+
+ final FlightSqlTestCertificates.CertKeyPair
+ clientMTlsCertKey = FlightSqlTestCertificates.exampleTlsCerts().get(1);
+
+ clientMTlsCertPath = clientMTlsCertKey.cert.getPath();
+ clientMTlsKeyPath = clientMTlsCertKey.key.getPath();
+
+ badClientMTlsCertPath = clientMTlsCertPath + ".bad";
+ badClientMTlsKeyPath = clientMTlsKeyPath + ".bad";
+
+ UserPasswordAuthentication authentication = new UserPasswordAuthentication.Builder()
+ .user(userTest, passTest)
+ .build();
+
+ FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder()
+ .authentication(authentication)
+ .useEncryption(certKey.cert, certKey.key)
+ .useMTlsClientVerification(serverMTlsCACert)
+ .producer(PRODUCER)
+ .build();
+ }
+
+ private BufferAllocator allocator;
+
+ @Before
+ public void setUp() throws Exception {
+ allocator = new RootAllocator(Long.MAX_VALUE);
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ allocator.getChildAllocators().forEach(BufferAllocator::close);
+ AutoCloseables.close(allocator);
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetEncryptedClientAuthenticated() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler client =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ assertNotNull(client);
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient providing a bad mTLS Cert Path. It's expected to
+ * receive the SQLException.
+ *
+ * @throws Exception on error.
+ */
+ @Test(expected = SQLException.class)
+ public void testGetEncryptedClientWithBadMTlsCertPath() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler ignored =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(badClientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ Assert.fail();
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient providing a bad mTLS Key Path. It's expected to
+ * receive the SQLException.
+ *
+ * @throws Exception on error.
+ */
+ @Test(expected = SQLException.class)
+ public void testGetEncryptedClientWithBadMTlsKeyPath() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler ignored =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(badClientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ Assert.fail();
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient without credentials.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetNonAuthenticatedEncryptedClientNoAuth() throws Exception {
+ try (ArrowFlightSqlClientHandler client =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ assertNotNull(client);
+ }
+ }
+
+ /**
+ * Check if an encrypted connection can be established successfully when the
+ * provided valid credentials and a valid TLS Root Certs path.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetEncryptedConnectionWithValidCredentialsAndTlsRootsPath() throws Exception {
+ final Properties properties = new Properties();
+
+ properties.put(ArrowFlightConnectionProperty.HOST.camelName(), "localhost");
+ properties.put(ArrowFlightConnectionProperty.PORT.camelName(),
+ FLIGHT_SERVER_TEST_RULE.getPort());
+ properties.put(ArrowFlightConnectionProperty.USER.camelName(),
+ userTest);
+ properties.put(ArrowFlightConnectionProperty.PASSWORD.camelName(),
+ passTest);
+ properties.put(ArrowFlightConnectionProperty.TLS_ROOT_CERTS.camelName(),
+ tlsRootCertsPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_CERTIFICATE.camelName(),
+ clientMTlsCertPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_KEY.camelName(),
+ clientMTlsKeyPath);
+
+ final ArrowFlightJdbcDataSource dataSource =
+ ArrowFlightJdbcDataSource.createNewDataSource(properties);
+ try (final Connection connection = dataSource.getConnection()) {
+ assert connection.isValid(300);
Review Comment:
Out of curiosity, why is there a 300 second timeout for `ArrowFlightJdbcDataSource` connections, but 0 second timeouts for `ArrowFlightJdbcDriver`/`DriverManager` connections?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1373945638
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -317,6 +340,15 @@ public Builder useTls(final File certChain, final File key) throws IOException {
return this;
}
+ /**
+ * Enable Client Verification via mTLS on the server.
+ * @param mTlsCACert The CA certificate to use for verifying clients.
+ */
+ public Builder useMTlsClientVerification(final File mTlsCACert) throws IOException {
+ this.mTlsCACert = new FileInputStream(mTlsCACert);
Review Comment:
Hey @jduo - I'm not a very good Java developer - but I'm learning (thanks for your patience). Would it be like this?
```java
public Builder useMTlsClientVerification(final File mTlsCACert) throws IOException {
if (this.mTlsCACert != null) {
this.mTlsCACert.close();
}
this.mTlsCACert = new FileInputStream(mTlsCACert);
```
Thanks for your help!
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1382152233
##########
java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionMutualTlsTest.java:
##########
@@ -0,0 +1,436 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.driver.jdbc;
+
+import static org.junit.Assert.assertNotNull;
+
+import java.io.File;
+import java.net.URLEncoder;
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.Properties;
+
+import org.apache.arrow.driver.jdbc.authentication.UserPasswordAuthentication;
+import org.apache.arrow.driver.jdbc.client.ArrowFlightSqlClientHandler;
+import org.apache.arrow.driver.jdbc.utils.ArrowFlightConnectionConfigImpl.ArrowFlightConnectionProperty;
+import org.apache.arrow.driver.jdbc.utils.FlightSqlTestCertificates;
+import org.apache.arrow.driver.jdbc.utils.MockFlightSqlProducer;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.util.AutoCloseables;
+import org.apache.calcite.avatica.org.apache.http.auth.UsernamePasswordCredentials;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+/**
+ * Tests encrypted connections.
+ */
+public class ConnectionMutualTlsTest {
+
+ @ClassRule
+ public static final FlightServerTestRule FLIGHT_SERVER_TEST_RULE;
+ private static final String tlsRootCertsPath;
+
+ private static final String serverMTlsCACertPath;
+ private static final String clientMTlsCertPath;
+ private static final String badClientMTlsCertPath;
+ private static final String clientMTlsKeyPath;
+ private static final String badClientMTlsKeyPath;
+ private static final MockFlightSqlProducer PRODUCER = new MockFlightSqlProducer();
+ private static final String userTest = "user1";
+ private static final String passTest = "pass1";
+
+ static {
+ final FlightSqlTestCertificates.CertKeyPair
+ certKey = FlightSqlTestCertificates.exampleTlsCerts().get(0);
+
+ tlsRootCertsPath = certKey.cert.getPath();
+
+ final File serverMTlsCACert = FlightSqlTestCertificates.exampleCACert();
+
+ serverMTlsCACertPath = serverMTlsCACert.getPath();
+
+ final FlightSqlTestCertificates.CertKeyPair
+ clientMTlsCertKey = FlightSqlTestCertificates.exampleTlsCerts().get(1);
+
+ clientMTlsCertPath = clientMTlsCertKey.cert.getPath();
+ clientMTlsKeyPath = clientMTlsCertKey.key.getPath();
+
+ badClientMTlsCertPath = clientMTlsCertPath + ".bad";
+ badClientMTlsKeyPath = clientMTlsKeyPath + ".bad";
+
+ UserPasswordAuthentication authentication = new UserPasswordAuthentication.Builder()
+ .user(userTest, passTest)
+ .build();
+
+ FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder()
+ .authentication(authentication)
+ .useEncryption(certKey.cert, certKey.key)
+ .useMTlsClientVerification(serverMTlsCACert)
+ .producer(PRODUCER)
+ .build();
+ }
+
+ private BufferAllocator allocator;
+
+ @Before
+ public void setUp() throws Exception {
+ allocator = new RootAllocator(Long.MAX_VALUE);
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ allocator.getChildAllocators().forEach(BufferAllocator::close);
+ AutoCloseables.close(allocator);
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetEncryptedClientAuthenticated() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler client =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ assertNotNull(client);
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient providing a bad mTLS Cert Path. It's expected to
+ * receive the SQLException.
+ *
+ * @throws Exception on error.
+ */
+ @Test(expected = SQLException.class)
Review Comment:
This should now be done.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1376317486
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -328,6 +360,15 @@ public Builder useTls(final InputStream certChain, final InputStream key) {
return this;
}
+ /**
+ * Enable mTLS on the server.
+ * @param mTlsCACert The CA certificate to use for verifying clients.
+ */
+ public Builder useMTlsClientVerification(final InputStream mTlsCACert) throws IOException {
+ this.mTlsCACert = mTlsCACert;
Review Comment:
@lidavidm I will file one. Will study a bit and file one.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "jduo (via GitHub)" <gi...@apache.org>.
jduo commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1373431764
##########
java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionMutualTlsTest.java:
##########
@@ -0,0 +1,434 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.driver.jdbc;
+
+import static org.junit.Assert.assertNotNull;
+
+import java.io.File;
+import java.net.URLEncoder;
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.Properties;
+
+import org.apache.arrow.driver.jdbc.authentication.UserPasswordAuthentication;
+import org.apache.arrow.driver.jdbc.client.ArrowFlightSqlClientHandler;
+import org.apache.arrow.driver.jdbc.utils.ArrowFlightConnectionConfigImpl.ArrowFlightConnectionProperty;
+import org.apache.arrow.driver.jdbc.utils.FlightSqlTestCertificates;
+import org.apache.arrow.driver.jdbc.utils.MockFlightSqlProducer;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.util.AutoCloseables;
+import org.apache.calcite.avatica.org.apache.http.auth.UsernamePasswordCredentials;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+/**
+ * Tests encrypted connections.
+ */
+public class ConnectionMutualTlsTest {
+
+ @ClassRule
+ public static final FlightServerTestRule FLIGHT_SERVER_TEST_RULE;
+ private static final String tlsRootCertsPath;
+
+ private static final String serverMTlsCACertPath;
+ private static final String clientMTlsCertPath;
+ private static final String badClientMTlsCertPath;
+ private static final String clientMTlsKeyPath;
+ private static final String badClientMTlsKeyPath;
+ private static final MockFlightSqlProducer PRODUCER = new MockFlightSqlProducer();
+ private static final String userTest = "user1";
+ private static final String passTest = "pass1";
+
+ static {
+ final FlightSqlTestCertificates.CertKeyPair
+ certKey = FlightSqlTestCertificates.exampleTlsCerts().get(0);
+
+ tlsRootCertsPath = certKey.cert.getPath();
+
+ final File serverMTlsCACert = FlightSqlTestCertificates.exampleCACert();
+
+ serverMTlsCACertPath = serverMTlsCACert.getPath();
+
+ final FlightSqlTestCertificates.CertKeyPair
+ clientMTlsCertKey = FlightSqlTestCertificates.exampleTlsCerts().get(1);
+
+ clientMTlsCertPath = clientMTlsCertKey.cert.getPath();
+ clientMTlsKeyPath = clientMTlsCertKey.key.getPath();
+
+ badClientMTlsCertPath = clientMTlsCertPath + ".bad";
+ badClientMTlsKeyPath = clientMTlsKeyPath + ".bad";
+
+ UserPasswordAuthentication authentication = new UserPasswordAuthentication.Builder()
+ .user(userTest, passTest)
+ .build();
+
+ FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder()
+ .authentication(authentication)
+ .useEncryption(certKey.cert, certKey.key)
+ .useMTlsClientVerification(serverMTlsCACert)
+ .producer(PRODUCER)
+ .build();
+ }
+
+ private BufferAllocator allocator;
+
+ @Before
+ public void setUp() throws Exception {
+ allocator = new RootAllocator(Long.MAX_VALUE);
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ allocator.getChildAllocators().forEach(BufferAllocator::close);
+ AutoCloseables.close(allocator);
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetEncryptedClientAuthenticated() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler client =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ assertNotNull(client);
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient providing a bad mTLS Cert Path. It's expected to
+ * receive the SQLException.
+ *
+ * @throws Exception on error.
+ */
+ @Test(expected = SQLException.class)
+ public void testGetEncryptedClientWithBadMTlsCertPath() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler ignored =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(badClientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ Assert.fail();
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient providing a bad mTLS Key Path. It's expected to
+ * receive the SQLException.
+ *
+ * @throws Exception on error.
+ */
+ @Test(expected = SQLException.class)
+ public void testGetEncryptedClientWithBadMTlsKeyPath() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler ignored =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(badClientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ Assert.fail();
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient without credentials.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetNonAuthenticatedEncryptedClientNoAuth() throws Exception {
+ try (ArrowFlightSqlClientHandler client =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ assertNotNull(client);
+ }
+ }
+
+ /**
+ * Check if an encrypted connection can be established successfully when the
+ * provided valid credentials and a valid TLS Root Certs path.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetEncryptedConnectionWithValidCredentialsAndTlsRootsPath() throws Exception {
+ final Properties properties = new Properties();
+
+ properties.put(ArrowFlightConnectionProperty.HOST.camelName(), "localhost");
+ properties.put(ArrowFlightConnectionProperty.PORT.camelName(),
+ FLIGHT_SERVER_TEST_RULE.getPort());
+ properties.put(ArrowFlightConnectionProperty.USER.camelName(),
+ userTest);
+ properties.put(ArrowFlightConnectionProperty.PASSWORD.camelName(),
+ passTest);
+ properties.put(ArrowFlightConnectionProperty.TLS_ROOT_CERTS.camelName(),
+ tlsRootCertsPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_CERTIFICATE.camelName(),
+ clientMTlsCertPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_KEY.camelName(),
+ clientMTlsKeyPath);
+
+ final ArrowFlightJdbcDataSource dataSource =
+ ArrowFlightJdbcDataSource.createNewDataSource(properties);
+ try (final Connection connection = dataSource.getConnection()) {
+ assert connection.isValid(300);
+ }
+ }
+
+ /**
+ * Check if an encrypted connection can be established successfully when not
+ * providing authentication.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetNonAuthenticatedEncryptedConnection() throws Exception {
+ final Properties properties = new Properties();
+
+ properties.put(ArrowFlightConnectionProperty.HOST.camelName(), FLIGHT_SERVER_TEST_RULE.getHost());
+ properties.put(ArrowFlightConnectionProperty.PORT.camelName(), FLIGHT_SERVER_TEST_RULE.getPort());
+ properties.put(ArrowFlightConnectionProperty.USE_ENCRYPTION.camelName(), true);
+ properties.put(ArrowFlightConnectionProperty.TLS_ROOT_CERTS.camelName(), tlsRootCertsPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_CERTIFICATE.camelName(), clientMTlsCertPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_KEY.camelName(), clientMTlsKeyPath);
+
+ final ArrowFlightJdbcDataSource dataSource = ArrowFlightJdbcDataSource.createNewDataSource(properties);
+ try (final Connection connection = dataSource.getConnection()) {
+ assert connection.isValid(300);
+ }
+ }
+
+ /**
+ * Check if an encrypted connection can be established successfully when connecting through
+ * the DriverManager using just a connection url.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testTLSConnectionPropertyTrueCorrectCastUrlWithDriverManager() throws Exception {
+ final Driver driver = new ArrowFlightJdbcDriver();
+ DriverManager.registerDriver(driver);
+
+ final Connection connection = DriverManager.getConnection(
+ String.format(
+ "jdbc:arrow-flight-sql://localhost:%s?user=%s&password=%s" +
+ "&useEncryption=true&%s=%s&%s=%s&%s=%s",
+ FLIGHT_SERVER_TEST_RULE.getPort(),
+ userTest,
+ passTest,
+ ArrowFlightConnectionProperty.TLS_ROOT_CERTS.camelName(),
+ URLEncoder.encode(tlsRootCertsPath, "UTF-8"),
+ ArrowFlightConnectionProperty.CLIENT_CERTIFICATE.camelName(),
+ URLEncoder.encode(clientMTlsCertPath, "UTF-8"),
+ ArrowFlightConnectionProperty.CLIENT_KEY.camelName(),
+ URLEncoder.encode(clientMTlsKeyPath, "UTF-8")));
+ Assert.assertTrue(connection.isValid(0));
+ connection.close();
Review Comment:
try-with-resources instead
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1373947747
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -328,6 +360,15 @@ public Builder useTls(final InputStream certChain, final InputStream key) {
return this;
}
+ /**
+ * Enable mTLS on the server.
+ * @param mTlsCACert The CA certificate to use for verifying clients.
+ */
+ public Builder useMTlsClientVerification(final InputStream mTlsCACert) throws IOException {
+ this.mTlsCACert = mTlsCACert;
Review Comment:
I mirrored the existing `useTls` method [here](https://github.com/apache/arrow/blob/fcbcb7dab71b14ff3436af7b21bd23132877e2fa/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java#L315)
Does that code have the issue as well?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "jduo (via GitHub)" <gi...@apache.org>.
jduo commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1373436764
##########
docs/source/java/flight_sql_jdbc_driver.rst:
##########
@@ -114,6 +114,21 @@ parameters are:
- null
- When TLS is enabled, the password for the certificate store
+ * - tlsRootCerts
+ - null
+ - Path to PEM-encoded root certificates for TLS - use this as
+ an alternative to ``trustStore``
+
+ * - clientCertificate
Review Comment:
It's a bit odd that these new parameters are raw PEM files instead of JKS files like the other TLS-related parameters.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1373942791
##########
java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionMutualTlsTest.java:
##########
@@ -0,0 +1,436 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.driver.jdbc;
+
+import static org.junit.Assert.assertNotNull;
+
+import java.io.File;
+import java.net.URLEncoder;
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.Properties;
+
+import org.apache.arrow.driver.jdbc.authentication.UserPasswordAuthentication;
+import org.apache.arrow.driver.jdbc.client.ArrowFlightSqlClientHandler;
+import org.apache.arrow.driver.jdbc.utils.ArrowFlightConnectionConfigImpl.ArrowFlightConnectionProperty;
+import org.apache.arrow.driver.jdbc.utils.FlightSqlTestCertificates;
+import org.apache.arrow.driver.jdbc.utils.MockFlightSqlProducer;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.util.AutoCloseables;
+import org.apache.calcite.avatica.org.apache.http.auth.UsernamePasswordCredentials;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+/**
+ * Tests encrypted connections.
+ */
+public class ConnectionMutualTlsTest {
+
+ @ClassRule
+ public static final FlightServerTestRule FLIGHT_SERVER_TEST_RULE;
+ private static final String tlsRootCertsPath;
+
+ private static final String serverMTlsCACertPath;
+ private static final String clientMTlsCertPath;
+ private static final String badClientMTlsCertPath;
+ private static final String clientMTlsKeyPath;
+ private static final String badClientMTlsKeyPath;
+ private static final MockFlightSqlProducer PRODUCER = new MockFlightSqlProducer();
+ private static final String userTest = "user1";
+ private static final String passTest = "pass1";
+
+ static {
+ final FlightSqlTestCertificates.CertKeyPair
+ certKey = FlightSqlTestCertificates.exampleTlsCerts().get(0);
+
+ tlsRootCertsPath = certKey.cert.getPath();
+
+ final File serverMTlsCACert = FlightSqlTestCertificates.exampleCACert();
+
+ serverMTlsCACertPath = serverMTlsCACert.getPath();
+
+ final FlightSqlTestCertificates.CertKeyPair
+ clientMTlsCertKey = FlightSqlTestCertificates.exampleTlsCerts().get(1);
+
+ clientMTlsCertPath = clientMTlsCertKey.cert.getPath();
+ clientMTlsKeyPath = clientMTlsCertKey.key.getPath();
+
+ badClientMTlsCertPath = clientMTlsCertPath + ".bad";
+ badClientMTlsKeyPath = clientMTlsKeyPath + ".bad";
+
+ UserPasswordAuthentication authentication = new UserPasswordAuthentication.Builder()
+ .user(userTest, passTest)
+ .build();
+
+ FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder()
+ .authentication(authentication)
+ .useEncryption(certKey.cert, certKey.key)
+ .useMTlsClientVerification(serverMTlsCACert)
+ .producer(PRODUCER)
+ .build();
+ }
+
+ private BufferAllocator allocator;
+
+ @Before
+ public void setUp() throws Exception {
+ allocator = new RootAllocator(Long.MAX_VALUE);
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ allocator.getChildAllocators().forEach(BufferAllocator::close);
+ AutoCloseables.close(allocator);
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetEncryptedClientAuthenticated() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler client =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ assertNotNull(client);
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient providing a bad mTLS Cert Path. It's expected to
+ * receive the SQLException.
+ *
+ * @throws Exception on error.
+ */
+ @Test(expected = SQLException.class)
+ public void testGetEncryptedClientWithBadMTlsCertPath() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler ignored =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(badClientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ Assert.fail();
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient providing a bad mTLS Key Path. It's expected to
+ * receive the SQLException.
+ *
+ * @throws Exception on error.
+ */
+ @Test(expected = SQLException.class)
+ public void testGetEncryptedClientWithBadMTlsKeyPath() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler ignored =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(badClientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ Assert.fail();
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient without credentials.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetNonAuthenticatedEncryptedClientNoAuth() throws Exception {
+ try (ArrowFlightSqlClientHandler client =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ assertNotNull(client);
+ }
+ }
+
+ /**
+ * Check if an encrypted connection can be established successfully when the
+ * provided valid credentials and a valid TLS Root Certs path.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetEncryptedConnectionWithValidCredentialsAndTlsRootsPath() throws Exception {
+ final Properties properties = new Properties();
+
+ properties.put(ArrowFlightConnectionProperty.HOST.camelName(), "localhost");
+ properties.put(ArrowFlightConnectionProperty.PORT.camelName(),
+ FLIGHT_SERVER_TEST_RULE.getPort());
+ properties.put(ArrowFlightConnectionProperty.USER.camelName(),
+ userTest);
+ properties.put(ArrowFlightConnectionProperty.PASSWORD.camelName(),
+ passTest);
+ properties.put(ArrowFlightConnectionProperty.TLS_ROOT_CERTS.camelName(),
+ tlsRootCertsPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_CERTIFICATE.camelName(),
+ clientMTlsCertPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_KEY.camelName(),
+ clientMTlsKeyPath);
+
+ final ArrowFlightJdbcDataSource dataSource =
+ ArrowFlightJdbcDataSource.createNewDataSource(properties);
+ try (final Connection connection = dataSource.getConnection()) {
+ assert connection.isValid(300);
Review Comment:
Very good question :) - I copied the tests from the `ConnectionTlsTest.java` code - while modifying the tests for mTLS of course. I'm not sure of the rationale for the timeout, but I'll research it.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1375559791
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -317,6 +340,15 @@ public Builder useTls(final File certChain, final File key) throws IOException {
return this;
}
+ /**
+ * Enable Client Verification via mTLS on the server.
+ * @param mTlsCACert The CA certificate to use for verifying clients.
+ */
+ public Builder useMTlsClientVerification(final File mTlsCACert) throws IOException {
+ this.mTlsCACert = new FileInputStream(mTlsCACert);
Review Comment:
@jduo just for my knowledge and to clarify a point, would it be more easier if we use try-with-resources here?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "jduo (via GitHub)" <gi...@apache.org>.
jduo commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1376313288
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -317,6 +340,15 @@ public Builder useTls(final File certChain, final File key) throws IOException {
return this;
}
+ /**
+ * Enable Client Verification via mTLS on the server.
+ * @param mTlsCACert The CA certificate to use for verifying clients.
+ */
+ public Builder useMTlsClientVerification(final File mTlsCACert) throws IOException {
+ this.mTlsCACert = new FileInputStream(mTlsCACert);
Review Comment:
@vibhatha , try-with-resources wouldn't help here since we need the stream to have a longer lifetime than this builder function. This is more of a case of tracking ownership.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1376444112
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -317,6 +340,15 @@ public Builder useTls(final File certChain, final File key) throws IOException {
return this;
}
+ /**
+ * Enable Client Verification via mTLS on the server.
+ * @param mTlsCACert The CA certificate to use for verifying clients.
+ */
+ public Builder useMTlsClientVerification(final File mTlsCACert) throws IOException {
+ this.mTlsCACert = new FileInputStream(mTlsCACert);
Review Comment:
@jduo - I've added small changes that detect if the InputStream attributes are not null, and if so - closes them. Could you please re-review?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on PR #38461:
URL: https://github.com/apache/arrow/pull/38461#issuecomment-1784313616
@prmoore77 I left a few comments and nice work with the test cases and comments. Thanks for working on this issue.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "jduo (via GitHub)" <gi...@apache.org>.
jduo commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1373920916
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -317,6 +340,15 @@ public Builder useTls(final File certChain, final File key) throws IOException {
return this;
}
+ /**
+ * Enable Client Verification via mTLS on the server.
+ * @param mTlsCACert The CA certificate to use for verifying clients.
+ */
+ public Builder useMTlsClientVerification(final File mTlsCACert) throws IOException {
+ this.mTlsCACert = new FileInputStream(mTlsCACert);
Review Comment:
If the user calls this method repeatedly we will leak File handles. Need to close the current one if it exists.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1376260089
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -317,6 +340,15 @@ public Builder useTls(final File certChain, final File key) throws IOException {
return this;
}
+ /**
+ * Enable Client Verification via mTLS on the server.
+ * @param mTlsCACert The CA certificate to use for verifying clients.
+ */
+ public Builder useMTlsClientVerification(final File mTlsCACert) throws IOException {
+ this.mTlsCACert = new FileInputStream(mTlsCACert);
Review Comment:
hi @jduo - this is the FlightServer - I'd rather not change the overall design of the current class's builder. It already uses this approach for `useTls` - and I think my change is consistent with that.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1376265362
##########
java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/client/ArrowFlightSqlClientHandler.java:
##########
@@ -463,8 +466,37 @@ public Builder withSystemTrustStore(final boolean useSystemTrustStore) {
return this;
}
+
+ public Builder withTlsRootCertificates(final String tlsRootCertificatesPath) {
Review Comment:
hi, @vibhatha - Noted - corrected and pushed a change just now.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "jduo (via GitHub)" <gi...@apache.org>.
jduo commented on PR #38461:
URL: https://github.com/apache/arrow/pull/38461#issuecomment-1795088483
@prmoore77 heads up, after #38521 a copy constructor was added to ArrowFlightSqlClientHandler.Builder
So any new fields that this PR adds will need to be added to that copy constructor.
#38580 also changes ArrowFligthSqlClientHandler to set defaults correctly and adds tests for verifying defaults, so if that goes in first, new fields will be added there too.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1381957485
##########
java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/client/utils/ClientAuthenticationUtils.java:
##########
@@ -235,6 +235,57 @@ public static InputStream getCertificateStream(final String keyStorePath,
return getSingleCertificateInputStream(keyStore);
}
+ /**
+ * Generates an {@link InputStream} that contains certificates for path-based
+ * TLS Root Certificates.
+ *
+ * @param tlsRootsCertificatesPath The path of the TLS Root Certificates.
+ * @return a new {code InputStream} containing the certificates.
+ * @throws GeneralSecurityException on error.
+ * @throws IOException on error.
+ */
+ public static InputStream getTlsRootCertificatesStream(final String tlsRootsCertificatesPath)
+ throws GeneralSecurityException, IOException {
+ Preconditions.checkNotNull(tlsRootsCertificatesPath, "TLS Root certificates path cannot be null!");
+
+ return Files
+ .newInputStream(Paths.get(Preconditions.checkNotNull(tlsRootsCertificatesPath)));
Review Comment:
Good point. I copied the approach from: https://github.com/apache/arrow/blob/cd6e63570f81e96375a4c51ef5d925b5f32f5a57/java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/client/utils/ClientAuthenticationUtils.java#L225 - should we remove there also?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1373704285
##########
java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionMutualTlsTest.java:
##########
@@ -0,0 +1,434 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.driver.jdbc;
+
+import static org.junit.Assert.assertNotNull;
+
+import java.io.File;
+import java.net.URLEncoder;
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.Properties;
+
+import org.apache.arrow.driver.jdbc.authentication.UserPasswordAuthentication;
+import org.apache.arrow.driver.jdbc.client.ArrowFlightSqlClientHandler;
+import org.apache.arrow.driver.jdbc.utils.ArrowFlightConnectionConfigImpl.ArrowFlightConnectionProperty;
+import org.apache.arrow.driver.jdbc.utils.FlightSqlTestCertificates;
+import org.apache.arrow.driver.jdbc.utils.MockFlightSqlProducer;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.util.AutoCloseables;
+import org.apache.calcite.avatica.org.apache.http.auth.UsernamePasswordCredentials;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+/**
+ * Tests encrypted connections.
+ */
+public class ConnectionMutualTlsTest {
+
+ @ClassRule
+ public static final FlightServerTestRule FLIGHT_SERVER_TEST_RULE;
+ private static final String tlsRootCertsPath;
+
+ private static final String serverMTlsCACertPath;
+ private static final String clientMTlsCertPath;
+ private static final String badClientMTlsCertPath;
+ private static final String clientMTlsKeyPath;
+ private static final String badClientMTlsKeyPath;
+ private static final MockFlightSqlProducer PRODUCER = new MockFlightSqlProducer();
+ private static final String userTest = "user1";
+ private static final String passTest = "pass1";
+
+ static {
+ final FlightSqlTestCertificates.CertKeyPair
+ certKey = FlightSqlTestCertificates.exampleTlsCerts().get(0);
+
+ tlsRootCertsPath = certKey.cert.getPath();
+
+ final File serverMTlsCACert = FlightSqlTestCertificates.exampleCACert();
+
+ serverMTlsCACertPath = serverMTlsCACert.getPath();
+
+ final FlightSqlTestCertificates.CertKeyPair
+ clientMTlsCertKey = FlightSqlTestCertificates.exampleTlsCerts().get(1);
+
+ clientMTlsCertPath = clientMTlsCertKey.cert.getPath();
+ clientMTlsKeyPath = clientMTlsCertKey.key.getPath();
+
+ badClientMTlsCertPath = clientMTlsCertPath + ".bad";
+ badClientMTlsKeyPath = clientMTlsKeyPath + ".bad";
+
+ UserPasswordAuthentication authentication = new UserPasswordAuthentication.Builder()
+ .user(userTest, passTest)
+ .build();
+
+ FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder()
+ .authentication(authentication)
+ .useEncryption(certKey.cert, certKey.key)
+ .useMTlsClientVerification(serverMTlsCACert)
+ .producer(PRODUCER)
+ .build();
+ }
+
+ private BufferAllocator allocator;
+
+ @Before
+ public void setUp() throws Exception {
+ allocator = new RootAllocator(Long.MAX_VALUE);
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ allocator.getChildAllocators().forEach(BufferAllocator::close);
+ AutoCloseables.close(allocator);
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetEncryptedClientAuthenticated() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler client =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ assertNotNull(client);
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient providing a bad mTLS Cert Path. It's expected to
+ * receive the SQLException.
+ *
+ * @throws Exception on error.
+ */
+ @Test(expected = SQLException.class)
+ public void testGetEncryptedClientWithBadMTlsCertPath() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler ignored =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(badClientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ Assert.fail();
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient providing a bad mTLS Key Path. It's expected to
+ * receive the SQLException.
+ *
+ * @throws Exception on error.
+ */
+ @Test(expected = SQLException.class)
+ public void testGetEncryptedClientWithBadMTlsKeyPath() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler ignored =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(badClientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ Assert.fail();
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient without credentials.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetNonAuthenticatedEncryptedClientNoAuth() throws Exception {
+ try (ArrowFlightSqlClientHandler client =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ assertNotNull(client);
+ }
+ }
+
+ /**
+ * Check if an encrypted connection can be established successfully when the
+ * provided valid credentials and a valid TLS Root Certs path.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetEncryptedConnectionWithValidCredentialsAndTlsRootsPath() throws Exception {
+ final Properties properties = new Properties();
+
+ properties.put(ArrowFlightConnectionProperty.HOST.camelName(), "localhost");
+ properties.put(ArrowFlightConnectionProperty.PORT.camelName(),
+ FLIGHT_SERVER_TEST_RULE.getPort());
+ properties.put(ArrowFlightConnectionProperty.USER.camelName(),
+ userTest);
+ properties.put(ArrowFlightConnectionProperty.PASSWORD.camelName(),
+ passTest);
+ properties.put(ArrowFlightConnectionProperty.TLS_ROOT_CERTS.camelName(),
+ tlsRootCertsPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_CERTIFICATE.camelName(),
+ clientMTlsCertPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_KEY.camelName(),
+ clientMTlsKeyPath);
+
+ final ArrowFlightJdbcDataSource dataSource =
+ ArrowFlightJdbcDataSource.createNewDataSource(properties);
+ try (final Connection connection = dataSource.getConnection()) {
+ assert connection.isValid(300);
+ }
+ }
+
+ /**
+ * Check if an encrypted connection can be established successfully when not
+ * providing authentication.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetNonAuthenticatedEncryptedConnection() throws Exception {
+ final Properties properties = new Properties();
+
+ properties.put(ArrowFlightConnectionProperty.HOST.camelName(), FLIGHT_SERVER_TEST_RULE.getHost());
+ properties.put(ArrowFlightConnectionProperty.PORT.camelName(), FLIGHT_SERVER_TEST_RULE.getPort());
+ properties.put(ArrowFlightConnectionProperty.USE_ENCRYPTION.camelName(), true);
+ properties.put(ArrowFlightConnectionProperty.TLS_ROOT_CERTS.camelName(), tlsRootCertsPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_CERTIFICATE.camelName(), clientMTlsCertPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_KEY.camelName(), clientMTlsKeyPath);
+
+ final ArrowFlightJdbcDataSource dataSource = ArrowFlightJdbcDataSource.createNewDataSource(properties);
+ try (final Connection connection = dataSource.getConnection()) {
+ assert connection.isValid(300);
+ }
+ }
+
+ /**
+ * Check if an encrypted connection can be established successfully when connecting through
+ * the DriverManager using just a connection url.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testTLSConnectionPropertyTrueCorrectCastUrlWithDriverManager() throws Exception {
+ final Driver driver = new ArrowFlightJdbcDriver();
+ DriverManager.registerDriver(driver);
+
+ final Connection connection = DriverManager.getConnection(
+ String.format(
+ "jdbc:arrow-flight-sql://localhost:%s?user=%s&password=%s" +
+ "&useEncryption=true&%s=%s&%s=%s&%s=%s",
+ FLIGHT_SERVER_TEST_RULE.getPort(),
+ userTest,
+ passTest,
+ ArrowFlightConnectionProperty.TLS_ROOT_CERTS.camelName(),
+ URLEncoder.encode(tlsRootCertsPath, "UTF-8"),
+ ArrowFlightConnectionProperty.CLIENT_CERTIFICATE.camelName(),
+ URLEncoder.encode(clientMTlsCertPath, "UTF-8"),
+ ArrowFlightConnectionProperty.CLIENT_KEY.camelName(),
+ URLEncoder.encode(clientMTlsKeyPath, "UTF-8")));
+ Assert.assertTrue(connection.isValid(0));
+ connection.close();
Review Comment:
Thanks for the tip! I'll make the change...
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1373698808
##########
docs/source/java/flight_sql_jdbc_driver.rst:
##########
@@ -114,6 +114,21 @@ parameters are:
- null
- When TLS is enabled, the password for the certificate store
+ * - tlsRootCerts
+ - null
+ - Path to PEM-encoded root certificates for TLS - use this as
+ an alternative to ``trustStore``
+
+ * - clientCertificate
Review Comment:
Hi @jduo - I was trying to add a convenience factor. For me, it is easier to work with PEM files - I added tlsRootCerts for folks that prefer PEM. I had been using PEM files with ADBC already - and it seemed so convenient, I wanted the option for JDBC...
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "jduo (via GitHub)" <gi...@apache.org>.
jduo commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1373923037
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -328,6 +360,15 @@ public Builder useTls(final InputStream certChain, final InputStream key) {
return this;
}
+ /**
+ * Enable mTLS on the server.
+ * @param mTlsCACert The CA certificate to use for verifying clients.
+ */
+ public Builder useMTlsClientVerification(final InputStream mTlsCACert) throws IOException {
+ this.mTlsCACert = mTlsCACert;
Review Comment:
Similar comment as above about this potentially leaking a file. Also, now it is ambiguous whether the Flight client should close() mTlsCACert (presumably it should).
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -328,6 +360,15 @@ public Builder useTls(final InputStream certChain, final InputStream key) {
return this;
}
+ /**
+ * Enable mTLS on the server.
+ * @param mTlsCACert The CA certificate to use for verifying clients.
+ */
+ public Builder useMTlsClientVerification(final InputStream mTlsCACert) throws IOException {
+ this.mTlsCACert = mTlsCACert;
Review Comment:
Similar comment as above about this potentially leaking a file. Also, now it is ambiguous whether the Flight client should close() mTlsCACert (presumably it should but there should be a comment).
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #38461:
URL: https://github.com/apache/arrow/pull/38461
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1377801521
##########
docs/source/java/flight_sql_jdbc_driver.rst:
##########
@@ -114,6 +114,21 @@ parameters are:
- null
- When TLS is enabled, the password for the certificate store
+ * - tlsRootCerts
+ - null
+ - Path to PEM-encoded root certificates for TLS - use this as
+ an alternative to ``trustStore``
+
+ * - clientCertificate
+ - null
+ - Path to PEM-encoded client mTLS certificate when the Flight
+ SQL server requires client verification.
+
+ * - clientKey
Review Comment:
Hello – it could be added, but the Java Flight client doesn’t yet support that to my knowledge – so I just wanted to catch up to that for now. We can add it in a later PR, unless you feel strongly that it should be included here.
##########
docs/source/java/flight_sql_jdbc_driver.rst:
##########
@@ -114,6 +114,21 @@ parameters are:
- null
- When TLS is enabled, the password for the certificate store
+ * - tlsRootCerts
+ - null
+ - Path to PEM-encoded root certificates for TLS - use this as
+ an alternative to ``trustStore``
+
+ * - clientCertificate
+ - null
+ - Path to PEM-encoded client mTLS certificate when the Flight
+ SQL server requires client verification.
+
+ * - clientKey
Review Comment:
Hello – it could be added, but the Java Flight client doesn’t yet support that to my knowledge – so I just wanted to catch up to that for now. We can add it in a later PR, unless you feel strongly that it should be included here.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1381950516
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -306,28 +329,63 @@ public Builder maxInboundMessageSize(int maxMessageSize) {
return this;
}
+ /**
+ * A small utility function to ensure that InputStream attributes.
+ * are closed if they are not null
+ * @param stream The InputStream to close (if it is not null).
+ */
+ private void closeInputStreamIfNotNull(InputStream stream) throws IOException {
+ if (stream != null) {
+ stream.close();
+ }
+ }
+
/**
* Enable TLS on the server.
* @param certChain The certificate chain to use.
* @param key The private key to use.
*/
public Builder useTls(final File certChain, final File key) throws IOException {
+ closeInputStreamIfNotNull(this.certChain);
+ closeInputStreamIfNotNull(this.key);
Review Comment:
would it be like this?:
```
private void closeInputStreamIfNotNull(InputStream stream) throws IOException {
if (stream != null) {
stream.close();
stream = null;
}
}
```
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1382433243
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -328,6 +360,15 @@ public Builder useTls(final InputStream certChain, final InputStream key) {
return this;
}
+ /**
+ * Enable mTLS on the server.
+ * @param mTlsCACert The CA certificate to use for verifying clients.
+ */
+ public Builder useMTlsClientVerification(final InputStream mTlsCACert) throws IOException {
+ this.mTlsCACert = mTlsCACert;
Review Comment:
Filed #38586
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "jduo (via GitHub)" <gi...@apache.org>.
jduo commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1374827659
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -317,6 +340,15 @@ public Builder useTls(final File certChain, final File key) throws IOException {
return this;
}
+ /**
+ * Enable Client Verification via mTLS on the server.
+ * @param mTlsCACert The CA certificate to use for verifying clients.
+ */
+ public Builder useMTlsClientVerification(final File mTlsCACert) throws IOException {
+ this.mTlsCACert = new FileInputStream(mTlsCACert);
Review Comment:
Yes this would work. However I'm leaning towards deferring opening mTlsCACert until the connection is made. It's odd for builders to have more logic than setting fields.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1375164054
##########
docs/source/java/flight_sql_jdbc_driver.rst:
##########
@@ -114,6 +114,21 @@ parameters are:
- null
- When TLS is enabled, the password for the certificate store
+ * - tlsRootCerts
+ - null
+ - Path to PEM-encoded root certificates for TLS - use this as
+ an alternative to ``trustStore``
+
+ * - clientCertificate
+ - null
+ - Path to PEM-encoded client mTLS certificate when the Flight
+ SQL server requires client verification.
+
+ * - clientKey
Review Comment:
Will it be necessary to provide an optional client key password argument if this client private key is password-protected?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1376315589
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -317,6 +340,15 @@ public Builder useTls(final File certChain, final File key) throws IOException {
return this;
}
+ /**
+ * Enable Client Verification via mTLS on the server.
+ * @param mTlsCACert The CA certificate to use for verifying clients.
+ */
+ public Builder useMTlsClientVerification(final File mTlsCACert) throws IOException {
+ this.mTlsCACert = new FileInputStream(mTlsCACert);
Review Comment:
Got it, thanks @jduo
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1373896544
##########
java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionMutualTlsTest.java:
##########
@@ -0,0 +1,436 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.driver.jdbc;
+
+import static org.junit.Assert.assertNotNull;
+
+import java.io.File;
+import java.net.URLEncoder;
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.Properties;
+
+import org.apache.arrow.driver.jdbc.authentication.UserPasswordAuthentication;
+import org.apache.arrow.driver.jdbc.client.ArrowFlightSqlClientHandler;
+import org.apache.arrow.driver.jdbc.utils.ArrowFlightConnectionConfigImpl.ArrowFlightConnectionProperty;
+import org.apache.arrow.driver.jdbc.utils.FlightSqlTestCertificates;
+import org.apache.arrow.driver.jdbc.utils.MockFlightSqlProducer;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.util.AutoCloseables;
+import org.apache.calcite.avatica.org.apache.http.auth.UsernamePasswordCredentials;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+/**
+ * Tests encrypted connections.
+ */
+public class ConnectionMutualTlsTest {
+
+ @ClassRule
+ public static final FlightServerTestRule FLIGHT_SERVER_TEST_RULE;
+ private static final String tlsRootCertsPath;
+
+ private static final String serverMTlsCACertPath;
+ private static final String clientMTlsCertPath;
+ private static final String badClientMTlsCertPath;
+ private static final String clientMTlsKeyPath;
+ private static final String badClientMTlsKeyPath;
+ private static final MockFlightSqlProducer PRODUCER = new MockFlightSqlProducer();
+ private static final String userTest = "user1";
+ private static final String passTest = "pass1";
+
+ static {
+ final FlightSqlTestCertificates.CertKeyPair
+ certKey = FlightSqlTestCertificates.exampleTlsCerts().get(0);
+
+ tlsRootCertsPath = certKey.cert.getPath();
+
+ final File serverMTlsCACert = FlightSqlTestCertificates.exampleCACert();
+
+ serverMTlsCACertPath = serverMTlsCACert.getPath();
+
+ final FlightSqlTestCertificates.CertKeyPair
+ clientMTlsCertKey = FlightSqlTestCertificates.exampleTlsCerts().get(1);
+
+ clientMTlsCertPath = clientMTlsCertKey.cert.getPath();
+ clientMTlsKeyPath = clientMTlsCertKey.key.getPath();
+
+ badClientMTlsCertPath = clientMTlsCertPath + ".bad";
+ badClientMTlsKeyPath = clientMTlsKeyPath + ".bad";
+
+ UserPasswordAuthentication authentication = new UserPasswordAuthentication.Builder()
+ .user(userTest, passTest)
+ .build();
+
+ FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder()
+ .authentication(authentication)
+ .useEncryption(certKey.cert, certKey.key)
+ .useMTlsClientVerification(serverMTlsCACert)
+ .producer(PRODUCER)
+ .build();
+ }
+
+ private BufferAllocator allocator;
+
+ @Before
+ public void setUp() throws Exception {
+ allocator = new RootAllocator(Long.MAX_VALUE);
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ allocator.getChildAllocators().forEach(BufferAllocator::close);
+ AutoCloseables.close(allocator);
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetEncryptedClientAuthenticated() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler client =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ assertNotNull(client);
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient providing a bad mTLS Cert Path. It's expected to
+ * receive the SQLException.
+ *
+ * @throws Exception on error.
+ */
+ @Test(expected = SQLException.class)
+ public void testGetEncryptedClientWithBadMTlsCertPath() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler ignored =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(badClientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ Assert.fail();
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient providing a bad mTLS Key Path. It's expected to
+ * receive the SQLException.
+ *
+ * @throws Exception on error.
+ */
+ @Test(expected = SQLException.class)
+ public void testGetEncryptedClientWithBadMTlsKeyPath() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler ignored =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(badClientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ Assert.fail();
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient without credentials.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetNonAuthenticatedEncryptedClientNoAuth() throws Exception {
+ try (ArrowFlightSqlClientHandler client =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ assertNotNull(client);
+ }
+ }
+
+ /**
+ * Check if an encrypted connection can be established successfully when the
+ * provided valid credentials and a valid TLS Root Certs path.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetEncryptedConnectionWithValidCredentialsAndTlsRootsPath() throws Exception {
+ final Properties properties = new Properties();
+
+ properties.put(ArrowFlightConnectionProperty.HOST.camelName(), "localhost");
+ properties.put(ArrowFlightConnectionProperty.PORT.camelName(),
+ FLIGHT_SERVER_TEST_RULE.getPort());
+ properties.put(ArrowFlightConnectionProperty.USER.camelName(),
+ userTest);
+ properties.put(ArrowFlightConnectionProperty.PASSWORD.camelName(),
+ passTest);
+ properties.put(ArrowFlightConnectionProperty.TLS_ROOT_CERTS.camelName(),
+ tlsRootCertsPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_CERTIFICATE.camelName(),
+ clientMTlsCertPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_KEY.camelName(),
+ clientMTlsKeyPath);
+
+ final ArrowFlightJdbcDataSource dataSource =
+ ArrowFlightJdbcDataSource.createNewDataSource(properties);
+ try (final Connection connection = dataSource.getConnection()) {
+ assert connection.isValid(300);
Review Comment:
Out of curiosity, why is there a 300 second timeout for ArrowFlightJdbcDataSource connections, but 0 second timeouts for `ArrowFlightJdbcDriver`/`DriverManager` connections?
##########
java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionMutualTlsTest.java:
##########
@@ -0,0 +1,436 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.driver.jdbc;
+
+import static org.junit.Assert.assertNotNull;
+
+import java.io.File;
+import java.net.URLEncoder;
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.Properties;
+
+import org.apache.arrow.driver.jdbc.authentication.UserPasswordAuthentication;
+import org.apache.arrow.driver.jdbc.client.ArrowFlightSqlClientHandler;
+import org.apache.arrow.driver.jdbc.utils.ArrowFlightConnectionConfigImpl.ArrowFlightConnectionProperty;
+import org.apache.arrow.driver.jdbc.utils.FlightSqlTestCertificates;
+import org.apache.arrow.driver.jdbc.utils.MockFlightSqlProducer;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.util.AutoCloseables;
+import org.apache.calcite.avatica.org.apache.http.auth.UsernamePasswordCredentials;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+/**
+ * Tests encrypted connections.
+ */
+public class ConnectionMutualTlsTest {
+
+ @ClassRule
+ public static final FlightServerTestRule FLIGHT_SERVER_TEST_RULE;
+ private static final String tlsRootCertsPath;
+
+ private static final String serverMTlsCACertPath;
+ private static final String clientMTlsCertPath;
+ private static final String badClientMTlsCertPath;
+ private static final String clientMTlsKeyPath;
+ private static final String badClientMTlsKeyPath;
+ private static final MockFlightSqlProducer PRODUCER = new MockFlightSqlProducer();
+ private static final String userTest = "user1";
+ private static final String passTest = "pass1";
+
+ static {
+ final FlightSqlTestCertificates.CertKeyPair
+ certKey = FlightSqlTestCertificates.exampleTlsCerts().get(0);
+
+ tlsRootCertsPath = certKey.cert.getPath();
+
+ final File serverMTlsCACert = FlightSqlTestCertificates.exampleCACert();
+
+ serverMTlsCACertPath = serverMTlsCACert.getPath();
+
+ final FlightSqlTestCertificates.CertKeyPair
+ clientMTlsCertKey = FlightSqlTestCertificates.exampleTlsCerts().get(1);
+
+ clientMTlsCertPath = clientMTlsCertKey.cert.getPath();
+ clientMTlsKeyPath = clientMTlsCertKey.key.getPath();
+
+ badClientMTlsCertPath = clientMTlsCertPath + ".bad";
+ badClientMTlsKeyPath = clientMTlsKeyPath + ".bad";
+
+ UserPasswordAuthentication authentication = new UserPasswordAuthentication.Builder()
+ .user(userTest, passTest)
+ .build();
+
+ FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder()
+ .authentication(authentication)
+ .useEncryption(certKey.cert, certKey.key)
+ .useMTlsClientVerification(serverMTlsCACert)
+ .producer(PRODUCER)
+ .build();
+ }
+
+ private BufferAllocator allocator;
+
+ @Before
+ public void setUp() throws Exception {
+ allocator = new RootAllocator(Long.MAX_VALUE);
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ allocator.getChildAllocators().forEach(BufferAllocator::close);
+ AutoCloseables.close(allocator);
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetEncryptedClientAuthenticated() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler client =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ assertNotNull(client);
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient providing a bad mTLS Cert Path. It's expected to
+ * receive the SQLException.
+ *
+ * @throws Exception on error.
+ */
+ @Test(expected = SQLException.class)
+ public void testGetEncryptedClientWithBadMTlsCertPath() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler ignored =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(badClientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ Assert.fail();
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient providing a bad mTLS Key Path. It's expected to
+ * receive the SQLException.
+ *
+ * @throws Exception on error.
+ */
+ @Test(expected = SQLException.class)
+ public void testGetEncryptedClientWithBadMTlsKeyPath() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler ignored =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(badClientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ Assert.fail();
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient without credentials.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetNonAuthenticatedEncryptedClientNoAuth() throws Exception {
+ try (ArrowFlightSqlClientHandler client =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ assertNotNull(client);
+ }
+ }
+
+ /**
+ * Check if an encrypted connection can be established successfully when the
+ * provided valid credentials and a valid TLS Root Certs path.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetEncryptedConnectionWithValidCredentialsAndTlsRootsPath() throws Exception {
+ final Properties properties = new Properties();
+
+ properties.put(ArrowFlightConnectionProperty.HOST.camelName(), "localhost");
+ properties.put(ArrowFlightConnectionProperty.PORT.camelName(),
+ FLIGHT_SERVER_TEST_RULE.getPort());
+ properties.put(ArrowFlightConnectionProperty.USER.camelName(),
+ userTest);
+ properties.put(ArrowFlightConnectionProperty.PASSWORD.camelName(),
+ passTest);
+ properties.put(ArrowFlightConnectionProperty.TLS_ROOT_CERTS.camelName(),
+ tlsRootCertsPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_CERTIFICATE.camelName(),
+ clientMTlsCertPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_KEY.camelName(),
+ clientMTlsKeyPath);
+
+ final ArrowFlightJdbcDataSource dataSource =
+ ArrowFlightJdbcDataSource.createNewDataSource(properties);
+ try (final Connection connection = dataSource.getConnection()) {
+ assert connection.isValid(300);
+ }
+ }
+
+ /**
+ * Check if an encrypted connection can be established successfully when not
+ * providing authentication.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetNonAuthenticatedEncryptedConnection() throws Exception {
+ final Properties properties = new Properties();
+
+ properties.put(ArrowFlightConnectionProperty.HOST.camelName(), FLIGHT_SERVER_TEST_RULE.getHost());
+ properties.put(ArrowFlightConnectionProperty.PORT.camelName(), FLIGHT_SERVER_TEST_RULE.getPort());
+ properties.put(ArrowFlightConnectionProperty.USE_ENCRYPTION.camelName(), true);
+ properties.put(ArrowFlightConnectionProperty.TLS_ROOT_CERTS.camelName(), tlsRootCertsPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_CERTIFICATE.camelName(), clientMTlsCertPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_KEY.camelName(), clientMTlsKeyPath);
+
+ final ArrowFlightJdbcDataSource dataSource = ArrowFlightJdbcDataSource.createNewDataSource(properties);
+ try (final Connection connection = dataSource.getConnection()) {
+ assert connection.isValid(300);
+ }
+ }
+
+ /**
+ * Check if an encrypted connection can be established successfully when connecting through
+ * the DriverManager using just a connection url.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testTLSConnectionPropertyTrueCorrectCastUrlWithDriverManager() throws Exception {
+ final Driver driver = new ArrowFlightJdbcDriver();
+ DriverManager.registerDriver(driver);
+
+ final String jdbcUrl = String.format(
+ "jdbc:arrow-flight-sql://localhost:%s?user=%s&password=%s" +
+ "&useEncryption=true&%s=%s&%s=%s&%s=%s",
+ FLIGHT_SERVER_TEST_RULE.getPort(),
+ userTest,
+ passTest,
+ ArrowFlightConnectionProperty.TLS_ROOT_CERTS.camelName(),
+ URLEncoder.encode(tlsRootCertsPath, "UTF-8"),
+ ArrowFlightConnectionProperty.CLIENT_CERTIFICATE.camelName(),
+ URLEncoder.encode(clientMTlsCertPath, "UTF-8"),
+ ArrowFlightConnectionProperty.CLIENT_KEY.camelName(),
+ URLEncoder.encode(clientMTlsKeyPath, "UTF-8"));
+
+ try (Connection connection = DriverManager.getConnection(jdbcUrl)) {
+ Assert.assertTrue(connection.isValid(0));
+ }
+ }
+
+ /**
+ * Check if an encrypted connection can be established successfully when connecting through the DriverManager using
+ * a connection url and properties with String K-V pairs.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testTLSConnectionPropertyTrueCorrectCastUrlAndPropertiesUsingSetPropertyWithDriverManager()
+ throws Exception {
+ final Driver driver = new ArrowFlightJdbcDriver();
+ DriverManager.registerDriver(driver);
+
+ Properties properties = new Properties();
+
+ properties.setProperty(ArrowFlightConnectionProperty.USER.camelName(), userTest);
+ properties.setProperty(ArrowFlightConnectionProperty.PASSWORD.camelName(), passTest);
+ properties.setProperty(ArrowFlightConnectionProperty.TLS_ROOT_CERTS.camelName(), tlsRootCertsPath);
+ properties.setProperty(ArrowFlightConnectionProperty.USE_ENCRYPTION.camelName(), "true");
+ properties.setProperty(ArrowFlightConnectionProperty.CLIENT_CERTIFICATE.camelName(), clientMTlsCertPath);
+ properties.setProperty(ArrowFlightConnectionProperty.CLIENT_KEY.camelName(), clientMTlsKeyPath);
+
+ final String jdbcUrl = String.format(
+ "jdbc:arrow-flight-sql://localhost:%s",
+ FLIGHT_SERVER_TEST_RULE.getPort());
+
+ try (Connection connection = DriverManager.getConnection(jdbcUrl, properties)) {
+ Assert.assertTrue(connection.isValid(0));
+ }
+ }
+
+ /**
+ * Check if an encrypted connection can be established successfully when connecting through the DriverManager using
+ * a connection url and properties with Object K-V pairs.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testTLSConnectionPropertyTrueCorrectCastUrlAndPropertiesUsingPutWithDriverManager()
+ throws Exception {
+ final Driver driver = new ArrowFlightJdbcDriver();
+ DriverManager.registerDriver(driver);
+
+ Properties properties = new Properties();
+
+ properties.put(ArrowFlightConnectionProperty.USER.camelName(), userTest);
+ properties.put(ArrowFlightConnectionProperty.PASSWORD.camelName(), passTest);
+ properties.put(ArrowFlightConnectionProperty.USE_ENCRYPTION.camelName(), true);
+ properties.put(ArrowFlightConnectionProperty.TLS_ROOT_CERTS.camelName(), tlsRootCertsPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_CERTIFICATE.camelName(), clientMTlsCertPath);
+ properties.put(ArrowFlightConnectionProperty.CLIENT_KEY.camelName(), clientMTlsKeyPath);
+
+ final String jdbcUrl = String.format(
+ "jdbc:arrow-flight-sql://localhost:%s",
+ FLIGHT_SERVER_TEST_RULE.getPort());
+
+ try (Connection connection = DriverManager.getConnection(jdbcUrl, properties)) {
+ Assert.assertTrue(connection.isValid(0));
+ }
+ }
+
+ /**
+ * Check if an encrypted connection can be established successfully when connecting through the DriverManager using
+ * just a connection url and using 0 and 1 as ssl values.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testTLSConnectionPropertyTrueIntegerCorrectCastUrlWithDriverManager()
+ throws Exception {
+ final Driver driver = new ArrowFlightJdbcDriver();
+ DriverManager.registerDriver(driver);
+
+ final String jdbcUrl = String.format(
+ "jdbc:arrow-flight-sql://localhost:%s?user=%s&password=%s" +
+ "&useEncryption=1&useSystemTrustStore=0&%s=%s&%s=%s&%s=%s",
+ FLIGHT_SERVER_TEST_RULE.getPort(),
+ userTest,
+ passTest,
+ ArrowFlightConnectionProperty.TLS_ROOT_CERTS.camelName(),
+ URLEncoder.encode(tlsRootCertsPath, "UTF-8"),
+ ArrowFlightConnectionProperty.CLIENT_CERTIFICATE.camelName(),
+ URLEncoder.encode(clientMTlsCertPath, "UTF-8"),
+ ArrowFlightConnectionProperty.CLIENT_KEY.camelName(),
+ URLEncoder.encode(clientMTlsKeyPath, "UTF-8"));
+
+ try (Connection connection = DriverManager.getConnection(jdbcUrl)) {
+ Assert.assertTrue(connection.isValid(0));
+ }
+ }
+
+ /**
+ * Check if an encrypted connection can be established successfully when connecting through the DriverManager using
+ * a connection url and properties with String K-V pairs and using 0 and 1 as ssl values.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testTLSConnectionPropertyTrueIntegerCorrectCastUrlAndPropertiesUsingSetPropertyWithDriverManager()
+ throws Exception {
+ final Driver driver = new ArrowFlightJdbcDriver();
+ DriverManager.registerDriver(driver);
+
+ Properties properties = new Properties();
+
+ properties.setProperty(ArrowFlightConnectionProperty.USER.camelName(), userTest);
+ properties.setProperty(ArrowFlightConnectionProperty.PASSWORD.camelName(), passTest);
+ properties.setProperty(ArrowFlightConnectionProperty.TLS_ROOT_CERTS.camelName(), tlsRootCertsPath);
+ properties.setProperty(ArrowFlightConnectionProperty.USE_ENCRYPTION.camelName(), "1");
+ properties.setProperty(ArrowFlightConnectionProperty.CLIENT_CERTIFICATE.camelName(), clientMTlsCertPath);
+ properties.setProperty(ArrowFlightConnectionProperty.CLIENT_KEY.camelName(), clientMTlsKeyPath);
+
+ final String jdbcUrl = String.format("jdbc:arrow-flight-sql://localhost:%s", FLIGHT_SERVER_TEST_RULE.getPort());
+
+ try (Connection connection = DriverManager.getConnection(jdbcUrl, properties)) {
+ Assert.assertTrue(connection.isValid(0));
+ }
+ }
+
+ /**
+ * Check if an encrypted connection can be established successfully when connecting through the DriverManager using
+ * a connection url and properties with Object K-V pairs and using 0 and 1 as ssl values.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testTLSConnectionPropertyTrueIntegerCorrectCastUrlAndPropertiesUsingPutWithDriverManager()
Review Comment:
We could use `@ParameterizedTest` to reduce some code duplication in the test files.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1381662045
##########
java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionMutualTlsTest.java:
##########
@@ -0,0 +1,436 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.driver.jdbc;
+
+import static org.junit.Assert.assertNotNull;
+
+import java.io.File;
+import java.net.URLEncoder;
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.Properties;
+
+import org.apache.arrow.driver.jdbc.authentication.UserPasswordAuthentication;
+import org.apache.arrow.driver.jdbc.client.ArrowFlightSqlClientHandler;
+import org.apache.arrow.driver.jdbc.utils.ArrowFlightConnectionConfigImpl.ArrowFlightConnectionProperty;
+import org.apache.arrow.driver.jdbc.utils.FlightSqlTestCertificates;
+import org.apache.arrow.driver.jdbc.utils.MockFlightSqlProducer;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.util.AutoCloseables;
+import org.apache.calcite.avatica.org.apache.http.auth.UsernamePasswordCredentials;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+/**
+ * Tests encrypted connections.
+ */
+public class ConnectionMutualTlsTest {
+
+ @ClassRule
+ public static final FlightServerTestRule FLIGHT_SERVER_TEST_RULE;
+ private static final String tlsRootCertsPath;
+
+ private static final String serverMTlsCACertPath;
+ private static final String clientMTlsCertPath;
+ private static final String badClientMTlsCertPath;
+ private static final String clientMTlsKeyPath;
+ private static final String badClientMTlsKeyPath;
+ private static final MockFlightSqlProducer PRODUCER = new MockFlightSqlProducer();
+ private static final String userTest = "user1";
+ private static final String passTest = "pass1";
+
+ static {
+ final FlightSqlTestCertificates.CertKeyPair
+ certKey = FlightSqlTestCertificates.exampleTlsCerts().get(0);
+
+ tlsRootCertsPath = certKey.cert.getPath();
+
+ final File serverMTlsCACert = FlightSqlTestCertificates.exampleCACert();
+
+ serverMTlsCACertPath = serverMTlsCACert.getPath();
+
+ final FlightSqlTestCertificates.CertKeyPair
+ clientMTlsCertKey = FlightSqlTestCertificates.exampleTlsCerts().get(1);
+
+ clientMTlsCertPath = clientMTlsCertKey.cert.getPath();
+ clientMTlsKeyPath = clientMTlsCertKey.key.getPath();
+
+ badClientMTlsCertPath = clientMTlsCertPath + ".bad";
+ badClientMTlsKeyPath = clientMTlsKeyPath + ".bad";
+
+ UserPasswordAuthentication authentication = new UserPasswordAuthentication.Builder()
+ .user(userTest, passTest)
+ .build();
+
+ FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder()
+ .authentication(authentication)
+ .useEncryption(certKey.cert, certKey.key)
+ .useMTlsClientVerification(serverMTlsCACert)
+ .producer(PRODUCER)
+ .build();
+ }
+
+ private BufferAllocator allocator;
+
+ @Before
+ public void setUp() throws Exception {
+ allocator = new RootAllocator(Long.MAX_VALUE);
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ allocator.getChildAllocators().forEach(BufferAllocator::close);
+ AutoCloseables.close(allocator);
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetEncryptedClientAuthenticated() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler client =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withClientCertificate(clientMTlsCertPath)
+ .withClientKey(clientMTlsKeyPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ assertNotNull(client);
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient providing a bad mTLS Cert Path. It's expected to
+ * receive the SQLException.
+ *
+ * @throws Exception on error.
+ */
+ @Test(expected = SQLException.class)
Review Comment:
Use assertThrows
##########
java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/client/utils/ClientAuthenticationUtils.java:
##########
@@ -235,6 +235,57 @@ public static InputStream getCertificateStream(final String keyStorePath,
return getSingleCertificateInputStream(keyStore);
}
+ /**
+ * Generates an {@link InputStream} that contains certificates for path-based
+ * TLS Root Certificates.
+ *
+ * @param tlsRootsCertificatesPath The path of the TLS Root Certificates.
+ * @return a new {code InputStream} containing the certificates.
+ * @throws GeneralSecurityException on error.
+ * @throws IOException on error.
+ */
+ public static InputStream getTlsRootCertificatesStream(final String tlsRootsCertificatesPath)
+ throws GeneralSecurityException, IOException {
+ Preconditions.checkNotNull(tlsRootsCertificatesPath, "TLS Root certificates path cannot be null!");
+
+ return Files
+ .newInputStream(Paths.get(Preconditions.checkNotNull(tlsRootsCertificatesPath)));
Review Comment:
Why the redundant checkNotNull?
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -328,6 +360,15 @@ public Builder useTls(final InputStream certChain, final InputStream key) {
return this;
}
+ /**
+ * Enable mTLS on the server.
+ * @param mTlsCACert The CA certificate to use for verifying clients.
+ */
+ public Builder useMTlsClientVerification(final InputStream mTlsCACert) throws IOException {
+ this.mTlsCACert = mTlsCACert;
Review Comment:
@vibhatha @davisusanibar
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -245,7 +254,21 @@ public FlightServer build() {
}
if (certChain != null) {
- builder.useTransportSecurity(certChain, key);
+ SslContextBuilder sslContextBuilder = GrpcSslContexts
+ .forServer(certChain, key);
+
+ if (mTlsCACert != null) {
+ sslContextBuilder
+ .clientAuth(ClientAuth.REQUIRE)
+ .trustManager(mTlsCACert);
+ }
+ try {
+ sslContext = sslContextBuilder.build();
+ } catch (SSLException e) {
+ throw new RuntimeException(e);
+ }
Review Comment:
https://netty.io/4.1/api/io/netty/handler/ssl/SslContextBuilder.html#trustManager-java.io.InputStream-
```suggestion
try {
sslContext = sslContextBuilder.build();
} catch (SSLException e) {
throw new RuntimeException(e);
} finally {
if (mTlsCACert != null) {
mTlsCACert.close();
}
certChain.close();
key.close();
}
```
##########
java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTlsRootCertsTest.java:
##########
@@ -0,0 +1,356 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.driver.jdbc;
+
+import static org.junit.Assert.assertNotNull;
+
+import java.net.URLEncoder;
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.Properties;
+
+import org.apache.arrow.driver.jdbc.authentication.UserPasswordAuthentication;
+import org.apache.arrow.driver.jdbc.client.ArrowFlightSqlClientHandler;
+import org.apache.arrow.driver.jdbc.utils.ArrowFlightConnectionConfigImpl.ArrowFlightConnectionProperty;
+import org.apache.arrow.driver.jdbc.utils.FlightSqlTestCertificates;
+import org.apache.arrow.driver.jdbc.utils.MockFlightSqlProducer;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.util.AutoCloseables;
+import org.apache.calcite.avatica.org.apache.http.auth.UsernamePasswordCredentials;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+
+/**
+ * Tests encrypted connections.
+ */
+public class ConnectionTlsRootCertsTest {
+
+ @ClassRule
+ public static final FlightServerTestRule FLIGHT_SERVER_TEST_RULE;
+ private static final String tlsRootCertsPath;
+ private static final String badTlsRootCertsPath;
+ private static final MockFlightSqlProducer PRODUCER = new MockFlightSqlProducer();
+ private static final String userTest = "user1";
+ private static final String passTest = "pass1";
+
+ static {
+ final FlightSqlTestCertificates.CertKeyPair
+ certKey = FlightSqlTestCertificates.exampleTlsCerts().get(0);
+
+ tlsRootCertsPath = certKey.cert.getPath();
+
+ badTlsRootCertsPath = certKey.cert.getPath() + ".bad";
+
+ UserPasswordAuthentication authentication = new UserPasswordAuthentication.Builder()
+ .user(userTest, passTest)
+ .build();
+
+ FLIGHT_SERVER_TEST_RULE = new FlightServerTestRule.Builder()
+ .authentication(authentication)
+ .useEncryption(certKey.cert, certKey.key)
+ .producer(PRODUCER)
+ .build();
+ }
+
+ private BufferAllocator allocator;
+
+ @Before
+ public void setUp() throws Exception {
+ allocator = new RootAllocator(Long.MAX_VALUE);
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ allocator.getChildAllocators().forEach(BufferAllocator::close);
+ AutoCloseables.close(allocator);
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testGetEncryptedClientAuthenticated() throws Exception {
+ final UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(
+ userTest, passTest);
+
+ try (ArrowFlightSqlClientHandler client =
+ new ArrowFlightSqlClientHandler.Builder()
+ .withHost(FLIGHT_SERVER_TEST_RULE.getHost())
+ .withPort(FLIGHT_SERVER_TEST_RULE.getPort())
+ .withUsername(credentials.getUserName())
+ .withPassword(credentials.getPassword())
+ .withTlsRootCertificates(tlsRootCertsPath)
+ .withBufferAllocator(allocator)
+ .withEncryption(true)
+ .build()) {
+ assertNotNull(client);
+ }
+ }
+
+ /**
+ * Try to instantiate an encrypted FlightClient providing a bad TLS Root Certs Path. It's expected to
+ * receive the SQLException.
+ *
+ * @throws Exception on error.
+ */
+ @Test(expected = SQLException.class)
Review Comment:
Use assertThrows
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1381950516
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -306,28 +329,63 @@ public Builder maxInboundMessageSize(int maxMessageSize) {
return this;
}
+ /**
+ * A small utility function to ensure that InputStream attributes.
+ * are closed if they are not null
+ * @param stream The InputStream to close (if it is not null).
+ */
+ private void closeInputStreamIfNotNull(InputStream stream) throws IOException {
+ if (stream != null) {
+ stream.close();
+ }
+ }
+
/**
* Enable TLS on the server.
* @param certChain The certificate chain to use.
* @param key The private key to use.
*/
public Builder useTls(final File certChain, final File key) throws IOException {
+ closeInputStreamIfNotNull(this.certChain);
+ closeInputStreamIfNotNull(this.key);
Review Comment:
would it be like this?:
```
private void closeInputStreamIfNotNull(InputStream stream) throws IOException {
if (stream != null) {
stream.close();
}
stream = null;
}
```
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1382152594
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -306,28 +329,63 @@ public Builder maxInboundMessageSize(int maxMessageSize) {
return this;
}
+ /**
+ * A small utility function to ensure that InputStream attributes.
+ * are closed if they are not null
+ * @param stream The InputStream to close (if it is not null).
+ */
+ private void closeInputStreamIfNotNull(InputStream stream) throws IOException {
+ if (stream != null) {
+ stream.close();
+ }
+ }
+
/**
* Enable TLS on the server.
* @param certChain The certificate chain to use.
* @param key The private key to use.
*/
public Builder useTls(final File certChain, final File key) throws IOException {
+ closeInputStreamIfNotNull(this.certChain);
+ closeInputStreamIfNotNull(this.key);
Review Comment:
I believe I've addressed these suggestions with updates to the code. Please confirm. Thanks.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #38461:
URL: https://github.com/apache/arrow/pull/38461#issuecomment-1797852616
After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 4ff1a29aae561cad6851a13666e4375a7645c6ef.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/18429100501) has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1373719110
##########
docs/source/java/flight_sql_jdbc_driver.rst:
##########
@@ -114,6 +114,21 @@ parameters are:
- null
- When TLS is enabled, the password for the certificate store
+ * - tlsRootCerts
+ - null
+ - Path to PEM-encoded root certificates for TLS - use this as
+ an alternative to ``trustStore``
+
+ * - clientCertificate
+ - null
+ - Path to PEM-encoded client mTLS certificate when the Flight
+ SQL server requires client verification.
+
+ * - clientKey
+ - null
+ - Path to PEM-encoded client mTLS key when the Flight
Review Comment:
Oh - I realized I used `clientKey` because the Java Flight client has it named that way:
https://github.com/apache/arrow/blob/57f643c2cecca729109daae18c7a64f3a37e76e4/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightClient.java#L661C1-L661C1
Are you ok with the name (for consistency's sake)?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "jduo (via GitHub)" <gi...@apache.org>.
jduo commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1373920916
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -317,6 +340,15 @@ public Builder useTls(final File certChain, final File key) throws IOException {
return this;
}
+ /**
+ * Enable Client Verification via mTLS on the server.
+ * @param mTlsCACert The CA certificate to use for verifying clients.
+ */
+ public Builder useMTlsClientVerification(final File mTlsCACert) throws IOException {
+ this.mTlsCACert = new FileInputStream(mTlsCACert);
Review Comment:
If the user calls this method repeatedly we will leak File handles. Need to close the current one if it exists. Perhaps it would be better to defer opening the physical file until starting the connection, though that delays notification if there's a problem opening it.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1373702088
##########
docs/source/java/flight_sql_jdbc_driver.rst:
##########
@@ -114,6 +114,21 @@ parameters are:
- null
- When TLS is enabled, the password for the certificate store
+ * - tlsRootCerts
+ - null
+ - Path to PEM-encoded root certificates for TLS - use this as
+ an alternative to ``trustStore``
+
+ * - clientCertificate
+ - null
+ - Path to PEM-encoded client mTLS certificate when the Flight
+ SQL server requires client verification.
+
+ * - clientKey
+ - null
+ - Path to PEM-encoded client mTLS key when the Flight
Review Comment:
agreed - I'll make the change...
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38461:
URL: https://github.com/apache/arrow/pull/38461#issuecomment-1779445213
:warning: GitHub issue #38460 **has been automatically assigned in GitHub** to PR creator.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on PR #38461:
URL: https://github.com/apache/arrow/pull/38461#issuecomment-1779529226
Thanks @prmoore77 ! @vibhatha @davisusanibar and I can take a look this week.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "jduo (via GitHub)" <gi...@apache.org>.
jduo commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1373437303
##########
docs/source/java/flight_sql_jdbc_driver.rst:
##########
@@ -114,6 +114,21 @@ parameters are:
- null
- When TLS is enabled, the password for the certificate store
+ * - tlsRootCerts
+ - null
+ - Path to PEM-encoded root certificates for TLS - use this as
+ an alternative to ``trustStore``
+
+ * - clientCertificate
+ - null
+ - Path to PEM-encoded client mTLS certificate when the Flight
+ SQL server requires client verification.
+
+ * - clientKey
+ - null
+ - Path to PEM-encoded client mTLS key when the Flight
Review Comment:
Maybe clientPrivateKey would be more clear?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #38461:
URL: https://github.com/apache/arrow/pull/38461#issuecomment-1781762517
Thanks for the PR. I'm doing a pass and testing as well.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1376280397
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -328,6 +360,15 @@ public Builder useTls(final InputStream certChain, final InputStream key) {
return this;
}
+ /**
+ * Enable mTLS on the server.
+ * @param mTlsCACert The CA certificate to use for verifying clients.
+ */
+ public Builder useMTlsClientVerification(final InputStream mTlsCACert) throws IOException {
+ this.mTlsCACert = mTlsCACert;
Review Comment:
@vibhatha @davisusanibar can you file an issue to fix this?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1377800976
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -328,6 +360,15 @@ public Builder useTls(final InputStream certChain, final InputStream key) {
return this;
}
+ /**
+ * Enable mTLS on the server.
+ * @param mTlsCACert The CA certificate to use for verifying clients.
+ */
+ public Builder useMTlsClientVerification(final InputStream mTlsCACert) throws IOException {
+ this.mTlsCACert = mTlsCACert;
Review Comment:
given that this issue will be filed separately - is it acceptable to approve/merge this PR?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on PR #38461:
URL: https://github.com/apache/arrow/pull/38461#issuecomment-1783941268
Hello – it could be added, but the Java Flight client doesn’t yet support that to my knowledge – so I just wanted to catch up to that for now. We can add it in a later PR, unless you feel strongly that it should be included here.
From: david dali susanibar arce ***@***.***>
Date: Saturday, October 28, 2023 at 7:20 PM
To: apache/arrow ***@***.***>
Cc: prmoore77 ***@***.***>, Mention ***@***.***>
Subject: Re: [apache/arrow] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver (PR #38461)
@davisusanibar commented on this pull request.
Thank you for the PR. I am adding a few minor questions.
________________________________
In docs/source/java/flight_sql_jdbc_driver.rst<https://github.com/apache/arrow/pull/38461#discussion_r1375164054>:
> @@ -114,6 +114,21 @@ parameters are:
- null
- When TLS is enabled, the password for the certificate store
+ * - tlsRootCerts
+ - null
+ - Path to PEM-encoded root certificates for TLS - use this as
+ an alternative to ``trustStore``
+
+ * - clientCertificate
+ - null
+ - Path to PEM-encoded client mTLS certificate when the Flight
+ SQL server requires client verification.
+
+ * - clientKey
Will it be necessary to provide an optional client key password argument if this client private key is password-protected?
—
Reply to this email directly, view it on GitHub<https://github.com/apache/arrow/pull/38461#pullrequestreview-1702696629>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AE6YADM4XNIWREIU7OLCZNDYBWHK7AVCNFSM6AAAAAA6PORM76VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMBSGY4TMNRSHE>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "jduo (via GitHub)" <gi...@apache.org>.
jduo commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1374829077
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -328,6 +360,15 @@ public Builder useTls(final InputStream certChain, final InputStream key) {
return this;
}
+ /**
+ * Enable mTLS on the server.
+ * @param mTlsCACert The CA certificate to use for verifying clients.
+ */
+ public Builder useMTlsClientVerification(final InputStream mTlsCACert) throws IOException {
+ this.mTlsCACert = mTlsCACert;
Review Comment:
Correct. It looks to have this issue too.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "prmoore77 (via GitHub)" <gi...@apache.org>.
prmoore77 commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1382152594
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -306,28 +329,63 @@ public Builder maxInboundMessageSize(int maxMessageSize) {
return this;
}
+ /**
+ * A small utility function to ensure that InputStream attributes.
+ * are closed if they are not null
+ * @param stream The InputStream to close (if it is not null).
+ */
+ private void closeInputStreamIfNotNull(InputStream stream) throws IOException {
+ if (stream != null) {
+ stream.close();
+ }
+ }
+
/**
* Enable TLS on the server.
* @param certChain The certificate chain to use.
* @param key The private key to use.
*/
public Builder useTls(final File certChain, final File key) throws IOException {
+ closeInputStreamIfNotNull(this.certChain);
+ closeInputStreamIfNotNull(this.key);
Review Comment:
I believe I've addressed these suggestions. Please confirm. Thanks.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38460: [Java][FlightRPC] Add mTLS support for Flight SQL JDBC driver [arrow]
Posted by "jduo (via GitHub)" <gi...@apache.org>.
jduo commented on code in PR #38461:
URL: https://github.com/apache/arrow/pull/38461#discussion_r1380623481
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -317,6 +340,15 @@ public Builder useTls(final File certChain, final File key) throws IOException {
return this;
}
+ /**
+ * Enable Client Verification via mTLS on the server.
+ * @param mTlsCACert The CA certificate to use for verifying clients.
+ */
+ public Builder useMTlsClientVerification(final File mTlsCACert) throws IOException {
+ this.mTlsCACert = new FileInputStream(mTlsCACert);
Review Comment:
Thanks @prmoore77 minor comment about another possible error scenario in useTls() that applies here.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org