[Backport 2.x] Refactor SSL Configuration (#4671)

build.gradle

@@ -691,6 +691,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">

src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java

@@ -646,7 +646,7 @@ public List<RestHandler> getRestHandlers(
                         threadPool,
                         Objects.requireNonNull(auditLog),
                         Objects.requireNonNull(userService),
-                        sks,
+                        sslSettingsManager,
                         sslCertReloadEnabled,
                         passwordHasher
                     )
@@ -1205,9 +1205,8 @@ public Collection<Object> createComponents(
         components.add(userService);
         components.add(passwordHasher);
 
-        if (!ExternalSecurityKeyStore.hasExternalSslContext(settings)) {
-            components.add(sks);
-        }
+        components.add(sslSettingsManager);
+
         final var allowDefaultInit = settings.getAsBoolean(SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX, false);
         final var useClusterState = useClusterStateToInitSecurityConfig(settings);
         if (!SSLConfig.isSslOnlyMode() && !isDisabled(settings) && allowDefaultInit && useClusterState) {
@@ -2165,7 +2164,13 @@ public PluginSubject getPluginSubject(Plugin plugin) {
     @Override
     public Optional<SecureSettingsFactory> getSecureSettingFactory(Settings settings) {
         return Optional.of(
-            new OpenSearchSecureSettingsFactory(threadPool, sks, evaluateSslExceptionHandler(), securityRestHandler, SSLConfig)
+            new OpenSearchSecureSettingsFactory(
+                threadPool,
+                sslSettingsManager,
+                evaluateSslExceptionHandler(),
+                securityRestHandler,
+                SSLConfig
+            )
         );
     }
 

src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java

@@ -25,7 +25,7 @@
 import org.opensearch.security.configuration.ConfigurationRepository;
 import org.opensearch.security.hasher.PasswordHasher;
 import org.opensearch.security.privileges.PrivilegesEvaluator;
-import org.opensearch.security.ssl.SecurityKeyStore;
+import org.opensearch.security.ssl.SslSettingsManager;
 import org.opensearch.security.ssl.transport.PrincipalExtractor;
 import org.opensearch.security.user.UserService;
 import org.opensearch.threadpool.ThreadPool;
@@ -47,7 +47,7 @@ public static Collection<RestHandler> getHandler(
         final ThreadPool threadPool,
         final AuditLog auditLog,
         final UserService userService,
-        final SecurityKeyStore securityKeyStore,
+        final SslSettingsManager sslSettingsManager,
         final boolean certificatesReloadEnabled,
         final PasswordHasher passwordHasher
     ) {
@@ -98,7 +98,13 @@ public static Collection<RestHandler> getHandler(
             new AuditApiAction(clusterService, threadPool, securityApiDependencies),
             new MultiTenancyConfigApiAction(clusterService, threadPool, securityApiDependencies),
             new ConfigUpgradeApiAction(clusterService, threadPool, securityApiDependencies),
-            new SecuritySSLCertsApiAction(clusterService, threadPool, securityKeyStore, certificatesReloadEnabled, securityApiDependencies),
+            new SecuritySSLCertsApiAction(
+                clusterService,
+                threadPool,
+                sslSettingsManager,
+                certificatesReloadEnabled,
+                securityApiDependencies
+            ),
             new CertificatesApiAction(clusterService, threadPool, securityApiDependencies)
         );
     }

src/main/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsApiAction.java

@@ -12,11 +12,10 @@
 package org.opensearch.security.dlic.rest.api;
 
 import java.io.IOException;
-import java.security.cert.X509Certificate;
-import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -31,8 +30,10 @@
 import org.opensearch.rest.RestRequest.Method;
 import org.opensearch.security.dlic.rest.validation.ValidationResult;
 import org.opensearch.security.securityconf.impl.CType;
-import org.opensearch.security.ssl.SecurityKeyStore;
-import org.opensearch.security.ssl.util.SSLConfigConstants;
+import org.opensearch.security.ssl.SslContextHandler;
+import org.opensearch.security.ssl.SslSettingsManager;
+import org.opensearch.security.ssl.config.CertType;
+import org.opensearch.security.ssl.config.Certificate;
 import org.opensearch.security.support.ConfigConstants;
 import org.opensearch.threadpool.ThreadPool;
 
@@ -62,23 +63,20 @@ public class SecuritySSLCertsApiAction extends AbstractApiAction {
         )
     );
 
-    private final SecurityKeyStore securityKeyStore;
+    private final SslSettingsManager sslSettingsManager;
 
     private final boolean certificatesReloadEnabled;
 
-    private final boolean httpsEnabled;
-
     public SecuritySSLCertsApiAction(
         final ClusterService clusterService,
         final ThreadPool threadPool,
-        final SecurityKeyStore securityKeyStore,
+        final SslSettingsManager sslSettingsManager,
         final boolean certificatesReloadEnabled,
         final SecurityApiDependencies securityApiDependencies
     ) {
         super(Endpoint.SSL, clusterService, threadPool, securityApiDependencies);
-        this.securityKeyStore = securityKeyStore;
+        this.sslSettingsManager = sslSettingsManager;
         this.certificatesReloadEnabled = certificatesReloadEnabled;
-        this.httpsEnabled = securityApiDependencies.settings().getAsBoolean(SSLConfigConstants.SECURITY_SSL_HTTP_ENABLED, true);
         this.requestHandlersBuilder.configureRequestHandlers(this::securitySSLCertsRequestHandlers);
     }
 
@@ -108,10 +106,10 @@ private void securitySSLCertsRequestHandlers(RequestHandler.RequestHandlersBuild
             .verifyAccessForAllMethods()
             .override(
                 Method.GET,
-                (channel, request, client) -> withSecurityKeyStore().valid(keyStore -> loadCertificates(channel, keyStore))
+                (channel, request, client) -> withSecurityKeyStore().valid(ignore -> loadCertificates(channel))
                     .error((status, toXContent) -> response(channel, status, toXContent))
             )
-            .override(Method.PUT, (channel, request, client) -> withSecurityKeyStore().valid(keyStore -> {
+            .override(Method.PUT, (channel, request, client) -> withSecurityKeyStore().valid(ignore -> {
                 if (!certificatesReloadEnabled) {
                     badRequest(
                         channel,
@@ -123,7 +121,7 @@ private void securitySSLCertsRequestHandlers(RequestHandler.RequestHandlersBuild
                         )
                     );
                 } else {
-                    reloadCertificates(channel, request, keyStore);
+                    reloadCertificates(channel, request);
                 }
             }).error((status, toXContent) -> response(channel, status, toXContent)));
     }
@@ -138,65 +136,70 @@ boolean accessHandler(final RestRequest request) {
         }
     }
 
-    ValidationResult<SecurityKeyStore> withSecurityKeyStore() {
-        if (securityKeyStore == null) {
+    ValidationResult<SslSettingsManager> withSecurityKeyStore() {
+        if (sslSettingsManager == null) {
             return ValidationResult.error(RestStatus.OK, badRequestMessage("keystore is not initialized"));
         }
-        return ValidationResult.success(securityKeyStore);
+        return ValidationResult.success(sslSettingsManager);
     }
 
-    protected void loadCertificates(final RestChannel channel, final SecurityKeyStore keyStore) throws IOException {
+    protected void loadCertificates(final RestChannel channel) throws IOException {
         ok(
             channel,
             (builder, params) -> builder.startObject()
-                .field("http_certificates_list", httpsEnabled ? generateCertDetailList(keyStore.getHttpCerts()) : null)
-                .field("transport_certificates_list", generateCertDetailList(keyStore.getTransportCerts()))
+                .field(
+                    "http_certificates_list",
+                    generateCertDetailList(
+                        sslSettingsManager.sslContextHandler(CertType.HTTP).map(SslContextHandler::keyMaterialCertificates).orElse(null)
+                    )
+                )
+                .field(
+                    "transport_certificates_list",
+                    generateCertDetailList(
+                        sslSettingsManager.sslContextHandler(CertType.TRANSPORT)
+                            .map(SslContextHandler::keyMaterialCertificates)
+                            .orElse(null)
+                    )
+                )
                 .endObject()
         );
     }
 
-    private List<Map<String, String>> generateCertDetailList(final X509Certificate[] certs) {
+    private List<Map<String, String>> generateCertDetailList(final Stream<Certificate> certs) {
         if (certs == null) {
             return null;
         }
-        return Arrays.stream(certs).map(cert -> {
-            final String issuerDn = cert != null && cert.getIssuerX500Principal() != null ? cert.getIssuerX500Principal().getName() : "";
-            final String subjectDn = cert != null && cert.getSubjectX500Principal() != null ? cert.getSubjectX500Principal().getName() : "";
-
-            final String san = securityKeyStore.getSubjectAlternativeNames(cert);
-
-            final String notBefore = cert != null && cert.getNotBefore() != null ? cert.getNotBefore().toInstant().toString() : "";
-            final String notAfter = cert != null && cert.getNotAfter() != null ? cert.getNotAfter().toInstant().toString() : "";
-            return ImmutableMap.of(
+        return certs.map(
+            c -> ImmutableMap.of(
                 "issuer_dn",
-                issuerDn,
+                c.issuer(),
                 "subject_dn",
-                subjectDn,
+                c.subject(),
                 "san",
-                san,
+                c.subjectAlternativeNames(),
                 "not_before",
-                notBefore,
+                c.notBefore(),
                 "not_after",
-                notAfter
-            );
-        }).collect(Collectors.toList());
+                c.notAfter()
+            )
+        ).collect(Collectors.toList());
     }
 
-    protected void reloadCertificates(final RestChannel channel, final RestRequest request, final SecurityKeyStore keyStore)
-        throws IOException {
+    protected void reloadCertificates(final RestChannel channel, final RestRequest request) throws IOException {
         final String certType = request.param("certType").toLowerCase().trim();
         try {
             switch (certType) {
                 case "http":
-                    if (!httpsEnabled) {
+                    if (sslSettingsManager.sslConfiguration(CertType.HTTP).isPresent()) {
+                        sslSettingsManager.reloadSslContext(CertType.HTTP);
+                        ok(channel, (builder, params) -> builder.startObject().field("message", "updated http certs").endObject());
+                    } else {
                         badRequest(channel, "SSL for HTTP is disabled");
-                        return;
                     }
-                    keyStore.initHttpSSLConfig();
-                    ok(channel, (builder, params) -> builder.startObject().field("message", "updated http certs").endObject());
                     break;
                 case "transport":
-                    keyStore.initTransportSSLConfig();
+                    sslSettingsManager.reloadSslContext(CertType.TRANSPORT);
+                    sslSettingsManager.reloadSslContext(CertType.TRANSPORT_CLIENT);
                     ok(channel, (builder, params) -> builder.startObject().field("message", "updated transport certs").endObject());
                     break;
                 default:

src/main/java/org/opensearch/security/dlic/rest/api/ssl/TransportCertificatesInfoNodesAction.java

@@ -12,23 +12,23 @@
 package org.opensearch.security.dlic.rest.api.ssl;
 
 import java.io.IOException;
-import java.security.cert.X509Certificate;
 import java.util.List;
 import java.util.Map;
-
-import com.google.common.collect.ImmutableList;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import org.opensearch.action.FailedNodeException;
 import org.opensearch.action.support.ActionFilters;
 import org.opensearch.action.support.nodes.BaseNodeRequest;
 import org.opensearch.action.support.nodes.TransportNodesAction;
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.inject.Inject;
-import org.opensearch.common.settings.Settings;
 import org.opensearch.core.common.io.stream.StreamInput;
 import org.opensearch.core.common.io.stream.StreamOutput;
-import org.opensearch.security.ssl.DefaultSecurityKeyStore;
-import org.opensearch.security.ssl.util.SSLConfigConstants;
+import org.opensearch.security.ssl.SslContextHandler;
+import org.opensearch.security.ssl.SslSettingsManager;
+import org.opensearch.security.ssl.config.CertType;
+import org.opensearch.security.ssl.config.Certificate;
 import org.opensearch.threadpool.ThreadPool;
 import org.opensearch.transport.TransportService;
 
@@ -38,18 +38,15 @@
     TransportCertificatesInfoNodesAction.NodeRequest,
     CertificatesNodesResponse.CertificatesNodeResponse> {
 
-    private final DefaultSecurityKeyStore securityKeyStore;
-
-    private final boolean httpsEnabled;
+    private final SslSettingsManager sslSettingsManager;
 
     @Inject
     public TransportCertificatesInfoNodesAction(
-        final Settings settings,
         final ThreadPool threadPool,
         final ClusterService clusterService,
         final TransportService transportService,
         final ActionFilters actionFilters,
-        final DefaultSecurityKeyStore securityKeyStore
+        final SslSettingsManager sslSettingsManager
     ) {
         super(
             CertificatesActionType.NAME,
@@ -62,8 +59,7 @@
             ThreadPool.Names.GENERIC,
             CertificatesNodesResponse.CertificatesNodeResponse.class
         );
-        this.httpsEnabled = settings.getAsBoolean(SSLConfigConstants.SECURITY_SSL_HTTP_ENABLED, true);
-        this.securityKeyStore = securityKeyStore;
+        this.sslSettingsManager = sslSettingsManager;
     }
 
     @Override
@@ -89,12 +85,6 @@
     protected CertificatesNodesResponse.CertificatesNodeResponse nodeOperation(final NodeRequest request) {
         final var sslCertRequest = request.sslCertsInfoNodesRequest;
 
-        if (securityKeyStore == null) {
-            return new CertificatesNodesResponse.CertificatesNodeResponse(
-                clusterService.localNode(),
-                new IllegalStateException("keystore is not initialized")
-            );
-        }
         try {
             return new CertificatesNodesResponse.CertificatesNodeResponse(
                 clusterService.localNode(),
@@ -109,23 +99,27 @@
         var httpCertificates = List.<CertificateInfo>of();
         var transportsCertificates = List.<CertificateInfo>of();
         if (CertificateType.isHttp(certificateType)) {
-            httpCertificates = httpsEnabled ? certificatesDetails(securityKeyStore.getHttpCerts()) : List.of();
+            httpCertificates = sslSettingsManager.sslContextHandler(CertType.HTTP)
+                .map(SslContextHandler::keyMaterialCertificates)
+                .map(this::certificatesDetails)
+                .orElse(List.of());
         }
         if (CertificateType.isTransport(certificateType)) {
-            transportsCertificates = certificatesDetails(securityKeyStore.getTransportCerts());
+            transportsCertificates = sslSettingsManager.sslContextHandler(CertType.TRANSPORT)
+                .map(SslContextHandler::keyMaterialCertificates)
+                .map(this::certificatesDetails)
+                .orElse(List.of());
         }
         return new CertificatesInfo(Map.of(CertificateType.HTTP, httpCertificates, CertificateType.TRANSPORT, transportsCertificates));
     }
 
-    private List<CertificateInfo> certificatesDetails(final X509Certificate[] certs) {
-        if (certs == null) {
+    private List<CertificateInfo> certificatesDetails(final Stream<Certificate> certificateStream) {
+        if (certificateStream == null) {
             return null;
         }
-        final var certificates = ImmutableList.<CertificateInfo>builder();
-        for (final var c : certs) {
-            certificates.add(CertificateInfo.from(c, securityKeyStore.getSubjectAlternativeNames(c)));
-        }
-        return certificates.build();
+        return certificateStream.map(
+            c -> new CertificateInfo(c.subject(), c.subjectAlternativeNames(), c.issuer(), c.notAfter(), c.notBefore())
+        ).collect(Collectors.toList());
     }
 
     public static class NodeRequest extends BaseNodeRequest {