Skip to content

KNOX-3245: Add new separators for SSL ciphers and protocols #1142

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

Merged
hanicz merged 2 commits into from
Jan 29, 2026
Merged

KNOX-3245: Add new separators for SSL ciphers and protocols #1142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
235a11f
KNOX-3245: Add new separators for SSL ciphers and protocols
hanicz Jan 29, 2026
24c3d6e
KNOX-3245: Fixed a bug where the ssl.include.protocols config wasn't …
hanicz Jan 29, 2026
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
28 changes: 13 additions & 15 deletions gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;

import org.apache.commons.codec.digest.HmacAlgorithms;
import org.apache.commons.io.FilenameUtils;
Expand Down Expand Up @@ -668,36 +669,33 @@ public String getFrontendUrl() {

@Override
public Set<String> getIncludedSSLProtocols() {
final Collection<String> includedSslProtocols = getTrimmedStringCollection(SSL_INCLUDE_PROTOCOLS);
final List<String> includedSslProtocols = splitConfigValueToList(SSL_INCLUDE_PROTOCOLS);
return includedSslProtocols == null ? Collections.emptySet() : new HashSet<>(includedSslProtocols);
}

@Override
public List<String> getExcludedSSLProtocols() {
List<String> protocols = null;
String value = get(SSL_EXCLUDE_PROTOCOLS);
if (!"none".equals(value)) {
protocols = Arrays.asList(value.split("\\s*,\\s*"));
}
return protocols;
return splitConfigValueToList(SSL_EXCLUDE_PROTOCOLS);
}

@Override
public List<String> getIncludedSSLCiphers() {
List<String> list = null;
String value = get(SSL_INCLUDE_CIPHERS);
if (value != null && !value.isEmpty() && !"none".equalsIgnoreCase(value.trim())) {
list = Arrays.asList(value.trim().split("\\s*,\\s*"));
}
return list;
return splitConfigValueToList(SSL_INCLUDE_CIPHERS);
}

@Override
public List<String> getExcludedSSLCiphers() {
return splitConfigValueToList(SSL_EXCLUDE_CIPHERS);
}

private List<String> splitConfigValueToList(String config) {
List<String> list = null;
String value = get(SSL_EXCLUDE_CIPHERS);
String value = get(config);
if (value != null && !value.isEmpty() && !"none".equalsIgnoreCase(value.trim())) {
list = Arrays.asList(value.trim().split("\\s*,\\s*"));
list = Arrays.stream(value.split("[,:\n]"))
.map(String::trim)
.filter(part -> !part.isEmpty())
.collect(Collectors.toList());
}
return list;
}
Expand Down
2 changes: 1 addition & 1 deletion ...-server/src/main/java/org/apache/knox/gateway/services/security/impl/JettySSLService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ public Object buildSslContextFactory(GatewayConfig config) throws AliasServiceEx
}

final Set<String> sslIncludeProtocols = config.getIncludedSSLProtocols();
if (sslIncludeProtocols != null && sslIncludeProtocols.isEmpty()) {
if (sslIncludeProtocols != null && !sslIncludeProtocols.isEmpty()) {
Copy link
Copy Markdown
Contributor

@smolnar82 smolnar82 Jan 29, 2026

Choose a reason for hiding this comment

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

Nice catch!

sslContextFactory.setIncludeProtocols(sslIncludeProtocols.toArray(new String[0]));
}

Expand Down
151 changes: 151 additions & 0 deletions gateway-server/src/test/java/org/apache/knox/gateway/config/impl/GatewayConfigImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@

import static org.apache.knox.gateway.services.security.impl.RemoteAliasService.REMOTE_ALIAS_SERVICE_TYPE;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.hasItems;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertEquals;
Expand All @@ -36,6 +38,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;

import org.apache.knox.gateway.config.GatewayConfig;
Expand Down Expand Up @@ -143,6 +146,30 @@ public void testSSLCiphers() {
config.set( "ssl.include.ciphers", " ONE , TWO , THREE " );
assertThat( config.getIncludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.include.ciphers", " ONE:TWO:THREE " );
assertThat( config.getIncludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.include.ciphers", " ONE:TWO,THREE " );
assertThat( config.getIncludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.include.ciphers", " ONE : TWO,THREE " );
assertThat( config.getIncludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.include.ciphers", " ONE : TWO\nTHREE " );
assertThat( config.getIncludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.include.ciphers", " ONE,TWO \n THREE " );
assertThat( config.getIncludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.include.ciphers", " ONE,TWO \n THREE :FOUR" );
assertThat( config.getIncludedSSLCiphers(), is(hasItems("ONE","TWO","THREE", "FOUR")) );

config.set( "ssl.include.ciphers", " ONE,TWO,,THREE" );
assertThat( config.getIncludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.include.ciphers", " ONE,TWO,,THREE" );
assertThat( config.getIncludedSSLCiphers(), not(hasItems("")) );

list = config.getExcludedSSLCiphers();
assertThat( list, is(nullValue()) );

Expand All @@ -166,6 +193,130 @@ public void testSSLCiphers() {

config.set( "ssl.exclude.ciphers", " ONE , TWO , THREE " );
assertThat( config.getExcludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.exclude.ciphers", " ONE:TWO:THREE " );
assertThat( config.getExcludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.exclude.ciphers", " ONE:TWO,THREE " );
assertThat( config.getExcludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.exclude.ciphers", " ONE : TWO,THREE " );
assertThat( config.getExcludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.exclude.ciphers", " ONE : TWO\nTHREE " );
assertThat( config.getExcludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.exclude.ciphers", " ONE,TWO \n THREE " );
assertThat( config.getExcludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.exclude.ciphers", " ONE,TWO \n THREE :FOUR" );
assertThat( config.getExcludedSSLCiphers(), is(hasItems("ONE","TWO","THREE", "FOUR")) );


config.set( "ssl.exclude.ciphers", " ONE,TWO,,THREE" );
assertThat( config.getExcludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.exclude.ciphers", " ONE,TWO,,THREE" );
assertThat( config.getExcludedSSLCiphers(), not(hasItems("")) );
}

@Test
public void testSSLProtocols() {
GatewayConfigImpl config = new GatewayConfigImpl();
Set<String> list;

list = config.getIncludedSSLProtocols();
assertThat( list, is(empty()) );

config.set( "ssl.include.protocols", "none" );
assertThat( config.getIncludedSSLProtocols(), is(empty()) );

config.set( "ssl.include.protocols", "" );
assertThat( config.getIncludedSSLProtocols(), is(empty()) );

config.set( "ssl.include.protocols", "ONE" );
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE")) );

config.set( "ssl.include.protocols", " ONE " );
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE")) );

config.set( "ssl.include.protocols", "ONE,TWO" );
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO")) );

config.set( "ssl.include.protocols", "ONE,TWO,THREE" );
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.include.protocols", " ONE , TWO , THREE " );
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.include.protocols", " ONE:TWO:THREE " );
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.include.protocols", " ONE:TWO,THREE " );
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.include.protocols", " ONE : TWO,THREE " );
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.include.protocols", " ONE : TWO\nTHREE " );
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.include.protocols", " ONE,TWO \n THREE " );
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.include.protocols", " ONE,TWO \n THREE :FOUR" );
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO","THREE", "FOUR")) );

config.set( "ssl.include.protocols", " ONE,TWO,,THREE" );
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.include.protocols", " ONE,TWO,,THREE" );
assertThat( config.getIncludedSSLProtocols(), not(hasItems("")) );

config.set( "ssl.exclude.protocols", "none" );
assertThat( config.getExcludedSSLProtocols(), is(nullValue()) );

config.set( "ssl.exclude.protocols", "" );
assertThat( config.getExcludedSSLProtocols(), is(nullValue()) );

config.set( "ssl.exclude.protocols", "ONE" );
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE")) );

config.set( "ssl.exclude.protocols", " ONE " );
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE")) );

config.set( "ssl.exclude.protocols", "ONE,TWO" );
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO")) );

config.set( "ssl.exclude.protocols", "ONE,TWO,THREE" );
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.exclude.protocols", " ONE , TWO , THREE " );
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.exclude.protocols", " ONE:TWO:THREE " );
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.exclude.protocols", " ONE:TWO,THREE " );
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.exclude.protocols", " ONE : TWO,THREE " );
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.exclude.protocols", " ONE : TWO\nTHREE " );
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.exclude.protocols", " ONE,TWO \n THREE " );
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.exclude.protocols", " ONE,TWO \n THREE :FOUR" );
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO","THREE", "FOUR")) );

config.set( "ssl.exclude.protocols", " ONE,TWO,,THREE" );
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );

config.set( "ssl.exclude.protocols", " ONE,TWO,,THREE" );
assertThat( config.getExcludedSSLProtocols(), not(hasItems("")) );
}

// KNOX-2772
Expand Down
71 changes: 66 additions & 5 deletions ...ver/src/test/java/org/apache/knox/gateway/services/security/impl/JettySSLServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,19 @@
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.replay;
import static org.easymock.EasyMock.verify;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import org.apache.knox.gateway.config.GatewayConfig;
import org.apache.knox.gateway.services.security.AliasService;
import org.apache.knox.gateway.services.security.AliasServiceException;
Expand Down Expand Up @@ -498,10 +504,65 @@ public void testExcludeTopologyFromClientAuthNoPolicy() {
assertFalse(sslContextFactory.getWantClientAuth());
}

@Test
public void testBuildSslContextFactoryCiphersAndProtocols() throws Exception {
String basedir = System.getProperty("basedir");
if (basedir == null) {
basedir = new File(".").getCanonicalPath();
}

Path identityKeystorePath = Paths.get(basedir, "target", "test-classes", "keystores", "server-keystore.jks");
String identityKeystoreType = "jks";
char[] identityKeystorePassword = "horton".toCharArray();
char[] identityKeyPassphrase = "horton".toCharArray();
String identityKeyAlias = "server";
Path truststorePath = Paths.get(basedir, "target", "test-classes", "keystores", "server-truststore.jks");
String truststoreType = "jks";
String truststorePasswordAlias = "trust_store_password";

List<String> includedCiphers = Arrays.asList("SSL_RSA_WITH_RC4_128_MD5", "SSL_RSA_WITH_RC4_128_SHA");
List<String> excludedCiphers = List.of("SSL_DHE_RSA_WITH_3DES_EDE_CBC_SHA");
List<String> excludedProtocols = List.of("SSLv3");
Set<String> includedProtocols = new HashSet<>(Arrays.asList("TLSv1.2", "TLSv1.3"));

GatewayConfig config = createGatewayConfig(false, false, identityKeystorePath, identityKeystoreType, identityKeyAlias, truststorePath, truststoreType, truststorePasswordAlias,
includedCiphers, excludedCiphers, includedProtocols, excludedProtocols);

AliasService aliasService = createMock(AliasService.class);
expect(aliasService.getGatewayIdentityKeystorePassword()).andReturn(identityKeystorePassword).atLeastOnce();
expect(aliasService.getGatewayIdentityPassphrase()).andReturn(identityKeyPassphrase).atLeastOnce();

KeystoreService keystoreService = createMock(KeystoreService.class);

replay(config, aliasService, keystoreService);

JettySSLService sslService = new JettySSLService();
sslService.setAliasService(aliasService);
sslService.setKeystoreService(keystoreService);

SslContextFactory sslContextFactory = (SslContextFactory) sslService.buildSslContextFactory(config);
assertArrayEquals(excludedCiphers.toArray(), sslContextFactory.getExcludeCipherSuites());
assertArrayEquals(includedCiphers.toArray(), sslContextFactory.getIncludeCipherSuites());
assertArrayEquals(excludedProtocols.toArray(), sslContextFactory.getExcludeProtocols());
assertArrayEquals(includedProtocols.toArray(), sslContextFactory.getIncludeProtocols());


verify(config, aliasService, keystoreService);
}

private GatewayConfig createGatewayConfig(boolean isClientAuthNeeded, boolean isExplicitTruststore,
Path identityKeystorePath, String identityKeystoreType,
String identityKeyAlias, Path truststorePath,
String truststoreType, String trustStorePasswordAlias) {
return createGatewayConfig(isClientAuthNeeded, isExplicitTruststore, identityKeystorePath, identityKeystoreType, identityKeyAlias, truststorePath, truststoreType, trustStorePasswordAlias, null, null, null, null);
}

private GatewayConfig createGatewayConfig(boolean isClientAuthNeeded, boolean isExplicitTruststore,
Path identityKeystorePath, String identityKeystoreType,
String identityKeyAlias, Path truststorePath,
String truststoreType, String trustStorePasswordAlias,
List<String> includedCiphers, List<String> excludedCiphers,
Set<String> includedProtocols, List<String> excludedProtocols) {
GatewayConfig config = createMock(GatewayConfig.class);
expect(config.getIdentityKeystorePath()).andReturn(identityKeystorePath.toString()).atLeastOnce();
expect(config.getIdentityKeystoreType()).andReturn(identityKeystoreType).atLeastOnce();
Expand All @@ -523,10 +584,10 @@ private GatewayConfig createGatewayConfig(boolean isClientAuthNeeded, boolean is

expect(config.isClientAuthWanted()).andReturn(false).atLeastOnce();
expect(config.getTrustAllCerts()).andReturn(false).atLeastOnce();
expect(config.getIncludedSSLCiphers()).andReturn(null).atLeastOnce();
expect(config.getExcludedSSLCiphers()).andReturn(null).atLeastOnce();
expect(config.getIncludedSSLProtocols()).andReturn(null).atLeastOnce();
expect(config.getExcludedSSLProtocols()).andReturn(null).atLeastOnce();
expect(config.getIncludedSSLCiphers()).andReturn(includedCiphers).atLeastOnce();
expect(config.getExcludedSSLCiphers()).andReturn(excludedCiphers).atLeastOnce();
expect(config.getIncludedSSLProtocols()).andReturn(includedProtocols).atLeastOnce();
expect(config.getExcludedSSLProtocols()).andReturn(excludedProtocols).atLeastOnce();
expect(config.isSSLRenegotiationAllowed()).andReturn(true).atLeastOnce();
return config;
}
Expand All @@ -538,4 +599,4 @@ private GatewayConfig createGatewayConfigForExcludeTopologyTest(boolean isClient
return config;
}

}
}
Loading