Skip to content

Commit

Permalink
Adapt disable dn verification on cert reload changes to SSL refactor (#…
Browse files Browse the repository at this point in the history
…4841)

Signed-off-by: Craig Perkins <cwperx@amazon.com>
  • Loading branch information
cwperks authored Oct 28, 2024
1 parent c8cacf1 commit 2e0e64f
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 19 deletions.
13 changes: 8 additions & 5 deletions src/main/java/org/opensearch/security/ssl/SslContextHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ void reloadSslContext() throws CertificateException {
if (sameCertificates(newCertificates)) {
return;
}
validateNewCertificates(newCertificates);
validateNewCertificates(newCertificates, sslConfiguration.sslParameters().shouldValidateNewCertDNs());
invalidateSessions();
if (sslContext.isClient()) {
sslContext = sslConfiguration.buildClientSslContext(false);
Expand Down Expand Up @@ -141,13 +141,16 @@ private void validateSans(final List<Certificate> newCertificates) throws Certif
}
}

private void validateNewCertificates(final List<Certificate> newCertificates) throws CertificateException {
private void validateNewCertificates(final List<Certificate> newCertificates, boolean shouldValidateNewCertDNs)
throws CertificateException {
for (final var certificate : newCertificates) {
certificate.x509Certificate().checkValidity();
}
validateSubjectDns(newCertificates);
validateIssuerDns(newCertificates);
validateSans(newCertificates);
if (shouldValidateNewCertDNs) {
validateSubjectDns(newCertificates);
validateIssuerDns(newCertificates);
validateSans(newCertificates);
}
}

private void invalidateSessions() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import static org.opensearch.security.ssl.util.SSLConfigConstants.ENABLED_CIPHERS;
import static org.opensearch.security.ssl.util.SSLConfigConstants.ENABLED_PROTOCOLS;
import static org.opensearch.security.ssl.util.SSLConfigConstants.ENABLE_OPENSSL_IF_AVAILABLE;
import static org.opensearch.security.ssl.util.SSLConfigConstants.ENFORCE_CERT_RELOAD_DN_VERIFICATION;
import static org.opensearch.security.ssl.util.SSLConfigConstants.OPENSSL_1_1_1_BETA_9;
import static org.opensearch.security.ssl.util.SSLConfigConstants.OPENSSL_AVAILABLE;

Expand All @@ -53,11 +54,20 @@ public class SslParameters {

private final List<String> ciphers;

private SslParameters(SslProvider provider, final ClientAuth clientAuth, List<String> protocols, List<String> ciphers) {
private final boolean validateCertDNsOnReload;

private SslParameters(
SslProvider provider,
final ClientAuth clientAuth,
List<String> protocols,
List<String> ciphers,
boolean validateCertDNsOnReload
) {
this.provider = provider;
this.ciphers = ciphers;
this.protocols = protocols;
this.clientAuth = clientAuth;
this.validateCertDNsOnReload = validateCertDNsOnReload;
}

public ClientAuth clientAuth() {
Expand All @@ -76,6 +86,10 @@ public List<String> allowedProtocols() {
return protocols;
}

public boolean shouldValidateNewCertDNs() {
return validateCertDNsOnReload;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
Expand Down Expand Up @@ -112,6 +126,10 @@ private SslProvider provider(final Settings settings) {
}
}

private boolean validateCertDNsOnReload(final Settings settings) {
return settings.getAsBoolean(ENFORCE_CERT_RELOAD_DN_VERIFICATION, true);
}

private List<String> protocols(final SslProvider provider, final Settings settings, boolean http) {
final var allowedProtocols = settings.getAsList(ENABLED_PROTOCOLS, List.of(ALLOWED_SSL_PROTOCOLS));
if (provider == SslProvider.OPENSSL) {
Expand Down Expand Up @@ -181,7 +199,8 @@ public SslParameters load(final boolean http) {
provider,
clientAuth,
protocols(provider, sslConfigSettings, http),
ciphers(provider, sslConfigSettings)
ciphers(provider, sslConfigSettings),
validateCertDNsOnReload(sslConfigSettings)
);
if (sslParameters.allowedProtocols().isEmpty()) {
throw new OpenSearchSecurityException("No ssl protocols for " + (http ? "HTTP" : "Transport") + " layer");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ public final class SSLConfigConstants {

public static final String CLIENT_AUTH_MODE = "clientauth_mode";

public static final String ENFORCE_CERT_RELOAD_DN_VERIFICATION = "enforce_cert_reload_dn_verification";

public static final String KEYSTORE_TYPE = "keystore_type";
public static final String KEYSTORE_ALIAS = "keystore_alias";
public static final String KEYSTORE_FILEPATH = "keystore_filepath";
Expand Down Expand Up @@ -82,8 +84,8 @@ public final class SSLConfigConstants {
public static final String SECURITY_SSL_HTTP_TRUSTSTORE_ALIAS = "plugins.security.ssl.http.truststore_alias";
public static final String SECURITY_SSL_HTTP_TRUSTSTORE_FILEPATH = "plugins.security.ssl.http.truststore_filepath";
public static final String SECURITY_SSL_HTTP_TRUSTSTORE_TYPE = "plugins.security.ssl.http.truststore_type";
public static final String SECURITY_SSL_HTTP_ENFORCE_CERT_RELOAD_DN_VERIFICATION =
"plugins.security.ssl.http.enforce_cert_reload_dn_verification";
public static final String SECURITY_SSL_HTTP_ENFORCE_CERT_RELOAD_DN_VERIFICATION = "plugins.security.ssl.http."
+ ENFORCE_CERT_RELOAD_DN_VERIFICATION;
public static final String SECURITY_SSL_TRANSPORT_ENABLE_OPENSSL_IF_AVAILABLE =
"plugins.security.ssl.transport.enable_openssl_if_available";
public static final String SECURITY_SSL_TRANSPORT_ENABLED = "plugins.security.ssl.transport.enabled";
Expand All @@ -93,8 +95,8 @@ public final class SSLConfigConstants {
public static final String SECURITY_SSL_TRANSPORT_ENFORCE_HOSTNAME_VERIFICATION_RESOLVE_HOST_NAME =
"plugins.security.ssl.transport.resolve_hostname";

public static final String SECURITY_SSL_TRANSPORT_ENFORCE_CERT_RELOAD_DN_VERIFICATION =
"plugins.security.ssl.transport.enforce_cert_reload_dn_verification";
public static final String SECURITY_SSL_TRANSPORT_ENFORCE_CERT_RELOAD_DN_VERIFICATION = "plugins.security.ssl.transport."
+ ENFORCE_CERT_RELOAD_DN_VERIFICATION;
public static final String SECURITY_SSL_TRANSPORT_KEYSTORE_ALIAS = "plugins.security.ssl.transport.keystore_alias";
public static final String SECURITY_SSL_TRANSPORT_SERVER_KEYSTORE_ALIAS = "plugins.security.ssl.transport.server.keystore_alias";
public static final String SECURITY_SSL_TRANSPORT_CLIENT_KEYSTORE_ALIAS = "plugins.security.ssl.transport.client.keystore_alias";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,9 @@ public void testReloadHttpCertDifferentTrustChain_noSkipDnValidationFail() throw
assertThat(
DefaultObjectMapper.readTree(reloadCertsResponse.getBody()).get("error").get("root_cause").get(0).get("reason").asText(),
is(
"OpenSearchSecurityException[Error while initializing http SSL layer from PEM: java.lang.Exception: "
+ "New Certs do not have valid Issuer DN, Subject DN or SAN.]; nested: Exception[New Certs do not have valid Issuer DN, Subject DN or SAN.];"
"java.security.cert.CertificateException: New certificates do not have valid Issuer DNs. "
+ "Current Issuer DNs: [CN=Example Com Inc. Signing CA,OU=Example Com Inc. Signing CA,O=Example Com Inc.,DC=example,DC=com] "
+ "new Issuer DNs: [CN=Example Com Inc. Secondary Signing CA,OU=Example Com Inc. Secondary Signing CA,O=Example Com Inc.,DC=example,DC=com]"
)
);
}
Expand All @@ -266,8 +267,9 @@ public void testReloadHttpCertDifferentTrustChain_defaultSettingValidationFail()
assertThat(
DefaultObjectMapper.readTree(reloadCertsResponse.getBody()).get("error").get("root_cause").get(0).get("reason").asText(),
is(
"OpenSearchSecurityException[Error while initializing http SSL layer from PEM: java.lang.Exception: "
+ "New Certs do not have valid Issuer DN, Subject DN or SAN.]; nested: Exception[New Certs do not have valid Issuer DN, Subject DN or SAN.];"
"java.security.cert.CertificateException: New certificates do not have valid Issuer DNs. "
+ "Current Issuer DNs: [CN=Example Com Inc. Signing CA,OU=Example Com Inc. Signing CA,O=Example Com Inc.,DC=example,DC=com] "
+ "new Issuer DNs: [CN=Example Com Inc. Secondary Signing CA,OU=Example Com Inc. Secondary Signing CA,O=Example Com Inc.,DC=example,DC=com]"
)
);
}
Expand Down Expand Up @@ -314,8 +316,9 @@ public void testReloadTransportCertDifferentTrustChain_noSkipDnValidationFail()
assertThat(
DefaultObjectMapper.readTree(reloadCertsResponse.getBody()).get("error").get("root_cause").get(0).get("reason").asText(),
is(
"OpenSearchSecurityException[Error while initializing transport SSL layer from PEM: java.lang.Exception: "
+ "New Certs do not have valid Issuer DN, Subject DN or SAN.]; nested: Exception[New Certs do not have valid Issuer DN, Subject DN or SAN.];"
"java.security.cert.CertificateException: New certificates do not have valid Issuer DNs. "
+ "Current Issuer DNs: [CN=Example Com Inc. Signing CA,OU=Example Com Inc. Signing CA,O=Example Com Inc.,DC=example,DC=com] "
+ "new Issuer DNs: [CN=Example Com Inc. Secondary Signing CA,OU=Example Com Inc. Secondary Signing CA,O=Example Com Inc.,DC=example,DC=com]"
)
);
}
Expand All @@ -337,8 +340,9 @@ public void testReloadTransportCertDifferentTrustChain_defaultSettingValidationF
assertThat(
DefaultObjectMapper.readTree(reloadCertsResponse.getBody()).get("error").get("root_cause").get(0).get("reason").asText(),
is(
"OpenSearchSecurityException[Error while initializing transport SSL layer from PEM: java.lang.Exception: "
+ "New Certs do not have valid Issuer DN, Subject DN or SAN.]; nested: Exception[New Certs do not have valid Issuer DN, Subject DN or SAN.];"
"java.security.cert.CertificateException: New certificates do not have valid Issuer DNs. "
+ "Current Issuer DNs: [CN=Example Com Inc. Signing CA,OU=Example Com Inc. Signing CA,O=Example Com Inc.,DC=example,DC=com] "
+ "new Issuer DNs: [CN=Example Com Inc. Secondary Signing CA,OU=Example Com Inc. Secondary Signing CA,O=Example Com Inc.,DC=example,DC=com]"
)
);
}
Expand Down

0 comments on commit 2e0e64f

Please sign in to comment.