Refactor DefaultKeyStore

Changes:

- Refactored DefaultKeyStore into specialized
  subclasses,
  each managing a distinct responsibility.

- Added missing tests for certificate loading,
  SSL parameter configuration,
  and related processes.

Signed-off-by: Andrey Pleskach <ples@aiven.io>

build.gradle

@@ -684,6 +684,9 @@ dependencies {
     testImplementation('org.awaitility:awaitility:4.2.2') {
         exclude(group: 'org.hamcrest', module: 'hamcrest')
     }
+    testImplementation "org.bouncycastle:bcpkix-jdk18on:${versions.bouncycastle}"
+    testImplementation "org.bouncycastle:bcutil-jdk18on:${versions.bouncycastle}"
+
     // Only osx-x86_64, osx-aarch_64, linux-x86_64, linux-aarch_64, windows-x86_64 are available
     if (osdetector.classifier in ["osx-x86_64", "osx-aarch_64", "linux-x86_64", "linux-aarch_64", "windows-x86_64"]) {
         testImplementation "io.netty:netty-tcnative-classes:2.0.61.Final"

checkstyle/checkstyle.xml

@@ -43,6 +43,13 @@
   <module name="BeforeExecutionExclusionFileFilter">
     <property name="fileNamePattern" value="src/main/java/com/amazon/dlic/auth/http/kerberos/HTTPSpnegoAuthenticator.java"/>
   </module>
+  <module name="BeforeExecutionExclusionFileFilter">
+    <property name="fileNamePattern" value="src/test/java/org/opensearch/security/ssl/SslContextHandlerTest.java"/>
+  </module>
+  <module name="BeforeExecutionExclusionFileFilter">
+    <property name="fileNamePattern" value="src/test/java/org/opensearch/security/ssl/CertificatesRule.java"/>
+  </module>
+
 
   <!-- https://checkstyle.org/config_filters.html#SuppressionFilter -->
   <module name="SuppressionFilter">
@@ -221,7 +228,7 @@
     <property name="severity" value="error"/>
   </module>
 
-  <module name="RegexpSingleline">
+      <module name="RegexpSingleline">
     <property name="format" value="extension"/>
     <property name="ignoreCase" value="true"/>
     <property name="message" value="Extension should only be used sparingly to keep implementations as generic as possible" />

src/main/java/org/opensearch/security/ssl/SecureSSLSettings.java

@@ -36,7 +36,7 @@
 public final class SecureSSLSettings {
     private static final Logger LOG = LogManager.getLogger(SecureSSLSettings.class);
 
-    private static final String SECURE_SUFFIX = "_secure";
+    public static final String SECURE_SUFFIX = "_secure";
     private static final String PREFIX = "plugins.security.ssl";
     private static final String HTTP_PREFIX = PREFIX + ".http";
     private static final String TRANSPORT_PREFIX = PREFIX + ".transport";

src/main/java/org/opensearch/security/ssl/SslConfiguration.java

@@ -0,0 +1,144 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ *
+ * Modifications Copyright OpenSearch Contributors. See
+ * GitHub history for details.
+ */
+
+package org.opensearch.security.ssl;
+
+import java.nio.file.Path;
+import java.security.AccessController;
+import java.security.PrivilegedActionException;
+import java.security.PrivilegedExceptionAction;
+import java.util.List;
+import java.util.Objects;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import org.opensearch.OpenSearchException;
+import org.opensearch.security.ssl.config.Certificate;
+import org.opensearch.security.ssl.config.KeyStoreConfiguration;
+import org.opensearch.security.ssl.config.SslParameters;
+import org.opensearch.security.ssl.config.TrustStoreConfiguration;
+
+import io.netty.handler.codec.http2.Http2SecurityUtil;
+import io.netty.handler.ssl.ApplicationProtocolConfig;
+import io.netty.handler.ssl.ApplicationProtocolNames;
+import io.netty.handler.ssl.SslContext;
+import io.netty.handler.ssl.SslContextBuilder;
+import io.netty.handler.ssl.SupportedCipherSuiteFilter;
+
+public class SslConfiguration {
+
+    private final static Logger LOGGER = LogManager.getLogger(SslConfiguration.class);
+
+    private final SslParameters sslParameters;
+
+    private final TrustStoreConfiguration trustStoreConfiguration;
+
+    private final KeyStoreConfiguration keyStoreConfiguration;
+
+    public SslConfiguration(
+        final SslParameters sslParameters,
+        final TrustStoreConfiguration trustStoreConfiguration,
+        final KeyStoreConfiguration keyStoreConfiguration
+    ) {
+        this.sslParameters = sslParameters;
+        this.trustStoreConfiguration = trustStoreConfiguration;
+        this.keyStoreConfiguration = keyStoreConfiguration;
+    }
+
+    public List<Path> dependentFiles() {
+        return Stream.concat(keyStoreConfiguration.files().stream(), Stream.of(trustStoreConfiguration.file()))
+            .collect(Collectors.toList());
+    }
+
+    public List<Certificate> certificates() {
+        return Stream.concat(trustStoreConfiguration.certificates().stream(), keyStoreConfiguration.certificates().stream())
+            .collect(Collectors.toList());
+    }
+
+    public SslParameters sslParameters() {
+        return sslParameters;
+    }
+
+    @SuppressWarnings("removal")
+    SslContext buildServerSslContext() {
+        try {
+            return AccessController.doPrivileged(
+                (PrivilegedExceptionAction<SslContext>) () -> SslContextBuilder.forServer(keyStoreConfiguration.createKeyManagerFactory())
+                    .sslProvider(sslParameters.provider())
+                    .clientAuth(sslParameters.clientAuth())
+                    .protocols(sslParameters.allowedProtocols().toArray(new String[0]))
+                    // TODO we always add all HTTP 2 ciphers, while maybe it is better to set them differently
+                    .ciphers(
+                        Stream.concat(Http2SecurityUtil.CIPHERS.stream(), sslParameters.allowedCiphers().stream())
+                            .sorted(String::compareTo)
+                            .collect(Collectors.toList()),
+                        SupportedCipherSuiteFilter.INSTANCE
+                    )
+                    .sessionCacheSize(0)
+                    .sessionTimeout(0)
+                    .applicationProtocolConfig(
+                        new ApplicationProtocolConfig(
+                            ApplicationProtocolConfig.Protocol.ALPN,
+                            // NO_ADVERTISE is currently the only mode supported by both OpenSsl and JDK providers.
+                            ApplicationProtocolConfig.SelectorFailureBehavior.NO_ADVERTISE,
+                            // ACCEPT is currently the only mode supported by both OpenSsl and JDK providers.
+                            ApplicationProtocolConfig.SelectedListenerFailureBehavior.ACCEPT,
+                            ApplicationProtocolNames.HTTP_2,
+                            ApplicationProtocolNames.HTTP_1_1
+                        )
+                    )
+                    .trustManager(trustStoreConfiguration.createTrustManagerFactory())
+                    .build()
+            );
+        } catch (PrivilegedActionException e) {
+            throw new OpenSearchException("Filed to build server SSL context", e);
+        }
+    }
+
+    @SuppressWarnings("removal")
+    SslContext buildClientSslContext() {
+        try {
+            return AccessController.doPrivileged(
+                (PrivilegedExceptionAction<SslContext>) () -> SslContextBuilder.forClient()
+                    .sslProvider(sslParameters.provider())
+                    .protocols(sslParameters.allowedProtocols())
+                    .ciphers(sslParameters.allowedCiphers())
+                    .applicationProtocolConfig(ApplicationProtocolConfig.DISABLED)
+                    .sessionCacheSize(0)
+                    .sessionTimeout(0)
+                    .sslProvider(sslParameters.provider())
+                    .keyManager(keyStoreConfiguration.createKeyManagerFactory())
+                    .trustManager(trustStoreConfiguration.createTrustManagerFactory())
+                    .build()
+            );
+        } catch (PrivilegedActionException e) {
+            throw new OpenSearchException("Filed to build client SSL context", e);
+        }
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        SslConfiguration that = (SslConfiguration) o;
+        return Objects.equals(sslParameters, that.sslParameters)
+            && Objects.equals(trustStoreConfiguration, that.trustStoreConfiguration)
+            && Objects.equals(keyStoreConfiguration, that.keyStoreConfiguration);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(sslParameters, trustStoreConfiguration, keyStoreConfiguration);
+    }
+}

src/main/java/org/opensearch/security/ssl/SslContextHandler.java

@@ -0,0 +1,181 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ *
+ * Modifications Copyright OpenSearch Contributors. See
+ * GitHub history for details.
+ */
+
+package org.opensearch.security.ssl;
+
+import java.nio.charset.StandardCharsets;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.util.Date;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import javax.net.ssl.SSLEngine;
+
+import org.opensearch.security.ssl.config.Certificate;
+import org.opensearch.transport.NettyAllocator;
+
+import io.netty.handler.ssl.SslContext;
+
+public class SslContextHandler {
+
+    private SslContext sslContext;
+
+    private final SslConfiguration sslConfiguration;
+
+    private final List<Certificate> loadedCertificates;
+
+    public SslContextHandler(final SslConfiguration sslConfiguration) {
+        this(sslConfiguration, false);
+    }
+
+    public SslContextHandler(final SslConfiguration sslConfiguration, final boolean client) {
+        this.sslContext = client ? sslConfiguration.buildClientSslContext() : sslConfiguration.buildServerSslContext();
+        this.sslConfiguration = sslConfiguration;
+        this.loadedCertificates = sslConfiguration.certificates();
+    }
+
+    public SSLEngine createSSLEngine() {
+        return sslContext.newEngine(NettyAllocator.getAllocator());
+    }
+
+    public SSLEngine createSSLEngine(final String hostname, final int port) {
+        return sslContext.newEngine(NettyAllocator.getAllocator(), hostname, port);
+    }
+
+    public SslConfiguration sslConfiguration() {
+        return sslConfiguration;
+    }
+
+    SslContext sslContext() {
+        return sslContext;
+    }
+
+    Stream<Certificate> keyMaterialCertificates() {
+        return keyMaterialCertificates(loadedCertificates);
+    }
+
+    Stream<Certificate> keyMaterialCertificates(final List<Certificate> certificates) {
+        return certificates.stream().filter(Certificate::hasKey);
+    }
+
+    void reloadSslContext() throws CertificateException {
+        final var newCertificates = sslConfiguration.certificates();
+        validateNewCertificates(newCertificates);
+        invalidateSessions();
+        if (sslContext.isClient()) {
+            sslContext = sslConfiguration.buildClientSslContext();
+        } else {
+            sslContext = sslConfiguration.buildServerSslContext();
+        }
+        loadedCertificates.clear();
+        loadedCertificates.addAll(newCertificates);
+    }
+
+    private boolean sameCertificates(final List<Certificate> newCertificates) {
+        final Set<String> currentCertSignatureSet = keyMaterialCertificates().map(Certificate::x509Certificate)
+            .map(X509Certificate::getSignature)
+            .map(s -> new String(s, StandardCharsets.UTF_8))
+            .collect(Collectors.toSet());
+        final Set<String> newCertSignatureSet = keyMaterialCertificates(newCertificates).map(Certificate::x509Certificate)
+            .map(X509Certificate::getSignature)
+            .map(s -> new String(s, StandardCharsets.UTF_8))
+            .collect(Collectors.toSet());
+        return currentCertSignatureSet.equals(newCertSignatureSet);
+    }
+
+    private boolean hasValidExpiryDates(final List<Certificate> newCertificates) {
+        // Get the earliest expiry date for current certificates
+        final Date earliestExpiryDate = keyMaterialCertificates().map(Certificate::x509Certificate)
+            .map(X509Certificate::getNotAfter)
+            .min(Date::compareTo)
+            .get();
+        // New certificates that expire before or on the same date as the current ones are invalid.
+        boolean newCertsExpireBeforeCurrentCerts = keyMaterialCertificates(newCertificates).map(Certificate::x509Certificate)
+            .anyMatch(c -> {
+                Date notAfterDate = c.getNotAfter();
+                return notAfterDate.before(earliestExpiryDate) || notAfterDate.equals(earliestExpiryDate);
+            });
+
+        return !newCertsExpireBeforeCurrentCerts;
+    }
+
+    private void validateSubjectDns(final List<Certificate> newCertificates) throws CertificateException {
+        final List<String> currentSubjectDNs = keyMaterialCertificates().map(Certificate::subject).sorted().collect(Collectors.toList());
+        final List<String> newSubjectDNs = keyMaterialCertificates(newCertificates).map(Certificate::subject)
+            .sorted()
+            .collect(Collectors.toList());
+        if (!currentSubjectDNs.equals(newSubjectDNs)) {
+            throw new CertificateException(
+                "New certificates do not have valid Subject DNs. Current Subject DNs "
+                    + currentSubjectDNs
+                    + " new Subject DNs "
+                    + newSubjectDNs
+            );
+        }
+    }
+
+    private void validateIssuerDns(final List<Certificate> newCertificates) throws CertificateException {
+        final List<String> currentIssuerDNs = keyMaterialCertificates().map(Certificate::issuer).sorted().collect(Collectors.toList());
+        final List<String> newIssuerDNs = keyMaterialCertificates(newCertificates).map(Certificate::issuer)
+            .sorted()
+            .collect(Collectors.toList());
+        if (!currentIssuerDNs.equals(newIssuerDNs)) {
+            throw new CertificateException(
+                "New certificates do not have valid Issuer DNs. Current Issuer DNs: "
+                    + currentIssuerDNs
+                    + " new Issuer DNs: "
+                    + newIssuerDNs
+            );
+        }
+    }
+
+    private void validateSans(final List<Certificate> newCertificates) throws CertificateException {
+        final List<String> currentSans = keyMaterialCertificates().map(Certificate::subjectAlternativeNames)
+            .sorted()
+            .collect(Collectors.toList());
+        final List<String> newSans = keyMaterialCertificates(newCertificates).map(Certificate::subjectAlternativeNames)
+            .sorted()
+            .collect(Collectors.toList());
+        if (!currentSans.equals(newSans)) {
+            throw new CertificateException(
+                "New certificates do not have valid SANs. Current SANs: " + currentSans + " new SANs: " + newSans
+            );
+        }
+    }
+
+    private void validateNewCertificates(final List<Certificate> newCertificates) throws CertificateException {
+        if (sameCertificates(newCertificates)) {
+            throw new CertificateException("Existing certificates are the same as new one");
+        }
+        if (!hasValidExpiryDates(newCertificates)) {
+            throw new CertificateException("New certificates should not expire before the current ones");
+        }
+        validateSubjectDns(newCertificates);
+        validateIssuerDns(newCertificates);
+        validateSans(newCertificates);
+    }
+
+    private void invalidateSessions() {
+        final var sessionContext = sslContext.sessionContext();
+        if (sessionContext != null) {
+            final var sessionsIds = sessionContext.getIds();
+            while (sessionsIds.hasMoreElements()) {
+                final var session = sessionContext.getSession(sessionsIds.nextElement());
+                if (session != null) {
+                    session.invalidate();
+                }
+            }
+        }
+    }
+
+}