Extended /certificates endpoint with additional SSL certs info

Enhanced the /certificates endpoint to provide more detailed information. The endpoint now lists all certificates used by each node, with additional properties for each certificate, including: - "format" - "alias" - "serial_number" - "has_private_key" Signed-off-by: Andrey Pleskach <[email protected]>
willyborankin · Nov 14, 2024 · a9d34d4 · a9d34d4
1 parent 4aa7b1c
commit a9d34d4
Show file tree

Hide file tree

Showing 10 changed files with 144 additions and 121 deletions.
diff --git a/src/integrationTest/java/org/opensearch/security/api/CertificatesRestApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/CertificatesRestApiIntegrationTest.java
@@ -14,6 +14,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
 import java.util.StringJoiner;
@@ -24,7 +25,7 @@
 import org.junit.Test;
 
 import org.opensearch.security.dlic.rest.api.Endpoint;
-import org.opensearch.security.dlic.rest.api.ssl.CertificateType;
+import org.opensearch.security.ssl.config.CertType;
 import org.opensearch.test.framework.TestSecurityConfig;
 import org.opensearch.test.framework.certificate.TestCertificates;
 import org.opensearch.test.framework.cluster.LocalOpenSearchCluster;
@@ -107,21 +108,13 @@ private void verifyTimeoutRequest(final TestRestClient client) throws Exception
     }
 
     private void verifySSLCertsInfo(final TestRestClient client) throws Exception {
-        assertSSLCertsInfo(
-            localCluster.nodes(),
-            Set.of(CertificateType.HTTP, CertificateType.TRANSPORT),
-            ok(() -> client.get(sslCertsPath()))
-        );
+        assertSSLCertsInfo(localCluster.nodes(), CertType.TYPES, ok(() -> client.get(sslCertsPath())));
         if (localCluster.nodes().size() > 1) {
             final var randomNodes = randomNodes();
             final var nodeIds = randomNodes.stream().map(n -> n.esNode().getNodeEnvironment().nodeId()).collect(Collectors.joining(","));
-            assertSSLCertsInfo(
-                randomNodes,
-                Set.of(CertificateType.HTTP, CertificateType.TRANSPORT),
-                ok(() -> client.get(sslCertsPath(nodeIds)))
-            );
+            assertSSLCertsInfo(randomNodes, CertType.TYPES, ok(() -> client.get(sslCertsPath(nodeIds))));
         }
-        final var randomCertType = randomFrom(List.of(CertificateType.HTTP, CertificateType.TRANSPORT));
+        final var randomCertType = randomFrom(List.copyOf(CertType.TYPES));
         assertSSLCertsInfo(
             localCluster.nodes(),
             Set.of(randomCertType),
@@ -132,7 +125,7 @@ private void verifySSLCertsInfo(final TestRestClient client) throws Exception {
 
     private void assertSSLCertsInfo(
         final List<LocalOpenSearchCluster.Node> expectedNode,
-        final Set<CertificateType> expectedCertTypes,
+        final Set<String> expectedCertTypes,
         final TestRestClient.HttpResponse response
     ) {
         final var body = response.bodyAsJsonNode();
@@ -152,14 +145,14 @@ private void assertSSLCertsInfo(
             assertThat(prettyStringBody, node.get("name").asText(), is(n.getNodeName()));
             assertThat(prettyStringBody, node.has("certificates"));
             final var certificates = node.get("certificates");
-            if (expectedCertTypes.contains(CertificateType.HTTP)) {
-                final var httpCertificates = certificates.get(CertificateType.HTTP.value());
+            if (expectedCertTypes.contains(CertType.HTTP.name().toUpperCase(Locale.ROOT))) {
+                final var httpCertificates = certificates.get(CertType.HTTP.name().toUpperCase(Locale.ROOT));
                 assertThat(prettyStringBody, httpCertificates.isArray());
                 assertThat(prettyStringBody, httpCertificates.size(), is(1));
                 verifyCertsJson(n.nodeNumber(), httpCertificates.get(0));
             }
-            if (expectedCertTypes.contains(CertificateType.TRANSPORT)) {
-                final var transportCertificates = certificates.get(CertificateType.TRANSPORT.value());
+            if (expectedCertTypes.contains(CertType.TRANSPORT_CLIENT.name().toUpperCase(Locale.ROOT))) {
+                final var transportCertificates = certificates.get(CertType.TRANSPORT.name().toUpperCase(Locale.ROOT));
                 assertThat(prettyStringBody, transportCertificates.isArray());
                 assertThat(prettyStringBody, transportCertificates.size(), is(1));
                 verifyCertsJson(n.nodeNumber(), transportCertificates.get(0));

diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/CertificatesApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/CertificatesApiAction.java
@@ -19,9 +19,9 @@
 
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.core.action.ActionListener;
+import org.opensearch.core.common.Strings;
 import org.opensearch.rest.RestRequest;
 import org.opensearch.rest.action.RestActions;
-import org.opensearch.security.dlic.rest.api.ssl.CertificateType;
 import org.opensearch.security.dlic.rest.api.ssl.CertificatesActionType;
 import org.opensearch.security.dlic.rest.api.ssl.CertificatesInfoNodesRequest;
 import org.opensearch.security.dlic.rest.api.ssl.CertificatesNodesResponse;
@@ -82,11 +82,9 @@ private void securitySSLCertsRequestHandlers(RequestHandler.RequestHandlersBuild
                 RestRequest.Method.GET,
                 (channel, request, client) -> client.execute(
                     CertificatesActionType.INSTANCE,
-                    new CertificatesInfoNodesRequest(
-                        CertificateType.from(request.param("cert_type")),
-                        true,
-                        request.paramAsStringArrayOrEmptyIfAll("nodeId")
-                    ).timeout(request.param("timeout")),
+                    new CertificatesInfoNodesRequest(certType(request), true, request.paramAsStringArrayOrEmptyIfAll("nodeId")).timeout(
+                        request.param("timeout")
+                    ),
                     new ActionListener<>() {
                         @Override
                         public void onResponse(final CertificatesNodesResponse response) {
@@ -110,6 +108,14 @@ public void onFailure(Exception e) {
             );
     }
 
+    private String certType(final RestRequest request) {
+        final var t = request.param("cert_type");
+        if (!Strings.isEmpty(t) && "all".equals(t)) {
+            return null;
+        }
+        return t;
+    }
+
     boolean accessHandler(final RestRequest request) {
         if (request.method() == RestRequest.Method.GET) {
             return securityApiDependencies.restApiAdminPrivilegesEvaluator().isCurrentUserAdminFor(endpoint, CERTS_INFO_ACTION);

diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ssl/CertificateInfo.java b/src/main/java/org/opensearch/security/dlic/rest/api/ssl/CertificateInfo.java
@@ -12,7 +12,6 @@
 package org.opensearch.security.dlic.rest.api.ssl;
 
 import java.io.IOException;
-import java.security.cert.X509Certificate;
 import java.util.Objects;
 
 import org.opensearch.core.common.io.stream.StreamInput;
@@ -33,7 +32,29 @@ public class CertificateInfo implements Writeable, ToXContent {
 
     private final String notBefore;
 
-    public CertificateInfo(String subject, String san, String issuer, String notAfter, String notBefore) {
+    private final String format;
+
+    private final String alias;
+
+    private final boolean hasPrivateKey;
+
+    private final String serialNumber;
+
+    public CertificateInfo(
+        String format,
+        String alias,
+        String serialNumber,
+        boolean hasPrivateKey,
+        String subject,
+        String san,
+        String issuer,
+        String notAfter,
+        String notBefore
+    ) {
+        this.format = format;
+        this.alias = alias;
+        this.serialNumber = serialNumber;
+        this.hasPrivateKey = hasPrivateKey;
         this.subject = subject;
         this.san = san;
         this.issuer = issuer;
@@ -47,6 +68,10 @@ public CertificateInfo(final StreamInput in) throws IOException {
         this.issuer = in.readOptionalString();
         this.notAfter = in.readOptionalString();
         this.notBefore = in.readOptionalString();
+        this.format = in.readOptionalString();
+        this.alias = in.readOptionalString();
+        this.serialNumber = in.readOptionalString();
+        this.hasPrivateKey = in.readBoolean();
     }
 
     @Override
@@ -56,41 +81,27 @@ public void writeTo(final StreamOutput out) throws IOException {
         out.writeOptionalString(issuer);
         out.writeOptionalString(notAfter);
         out.writeOptionalString(notBefore);
+        out.writeOptionalString(format);
+        out.writeOptionalString(alias);
+        out.writeOptionalString(serialNumber);
+        out.writeBoolean(hasPrivateKey);
     }
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException {
         return builder.startObject()
+            .field("format", format)
+            .field("alias", alias)
             .field("subject_dn", subject)
             .field("san", san)
+            .field("serial_number", serialNumber)
             .field("issuer_dn", issuer)
+            .field("has_private_key", hasPrivateKey)
             .field("not_after", notAfter)
             .field("not_before", notAfter)
             .endObject();
     }
 
-    public static CertificateInfo from(final X509Certificate x509Certificate, final String subjectAlternativeNames) {
-        String subject = null;
-        String issuer = null;
-        String notAfter = null;
-        String notBefore = null;
-        if (x509Certificate != null) {
-            if (x509Certificate.getSubjectX500Principal() != null) {
-                subject = x509Certificate.getSubjectX500Principal().getName();
-            }
-            if (x509Certificate.getIssuerX500Principal() != null) {
-                issuer = x509Certificate.getIssuerX500Principal().getName();
-            }
-            if (x509Certificate.getNotAfter() != null) {
-                notAfter = x509Certificate.getNotAfter().toInstant().toString();
-            }
-            if (x509Certificate.getNotBefore() != null) {
-                notBefore = x509Certificate.getNotBefore().toInstant().toString();
-            }
-        }
-        return new CertificateInfo(subject, subjectAlternativeNames, issuer, notAfter, notBefore);
-    }
-
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;

diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ssl/CertificateType.java b/src/main/java/org/opensearch/security/dlic/rest/api/ssl/CertificateType.java
diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ssl/CertificatesInfo.java b/src/main/java/org/opensearch/security/dlic/rest/api/ssl/CertificatesInfo.java
@@ -13,24 +13,26 @@
 
 import java.io.IOException;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 
 import org.opensearch.core.common.io.stream.StreamInput;
 import org.opensearch.core.common.io.stream.StreamOutput;
 import org.opensearch.core.common.io.stream.Writeable;
 import org.opensearch.core.xcontent.ToXContent;
 import org.opensearch.core.xcontent.XContentBuilder;
+import org.opensearch.security.ssl.config.CertType;
 
 public class CertificatesInfo implements Writeable, ToXContent {
 
-    private final Map<CertificateType, List<CertificateInfo>> certificates;
+    private final Map<CertType, List<CertificateInfo>> certificates;
 
-    public CertificatesInfo(final Map<CertificateType, List<CertificateInfo>> certificates) {
+    public CertificatesInfo(final Map<CertType, List<CertificateInfo>> certificates) {
         this.certificates = certificates;
     }
 
     public CertificatesInfo(final StreamInput in) throws IOException {
-        certificates = in.readMap(keyIn -> keyIn.readEnum(CertificateType.class), listIn -> listIn.readList(CertificateInfo::new));
+        certificates = in.readMap(keyIn -> keyIn.readEnum(CertType.class), listIn -> listIn.readList(CertificateInfo::new));
     }
 
     @Override
@@ -41,8 +43,9 @@ public void writeTo(StreamOutput out) throws IOException {
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         return builder.startObject("certificates")
-            .field(CertificateType.HTTP.value(), certificates.get(CertificateType.HTTP))
-            .field(CertificateType.TRANSPORT.value(), certificates.get(CertificateType.TRANSPORT))
+            .field(CertType.HTTP.name().toLowerCase(Locale.ROOT), certificates.get(CertType.HTTP))
+            .field(CertType.TRANSPORT.name().toLowerCase(Locale.ROOT), certificates.get(CertType.TRANSPORT))
+            .field(CertType.TRANSPORT_CLIENT.name().toLowerCase(Locale.ROOT), certificates.get(CertType.TRANSPORT_CLIENT))
             .endObject();
     }
 }
diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ssl/CertificatesInfoNodesRequest.java b/src/main/java/org/opensearch/security/dlic/rest/api/ssl/CertificatesInfoNodesRequest.java
@@ -12,31 +12,35 @@
 package org.opensearch.security.dlic.rest.api.ssl;
 
 import java.io.IOException;
+import java.util.Optional;
 
+import org.opensearch.action.ActionRequestValidationException;
 import org.opensearch.action.support.nodes.BaseNodesRequest;
+import org.opensearch.core.common.Strings;
 import org.opensearch.core.common.io.stream.StreamInput;
 import org.opensearch.core.common.io.stream.StreamOutput;
+import org.opensearch.security.ssl.config.CertType;
 
 public class CertificatesInfoNodesRequest extends BaseNodesRequest<CertificatesInfoNodesRequest> {
 
-    private final CertificateType certificateType;
+    private final String certificateType;
 
     private final boolean inMemory;
 
-    public CertificatesInfoNodesRequest(CertificateType certificateType, boolean inMemory, String... nodesIds) {
+    public CertificatesInfoNodesRequest(String certificateType, boolean inMemory, String... nodesIds) {
         super(nodesIds);
         this.certificateType = certificateType;
         this.inMemory = inMemory;
     }
 
     public CertificatesInfoNodesRequest(final StreamInput in) throws IOException {
         super(in);
-        certificateType = in.readEnum(CertificateType.class);
+        certificateType = in.readOptionalString();
         inMemory = in.readBoolean();
     }
 
-    public CertificateType certificateType() {
-        return certificateType;
+    public Optional<String> certificateType() {
+        return Optional.ofNullable(certificateType);
     }
 
     public boolean inMemory() {
@@ -46,7 +50,17 @@ public boolean inMemory() {
     @Override
     public void writeTo(final StreamOutput out) throws IOException {
         super.writeTo(out);
-        out.writeEnum(certificateType);
+        out.writeOptionalString(certificateType);
         out.writeBoolean(inMemory);
     }
+
+    @Override
+    public ActionRequestValidationException validate() {
+        if (!Strings.isEmpty(certificateType) && !CertType.TYPES.contains(certificateType)) {
+            final var errorMessage = new ActionRequestValidationException();
+            errorMessage.addValidationError("wrong certificate type " + certificateType + ". Please use one of " + CertType.TYPES);
+            return errorMessage;
+        }
+        return super.validate();
+    }
 }