From bab21d12892a08ac86d256545cbf853e68e327db Mon Sep 17 00:00:00 2001 From: Andrey Pleskach Date: Thu, 7 Nov 2024 17:53:56 +0100 Subject: [PATCH] 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 --- .../CertificatesRestApiIntegrationTest.java | 27 ++++----- .../dlic/rest/api/CertificatesApiAction.java | 18 ++++-- .../dlic/rest/api/ssl/CertificateInfo.java | 59 +++++++++++-------- .../dlic/rest/api/ssl/CertificateType.java | 48 --------------- .../dlic/rest/api/ssl/CertificatesInfo.java | 13 ++-- .../api/ssl/CertificatesInfoNodesRequest.java | 26 ++++++-- .../TransportCertificatesInfoNodesAction.java | 50 ++++++++++++---- .../security/ssl/SslContextHandler.java | 9 ++- .../security/ssl/config/CertType.java | 9 +++ .../security/ssl/config/Certificate.java | 6 +- 10 files changed, 144 insertions(+), 121 deletions(-) delete mode 100644 src/main/java/org/opensearch/security/dlic/rest/api/ssl/CertificateType.java diff --git a/src/integrationTest/java/org/opensearch/security/api/CertificatesRestApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/CertificatesRestApiIntegrationTest.java index 43ba0ce807..175eb109e8 100644 --- 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 expectedNode, - final Set expectedCertTypes, + final Set 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 index 1a769a9b71..61f1695b21 100644 --- 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 index ce757286e3..cae9764ae6 100644 --- 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; @@ -42,6 +63,10 @@ public CertificateInfo(String subject, String san, String issuer, String notAfte } public CertificateInfo(final StreamInput in) throws IOException { + this.format = in.readOptionalString(); + this.alias = in.readOptionalString(); + this.serialNumber = in.readOptionalString(); + this.hasPrivateKey = in.readBoolean(); this.subject = in.readOptionalString(); this.san = in.readOptionalString(); this.issuer = in.readOptionalString(); @@ -51,6 +76,10 @@ public CertificateInfo(final StreamInput in) throws IOException { @Override public void writeTo(final StreamOutput out) throws IOException { + out.writeOptionalString(format); + out.writeOptionalString(alias); + out.writeOptionalString(serialNumber); + out.writeBoolean(hasPrivateKey); out.writeOptionalString(subject); out.writeOptionalString(san); out.writeOptionalString(issuer); @@ -61,36 +90,18 @@ public void writeTo(final StreamOutput out) throws IOException { @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 deleted file mode 100644 index 0158494869..0000000000 --- a/src/main/java/org/opensearch/security/dlic/rest/api/ssl/CertificateType.java +++ /dev/null @@ -1,48 +0,0 @@ -/* - * 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.dlic.rest.api.ssl; - -import java.util.Locale; - -public enum CertificateType { - HTTP("http"), - TRANSPORT("transport"), - ALL("all"); - - private final String value; - - private CertificateType(String value) { - this.value = value; - } - - public static boolean isHttp(final CertificateType certificateType) { - return certificateType == HTTP || certificateType == ALL; - } - - public static boolean isTransport(final CertificateType certificateType) { - return certificateType == TRANSPORT || certificateType == ALL; - } - - public String value() { - return value.toLowerCase(Locale.ROOT); - } - - public static CertificateType from(final String certType) { - if (certType == null) { - return ALL; - } - for (final var t : values()) - if (t.value.equalsIgnoreCase(certType)) return t; - throw new IllegalArgumentException("Invalid certificate type: " + certType); - } - -} 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 index ca15216b80..f529cd4731 100644 --- 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,6 +13,7 @@ import java.io.IOException; import java.util.List; +import java.util.Locale; import java.util.Map; import org.opensearch.core.common.io.stream.StreamInput; @@ -20,17 +21,18 @@ 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> certificates; + private final Map> certificates; - public CertificatesInfo(final Map> certificates) { + public CertificatesInfo(final Map> 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 index f7b971daec..fbe81225c5 100644 --- 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,18 +12,22 @@ 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 { - 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; @@ -31,12 +35,12 @@ public CertificatesInfoNodesRequest(CertificateType certificateType, boolean inM 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 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(); + } } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ssl/TransportCertificatesInfoNodesAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/ssl/TransportCertificatesInfoNodesAction.java index 39edfd570f..77305f91ed 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/ssl/TransportCertificatesInfoNodesAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/ssl/TransportCertificatesInfoNodesAction.java @@ -13,7 +13,9 @@ import java.io.IOException; import java.util.List; +import java.util.Locale; import java.util.Map; +import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -95,30 +97,54 @@ protected CertificatesNodesResponse.CertificatesNodeResponse nodeOperation(final } } - protected CertificatesInfo loadCertificates(final CertificateType certificateType) { + protected CertificatesInfo loadCertificates(final Optional certificateType) { var httpCertificates = List.of(); - var transportsCertificates = List.of(); - if (CertificateType.isHttp(certificateType)) { + var transportCertificates = List.of(); + var transportClientCertificates = List.of(); + final var certType = certificateType.map(t -> CertType.valueOf(t.toUpperCase(Locale.ROOT))).orElse(null); + if (certType == null || certType == CertType.HTTP) { httpCertificates = sslSettingsManager.sslContextHandler(CertType.HTTP) - .map(SslContextHandler::keyMaterialCertificates) + .map(SslContextHandler::certificates) .map(this::certificatesDetails) .orElse(List.of()); } - if (CertificateType.isTransport(certificateType)) { - transportsCertificates = sslSettingsManager.sslContextHandler(CertType.TRANSPORT) - .map(SslContextHandler::keyMaterialCertificates) + if (certType == null || certType == CertType.TRANSPORT) { + transportCertificates = sslSettingsManager.sslContextHandler(CertType.TRANSPORT) + .map(SslContextHandler::certificates) .map(this::certificatesDetails) .orElse(List.of()); } - return new CertificatesInfo(Map.of(CertificateType.HTTP, httpCertificates, CertificateType.TRANSPORT, transportsCertificates)); + if (certType == null || certType == CertType.TRANSPORT_CLIENT) { + transportClientCertificates = sslSettingsManager.sslContextHandler(CertType.TRANSPORT_CLIENT) + .map(SslContextHandler::certificates) + .map(this::certificatesDetails) + .orElse(List.of()); + } + return new CertificatesInfo( + Map.of( + CertType.HTTP, + httpCertificates, + CertType.TRANSPORT, + transportCertificates, + CertType.TRANSPORT_CLIENT, + transportClientCertificates + ) + ); } private List certificatesDetails(final Stream certificateStream) { - if (certificateStream == null) { - return null; - } return certificateStream.map( - c -> new CertificateInfo(c.subject(), c.subjectAlternativeNames(), c.issuer(), c.notAfter(), c.notBefore()) + c -> new CertificateInfo( + c.format(), + c.alias(), + c.serialNumber(), + c.hasPrivateKey(), + c.subject(), + c.subjectAlternativeNames(), + c.issuer(), + c.notAfter(), + c.notBefore() + ) ).collect(Collectors.toList()); } diff --git a/src/main/java/org/opensearch/security/ssl/SslContextHandler.java b/src/main/java/org/opensearch/security/ssl/SslContextHandler.java index fae9cb27ba..cade5abbdd 100644 --- a/src/main/java/org/opensearch/security/ssl/SslContextHandler.java +++ b/src/main/java/org/opensearch/security/ssl/SslContextHandler.java @@ -62,12 +62,17 @@ SslContext sslContext() { return sslContext; } + public Stream certificates() { + return Stream.concat(authorityCertificates(), keyMaterialCertificates()) + .sorted((c1, c2) -> Boolean.compare(c1.hasPrivateKey(), c2.hasPrivateKey())); + } + public Stream authorityCertificates() { return authorityCertificates(loadedCertificates); } Stream authorityCertificates(final List certificates) { - return certificates.stream().filter(not(Certificate::hasKey)); + return certificates.stream().filter(not(Certificate::hasPrivateKey)); } public Stream keyMaterialCertificates() { @@ -75,7 +80,7 @@ public Stream keyMaterialCertificates() { } Stream keyMaterialCertificates(final List certificates) { - return certificates.stream().filter(Certificate::hasKey); + return certificates.stream().filter(Certificate::hasPrivateKey); } void reloadSslContext() throws CertificateException { diff --git a/src/main/java/org/opensearch/security/ssl/config/CertType.java b/src/main/java/org/opensearch/security/ssl/config/CertType.java index 09a8dcfae9..0c7a698ede 100644 --- a/src/main/java/org/opensearch/security/ssl/config/CertType.java +++ b/src/main/java/org/opensearch/security/ssl/config/CertType.java @@ -11,6 +11,10 @@ package org.opensearch.security.ssl.config; +import java.util.Arrays; +import java.util.Set; +import java.util.stream.Collectors; + import static org.opensearch.security.ssl.util.SSLConfigConstants.SSL_HTTP_PREFIX; import static org.opensearch.security.ssl.util.SSLConfigConstants.SSL_TRANSPORT_CLIENT_PREFIX; import static org.opensearch.security.ssl.util.SSLConfigConstants.SSL_TRANSPORT_PREFIX; @@ -20,6 +24,11 @@ public enum CertType { TRANSPORT(SSL_TRANSPORT_PREFIX), TRANSPORT_CLIENT(SSL_TRANSPORT_CLIENT_PREFIX); + public static Set TYPES = Arrays.stream(CertType.values()) + .map(CertType::name) + .map(String::toLowerCase) + .collect(Collectors.toSet()); + private final String sslConfigPrefix; private CertType(String sslConfigPrefix) { diff --git a/src/main/java/org/opensearch/security/ssl/config/Certificate.java b/src/main/java/org/opensearch/security/ssl/config/Certificate.java index 534148db57..de535ad8cd 100644 --- a/src/main/java/org/opensearch/security/ssl/config/Certificate.java +++ b/src/main/java/org/opensearch/security/ssl/config/Certificate.java @@ -68,7 +68,7 @@ public String alias() { return alias; } - public boolean hasKey() { + public boolean hasPrivateKey() { return hasKey; } @@ -76,6 +76,10 @@ public String subjectAlternativeNames() { return loadSubjectAlternativeNames(); } + public byte[] signature() { + return certificate.getSignature(); + } + @Deprecated(since = "since JDK 21", forRemoval = true) public String loadSubjectAlternativeNames() { String san = "";