gaborgsomogyi commented on code in PR #24919:
URL: https://github.com/apache/flink/pull/24919#discussion_r1634861389


##########
flink-runtime/src/main/java/org/apache/flink/runtime/net/SSLUtils.java:
##########
@@ -285,7 +291,14 @@ private static KeyManagerFactory getKeyManagerFactory(
                                 : SecurityOptions.SSL_REST_KEY_PASSWORD,
                         SecurityOptions.SSL_KEY_PASSWORD);
 
-        KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType());
+        // do not use getAndCheckOption here as there is no fallback option 
and a default is
+        // specified
+        String keystoreType =
+                internal
+                        ? 
config.get(SecurityOptions.SSL_INTERNAL_KEYSTORE_TYPE)
+                        : config.get(SecurityOptions.SSL_REST_KEYSTORE_TYPE);

Review Comment:
   Just a clarification for other reviewers, since there is default value it 
just doesn't make sense to provide fallback.



##########
flink-rpc/flink-rpc-akka/src/main/java/org/apache/flink/runtime/rpc/pekko/CustomSSLEngineProvider.java:
##########
@@ -59,13 +68,35 @@ public TrustManager[] trustManagers() {
                                     .fingerprints(sslCertFingerprints)
                                     .build();
 
-            trustManagerFactory.init(loadKeystore(sslTrustStore, 
sslTrustStorePassword));
+            trustManagerFactory.init(
+                    loadKeystore(sslTrustStore, sslTrustStorePassword, 
sslTrustStoreType));
             return trustManagerFactory.getTrustManagers();
-        } catch (GeneralSecurityException e) {
+        } catch (GeneralSecurityException | IOException e) {
             // replicate exception handling from SSLEngineProvider
             throw new RemoteTransportException(
                     "Server SSL connection could not be established because 
SSL context could not be constructed",
                     e);
         }
     }
+
+    @Override
+    public KeyStore loadKeystore(String filename, String password) {
+        try {
+            return loadKeystore(filename, password, sslKeyStoreType);
+        } catch (IOException | GeneralSecurityException e) {
+            throw new RemoteTransportException(
+                    "Server SSL connection could not be established because 
SSL context could not be constructed",

Review Comment:
   Here we can be more specific: "Server SSL connection could not be 
established because keystore could not be loaded"



-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to