Copilot commented on code in PR #17854: URL: https://github.com/apache/iotdb/pull/17854#discussion_r3432761737
########## iotdb-client/service-rpc/src/main/java/org/apache/iotdb/rpc/RpcSslUtils.java: ########## @@ -0,0 +1,261 @@ +/* + * 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.iotdb.rpc; + +import org.apache.thrift.transport.TSSLTransportFactory; +import org.apache.thrift.transport.TTransportException; + +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 java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.nio.file.AccessDeniedException; +import java.security.GeneralSecurityException; +import java.security.KeyStore; +import java.security.cert.X509Certificate; +import java.util.Enumeration; +import java.util.Locale; +import java.util.stream.Stream; + +public final class RpcSslUtils { + + private static final String DEFAULT_PROTOCOL = "TLS"; + private static final String PKCS12_STORE_TYPE = "PKCS12"; + private static final String JKS_STORE_TYPE = "JKS"; + + private static volatile String protocol = DEFAULT_PROTOCOL; + + private RpcSslUtils() {} + + public static void configure(String sslProtocol) { + protocol = normalizeProtocol(sslProtocol); + } + + public static TSSLTransportFactory.TSSLTransportParameters createTSSLTransportParameters() { + return createTSSLTransportParameters(protocol); + } + + public static TSSLTransportFactory.TSSLTransportParameters createTSSLTransportParameters( + String sslProtocol) { + return new TSSLTransportFactory.TSSLTransportParameters(normalizeProtocol(sslProtocol), null); + } + + public static void setKeyStore( + TSSLTransportFactory.TSSLTransportParameters params, String keyStorePath, String keyStorePwd) + throws TTransportException { + try { + params.setKeyStore( + keyStorePath, + keyStorePwd, + KeyManagerFactory.getDefaultAlgorithm(), + detectStoreType(keyStorePath, keyStorePwd)); + } catch (GeneralSecurityException | IOException e) { + throw new TTransportException(e); + } + } + + public static void setTrustStore( + TSSLTransportFactory.TSSLTransportParameters params, + String trustStorePath, + String trustStorePwd) + throws TTransportException { + try { + params.setTrustStore( + trustStorePath, + trustStorePwd, + TrustManagerFactory.getDefaultAlgorithm(), + detectStoreType(trustStorePath, trustStorePwd)); + } catch (GeneralSecurityException | IOException e) { + throw new TTransportException(e); + } + } + + public static SSLContext createSSLContext( + String keyStorePath, + String keyStorePassword, + String trustStorePath, + String trustStorePassword) + throws GeneralSecurityException, IOException { + return createSSLContext( + keyStorePath, keyStorePassword, trustStorePath, trustStorePassword, protocol); + } + + public static SSLContext createSSLContext( + String keyStorePath, + String keyStorePassword, + String trustStorePath, + String trustStorePassword, + String sslProtocol) + throws GeneralSecurityException, IOException { + SSLContext context = SSLContext.getInstance(normalizeProtocol(sslProtocol)); + KeyManager[] keyManagers = + hasText(keyStorePath) ? loadKeyManagers(keyStorePath, keyStorePassword) : null; + TrustManager[] trustManagers = + hasText(trustStorePath) ? loadTrustManagers(trustStorePath, trustStorePassword) : null; + context.init(keyManagers, trustManagers, null); + return context; + } + + public static KeyManager[] createKeyManagers(String keyStorePath, String keyStorePassword) + throws GeneralSecurityException, IOException { + return loadKeyManagers(keyStorePath, keyStorePassword); + } + + public static TrustManager[] createTrustManagers(String trustStorePath, String trustStorePassword) + throws GeneralSecurityException, IOException { + return loadTrustManagers(trustStorePath, trustStorePassword); + } + + public static String getProtocol() { + return protocol; + } + + public static boolean isSpecificProtocol(String sslProtocol) { + String trimmed = trimToEmpty(sslProtocol); + return !trimmed.isEmpty() && !DEFAULT_PROTOCOL.equals(trimmed.toUpperCase(Locale.ROOT)); + } + + public static String normalizeStandardTlsProtocol(String sslProtocol) { + String protocol = normalizeProtocol(sslProtocol); + if (!isStandardTlsProtocol(protocol)) { + throw new IllegalArgumentException( + "Unsupported SSL protocol " + protocol + ". Only standard TLS protocols are supported."); + } + return protocol; + } + + public static boolean isStandardTlsProtocol(String sslProtocol) { + String protocol = normalizeProtocol(sslProtocol).toUpperCase(Locale.ROOT); + return DEFAULT_PROTOCOL.equals(protocol) || protocol.matches("TLSV\\d+(\\.\\d+)*"); + } Review Comment: `isStandardTlsProtocol` currently treats any `TLSv<digits>[.<digits>...]` value as "standard" (e.g., `TLSv2`, `TLSv9`). This makes `normalizeStandardTlsProtocol` accept protocols that are not real/standard TLS protocol names and will later fail at runtime when creating the SSLContext/Thrift transport. ########## iotdb-client/service-rpc/src/main/java/org/apache/iotdb/rpc/RpcSslUtils.java: ########## @@ -0,0 +1,261 @@ +/* + * 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.iotdb.rpc; + +import org.apache.thrift.transport.TSSLTransportFactory; +import org.apache.thrift.transport.TTransportException; + +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 java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.nio.file.AccessDeniedException; +import java.security.GeneralSecurityException; +import java.security.KeyStore; +import java.security.cert.X509Certificate; +import java.util.Enumeration; +import java.util.Locale; +import java.util.stream.Stream; + +public final class RpcSslUtils { + + private static final String DEFAULT_PROTOCOL = "TLS"; + private static final String PKCS12_STORE_TYPE = "PKCS12"; + private static final String JKS_STORE_TYPE = "JKS"; + + private static volatile String protocol = DEFAULT_PROTOCOL; + + private RpcSslUtils() {} + + public static void configure(String sslProtocol) { + protocol = normalizeProtocol(sslProtocol); + } + + public static TSSLTransportFactory.TSSLTransportParameters createTSSLTransportParameters() { + return createTSSLTransportParameters(protocol); + } + + public static TSSLTransportFactory.TSSLTransportParameters createTSSLTransportParameters( + String sslProtocol) { + return new TSSLTransportFactory.TSSLTransportParameters(normalizeProtocol(sslProtocol), null); + } + + public static void setKeyStore( + TSSLTransportFactory.TSSLTransportParameters params, String keyStorePath, String keyStorePwd) + throws TTransportException { + try { + params.setKeyStore( + keyStorePath, + keyStorePwd, + KeyManagerFactory.getDefaultAlgorithm(), + detectStoreType(keyStorePath, keyStorePwd)); + } catch (GeneralSecurityException | IOException e) { + throw new TTransportException(e); + } + } + + public static void setTrustStore( + TSSLTransportFactory.TSSLTransportParameters params, + String trustStorePath, + String trustStorePwd) + throws TTransportException { + try { + params.setTrustStore( + trustStorePath, + trustStorePwd, + TrustManagerFactory.getDefaultAlgorithm(), + detectStoreType(trustStorePath, trustStorePwd)); + } catch (GeneralSecurityException | IOException e) { + throw new TTransportException(e); + } + } + + public static SSLContext createSSLContext( + String keyStorePath, + String keyStorePassword, + String trustStorePath, + String trustStorePassword) + throws GeneralSecurityException, IOException { + return createSSLContext( + keyStorePath, keyStorePassword, trustStorePath, trustStorePassword, protocol); + } + + public static SSLContext createSSLContext( + String keyStorePath, + String keyStorePassword, + String trustStorePath, + String trustStorePassword, + String sslProtocol) + throws GeneralSecurityException, IOException { + SSLContext context = SSLContext.getInstance(normalizeProtocol(sslProtocol)); + KeyManager[] keyManagers = + hasText(keyStorePath) ? loadKeyManagers(keyStorePath, keyStorePassword) : null; + TrustManager[] trustManagers = + hasText(trustStorePath) ? loadTrustManagers(trustStorePath, trustStorePassword) : null; + context.init(keyManagers, trustManagers, null); + return context; + } + + public static KeyManager[] createKeyManagers(String keyStorePath, String keyStorePassword) + throws GeneralSecurityException, IOException { + return loadKeyManagers(keyStorePath, keyStorePassword); + } + + public static TrustManager[] createTrustManagers(String trustStorePath, String trustStorePassword) + throws GeneralSecurityException, IOException { + return loadTrustManagers(trustStorePath, trustStorePassword); + } + + public static String getProtocol() { + return protocol; + } + + public static boolean isSpecificProtocol(String sslProtocol) { + String trimmed = trimToEmpty(sslProtocol); + return !trimmed.isEmpty() && !DEFAULT_PROTOCOL.equals(trimmed.toUpperCase(Locale.ROOT)); + } + + public static String normalizeStandardTlsProtocol(String sslProtocol) { + String protocol = normalizeProtocol(sslProtocol); + if (!isStandardTlsProtocol(protocol)) { + throw new IllegalArgumentException( + "Unsupported SSL protocol " + protocol + ". Only standard TLS protocols are supported."); + } + return protocol; + } + + public static boolean isStandardTlsProtocol(String sslProtocol) { + String protocol = normalizeProtocol(sslProtocol).toUpperCase(Locale.ROOT); + return DEFAULT_PROTOCOL.equals(protocol) || protocol.matches("TLSV\\d+(\\.\\d+)*"); + } + + public static void validateKeyStore(String keyStorePath, String keyStorePassword) + throws TTransportException { + validateStore(keyStorePath, keyStorePassword); + } + + public static void validateTrustStore(String trustStorePath, String trustStorePassword) + throws TTransportException { + validateStore(trustStorePath, trustStorePassword); + } + + private static KeyManager[] loadKeyManagers(String keyStorePath, String keyStorePassword) + throws GeneralSecurityException, IOException { + KeyStore keyStore = loadStore(keyStorePath, keyStorePassword); + KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()); + kmf.init(keyStore, toPassword(keyStorePassword)); + return kmf.getKeyManagers(); + } + + private static TrustManager[] loadTrustManagers(String trustStorePath, String trustStorePassword) + throws GeneralSecurityException, IOException { + KeyStore trustStore = loadStore(trustStorePath, trustStorePassword); + TrustManagerFactory tmf = + TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); + tmf.init(trustStore); + return tmf.getTrustManagers(); + } + + private static String detectStoreType(String storePath, String storePassword) + throws GeneralSecurityException, IOException { + return loadStore(storePath, storePassword).getType(); + } + + private static KeyStore loadStore(String storePath, String storePassword) + throws GeneralSecurityException, IOException { + Exception lastException = null; + for (String storeType : storeTypeCandidates()) { + try { + return loadStore(storePath, storePassword, storeType); + } catch (AccessDeniedException | FileNotFoundException e) { + throw e; + } catch (GeneralSecurityException | IOException e) { + lastException = e; + } + } + if (lastException instanceof GeneralSecurityException) { + throw (GeneralSecurityException) lastException; + } + if (lastException instanceof IOException) { + throw (IOException) lastException; + } + throw new IOException("No supported keystore or truststore type is available"); + } + + private static KeyStore loadStore(String storePath, String storePassword, String storeType) + throws GeneralSecurityException, IOException { + KeyStore store = KeyStore.getInstance(storeType); + try (FileInputStream fis = new FileInputStream(storePath)) { + store.load(fis, toPassword(storePassword)); + } catch (AccessDeniedException e) { + throw new AccessDeniedException("Failed to load keystore or truststore file"); + } catch (FileNotFoundException e) { + throw new FileNotFoundException("keystore or truststore file not found: " + storePath); + } + return store; + } Review Comment: `loadStore(..., storeType)` uses `new FileInputStream(storePath)` but tries to handle `java.nio.file.AccessDeniedException`. `FileInputStream` will typically throw `FileNotFoundException` (including for permission denied), so the `AccessDeniedException` path is effectively unreachable and access-denied cases will be misreported as "not found"/generic failures. ########## iotdb-client/jdbc/src/main/java/org/apache/iotdb/jdbc/Utils.java: ########## @@ -136,6 +136,9 @@ static IoTDBConnectionParams parseUrl(String url, Properties info) throws IoTDBU if (info.containsKey(Config.TRUST_STORE_PWD)) { params.setTrustStorePwd(info.getProperty(Config.TRUST_STORE_PWD)); } + if (info.containsKey(Config.SSL_PROTOCOL)) { + params.setSslProtocol(info.getProperty(Config.SSL_PROTOCOL)); + } Review Comment: JDBC URL parsing currently accepts any `ssl_protocol` value verbatim (e.g., `CUSTOMv1`). With the new `RpcSslUtils.normalizeStandardTlsProtocol(...)` and the server-side config/template stating only standard TLS protocol names are supported, it would be better to validate early here and fail with a clear `IoTDBURLException` instead of letting the connection fail later with a low-level `NoSuchAlgorithmException`/TLS error. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
