Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove OpenSSL provider as an option. JDK SSL Provider will be used by default. #2298

Closed
wants to merge 4 commits into from
Closed

Remove OpenSSL provider as an option. JDK SSL Provider will be used by default. #2298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d374543
Remove OpenSSL provider
cwperks Nov 23, 2022
5598df5
Remove additional references to OpenSSL
cwperks Nov 23, 2022
b8f2f8c
Remove unused imports
cwperks Nov 23, 2022
036490a
Merge branch 'main' into remove-openssl
cwperks Dec 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/integration-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jobs:

- uses: actions/checkout@v2

- run: OPENDISTRO_SECURITY_TEST_OPENSSL_OPT=true ./gradlew test
- run: ./gradlew test

- uses: actions/upload-artifact@v3
if: always()
Expand Down
116 changes: 6 additions & 110 deletions src/main/java/org/opensearch/security/ssl/DefaultSecurityKeyStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;
Expand All @@ -59,12 +58,10 @@
import io.netty.handler.ssl.ApplicationProtocolConfig.SelectorFailureBehavior;
import io.netty.handler.ssl.ApplicationProtocolNames;
import io.netty.handler.ssl.ClientAuth;
import io.netty.handler.ssl.OpenSsl;
import io.netty.handler.ssl.SslContext;
import io.netty.handler.ssl.SslContextBuilder;
import io.netty.handler.ssl.SslProvider;
import io.netty.handler.ssl.SupportedCipherSuiteFilter;
import io.netty.util.internal.PlatformDependent;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.bouncycastle.asn1.ASN1InputStream;
Expand Down Expand Up @@ -114,14 +111,10 @@ private void printJCEWarnings() {
private final boolean transportSSLEnabled;

private List<String> enabledHttpCiphersJDKProvider;
private List<String> enabledHttpCiphersOpenSSLProvider;
private List<String> enabledTransportCiphersJDKProvider;
private List<String> enabledTransportCiphersOpenSSLProvider;

private List<String> enabledHttpProtocolsJDKProvider;
private List<String> enabledHttpProtocolsOpenSSLProvider;
private List<String> enabledTransportProtocolsJDKProvider;
private List<String> enabledTransportProtocolsOpenSSLProvider;

private SslContext httpSslContext;
private SslContext transportServerSslContext;
Expand All @@ -144,38 +137,14 @@ public DefaultSecurityKeyStore(final Settings settings, final Path configPath) {
SSLConfigConstants.SECURITY_SSL_HTTP_ENABLED_DEFAULT);
transportSSLEnabled = settings.getAsBoolean(SSLConfigConstants.SECURITY_SSL_TRANSPORT_ENABLED,
SSLConfigConstants.SECURITY_SSL_TRANSPORT_ENABLED_DEFAULT);
final boolean useOpenSSLForHttpIfAvailable = OpenSearchSecuritySSLPlugin.OPENSSL_SUPPORTED && settings
.getAsBoolean(SSLConfigConstants.SECURITY_SSL_HTTP_ENABLE_OPENSSL_IF_AVAILABLE, true);
final boolean useOpenSSLForTransportIfAvailable = OpenSearchSecuritySSLPlugin.OPENSSL_SUPPORTED && settings
.getAsBoolean(SSLConfigConstants.SECURITY_SSL_TRANSPORT_ENABLE_OPENSSL_IF_AVAILABLE, true);

if(!OpenSearchSecuritySSLPlugin.OPENSSL_SUPPORTED && OpenSsl.isAvailable() && (settings.getAsBoolean(SSLConfigConstants.SECURITY_SSL_HTTP_ENABLE_OPENSSL_IF_AVAILABLE, true) || settings.getAsBoolean(SSLConfigConstants.SECURITY_SSL_TRANSPORT_ENABLE_OPENSSL_IF_AVAILABLE, true) )) {
if (PlatformDependent.javaVersion() < 12) {
log.warn("Support for OpenSSL with Java 11 or prior versions require using Netty allocator. Set 'opensearch.unsafe.use_netty_default_allocator' system property to true");
} else {
log.warn("Support for OpenSSL with Java 12+ has been removed from Open Distro Security since Elasticsearch 7.4.0. Using JDK SSL instead.");
}
}

boolean openSSLInfoLogged = false;

if (httpSSLEnabled && useOpenSSLForHttpIfAvailable) {
sslHTTPProvider = SslContext.defaultServerProvider();
logOpenSSLInfos();
openSSLInfoLogged = true;
} else if (httpSSLEnabled) {
if (httpSSLEnabled) {
sslHTTPProvider = SslProvider.JDK;
} else {
sslHTTPProvider = null;
}

if (transportSSLEnabled && useOpenSSLForTransportIfAvailable) {
sslTransportClientProvider = SslContext.defaultClientProvider();
sslTransportServerProvider = SslContext.defaultServerProvider();
if (!openSSLInfoLogged) {
logOpenSSLInfos();
}
} else if (transportSSLEnabled) {
if (transportSSLEnabled) {
sslTransportClientProvider = sslTransportServerProvider = SslProvider.JDK;
} else {
sslTransportClientProvider = sslTransportServerProvider = null;
Expand Down Expand Up @@ -729,37 +698,15 @@ private void setHttpSSLCerts(X509Certificate[] certs) {
this.httpCerts = certs;
}

private void logOpenSSLInfos() {
if (OpenSearchSecuritySSLPlugin.OPENSSL_SUPPORTED && OpenSsl.isAvailable()) {
log.info("OpenSSL {} ({}) available", OpenSsl.versionString(), OpenSsl.version());

if (OpenSsl.version() < 0x10002000L) {
log.warn(
"Outdated OpenSSL version detected. You should update to 1.0.2k or later. Currently installed: {}",
OpenSsl.versionString());
}

if (!OpenSsl.supportsHostnameValidation()) {
log.warn("Your OpenSSL version {} does not support hostname verification. You should update to 1.0.2k or later.", OpenSsl.versionString());
}

log.debug("OpenSSL available ciphers {}", OpenSsl.availableOpenSslCipherSuites());
} else {
log.info("OpenSSL not available (this is not an error, we simply fallback to built-in JDK SSL) because of "
+ OpenSsl.unavailabilityCause());
}
}

private List<String> getEnabledSSLCiphers(final SslProvider provider, boolean http) {
if (provider == null) {
return Collections.emptyList();
}

if (http) {
return provider == SslProvider.JDK ? enabledHttpCiphersJDKProvider : enabledHttpCiphersOpenSSLProvider;
return enabledHttpCiphersJDKProvider;
} else {
return provider == SslProvider.JDK ? enabledTransportCiphersJDKProvider
: enabledTransportCiphersOpenSSLProvider;
return enabledTransportCiphersJDKProvider;
}

}
Expand All @@ -770,10 +717,9 @@ private String[] getEnabledSSLProtocols(final SslProvider provider, boolean http
}

if (http) {
return (provider == SslProvider.JDK ? enabledHttpProtocolsJDKProvider : enabledHttpProtocolsOpenSSLProvider).toArray(new String[0]);
return enabledHttpProtocolsJDKProvider.toArray(new String[0]);
} else {
return (provider == SslProvider.JDK ? enabledTransportProtocolsJDKProvider
: enabledTransportProtocolsOpenSSLProvider).toArray(new String[0]);
return enabledTransportProtocolsJDKProvider.toArray(new String[0]);
}

}
Expand All @@ -786,56 +732,6 @@ private void initEnabledSSLCiphers() {
final List<String> secureHttpSSLProtocols = Arrays.asList(SSLConfigConstants.getSecureSSLProtocols(settings, true));
final List<String> secureTransportSSLProtocols = Arrays.asList(SSLConfigConstants.getSecureSSLProtocols(settings, false));

if (OpenSearchSecuritySSLPlugin.OPENSSL_SUPPORTED && OpenSsl.isAvailable()) {
final Set<String> openSSLSecureHttpCiphers = new HashSet<>();
for (final String secure : secureHttpSSLCiphers) {
if (OpenSsl.isCipherSuiteAvailable(secure)) {
openSSLSecureHttpCiphers.add(secure);
}
}


log.debug("OPENSSL {} supports the following ciphers (java-style) {}", OpenSsl.versionString(), OpenSsl.availableJavaCipherSuites());
log.debug("OPENSSL {} supports the following ciphers (openssl-style) {}", OpenSsl.versionString(), OpenSsl.availableOpenSslCipherSuites());

enabledHttpCiphersOpenSSLProvider = Collections
.unmodifiableList(new ArrayList<String>(openSSLSecureHttpCiphers));
} else {
enabledHttpCiphersOpenSSLProvider = Collections.emptyList();
}

if (OpenSearchSecuritySSLPlugin.OPENSSL_SUPPORTED && OpenSsl.isAvailable()) {
final Set<String> openSSLSecureTransportCiphers = new HashSet<>();
for (final String secure : secureTransportSSLCiphers) {
if (OpenSsl.isCipherSuiteAvailable(secure)) {
openSSLSecureTransportCiphers.add(secure);
}
}

enabledTransportCiphersOpenSSLProvider = Collections
.unmodifiableList(new ArrayList<String>(openSSLSecureTransportCiphers));
} else {
enabledTransportCiphersOpenSSLProvider = Collections.emptyList();
}

if(OpenSearchSecuritySSLPlugin.OPENSSL_SUPPORTED && OpenSsl.isAvailable() && OpenSsl.version() > 0x10101009L) {
enabledHttpProtocolsOpenSSLProvider = new ArrayList(Arrays.asList("TLSv1.3","TLSv1.2","TLSv1.1","TLSv1"));
enabledHttpProtocolsOpenSSLProvider.retainAll(secureHttpSSLProtocols);
enabledTransportProtocolsOpenSSLProvider = new ArrayList(Arrays.asList("TLSv1.3","TLSv1.2","TLSv1.1"));
enabledTransportProtocolsOpenSSLProvider.retainAll(secureTransportSSLProtocols);

log.info("OpenSSL supports TLSv1.3");

} else if(OpenSearchSecuritySSLPlugin.OPENSSL_SUPPORTED && OpenSsl.isAvailable()){
enabledHttpProtocolsOpenSSLProvider = new ArrayList(Arrays.asList("TLSv1.2","TLSv1.1","TLSv1"));
enabledHttpProtocolsOpenSSLProvider.retainAll(secureHttpSSLProtocols);
enabledTransportProtocolsOpenSSLProvider = new ArrayList(Arrays.asList("TLSv1.2","TLSv1.1"));
enabledTransportProtocolsOpenSSLProvider.retainAll(secureTransportSSLProtocols);
} else {
enabledHttpProtocolsOpenSSLProvider = Collections.emptyList();
enabledTransportProtocolsOpenSSLProvider = Collections.emptyList();
}

SSLEngine engine = null;
List<String> jdkSupportedCiphers = null;
List<String> jdkSupportedProtocols = null;
Expand Down
5 changes: 0 additions & 5 deletions src/main/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.cluster.node.DiscoveryNodes;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.Booleans;
import org.opensearch.common.io.stream.NamedWriteableRegistry;
import org.opensearch.common.network.NetworkModule;
import org.opensearch.common.network.NetworkService;
Expand Down Expand Up @@ -89,8 +88,6 @@
//For ES5 this class has only effect when SSL only plugin is installed
public class OpenSearchSecuritySSLPlugin extends Plugin implements SystemIndexPlugin, NetworkPlugin {

private static boolean USE_NETTY_DEFAULT_ALLOCATOR = Booleans.parseBoolean(System.getProperty("opensearch.unsafe.use_netty_default_allocator"), false);
public static final boolean OPENSSL_SUPPORTED = (PlatformDependent.javaVersion() < 12) && USE_NETTY_DEFAULT_ALLOCATOR;
protected final Logger log = LogManager.getLogger(this.getClass());
protected static final String CLIENT_TYPE = "client.type";
protected final boolean client;
Expand Down Expand Up @@ -328,9 +325,7 @@ public List<Setting<?>> getSettings() {
settings.add(Setting.simpleString(SSLConfigConstants.SECURITY_SSL_HTTP_TRUSTSTORE_FILEPATH, Property.NodeScope, Property.Filtered));
settings.add(Setting.simpleString(SSLConfigConstants.SECURITY_SSL_HTTP_TRUSTSTORE_PASSWORD, Property.NodeScope, Property.Filtered));
settings.add(Setting.simpleString(SSLConfigConstants.SECURITY_SSL_HTTP_TRUSTSTORE_TYPE, Property.NodeScope, Property.Filtered));
settings.add(Setting.boolSetting(SSLConfigConstants.SECURITY_SSL_HTTP_ENABLE_OPENSSL_IF_AVAILABLE, OPENSSL_SUPPORTED, Property.NodeScope, Property.Filtered));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is removed will this create an error before the OpenSearch starts up?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not certain if I would want this failure to happen, it could be seen as non-bwc compatible. Instead logging a notice that the feature doesn't work and the setting wouldn't be used could be better in the interim until 3.0.0 when we'd could force this as a breaking change.

settings.add(Setting.boolSetting(SSLConfigConstants.SECURITY_SSL_HTTP_ENABLED, SSLConfigConstants.SECURITY_SSL_HTTP_ENABLED_DEFAULT, Property.NodeScope, Property.Filtered));
settings.add(Setting.boolSetting(SSLConfigConstants.SECURITY_SSL_TRANSPORT_ENABLE_OPENSSL_IF_AVAILABLE, OPENSSL_SUPPORTED,Property.NodeScope, Property.Filtered));
settings.add(Setting.boolSetting(SSLConfigConstants.SECURITY_SSL_TRANSPORT_ENABLED, SSLConfigConstants.SECURITY_SSL_TRANSPORT_ENABLED_DEFAULT, Property.NodeScope, Property.Filtered));
settings.add(Setting.boolSetting(SSLConfigConstants.SECURITY_SSL_TRANSPORT_ENFORCE_HOSTNAME_VERIFICATION, true, Property.NodeScope, Property.Filtered));
settings.add(Setting.boolSetting(SSLConfigConstants.SECURITY_SSL_TRANSPORT_ENFORCE_HOSTNAME_VERIFICATION_RESOLVE_HOST_NAME, true, Property.NodeScope, Property.Filtered));
Expand Down
2 changes: 0 additions & 2 deletions src/main/java/org/opensearch/security/ssl/util/SSLConfigConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@

public final class SSLConfigConstants {

public static final String SECURITY_SSL_HTTP_ENABLE_OPENSSL_IF_AVAILABLE = "plugins.security.ssl.http.enable_openssl_if_available";
public static final String SECURITY_SSL_HTTP_ENABLED = "plugins.security.ssl.http.enabled";
public static final boolean SECURITY_SSL_HTTP_ENABLED_DEFAULT = false;
public static final String SECURITY_SSL_HTTP_CLIENTAUTH_MODE = "plugins.security.ssl.http.clientauth_mode";
Expand All @@ -42,7 +41,6 @@ public final class SSLConfigConstants {
public static final String SECURITY_SSL_HTTP_TRUSTSTORE_FILEPATH = "plugins.security.ssl.http.truststore_filepath";
public static final String SECURITY_SSL_HTTP_TRUSTSTORE_PASSWORD = "plugins.security.ssl.http.truststore_password";
public static final String SECURITY_SSL_HTTP_TRUSTSTORE_TYPE = "plugins.security.ssl.http.truststore_type";
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";
public static final boolean SECURITY_SSL_TRANSPORT_ENABLED_DEFAULT = true;
public static final String SECURITY_SSL_TRANSPORT_ENFORCE_HOSTNAME_VERIFICATION = "plugins.security.ssl.transport.enforce_hostname_verification";
Expand Down
8 changes: 0 additions & 8 deletions src/test/java/org/opensearch/security/IntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,9 @@
import java.util.TreeSet;

import com.fasterxml.jackson.databind.JsonNode;
import io.netty.handler.ssl.OpenSsl;
import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.message.BasicHeader;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Test;

import org.opensearch.action.admin.indices.alias.IndicesAliasesRequest;
Expand Down Expand Up @@ -173,12 +171,6 @@ public void testDNSpecials1() throws Exception {
Assert.assertEquals(HttpStatus.SC_OK, rh.executeGetRequest("", encodeBasicHeader("worf", "worf")).getStatusCode());
}

@Test
public void testEnsureOpenSSLAvailability() {
Assume.assumeTrue(allowOpenSSL);
Assert.assertTrue(String.valueOf(OpenSsl.unavailabilityCause()), OpenSsl.isAvailable());
}

@Test
public void testMultiget() throws Exception {

Expand Down
2 changes: 0 additions & 2 deletions src/test/java/org/opensearch/security/httpclient/HttpClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ public void testSslConnection() throws Exception {

final Settings settings = Settings.builder()
.put("plugins.security.ssl.http.enabled", true)
.put(SSLConfigConstants.SECURITY_SSL_HTTP_ENABLE_OPENSSL_IF_AVAILABLE, false)
.put(SSLConfigConstants.SECURITY_SSL_HTTP_KEYSTORE_ALIAS, "node-0")
.put("plugins.security.ssl.http.keystore_filepath", FileHelper.getAbsoluteFilePathFromClassPath("auditlog/node-0-keystore.jks"))
.put("plugins.security.ssl.http.truststore_filepath", FileHelper.getAbsoluteFilePathFromClassPath("auditlog/truststore.jks"))
Expand Down Expand Up @@ -107,7 +106,6 @@ public void testSslConnectionPKIAuth() throws Exception {
final Settings settings = Settings.builder()
.put("plugins.security.ssl.http.enabled", true)
.put("plugins.security.ssl.http.clientauth_mode", "REQUIRE")
.put(SSLConfigConstants.SECURITY_SSL_HTTP_ENABLE_OPENSSL_IF_AVAILABLE, false)
.put(SSLConfigConstants.SECURITY_SSL_HTTP_KEYSTORE_ALIAS, "node-0")
.put("plugins.security.ssl.http.keystore_filepath", FileHelper.getAbsoluteFilePathFromClassPath("auditlog/node-0-keystore.jks"))
.put("plugins.security.ssl.http.truststore_filepath", FileHelper.getAbsoluteFilePathFromClassPath("auditlog/truststore.jks"))
Expand Down
Loading