Skip to content

Commit

Permalink
Bug Fixes
Browse files Browse the repository at this point in the history 
- Switch to use SslSettingsManager
  instead of DefaultKeyStore
- Resolved issues with default password
  settings for JDK trust/key stores
- Resolved issues with default key password
  settings for PEM

Signed-off-by: Andrey Pleskach <[email protected]>
  • Loading branch information
@willyborankin
willyborankin committed Aug 22, 2024
1 parent 58074cb commit 750d140
Show file tree
Hide file tree
Showing 14 changed files with 154 additions and 143 deletions.
8 changes: 3 additions & 5 deletions src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ public List<RestHandler> getRestHandlers(
evaluator,
threadPool,
Objects.requireNonNull(auditLog),
sks,
sslSettingsManager,
Objects.requireNonNull(userService),
sslCertReloadEnabled,
passwordHasher
Expand Down Expand Up @@ -1204,9 +1204,7 @@ 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) {
Expand Down Expand Up @@ -2114,7 +2112,7 @@ public SecurityTokenManager getTokenManager() {

@Override
public Optional<SecureSettingsFactory> getSecureSettingFactory(Settings settings) {
return Optional.of(new OpenSearchSecureSettingsFactory(threadPool, sks, sslExceptionHandler, securityRestHandler));
return Optional.of(new OpenSearchSecureSettingsFactory(threadPool, sslSettingsManager, sslExceptionHandler, securityRestHandler));
}

@SuppressWarnings("removal")
Expand Down
12 changes: 9 additions & 3 deletions src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -46,7 +46,7 @@ public static Collection<RestHandler> getHandler(
final PrivilegesEvaluator evaluator,
final ThreadPool threadPool,
final AuditLog auditLog,
final SecurityKeyStore securityKeyStore,
final SslSettingsManager sslSettingsManager,
final UserService userService,
final boolean certificatesReloadEnabled,
final PasswordHasher passwordHasher
Expand Down Expand Up @@ -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)
);
}
Expand Down
91 changes: 48 additions & 43 deletions src/main/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsApiAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.Certificate;
import org.opensearch.security.ssl.config.SslContextType;
import org.opensearch.security.support.ConfigConstants;
import org.opensearch.threadpool.ThreadPool;

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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,
Expand All @@ -123,7 +121,7 @@ private void securitySSLCertsRequestHandlers(RequestHandler.RequestHandlersBuild
)
);
} else {
reloadCertificates(channel, request, keyStore);
reloadCertificates(channel, request);
}
}).error((status, toXContent) -> response(channel, status, toXContent)));
}
Expand All @@ -138,65 +136,72 @@ 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(SslContextType.HTTP)
.map(SslContextHandler::keyMaterialCertificates)
.orElse(null)
)
)
.field(
"transport_certificates_list",
generateCertDetailList(
sslSettingsManager.sslContextHandler(SslContextType.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(SslContextType.HTTP).isPresent()) {
sslSettingsManager.reloadSslContext(SslContextType.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(SslContextType.TRANSPORT);
sslSettingsManager.reloadSslContext(SslContextType.CLIENT);
ok(channel, (builder, params) -> builder.startObject().field("message", "updated transport certs").endObject());
break;
default:
Expand Down
23 changes: 0 additions & 23 deletions src/main/java/org/opensearch/security/dlic/rest/api/ssl/CertificateInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -69,28 +68,6 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par
.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;
Expand Down
50 changes: 22 additions & 28 deletions .../java/org/opensearch/security/dlic/rest/api/ssl/TransportCertificatesInfoNodesAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,22 @@
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.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.Certificate;
import org.opensearch.security.ssl.config.SslContextType;
import org.opensearch.threadpool.ThreadPool;
import org.opensearch.transport.TransportRequest;
import org.opensearch.transport.TransportService;
Expand All @@ -38,18 +38,15 @@ public class TransportCertificatesInfoNodesAction extends TransportNodesAction<
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,
Expand All @@ -62,8 +59,7 @@ public TransportCertificatesInfoNodesAction(
ThreadPool.Names.GENERIC,
CertificatesNodesResponse.CertificatesNodeResponse.class
);
this.httpsEnabled = settings.getAsBoolean(SSLConfigConstants.SECURITY_SSL_HTTP_ENABLED, true);
this.securityKeyStore = securityKeyStore;
this.sslSettingsManager = sslSettingsManager;
}

@Override
Expand All @@ -89,12 +85,6 @@ protected CertificatesNodesResponse.CertificatesNodeResponse newNodeResponse(fin
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(),
Expand All @@ -109,23 +99,27 @@ protected CertificatesInfo loadCertificates(final CertificateType certificateTyp
var httpCertificates = List.<CertificateInfo>of();
var transportsCertificates = List.<CertificateInfo>of();
if (CertificateType.isHttp(certificateType)) {
httpCertificates = httpsEnabled ? certificatesDetails(securityKeyStore.getHttpCerts()) : List.of();
httpCertificates = sslSettingsManager.sslContextHandler(SslContextType.HTTP)
.map(SslContextHandler::keyMaterialCertificates)
.map(this::certificatesDetails)
.orElse(List.of());
}
if (CertificateType.isTransport(certificateType)) {
transportsCertificates = certificatesDetails(securityKeyStore.getTransportCerts());
transportsCertificates = sslSettingsManager.sslContextHandler(SslContextType.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 TransportRequest {
Expand Down
Loading

0 comments on commit 750d140

Please sign in to comment.