Skip to content

Commit

Permalink
Revert "Allow skipping hot reload dn validation (#4752)"
Browse files Browse the repository at this point in the history
This reverts commit c8cacf1.

Signed-off-by: Craig Perkins <[email protected]>
  • Loading branch information
cwperks committed Oct 24, 2024
1 parent c8cacf1 commit fecbf34
Show file tree
Hide file tree
Showing 10 changed files with 34 additions and 432 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,7 @@ private void printJCEWarnings() {
public final SslProvider sslTransportServerProvider;
public final SslProvider sslTransportClientProvider;
private final boolean httpSSLEnabled;
private final boolean httpSSLEnforceCertReloadDnVerification;
private final boolean transportSSLEnabled;
private final boolean transportSSLEnforceCertReloadDnVerification;

private ArrayList<String> enabledHttpCiphersJDKProvider;
private ArrayList<String> enabledHttpCiphersOpenSSLProvider;
Expand Down Expand Up @@ -168,18 +166,10 @@ public DefaultSecurityKeyStore(final Settings settings, final Path configPath) {
SSLConfigConstants.SECURITY_SSL_HTTP_ENABLED,
SSLConfigConstants.SECURITY_SSL_HTTP_ENABLED_DEFAULT
);
httpSSLEnforceCertReloadDnVerification = settings.getAsBoolean(
SSLConfigConstants.SECURITY_SSL_HTTP_ENFORCE_CERT_RELOAD_DN_VERIFICATION,
true
);
transportSSLEnabled = settings.getAsBoolean(
SSLConfigConstants.SECURITY_SSL_TRANSPORT_ENABLED,
SSLConfigConstants.SECURITY_SSL_TRANSPORT_ENABLED_DEFAULT
);
transportSSLEnforceCertReloadDnVerification = settings.getAsBoolean(
SSLConfigConstants.SECURITY_SSL_TRANSPORT_ENFORCE_CERT_RELOAD_DN_VERIFICATION,
true
);
final boolean useOpenSSLForHttpIfAvailable = OpenSearchSecuritySSLPlugin.OPENSSL_SUPPORTED
&& settings.getAsBoolean(SSLConfigConstants.SECURITY_SSL_HTTP_ENABLE_OPENSSL_IF_AVAILABLE, true);
final boolean useOpenSSLForTransportIfAvailable = OpenSearchSecuritySSLPlugin.OPENSSL_SUPPORTED
Expand Down Expand Up @@ -432,7 +422,7 @@ public void initTransportSSLConfig() {
certFromTruststore = new CertFromTruststore(truststoreProps, truststoreAlias);
}

validateNewCerts(transportCerts, certFromKeystore.getCerts(), transportSSLEnforceCertReloadDnVerification);
validateNewCerts(transportCerts, certFromKeystore.getCerts());

Check warning on line 425 in src/main/java/org/opensearch/security/ssl/DefaultSecurityKeyStore.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/ssl/DefaultSecurityKeyStore.java#L425

Added line #L425 was not covered by tests
transportServerSslContext = buildSSLServerContext(
certFromKeystore.getServerKey(),
certFromKeystore.getServerCert(),
Expand Down Expand Up @@ -483,7 +473,7 @@ public void initTransportSSLConfig() {
certFromFile = new CertFromFile(certProps);
}

validateNewCerts(transportCerts, certFromFile.getCerts(), transportSSLEnforceCertReloadDnVerification);
validateNewCerts(transportCerts, certFromFile.getCerts());

Check warning on line 476 in src/main/java/org/opensearch/security/ssl/DefaultSecurityKeyStore.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/ssl/DefaultSecurityKeyStore.java#L476

Added line #L476 was not covered by tests
transportServerSslContext = buildSSLServerContext(
certFromFile.getServerPemKey(),
certFromFile.getServerPemCert(),
Expand Down Expand Up @@ -581,7 +571,7 @@ public void initHttpSSLConfig() {
certFromTruststore = new CertFromTruststore(truststoreProps, truststoreAlias);
}

validateNewCerts(httpCerts, certFromKeystore.getCerts(), httpSSLEnforceCertReloadDnVerification);
validateNewCerts(httpCerts, certFromKeystore.getCerts());
httpSslContext = buildSSLServerContext(
certFromKeystore.getServerKey(),
certFromKeystore.getServerCert(),
Expand Down Expand Up @@ -612,7 +602,7 @@ public void initHttpSSLConfig() {
);
CertFromFile certFromFile = new CertFromFile(certFileProps);

validateNewCerts(httpCerts, certFromFile.getCerts(), httpSSLEnforceCertReloadDnVerification);
validateNewCerts(httpCerts, certFromFile.getCerts());

Check warning on line 605 in src/main/java/org/opensearch/security/ssl/DefaultSecurityKeyStore.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/ssl/DefaultSecurityKeyStore.java#L605

Added line #L605 was not covered by tests
httpSslContext = buildSSLServerContext(
certFromFile.getServerPemKey(),
certFromFile.getServerPemCert(),
Expand Down Expand Up @@ -643,16 +633,11 @@ public void initHttpSSLConfig() {
* If the current and new certificates are same, skip remaining checks.
* For new X509 cert to be valid Issuer, Subject DN must be the same and
* new certificates should expire after current ones.
* @param currentX509Certs Array of current x509 certificates
* @param newX509Certs Array of x509 certificates which will replace our current cert
* @param verifyValidDNs Whether to verify that new certs have valid IssuerDN, SubjectDN and SAN
* @param currentX509Certs Array of current x509 certificates
* @param newX509Certs Array of x509 certificates which will replace our current cert
* @throws Exception if certificate is invalid
*/
private void validateNewCerts(
final X509Certificate[] currentX509Certs,
final X509Certificate[] newX509Certs,
final boolean verifyValidDNs
) throws Exception {
private void validateNewCerts(final X509Certificate[] currentX509Certs, final X509Certificate[] newX509Certs) throws Exception {

// First time we init certs ignore validity check
if (currentX509Certs == null) {
Expand All @@ -669,7 +654,7 @@ private void validateNewCerts(
}

// Check if new X509 certs have valid IssuerDN, SubjectDN or SAN
if (verifyValidDNs && !hasValidDNs(currentX509Certs, newX509Certs)) {
if (!hasValidDNs(currentX509Certs, newX509Certs)) {
throw new Exception("New Certs do not have valid Issuer DN, Subject DN or SAN.");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -634,23 +634,6 @@ public List<Setting<?>> getSettings() {
Setting.longSetting(SSLConfigConstants.SECURITY_SSL_HTTP_CRL_VALIDATION_DATE, -1, -1, Property.NodeScope, Property.Filtered)
);

settings.add(
Setting.boolSetting(
SSLConfigConstants.SECURITY_SSL_HTTP_ENFORCE_CERT_RELOAD_DN_VERIFICATION,
true,
Property.NodeScope,
Property.Filtered
)
);
settings.add(
Setting.boolSetting(
SSLConfigConstants.SECURITY_SSL_TRANSPORT_ENFORCE_CERT_RELOAD_DN_VERIFICATION,
true,
Property.NodeScope,
Property.Filtered
)
);

return settings;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ 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_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 +91,6 @@ 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_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
Loading

0 comments on commit fecbf34

Please sign in to comment.