Skip to content

Commit db6e7dc

Browse files
willyborankinwillyborankin
authored
Refactor SSL Configuration (opensearch-project#4671)
1 parent 811f26d commit db6e7dc

33 files changed

+3887
-123
lines changed

‎build.gradle

+3
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,9 @@ dependencies {
686686
testImplementation('org.awaitility:awaitility:4.2.2') {
687687
exclude(group: 'org.hamcrest', module: 'hamcrest')
688688
}
689+
testImplementation "org.bouncycastle:bcpkix-jdk18on:${versions.bouncycastle}"
690+
testImplementation "org.bouncycastle:bcutil-jdk18on:${versions.bouncycastle}"
691+
689692
// Only osx-x86_64, osx-aarch_64, linux-x86_64, linux-aarch_64, windows-x86_64 are available
690693
if (osdetector.classifier in ["osx-x86_64", "osx-aarch_64", "linux-x86_64", "linux-aarch_64", "windows-x86_64"]) {
691694
testImplementation "io.netty:netty-tcnative-classes:2.0.61.Final"

‎checkstyle/checkstyle.xml

+7
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,13 @@
4343
<module name="BeforeExecutionExclusionFileFilter">
4444
<property name="fileNamePattern" value="src/main/java/com/amazon/dlic/auth/http/kerberos/HTTPSpnegoAuthenticator.java"/>
4545
</module>
46+
<module name="BeforeExecutionExclusionFileFilter">
47+
<property name="fileNamePattern" value="src/test/java/org/opensearch/security/ssl/SslContextHandlerTest.java"/>
48+
</module>
49+
<module name="BeforeExecutionExclusionFileFilter">
50+
<property name="fileNamePattern" value="src/test/java/org/opensearch/security/ssl/CertificatesRule.java"/>
51+
</module>
52+
4653

4754
<!-- https://checkstyle.org/config_filters.html#SuppressionFilter -->
4855
<module name="SuppressionFilter">

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

+10-5
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,7 @@ public List<RestHandler> getRestHandlers(
648648
evaluator,
649649
threadPool,
650650
Objects.requireNonNull(auditLog),
651-
sks,
651+
sslSettingsManager,
652652
Objects.requireNonNull(userService),
653653
sslCertReloadEnabled,
654654
passwordHasher
@@ -1207,9 +1207,8 @@ public Collection<Object> createComponents(
12071207
components.add(userService);
12081208
components.add(passwordHasher);
12091209

1210-
if (!ExternalSecurityKeyStore.hasExternalSslContext(settings)) {
1211-
components.add(sks);
1212-
}
1210+
components.add(sslSettingsManager);
1211+
12131212
final var allowDefaultInit = settings.getAsBoolean(SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX, false);
12141213
final var useClusterState = useClusterStateToInitSecurityConfig(settings);
12151214
if (!SSLConfig.isSslOnlyMode() && !isDisabled(settings) && allowDefaultInit && useClusterState) {
@@ -2167,7 +2166,13 @@ public PluginSubject getPluginSubject(Plugin plugin) {
21672166
@Override
21682167
public Optional<SecureSettingsFactory> getSecureSettingFactory(Settings settings) {
21692168
return Optional.of(
2170-
new OpenSearchSecureSettingsFactory(threadPool, sks, evaluateSslExceptionHandler(), securityRestHandler, SSLConfig)
2169+
new OpenSearchSecureSettingsFactory(
2170+
threadPool,
2171+
sslSettingsManager,
2172+
evaluateSslExceptionHandler(),
2173+
securityRestHandler,
2174+
SSLConfig
2175+
)
21712176
);
21722177
}
21732178

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

+9-3
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import org.opensearch.security.configuration.ConfigurationRepository;
2626
import org.opensearch.security.hasher.PasswordHasher;
2727
import org.opensearch.security.privileges.PrivilegesEvaluator;
28-
import org.opensearch.security.ssl.SecurityKeyStore;
28+
import org.opensearch.security.ssl.SslSettingsManager;
2929
import org.opensearch.security.ssl.transport.PrincipalExtractor;
3030
import org.opensearch.security.user.UserService;
3131
import org.opensearch.threadpool.ThreadPool;
@@ -46,7 +46,7 @@ public static Collection<RestHandler> getHandler(
4646
final PrivilegesEvaluator evaluator,
4747
final ThreadPool threadPool,
4848
final AuditLog auditLog,
49-
final SecurityKeyStore securityKeyStore,
49+
final SslSettingsManager sslSettingsManager,
5050
final UserService userService,
5151
final boolean certificatesReloadEnabled,
5252
final PasswordHasher passwordHasher
@@ -97,7 +97,13 @@ public static Collection<RestHandler> getHandler(
9797
new MultiTenancyConfigApiAction(clusterService, threadPool, securityApiDependencies),
9898
new RateLimitersApiAction(clusterService, threadPool, securityApiDependencies),
9999
new ConfigUpgradeApiAction(clusterService, threadPool, securityApiDependencies),
100-
new SecuritySSLCertsApiAction(clusterService, threadPool, securityKeyStore, certificatesReloadEnabled, securityApiDependencies),
100+
new SecuritySSLCertsApiAction(
101+
clusterService,
102+
threadPool,
103+
sslSettingsManager,
104+
certificatesReloadEnabled,
105+
securityApiDependencies
106+
),
101107
new CertificatesApiAction(clusterService, threadPool, securityApiDependencies)
102108
);
103109
}

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

+46-43
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,10 @@
1212
package org.opensearch.security.dlic.rest.api;
1313

1414
import java.io.IOException;
15-
import java.security.cert.X509Certificate;
16-
import java.util.Arrays;
1715
import java.util.List;
1816
import java.util.Map;
1917
import java.util.stream.Collectors;
18+
import java.util.stream.Stream;
2019

2120
import com.google.common.collect.ImmutableList;
2221
import com.google.common.collect.ImmutableMap;
@@ -31,8 +30,10 @@
3130
import org.opensearch.rest.RestRequest.Method;
3231
import org.opensearch.security.dlic.rest.validation.ValidationResult;
3332
import org.opensearch.security.securityconf.impl.CType;
34-
import org.opensearch.security.ssl.SecurityKeyStore;
35-
import org.opensearch.security.ssl.util.SSLConfigConstants;
33+
import org.opensearch.security.ssl.SslContextHandler;
34+
import org.opensearch.security.ssl.SslSettingsManager;
35+
import org.opensearch.security.ssl.config.CertType;
36+
import org.opensearch.security.ssl.config.Certificate;
3637
import org.opensearch.security.support.ConfigConstants;
3738
import org.opensearch.threadpool.ThreadPool;
3839

@@ -62,23 +63,20 @@ public class SecuritySSLCertsApiAction extends AbstractApiAction {
6263
)
6364
);
6465

65-
private final SecurityKeyStore securityKeyStore;
66+
private final SslSettingsManager sslSettingsManager;
6667

6768
private final boolean certificatesReloadEnabled;
6869

69-
private final boolean httpsEnabled;
70-
7170
public SecuritySSLCertsApiAction(
7271
final ClusterService clusterService,
7372
final ThreadPool threadPool,
74-
final SecurityKeyStore securityKeyStore,
73+
final SslSettingsManager sslSettingsManager,
7574
final boolean certificatesReloadEnabled,
7675
final SecurityApiDependencies securityApiDependencies
7776
) {
7877
super(Endpoint.SSL, clusterService, threadPool, securityApiDependencies);
79-
this.securityKeyStore = securityKeyStore;
78+
this.sslSettingsManager = sslSettingsManager;
8079
this.certificatesReloadEnabled = certificatesReloadEnabled;
81-
this.httpsEnabled = securityApiDependencies.settings().getAsBoolean(SSLConfigConstants.SECURITY_SSL_HTTP_ENABLED, true);
8280
this.requestHandlersBuilder.configureRequestHandlers(this::securitySSLCertsRequestHandlers);
8381
}
8482

@@ -108,10 +106,10 @@ private void securitySSLCertsRequestHandlers(RequestHandler.RequestHandlersBuild
108106
.verifyAccessForAllMethods()
109107
.override(
110108
Method.GET,
111-
(channel, request, client) -> withSecurityKeyStore().valid(keyStore -> loadCertificates(channel, keyStore))
109+
(channel, request, client) -> withSecurityKeyStore().valid(ignore -> loadCertificates(channel))
112110
.error((status, toXContent) -> response(channel, status, toXContent))
113111
)
114-
.override(Method.PUT, (channel, request, client) -> withSecurityKeyStore().valid(keyStore -> {
112+
.override(Method.PUT, (channel, request, client) -> withSecurityKeyStore().valid(ignore -> {
115113
if (!certificatesReloadEnabled) {
116114
badRequest(
117115
channel,
@@ -123,7 +121,7 @@ private void securitySSLCertsRequestHandlers(RequestHandler.RequestHandlersBuild
123121
)
124122
);
125123
} else {
126-
reloadCertificates(channel, request, keyStore);
124+
reloadCertificates(channel, request);
127125
}
128126
}).error((status, toXContent) -> response(channel, status, toXContent)));
129127
}
@@ -138,65 +136,70 @@ boolean accessHandler(final RestRequest request) {
138136
}
139137
}
140138

141-
ValidationResult<SecurityKeyStore> withSecurityKeyStore() {
142-
if (securityKeyStore == null) {
139+
ValidationResult<SslSettingsManager> withSecurityKeyStore() {
140+
if (sslSettingsManager == null) {
143141
return ValidationResult.error(RestStatus.OK, badRequestMessage("keystore is not initialized"));
144142
}
145-
return ValidationResult.success(securityKeyStore);
143+
return ValidationResult.success(sslSettingsManager);
146144
}
147145

148-
protected void loadCertificates(final RestChannel channel, final SecurityKeyStore keyStore) throws IOException {
146+
protected void loadCertificates(final RestChannel channel) throws IOException {
149147
ok(
150148
channel,
151149
(builder, params) -> builder.startObject()
152-
.field("http_certificates_list", httpsEnabled ? generateCertDetailList(keyStore.getHttpCerts()) : null)
153-
.field("transport_certificates_list", generateCertDetailList(keyStore.getTransportCerts()))
150+
.field(
151+
"http_certificates_list",
152+
generateCertDetailList(
153+
sslSettingsManager.sslContextHandler(CertType.HTTP).map(SslContextHandler::keyMaterialCertificates).orElse(null)
154+
)
155+
)
156+
.field(
157+
"transport_certificates_list",
158+
generateCertDetailList(
159+
sslSettingsManager.sslContextHandler(CertType.TRANSPORT)
160+
.map(SslContextHandler::keyMaterialCertificates)
161+
.orElse(null)
162+
)
163+
)
154164
.endObject()
155165
);
156166
}
157167

158-
private List<Map<String, String>> generateCertDetailList(final X509Certificate[] certs) {
168+
private List<Map<String, String>> generateCertDetailList(final Stream<Certificate> certs) {
159169
if (certs == null) {
160170
return null;
161171
}
162-
return Arrays.stream(certs).map(cert -> {
163-
final String issuerDn = cert != null && cert.getIssuerX500Principal() != null ? cert.getIssuerX500Principal().getName() : "";
164-
final String subjectDn = cert != null && cert.getSubjectX500Principal() != null ? cert.getSubjectX500Principal().getName() : "";
165-
166-
final String san = securityKeyStore.getSubjectAlternativeNames(cert);
167-
168-
final String notBefore = cert != null && cert.getNotBefore() != null ? cert.getNotBefore().toInstant().toString() : "";
169-
final String notAfter = cert != null && cert.getNotAfter() != null ? cert.getNotAfter().toInstant().toString() : "";
170-
return ImmutableMap.of(
172+
return certs.map(
173+
c -> ImmutableMap.of(
171174
"issuer_dn",
172-
issuerDn,
175+
c.issuer(),
173176
"subject_dn",
174-
subjectDn,
177+
c.subject(),
175178
"san",
176-
san,
179+
c.subjectAlternativeNames(),
177180
"not_before",
178-
notBefore,
181+
c.notBefore(),
179182
"not_after",
180-
notAfter
181-
);
182-
}).collect(Collectors.toList());
183+
c.notAfter()
184+
)
185+
).collect(Collectors.toList());
183186
}
184187

185-
protected void reloadCertificates(final RestChannel channel, final RestRequest request, final SecurityKeyStore keyStore)
186-
throws IOException {
188+
protected void reloadCertificates(final RestChannel channel, final RestRequest request) throws IOException {
187189
final String certType = request.param("certType").toLowerCase().trim();
188190
try {
189191
switch (certType) {
190192
case "http":
191-
if (!httpsEnabled) {
193+
if (sslSettingsManager.sslConfiguration(CertType.HTTP).isPresent()) {
194+
sslSettingsManager.reloadSslContext(CertType.HTTP);
195+
ok(channel, (builder, params) -> builder.startObject().field("message", "updated http certs").endObject());
196+
} else {
192197
badRequest(channel, "SSL for HTTP is disabled");
193-
return;
194198
}
195-
keyStore.initHttpSSLConfig();
196-
ok(channel, (builder, params) -> builder.startObject().field("message", "updated http certs").endObject());
197199
break;
198200
case "transport":
199-
keyStore.initTransportSSLConfig();
201+
sslSettingsManager.reloadSslContext(CertType.TRANSPORT);
202+
sslSettingsManager.reloadSslContext(CertType.TRANSPORT_CLIENT);
200203
ok(channel, (builder, params) -> builder.startObject().field("message", "updated transport certs").endObject());
201204
break;
202205
default:

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

+22-28
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,22 @@
1212
package org.opensearch.security.dlic.rest.api.ssl;
1313

1414
import java.io.IOException;
15-
import java.security.cert.X509Certificate;
1615
import java.util.List;
1716
import java.util.Map;
18-
19-
import com.google.common.collect.ImmutableList;
17+
import java.util.stream.Collectors;
18+
import java.util.stream.Stream;
2019

2120
import org.opensearch.action.FailedNodeException;
2221
import org.opensearch.action.support.ActionFilters;
2322
import org.opensearch.action.support.nodes.TransportNodesAction;
2423
import org.opensearch.cluster.service.ClusterService;
2524
import org.opensearch.common.inject.Inject;
26-
import org.opensearch.common.settings.Settings;
2725
import org.opensearch.core.common.io.stream.StreamInput;
2826
import org.opensearch.core.common.io.stream.StreamOutput;
29-
import org.opensearch.security.ssl.DefaultSecurityKeyStore;
30-
import org.opensearch.security.ssl.util.SSLConfigConstants;
27+
import org.opensearch.security.ssl.SslContextHandler;
28+
import org.opensearch.security.ssl.SslSettingsManager;
29+
import org.opensearch.security.ssl.config.CertType;
30+
import org.opensearch.security.ssl.config.Certificate;
3131
import org.opensearch.threadpool.ThreadPool;
3232
import org.opensearch.transport.TransportRequest;
3333
import org.opensearch.transport.TransportService;
@@ -38,18 +38,15 @@ public class TransportCertificatesInfoNodesAction extends TransportNodesAction<
3838
TransportCertificatesInfoNodesAction.NodeRequest,
3939
CertificatesNodesResponse.CertificatesNodeResponse> {
4040

41-
private final DefaultSecurityKeyStore securityKeyStore;
42-
43-
private final boolean httpsEnabled;
41+
private final SslSettingsManager sslSettingsManager;
4442

4543
@Inject
4644
public TransportCertificatesInfoNodesAction(
47-
final Settings settings,
4845
final ThreadPool threadPool,
4946
final ClusterService clusterService,
5047
final TransportService transportService,
5148
final ActionFilters actionFilters,
52-
final DefaultSecurityKeyStore securityKeyStore
49+
final SslSettingsManager sslSettingsManager
5350
) {
5451
super(
5552
CertificatesActionType.NAME,
@@ -62,8 +59,7 @@ public TransportCertificatesInfoNodesAction(
6259
ThreadPool.Names.GENERIC,
6360
CertificatesNodesResponse.CertificatesNodeResponse.class
6461
);
65-
this.httpsEnabled = settings.getAsBoolean(SSLConfigConstants.SECURITY_SSL_HTTP_ENABLED, true);
66-
this.securityKeyStore = securityKeyStore;
62+
this.sslSettingsManager = sslSettingsManager;
6763
}
6864

6965
@Override
@@ -89,12 +85,6 @@ protected CertificatesNodesResponse.CertificatesNodeResponse newNodeResponse(fin
8985
protected CertificatesNodesResponse.CertificatesNodeResponse nodeOperation(final NodeRequest request) {
9086
final var sslCertRequest = request.sslCertsInfoNodesRequest;
9187

92-
if (securityKeyStore == null) {
93-
return new CertificatesNodesResponse.CertificatesNodeResponse(
94-
clusterService.localNode(),
95-
new IllegalStateException("keystore is not initialized")
96-
);
97-
}
9888
try {
9989
return new CertificatesNodesResponse.CertificatesNodeResponse(
10090
clusterService.localNode(),
@@ -109,23 +99,27 @@ protected CertificatesInfo loadCertificates(final CertificateType certificateTyp
10999
var httpCertificates = List.<CertificateInfo>of();
110100
var transportsCertificates = List.<CertificateInfo>of();
111101
if (CertificateType.isHttp(certificateType)) {
112-
httpCertificates = httpsEnabled ? certificatesDetails(securityKeyStore.getHttpCerts()) : List.of();
102+
httpCertificates = sslSettingsManager.sslContextHandler(CertType.HTTP)
103+
.map(SslContextHandler::keyMaterialCertificates)
104+
.map(this::certificatesDetails)
105+
.orElse(List.of());
113106
}
114107
if (CertificateType.isTransport(certificateType)) {
115-
transportsCertificates = certificatesDetails(securityKeyStore.getTransportCerts());
108+
transportsCertificates = sslSettingsManager.sslContextHandler(CertType.TRANSPORT)
109+
.map(SslContextHandler::keyMaterialCertificates)
110+
.map(this::certificatesDetails)
111+
.orElse(List.of());
116112
}
117113
return new CertificatesInfo(Map.of(CertificateType.HTTP, httpCertificates, CertificateType.TRANSPORT, transportsCertificates));
118114
}
119115

120-
private List<CertificateInfo> certificatesDetails(final X509Certificate[] certs) {
121-
if (certs == null) {
116+
private List<CertificateInfo> certificatesDetails(final Stream<Certificate> certificateStream) {
117+
if (certificateStream == null) {
122118
return null;
123119
}
124-
final var certificates = ImmutableList.<CertificateInfo>builder();
125-
for (final var c : certs) {
126-
certificates.add(CertificateInfo.from(c, securityKeyStore.getSubjectAlternativeNames(c)));
127-
}
128-
return certificates.build();
120+
return certificateStream.map(
121+
c -> new CertificateInfo(c.subject(), c.subjectAlternativeNames(), c.issuer(), c.notAfter(), c.notBefore())
122+
).collect(Collectors.toList());
129123
}
130124

131125
public static class NodeRequest extends TransportRequest {

0 commit comments

Comments
 (0)