vy commented on code in PR #2767:
URL: https://github.com/apache/logging-log4j2/pull/2767#discussion_r1746978471
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/SslConfiguration.java:
##########
@@ -181,61 +123,43 @@ private static SSLContext createDefaultSslContext(final
String protocol) {
private static SSLContext createSslContext(
final String protocol,
- final KeyStoreConfiguration keyStoreConfig,
- final TrustStoreConfiguration trustStoreConfig)
- throws KeyStoreConfigurationException,
TrustStoreConfigurationException {
+ @Nullable final KeyStoreConfiguration keyStoreConfig,
+ @Nullable final TrustStoreConfiguration trustStoreConfig) {
try {
- KeyManager[] kManagers = null;
- TrustManager[] tManagers = null;
-
- final SSLContext newSslContext = SSLContext.getInstance(protocol);
- if (keyStoreConfig != null) {
- final KeyManagerFactory kmFactory =
loadKeyManagerFactory(keyStoreConfig);
- kManagers = kmFactory.getKeyManagers();
- }
- if (trustStoreConfig != null) {
- final TrustManagerFactory tmFactory =
loadTrustManagerFactory(trustStoreConfig);
- tManagers = tmFactory.getTrustManagers();
- }
-
- newSslContext.init(kManagers, tManagers, null);
- return newSslContext;
- } catch (final NoSuchAlgorithmException e) {
- LOGGER.error("No Provider supports a TrustManagerFactorySpi
implementation for the specified protocol", e);
- throw new TrustStoreConfigurationException(e);
- } catch (final KeyManagementException e) {
- LOGGER.error("Failed to initialize the SSLContext", e);
- throw new KeyStoreConfigurationException(e);
+ final SSLContext sslContext = SSLContext.getInstance(protocol);
+ final KeyManager[] keyManagers = loadKeyManagers(keyStoreConfig);
+ final TrustManager[] trustManagers =
loadTrustManagers(trustStoreConfig);
+ sslContext.init(keyManagers, trustManagers, null);
+ return sslContext;
+ } catch (final Exception error) {
+ LOGGER.error(
+ "Failed to create an `SSLContext` using the provided
configuration, falling back to a default instance",
+ error);
+ return createDefaultSslContext(protocol);
Review Comment:
I don't really have a particular preference. Wouldn't providing an empty
trust store here break the system of users configuring only a key store, but
leaving the trust store configuration out? That is, if we provide an empty
trust store by default, such systems won't be able to connect to hosts they
were earlier able to. What do you think?
--
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]
