ppkarwasz commented on code in PR #2767:
URL: https://github.com/apache/logging-log4j2/pull/2767#discussion_r1746966262
##########
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:
Not sure if this is the correct default.
The default `SSLContext` trusts more than a 100 public Certification
Authorities. I always have a problem with such a trust store. Maybe the "empty"
context is a better fallback? It will certainly cause a failure that the user
will detect.
--
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]
