You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "alessandro-nori (via GitHub)" <gi...@apache.org> on 2026/09/01 16:47:58 UTC
[PR] Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
alessandro-nori opened a new pull request, #15235:
URL: https://github.com/apache/iceberg/pull/15235
This is a follow up to https://github.com/apache/iceberg/pull/13190.
(I have an older PR https://github.com/apache/iceberg/pull/13483 but I don't think I have the rights to reopen)
Implements a BasicTLSConfigurer class that provides TLS configuration with custom keystore and truststore support for REST client authentication.
Property-based configuration with support for:
`rest.client.tls.keystore.path`
`rest.client.tls.keystore.password`
`rest.client.tls.keystore.type` - (default: JKS)
`rest.client.tls.truststore.path`
`rest.client.tls.truststore.password`
`rest.client.tls.truststore.type` - (default: JKS)
## Extra notes on the use-case
In environments where the REST catalog requires `mTLS`, `BasicTLSConfigurer` enables setting the client certificate and CA trust explicitly for the Iceberg RestCatalog.
This avoids configuring JVM-wide TLS settings via javaoptions, ensuring that certificates are limited in scope to Iceberg and do not impact other applications running in the same JVM.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "roryqi (via GitHub)" <gi...@apache.org>.
roryqi commented on code in PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#discussion_r2769351742
##########
core/src/main/java/org/apache/iceberg/rest/auth/BasicTLSConfigurer.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.iceberg.rest.auth;
+
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.security.KeyManagementException;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.util.Map;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+
+/** A TLS configurer that supports custom keystore and truststore configuration. */
+public class BasicTLSConfigurer implements TLSConfigurer {
+
+ public static final String TLS_KEYSTORE_PATH = "rest.client.tls.keystore.path";
+ public static final String TLS_KEYSTORE_PASSWORD = "rest.client.tls.keystore.password";
+ public static final String TLS_KEYSTORE_TYPE = "rest.client.tls.keystore.type";
+ public static final String TLS_TRUSTSTORE_PATH = "rest.client.tls.truststore.path";
+ public static final String TLS_TRUSTSTORE_PASSWORD = "rest.client.tls.truststore.password";
+ public static final String TLS_TRUSTSTORE_TYPE = "rest.client.tls.truststore.type";
+
+ private static final String DEFAULT_KEYSTORE_TYPE = "JKS";
+ private static final String DEFAULT_SSL_PROTOCOL = "TLS";
+
+ private SSLContext sslContext;
+
+ @Override
+ public void initialize(Map<String, String> properties) {
+ String keystorePath = properties.get(TLS_KEYSTORE_PATH);
+ String keystorePassword = properties.get(TLS_KEYSTORE_PASSWORD);
+ String keystoreType = properties.getOrDefault(TLS_KEYSTORE_TYPE, DEFAULT_KEYSTORE_TYPE);
+ String truststorePath = properties.get(TLS_TRUSTSTORE_PATH);
+ String truststorePassword = properties.get(TLS_TRUSTSTORE_PASSWORD);
+ String truststoreType = properties.getOrDefault(TLS_TRUSTSTORE_TYPE, DEFAULT_KEYSTORE_TYPE);
+
+ try {
+ SSLContext context = SSLContext.getInstance(DEFAULT_SSL_PROTOCOL);
+
+ KeyManager[] keyManagers = null;
+ if (keystorePath != null) {
+ char[] keystorePasswordChars =
+ keystorePassword != null ? keystorePassword.toCharArray() : null;
+ KeyStore keyStore = loadKeyStore(keystorePath, keystorePasswordChars, keystoreType);
+ KeyManagerFactory keyManagerFactory =
+ KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+ keyManagerFactory.init(keyStore, keystorePasswordChars);
+ keyManagers = keyManagerFactory.getKeyManagers();
+ }
+
+ TrustManager[] trustManagers = null;
+ if (truststorePath != null) {
+ char[] truststorePasswordChars =
+ truststorePassword != null ? truststorePassword.toCharArray() : null;
+ KeyStore trustStore = loadKeyStore(truststorePath, truststorePasswordChars, truststoreType);
+ TrustManagerFactory trustManagerFactory =
+ TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
+ trustManagerFactory.init(trustStore);
+ trustManagers = trustManagerFactory.getTrustManagers();
+ }
+
+ context.init(keyManagers, trustManagers, null);
+
+ this.sslContext = context;
+ } catch (NoSuchAlgorithmException
+ | KeyManagementException
+ | KeyStoreException
+ | UnrecoverableKeyException e) {
+ throw new IllegalStateException("Failed to create SSL context", e);
+ }
+ }
+
+ @Override
+ public SSLContext sslContext() {
+ try {
+ return sslContext != null ? sslContext : SSLContext.getDefault();
Review Comment:
If `TLSConfigurer` is initialized, it won't be null. Maybe we should use `Preconditions.checkState` to throw an IllegalStateException if sslContext is 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "roryqi (via GitHub)" <gi...@apache.org>.
roryqi commented on PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#issuecomment-3877629492
Maybe we can refer to some industry implementation like Spark.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#issuecomment-4185786770
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "roryqi (via GitHub)" <gi...@apache.org>.
roryqi commented on code in PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#discussion_r2769383325
##########
core/src/main/java/org/apache/iceberg/rest/auth/BasicTLSConfigurer.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.iceberg.rest.auth;
+
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.security.KeyManagementException;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.util.Map;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+
+/** A TLS configurer that supports custom keystore and truststore configuration. */
+public class BasicTLSConfigurer implements TLSConfigurer {
+
+ public static final String TLS_KEYSTORE_PATH = "rest.client.tls.keystore.path";
+ public static final String TLS_KEYSTORE_PASSWORD = "rest.client.tls.keystore.password";
+ public static final String TLS_KEYSTORE_TYPE = "rest.client.tls.keystore.type";
+ public static final String TLS_TRUSTSTORE_PATH = "rest.client.tls.truststore.path";
+ public static final String TLS_TRUSTSTORE_PASSWORD = "rest.client.tls.truststore.password";
+ public static final String TLS_TRUSTSTORE_TYPE = "rest.client.tls.truststore.type";
+
+ private static final String DEFAULT_KEYSTORE_TYPE = "JKS";
+ private static final String DEFAULT_SSL_PROTOCOL = "TLS";
+
+ private SSLContext sslContext;
+
+ @Override
+ public void initialize(Map<String, String> properties) {
+ String keystorePath = properties.get(TLS_KEYSTORE_PATH);
+ String keystorePassword = properties.get(TLS_KEYSTORE_PASSWORD);
+ String keystoreType = properties.getOrDefault(TLS_KEYSTORE_TYPE, DEFAULT_KEYSTORE_TYPE);
+ String truststorePath = properties.get(TLS_TRUSTSTORE_PATH);
+ String truststorePassword = properties.get(TLS_TRUSTSTORE_PASSWORD);
+ String truststoreType = properties.getOrDefault(TLS_TRUSTSTORE_TYPE, DEFAULT_KEYSTORE_TYPE);
+
+ try {
+ SSLContext context = SSLContext.getInstance(DEFAULT_SSL_PROTOCOL);
+
+ KeyManager[] keyManagers = null;
+ if (keystorePath != null) {
+ char[] keystorePasswordChars =
+ keystorePassword != null ? keystorePassword.toCharArray() : null;
+ KeyStore keyStore = loadKeyStore(keystorePath, keystorePasswordChars, keystoreType);
+ KeyManagerFactory keyManagerFactory =
+ KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+ keyManagerFactory.init(keyStore, keystorePasswordChars);
+ keyManagers = keyManagerFactory.getKeyManagers();
+ }
+
+ TrustManager[] trustManagers = null;
+ if (truststorePath != null) {
+ char[] truststorePasswordChars =
+ truststorePassword != null ? truststorePassword.toCharArray() : null;
+ KeyStore trustStore = loadKeyStore(truststorePath, truststorePasswordChars, truststoreType);
+ TrustManagerFactory trustManagerFactory =
+ TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
+ trustManagerFactory.init(trustStore);
+ trustManagers = trustManagerFactory.getTrustManagers();
+ }
+
+ context.init(keyManagers, trustManagers, null);
Review Comment:
Maybe we should return `SSLContext.getDefault()` if keyManagers and trustManagers are 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "git-rbanerjee (via GitHub)" <gi...@apache.org>.
git-rbanerjee commented on code in PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#discussion_r2881864035
##########
core/src/main/java/org/apache/iceberg/rest/auth/BasicTLSConfigurer.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.iceberg.rest.auth;
+
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.security.KeyManagementException;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.util.Map;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.base.Strings;
+
+/** A TLS configurer that supports custom keystore and truststore configuration. */
+public class BasicTLSConfigurer implements TLSConfigurer {
+
+ public static final String TLS_KEYSTORE_PATH = "rest.client.tls.keystore.path";
+ public static final String TLS_KEYSTORE_PASSWORD = "rest.client.tls.keystore.password";
+ public static final String TLS_KEYSTORE_TYPE = "rest.client.tls.keystore.type";
+ public static final String TLS_TRUSTSTORE_PATH = "rest.client.tls.truststore.path";
+ public static final String TLS_TRUSTSTORE_PASSWORD = "rest.client.tls.truststore.password";
+ public static final String TLS_TRUSTSTORE_TYPE = "rest.client.tls.truststore.type";
+
+ private static final String DEFAULT_KEYSTORE_TYPE = "JKS";
+ private static final String DEFAULT_SSL_PROTOCOL = "TLS";
Review Comment:
Allow configuring this. Maybe with something like `rest.client.tls.protocol`
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "roryqi (via GitHub)" <gi...@apache.org>.
roryqi commented on PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#issuecomment-3863988881
> It would really nice if we can enumerate the use case it will enable too !
View definer and read restriction require a trusted client. It's a simple way to use TLS. We can verify the certificate of the trusted client in the REST server side.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "roryqi (via GitHub)" <gi...@apache.org>.
roryqi commented on code in PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#discussion_r2787967818
##########
core/src/main/java/org/apache/iceberg/rest/auth/BasicTLSConfigurer.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.iceberg.rest.auth;
+
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.security.KeyManagementException;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.util.Map;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.base.Strings;
+
+/** A TLS configurer that supports custom keystore and truststore configuration. */
+public class BasicTLSConfigurer implements TLSConfigurer {
+
+ public static final String TLS_KEYSTORE_PATH = "rest.client.tls.keystore.path";
+ public static final String TLS_KEYSTORE_PASSWORD = "rest.client.tls.keystore.password";
+ public static final String TLS_KEYSTORE_TYPE = "rest.client.tls.keystore.type";
+ public static final String TLS_TRUSTSTORE_PATH = "rest.client.tls.truststore.path";
+ public static final String TLS_TRUSTSTORE_PASSWORD = "rest.client.tls.truststore.password";
+ public static final String TLS_TRUSTSTORE_TYPE = "rest.client.tls.truststore.type";
+
+ private static final String DEFAULT_KEYSTORE_TYPE = "JKS";
+ private static final String DEFAULT_SSL_PROTOCOL = "TLS";
+
+ private SSLContext sslContext;
+
+ @Override
+ public void initialize(Map<String, String> properties) {
+ String keystorePath = properties.get(TLS_KEYSTORE_PATH);
+ String keystorePassword = properties.get(TLS_KEYSTORE_PASSWORD);
+ String keystoreType = properties.getOrDefault(TLS_KEYSTORE_TYPE, DEFAULT_KEYSTORE_TYPE);
+ String truststorePath = properties.get(TLS_TRUSTSTORE_PATH);
+ String truststorePassword = properties.get(TLS_TRUSTSTORE_PASSWORD);
+ String truststoreType = properties.getOrDefault(TLS_TRUSTSTORE_TYPE, DEFAULT_KEYSTORE_TYPE);
+
+ try {
+ SSLContext context = SSLContext.getInstance(DEFAULT_SSL_PROTOCOL);
+
+ KeyManager[] keyManagers = null;
+ if (!Strings.isNullOrEmpty(keystorePath)) {
+ char[] keystorePasswordChars =
+ keystorePassword != null ? keystorePassword.toCharArray() : null;
+ KeyStore keyStore = loadKeyStore(keystorePath, keystorePasswordChars, keystoreType);
+ KeyManagerFactory keyManagerFactory =
+ KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+ keyManagerFactory.init(keyStore, keystorePasswordChars);
+ keyManagers = keyManagerFactory.getKeyManagers();
+ }
+
+ TrustManager[] trustManagers = null;
+ if (!Strings.isNullOrEmpty(truststorePath)) {
+ char[] truststorePasswordChars =
+ truststorePassword != null ? truststorePassword.toCharArray() : null;
+ KeyStore trustStore = loadKeyStore(truststorePath, truststorePasswordChars, truststoreType);
+ TrustManagerFactory trustManagerFactory =
+ TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
+ trustManagerFactory.init(trustStore);
+ trustManagers = trustManagerFactory.getTrustManagers();
+ }
+
+ if (keyManagers == null && trustManagers == null) {
+ this.sslContext = SSLContext.getDefault();
+ } else {
+ context.init(keyManagers, trustManagers, null);
Review Comment:
What will it happen if keyManagers is null or trustManagers is 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "roryqi (via GitHub)" <gi...@apache.org>.
roryqi commented on code in PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#discussion_r2769373449
##########
core/src/main/java/org/apache/iceberg/rest/auth/BasicTLSConfigurer.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.iceberg.rest.auth;
+
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.security.KeyManagementException;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.util.Map;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+
+/** A TLS configurer that supports custom keystore and truststore configuration. */
+public class BasicTLSConfigurer implements TLSConfigurer {
+
+ public static final String TLS_KEYSTORE_PATH = "rest.client.tls.keystore.path";
+ public static final String TLS_KEYSTORE_PASSWORD = "rest.client.tls.keystore.password";
+ public static final String TLS_KEYSTORE_TYPE = "rest.client.tls.keystore.type";
+ public static final String TLS_TRUSTSTORE_PATH = "rest.client.tls.truststore.path";
+ public static final String TLS_TRUSTSTORE_PASSWORD = "rest.client.tls.truststore.password";
+ public static final String TLS_TRUSTSTORE_TYPE = "rest.client.tls.truststore.type";
+
+ private static final String DEFAULT_KEYSTORE_TYPE = "JKS";
+ private static final String DEFAULT_SSL_PROTOCOL = "TLS";
+
+ private SSLContext sslContext;
+
+ @Override
+ public void initialize(Map<String, String> properties) {
+ String keystorePath = properties.get(TLS_KEYSTORE_PATH);
+ String keystorePassword = properties.get(TLS_KEYSTORE_PASSWORD);
+ String keystoreType = properties.getOrDefault(TLS_KEYSTORE_TYPE, DEFAULT_KEYSTORE_TYPE);
+ String truststorePath = properties.get(TLS_TRUSTSTORE_PATH);
+ String truststorePassword = properties.get(TLS_TRUSTSTORE_PASSWORD);
+ String truststoreType = properties.getOrDefault(TLS_TRUSTSTORE_TYPE, DEFAULT_KEYSTORE_TYPE);
+
+ try {
+ SSLContext context = SSLContext.getInstance(DEFAULT_SSL_PROTOCOL);
+
+ KeyManager[] keyManagers = null;
+ if (keystorePath != null) {
Review Comment:
Maybe it's better to use `StringUtils.isNotBlank`
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "alessandro-nori (via GitHub)" <gi...@apache.org>.
alessandro-nori commented on code in PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#discussion_r3923219259
##########
core/src/main/java/org/apache/iceberg/rest/auth/BasicTLSConfigurer.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.iceberg.rest.auth;
+
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.security.KeyManagementException;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.util.Map;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.base.Strings;
+
+/** A TLS configurer that supports custom keystore and truststore configuration. */
+public class BasicTLSConfigurer implements TLSConfigurer {
+
+ public static final String TLS_KEYSTORE_PATH = "rest.client.tls.keystore.path";
+ public static final String TLS_KEYSTORE_PASSWORD = "rest.client.tls.keystore.password";
+ public static final String TLS_KEYSTORE_TYPE = "rest.client.tls.keystore.type";
+ public static final String TLS_TRUSTSTORE_PATH = "rest.client.tls.truststore.path";
+ public static final String TLS_TRUSTSTORE_PASSWORD = "rest.client.tls.truststore.password";
+ public static final String TLS_TRUSTSTORE_TYPE = "rest.client.tls.truststore.type";
+
+ private static final String DEFAULT_KEYSTORE_TYPE = "JKS";
+ private static final String DEFAULT_SSL_PROTOCOL = "TLS";
+
+ private SSLContext sslContext;
+
+ @Override
+ public void initialize(Map<String, String> properties) {
+ String keystorePath = properties.get(TLS_KEYSTORE_PATH);
+ String keystorePassword = properties.get(TLS_KEYSTORE_PASSWORD);
+ String keystoreType = properties.getOrDefault(TLS_KEYSTORE_TYPE, DEFAULT_KEYSTORE_TYPE);
+ String truststorePath = properties.get(TLS_TRUSTSTORE_PATH);
+ String truststorePassword = properties.get(TLS_TRUSTSTORE_PASSWORD);
+ String truststoreType = properties.getOrDefault(TLS_TRUSTSTORE_TYPE, DEFAULT_KEYSTORE_TYPE);
+
+ try {
+ SSLContext context = SSLContext.getInstance(DEFAULT_SSL_PROTOCOL);
+
+ KeyManager[] keyManagers = null;
+ if (!Strings.isNullOrEmpty(keystorePath)) {
+ char[] keystorePasswordChars =
+ keystorePassword != null ? keystorePassword.toCharArray() : null;
+ KeyStore keyStore = loadKeyStore(keystorePath, keystorePasswordChars, keystoreType);
+ KeyManagerFactory keyManagerFactory =
+ KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+ keyManagerFactory.init(keyStore, keystorePasswordChars);
+ keyManagers = keyManagerFactory.getKeyManagers();
+ }
+
+ TrustManager[] trustManagers = null;
+ if (!Strings.isNullOrEmpty(truststorePath)) {
+ char[] truststorePasswordChars =
+ truststorePassword != null ? truststorePassword.toCharArray() : null;
+ KeyStore trustStore = loadKeyStore(truststorePath, truststorePasswordChars, truststoreType);
+ TrustManagerFactory trustManagerFactory =
+ TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
+ trustManagerFactory.init(trustStore);
+ trustManagers = trustManagerFactory.getTrustManagers();
+ }
+
+ if (keyManagers == null && trustManagers == null) {
+ this.sslContext = SSLContext.getDefault();
+ } else {
+ context.init(keyManagers, trustManagers, null);
+ this.sslContext = context;
+ }
+ } catch (NoSuchAlgorithmException
+ | KeyManagementException
+ | KeyStoreException
+ | UnrecoverableKeyException e) {
+ throw new IllegalStateException("Failed to create SSL context", e);
+ }
+ }
+
+ @Override
+ public SSLContext sslContext() {
+ Preconditions.checkState(sslContext != null, "TLSConfigurer must be initialized before use");
+ return sslContext;
+ }
+
+ @Override
+ public String[] supportedProtocols() {
+ return null;
+ }
+
+ @Override
+ public String[] supportedCipherSuites() {
+ return null;
Review Comment:
I was not planning on adding cyphers in this PR so that why I didn't initially override this method
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "roryqi (via GitHub)" <gi...@apache.org>.
roryqi commented on code in PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#discussion_r2769571155
##########
core/src/main/java/org/apache/iceberg/rest/auth/BasicTLSConfigurer.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.iceberg.rest.auth;
+
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.security.KeyManagementException;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.util.Map;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+
+/** A TLS configurer that supports custom keystore and truststore configuration. */
+public class BasicTLSConfigurer implements TLSConfigurer {
+
+ public static final String TLS_KEYSTORE_PATH = "rest.client.tls.keystore.path";
+ public static final String TLS_KEYSTORE_PASSWORD = "rest.client.tls.keystore.password";
+ public static final String TLS_KEYSTORE_TYPE = "rest.client.tls.keystore.type";
+ public static final String TLS_TRUSTSTORE_PATH = "rest.client.tls.truststore.path";
+ public static final String TLS_TRUSTSTORE_PASSWORD = "rest.client.tls.truststore.password";
+ public static final String TLS_TRUSTSTORE_TYPE = "rest.client.tls.truststore.type";
+
+ private static final String DEFAULT_KEYSTORE_TYPE = "JKS";
+ private static final String DEFAULT_SSL_PROTOCOL = "TLS";
+
+ private SSLContext sslContext;
+
+ @Override
+ public void initialize(Map<String, String> properties) {
+ String keystorePath = properties.get(TLS_KEYSTORE_PATH);
+ String keystorePassword = properties.get(TLS_KEYSTORE_PASSWORD);
+ String keystoreType = properties.getOrDefault(TLS_KEYSTORE_TYPE, DEFAULT_KEYSTORE_TYPE);
+ String truststorePath = properties.get(TLS_TRUSTSTORE_PATH);
+ String truststorePassword = properties.get(TLS_TRUSTSTORE_PASSWORD);
+ String truststoreType = properties.getOrDefault(TLS_TRUSTSTORE_TYPE, DEFAULT_KEYSTORE_TYPE);
+
+ try {
+ SSLContext context = SSLContext.getInstance(DEFAULT_SSL_PROTOCOL);
+
+ KeyManager[] keyManagers = null;
+ if (keystorePath != null) {
Review Comment:
You should check whether the path 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "roryqi (via GitHub)" <gi...@apache.org>.
roryqi commented on code in PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#discussion_r2787969755
##########
core/src/main/java/org/apache/iceberg/rest/auth/BasicTLSConfigurer.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.iceberg.rest.auth;
+
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.security.KeyManagementException;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.util.Map;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.base.Strings;
+
+/** A TLS configurer that supports custom keystore and truststore configuration. */
+public class BasicTLSConfigurer implements TLSConfigurer {
+
+ public static final String TLS_KEYSTORE_PATH = "rest.client.tls.keystore.path";
+ public static final String TLS_KEYSTORE_PASSWORD = "rest.client.tls.keystore.password";
+ public static final String TLS_KEYSTORE_TYPE = "rest.client.tls.keystore.type";
+ public static final String TLS_TRUSTSTORE_PATH = "rest.client.tls.truststore.path";
+ public static final String TLS_TRUSTSTORE_PASSWORD = "rest.client.tls.truststore.password";
+ public static final String TLS_TRUSTSTORE_TYPE = "rest.client.tls.truststore.type";
+
+ private static final String DEFAULT_KEYSTORE_TYPE = "JKS";
+ private static final String DEFAULT_SSL_PROTOCOL = "TLS";
+
+ private SSLContext sslContext;
+
+ @Override
+ public void initialize(Map<String, String> properties) {
+ String keystorePath = properties.get(TLS_KEYSTORE_PATH);
+ String keystorePassword = properties.get(TLS_KEYSTORE_PASSWORD);
+ String keystoreType = properties.getOrDefault(TLS_KEYSTORE_TYPE, DEFAULT_KEYSTORE_TYPE);
+ String truststorePath = properties.get(TLS_TRUSTSTORE_PATH);
+ String truststorePassword = properties.get(TLS_TRUSTSTORE_PASSWORD);
+ String truststoreType = properties.getOrDefault(TLS_TRUSTSTORE_TYPE, DEFAULT_KEYSTORE_TYPE);
+
+ try {
+ SSLContext context = SSLContext.getInstance(DEFAULT_SSL_PROTOCOL);
+
+ KeyManager[] keyManagers = null;
+ if (!Strings.isNullOrEmpty(keystorePath)) {
+ char[] keystorePasswordChars =
+ keystorePassword != null ? keystorePassword.toCharArray() : null;
+ KeyStore keyStore = loadKeyStore(keystorePath, keystorePasswordChars, keystoreType);
+ KeyManagerFactory keyManagerFactory =
+ KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+ keyManagerFactory.init(keyStore, keystorePasswordChars);
+ keyManagers = keyManagerFactory.getKeyManagers();
+ }
+
+ TrustManager[] trustManagers = null;
+ if (!Strings.isNullOrEmpty(truststorePath)) {
+ char[] truststorePasswordChars =
+ truststorePassword != null ? truststorePassword.toCharArray() : null;
+ KeyStore trustStore = loadKeyStore(truststorePath, truststorePasswordChars, truststoreType);
+ TrustManagerFactory trustManagerFactory =
+ TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
+ trustManagerFactory.init(trustStore);
+ trustManagers = trustManagerFactory.getTrustManagers();
+ }
+
+ if (keyManagers == null && trustManagers == null) {
+ this.sslContext = SSLContext.getDefault();
+ } else {
+ context.init(keyManagers, trustManagers, null);
+ this.sslContext = context;
+ }
+ } catch (NoSuchAlgorithmException
+ | KeyManagementException
+ | KeyStoreException
+ | UnrecoverableKeyException e) {
+ throw new IllegalStateException("Failed to create SSL context", e);
+ }
+ }
+
+ @Override
+ public SSLContext sslContext() {
+ Preconditions.checkState(sslContext != null, "TLSConfigurer must be initialized before use");
+ return sslContext;
+ }
+
+ @Override
+ public String[] supportedProtocols() {
+ return null;
Review Comment:
Why do we return null 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#issuecomment-5497353080
@alessandro-nori @roryqi @git-rbanerjee i want to help here, please let me know if you all still want to continue with 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#issuecomment-6205129613
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "git-rbanerjee (via GitHub)" <gi...@apache.org>.
git-rbanerjee commented on PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#issuecomment-3995349470
In `AuthProperties` , `rest.auth.type` should have a "mTLS" option.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#issuecomment-3854761658
It would really nice if we can enumerate the use case it will enable 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#issuecomment-4227455153
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #15235: Add BasicTLSConfigurer for custom keystore and truststore support
URL: https://github.com/apache/iceberg/pull/15235
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#issuecomment-5685046433
i wonder why not have a custom implementation of this class `BasicTLSConfigurer` and then injecting that to class path would be sufficient for this scenario rather than opening these bunch of config ?
rest.client.tls.configurer-impl = <custom impl with custom keystore and trust store support>
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #15235: Core: Add BasicTLSConfigurer for custom keystore and truststore support
URL: https://github.com/apache/iceberg/pull/15235
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "roryqi (via GitHub)" <gi...@apache.org>.
roryqi commented on PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#issuecomment-3854151229
Maybe you should override
```
default String[] supportedProtocols() {
return null;
}
default String[] supportedCipherSuites() {
return 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "roryqi (via GitHub)" <gi...@apache.org>.
roryqi commented on code in PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#discussion_r2787971077
##########
core/src/main/java/org/apache/iceberg/rest/auth/BasicTLSConfigurer.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.iceberg.rest.auth;
+
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.security.KeyManagementException;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.util.Map;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.base.Strings;
+
+/** A TLS configurer that supports custom keystore and truststore configuration. */
+public class BasicTLSConfigurer implements TLSConfigurer {
+
+ public static final String TLS_KEYSTORE_PATH = "rest.client.tls.keystore.path";
+ public static final String TLS_KEYSTORE_PASSWORD = "rest.client.tls.keystore.password";
+ public static final String TLS_KEYSTORE_TYPE = "rest.client.tls.keystore.type";
+ public static final String TLS_TRUSTSTORE_PATH = "rest.client.tls.truststore.path";
+ public static final String TLS_TRUSTSTORE_PASSWORD = "rest.client.tls.truststore.password";
+ public static final String TLS_TRUSTSTORE_TYPE = "rest.client.tls.truststore.type";
+
+ private static final String DEFAULT_KEYSTORE_TYPE = "JKS";
+ private static final String DEFAULT_SSL_PROTOCOL = "TLS";
+
+ private SSLContext sslContext;
+
+ @Override
+ public void initialize(Map<String, String> properties) {
+ String keystorePath = properties.get(TLS_KEYSTORE_PATH);
+ String keystorePassword = properties.get(TLS_KEYSTORE_PASSWORD);
+ String keystoreType = properties.getOrDefault(TLS_KEYSTORE_TYPE, DEFAULT_KEYSTORE_TYPE);
+ String truststorePath = properties.get(TLS_TRUSTSTORE_PATH);
+ String truststorePassword = properties.get(TLS_TRUSTSTORE_PASSWORD);
+ String truststoreType = properties.getOrDefault(TLS_TRUSTSTORE_TYPE, DEFAULT_KEYSTORE_TYPE);
+
+ try {
+ SSLContext context = SSLContext.getInstance(DEFAULT_SSL_PROTOCOL);
+
+ KeyManager[] keyManagers = null;
+ if (!Strings.isNullOrEmpty(keystorePath)) {
+ char[] keystorePasswordChars =
+ keystorePassword != null ? keystorePassword.toCharArray() : null;
+ KeyStore keyStore = loadKeyStore(keystorePath, keystorePasswordChars, keystoreType);
+ KeyManagerFactory keyManagerFactory =
+ KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+ keyManagerFactory.init(keyStore, keystorePasswordChars);
+ keyManagers = keyManagerFactory.getKeyManagers();
+ }
+
+ TrustManager[] trustManagers = null;
+ if (!Strings.isNullOrEmpty(truststorePath)) {
+ char[] truststorePasswordChars =
+ truststorePassword != null ? truststorePassword.toCharArray() : null;
+ KeyStore trustStore = loadKeyStore(truststorePath, truststorePasswordChars, truststoreType);
+ TrustManagerFactory trustManagerFactory =
+ TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
+ trustManagerFactory.init(trustStore);
+ trustManagers = trustManagerFactory.getTrustManagers();
+ }
+
+ if (keyManagers == null && trustManagers == null) {
+ this.sslContext = SSLContext.getDefault();
+ } else {
+ context.init(keyManagers, trustManagers, null);
+ this.sslContext = context;
+ }
+ } catch (NoSuchAlgorithmException
+ | KeyManagementException
+ | KeyStoreException
+ | UnrecoverableKeyException e) {
+ throw new IllegalStateException("Failed to create SSL context", e);
+ }
+ }
+
+ @Override
+ public SSLContext sslContext() {
+ Preconditions.checkState(sslContext != null, "TLSConfigurer must be initialized before use");
+ return sslContext;
+ }
+
+ @Override
+ public String[] supportedProtocols() {
+ return null;
+ }
+
+ @Override
+ public String[] supportedCipherSuites() {
+ return null;
Review Comment:
Why do we return null 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "alessandro-nori (via GitHub)" <gi...@apache.org>.
alessandro-nori commented on PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#issuecomment-5523553117
Hi @singhpk234 , thanks for the help and sorry for leaving this going stale. I'll address the comments.
Is there anything else missing in your opinion?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "alessandro-nori (via GitHub)" <gi...@apache.org>.
alessandro-nori commented on code in PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#discussion_r3923207921
##########
core/src/main/java/org/apache/iceberg/rest/auth/BasicTLSConfigurer.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.iceberg.rest.auth;
+
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.security.KeyManagementException;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.util.Map;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.base.Strings;
+
+/** A TLS configurer that supports custom keystore and truststore configuration. */
+public class BasicTLSConfigurer implements TLSConfigurer {
+
+ public static final String TLS_KEYSTORE_PATH = "rest.client.tls.keystore.path";
+ public static final String TLS_KEYSTORE_PASSWORD = "rest.client.tls.keystore.password";
+ public static final String TLS_KEYSTORE_TYPE = "rest.client.tls.keystore.type";
+ public static final String TLS_TRUSTSTORE_PATH = "rest.client.tls.truststore.path";
+ public static final String TLS_TRUSTSTORE_PASSWORD = "rest.client.tls.truststore.password";
+ public static final String TLS_TRUSTSTORE_TYPE = "rest.client.tls.truststore.type";
+
+ private static final String DEFAULT_KEYSTORE_TYPE = "JKS";
+ private static final String DEFAULT_SSL_PROTOCOL = "TLS";
+
+ private SSLContext sslContext;
+
+ @Override
+ public void initialize(Map<String, String> properties) {
+ String keystorePath = properties.get(TLS_KEYSTORE_PATH);
+ String keystorePassword = properties.get(TLS_KEYSTORE_PASSWORD);
+ String keystoreType = properties.getOrDefault(TLS_KEYSTORE_TYPE, DEFAULT_KEYSTORE_TYPE);
+ String truststorePath = properties.get(TLS_TRUSTSTORE_PATH);
+ String truststorePassword = properties.get(TLS_TRUSTSTORE_PASSWORD);
+ String truststoreType = properties.getOrDefault(TLS_TRUSTSTORE_TYPE, DEFAULT_KEYSTORE_TYPE);
+
+ try {
+ SSLContext context = SSLContext.getInstance(DEFAULT_SSL_PROTOCOL);
+
+ KeyManager[] keyManagers = null;
+ if (!Strings.isNullOrEmpty(keystorePath)) {
+ char[] keystorePasswordChars =
+ keystorePassword != null ? keystorePassword.toCharArray() : null;
+ KeyStore keyStore = loadKeyStore(keystorePath, keystorePasswordChars, keystoreType);
+ KeyManagerFactory keyManagerFactory =
+ KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+ keyManagerFactory.init(keyStore, keystorePasswordChars);
+ keyManagers = keyManagerFactory.getKeyManagers();
+ }
+
+ TrustManager[] trustManagers = null;
+ if (!Strings.isNullOrEmpty(truststorePath)) {
+ char[] truststorePasswordChars =
+ truststorePassword != null ? truststorePassword.toCharArray() : null;
+ KeyStore trustStore = loadKeyStore(truststorePath, truststorePasswordChars, truststoreType);
+ TrustManagerFactory trustManagerFactory =
+ TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
+ trustManagerFactory.init(trustStore);
+ trustManagers = trustManagerFactory.getTrustManagers();
+ }
+
+ if (keyManagers == null && trustManagers == null) {
+ this.sslContext = SSLContext.getDefault();
+ } else {
+ context.init(keyManagers, trustManagers, null);
Review Comment:
it would fall back to the default implementation for the one that the user didn't configure
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Add BasicTLSConfigurer for custom keystore and truststore support [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #15235:
URL: https://github.com/apache/iceberg/pull/15235#issuecomment-6354366326
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org