Skip to content

Commit 420b65b

Browse files
haniczhanicz
authored
KNOX-3245: Add new separators for SSL ciphers and protocols (#1142)
* KNOX-3245: Add new separators for SSL ciphers and protocols * KNOX-3245: Fixed a bug where the ssl.include.protocols config wasn't applied to the internal Jetty server
1 parent 60b021c commit 420b65b

4 files changed

Lines changed: 231 additions & 21 deletions

File tree

‎gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import java.util.Set;
3939
import java.util.concurrent.ConcurrentHashMap;
4040
import java.util.concurrent.TimeUnit;
41+
import java.util.stream.Collectors;
4142

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

669670
@Override
670671
public Set<String> getIncludedSSLProtocols() {
671-
final Collection<String> includedSslProtocols = getTrimmedStringCollection(SSL_INCLUDE_PROTOCOLS);
672+
final List<String> includedSslProtocols = splitConfigValueToList(SSL_INCLUDE_PROTOCOLS);
672673
return includedSslProtocols == null ? Collections.emptySet() : new HashSet<>(includedSslProtocols);
673674
}
674675

675676
@Override
676677
public List<String> getExcludedSSLProtocols() {
677-
List<String> protocols = null;
678-
String value = get(SSL_EXCLUDE_PROTOCOLS);
679-
if (!"none".equals(value)) {
680-
protocols = Arrays.asList(value.split("\\s*,\\s*"));
681-
}
682-
return protocols;
678+
return splitConfigValueToList(SSL_EXCLUDE_PROTOCOLS);
683679
}
684680

685681
@Override
686682
public List<String> getIncludedSSLCiphers() {
687-
List<String> list = null;
688-
String value = get(SSL_INCLUDE_CIPHERS);
689-
if (value != null && !value.isEmpty() && !"none".equalsIgnoreCase(value.trim())) {
690-
list = Arrays.asList(value.trim().split("\\s*,\\s*"));
691-
}
692-
return list;
683+
return splitConfigValueToList(SSL_INCLUDE_CIPHERS);
693684
}
694685

695686
@Override
696687
public List<String> getExcludedSSLCiphers() {
688+
return splitConfigValueToList(SSL_EXCLUDE_CIPHERS);
689+
}
690+
691+
private List<String> splitConfigValueToList(String config) {
697692
List<String> list = null;
698-
String value = get(SSL_EXCLUDE_CIPHERS);
693+
String value = get(config);
699694
if (value != null && !value.isEmpty() && !"none".equalsIgnoreCase(value.trim())) {
700-
list = Arrays.asList(value.trim().split("\\s*,\\s*"));
695+
list = Arrays.stream(value.split("[,:\n]"))
696+
.map(String::trim)
697+
.filter(part -> !part.isEmpty())
698+
.collect(Collectors.toList());
701699
}
702700
return list;
703701
}

‎gateway-server/src/main/java/org/apache/knox/gateway/services/security/impl/JettySSLService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ public Object buildSslContextFactory(GatewayConfig config) throws AliasServiceEx
228228
}
229229

230230
final Set<String> sslIncludeProtocols = config.getIncludedSSLProtocols();
231-
if (sslIncludeProtocols != null && sslIncludeProtocols.isEmpty()) {
231+
if (sslIncludeProtocols != null && !sslIncludeProtocols.isEmpty()) {
232232
sslContextFactory.setIncludeProtocols(sslIncludeProtocols.toArray(new String[0]));
233233
}
234234

‎gateway-server/src/test/java/org/apache/knox/gateway/config/impl/GatewayConfigImplTest.java‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,10 @@
1818

1919
import static org.apache.knox.gateway.services.security.impl.RemoteAliasService.REMOTE_ALIAS_SERVICE_TYPE;
2020
import static org.hamcrest.CoreMatchers.is;
21+
import static org.hamcrest.CoreMatchers.not;
2122
import static org.hamcrest.CoreMatchers.notNullValue;
2223
import static org.hamcrest.MatcherAssert.assertThat;
24+
import static org.hamcrest.Matchers.empty;
2325
import static org.hamcrest.Matchers.hasItems;
2426
import static org.hamcrest.Matchers.nullValue;
2527
import static org.junit.Assert.assertEquals;
@@ -36,6 +38,7 @@
3638
import java.util.HashSet;
3739
import java.util.List;
3840
import java.util.Map;
41+
import java.util.Set;
3942
import java.util.concurrent.TimeUnit;
4043

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

149+
config.set( "ssl.include.ciphers", " ONE:TWO:THREE " );
150+
assertThat( config.getIncludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );
151+
152+
config.set( "ssl.include.ciphers", " ONE:TWO,THREE " );
153+
assertThat( config.getIncludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );
154+
155+
config.set( "ssl.include.ciphers", " ONE : TWO,THREE " );
156+
assertThat( config.getIncludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );
157+
158+
config.set( "ssl.include.ciphers", " ONE : TWO\nTHREE " );
159+
assertThat( config.getIncludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );
160+
161+
config.set( "ssl.include.ciphers", " ONE,TWO \n THREE " );
162+
assertThat( config.getIncludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );
163+
164+
config.set( "ssl.include.ciphers", " ONE,TWO \n THREE :FOUR" );
165+
assertThat( config.getIncludedSSLCiphers(), is(hasItems("ONE","TWO","THREE", "FOUR")) );
166+
167+
config.set( "ssl.include.ciphers", " ONE,TWO,,THREE" );
168+
assertThat( config.getIncludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );
169+
170+
config.set( "ssl.include.ciphers", " ONE,TWO,,THREE" );
171+
assertThat( config.getIncludedSSLCiphers(), not(hasItems("")) );
172+
146173
list = config.getExcludedSSLCiphers();
147174
assertThat( list, is(nullValue()) );
148175

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

167194
config.set( "ssl.exclude.ciphers", " ONE , TWO , THREE " );
168195
assertThat( config.getExcludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );
196+
197+
config.set( "ssl.exclude.ciphers", " ONE:TWO:THREE " );
198+
assertThat( config.getExcludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );
199+
200+
config.set( "ssl.exclude.ciphers", " ONE:TWO,THREE " );
201+
assertThat( config.getExcludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );
202+
203+
config.set( "ssl.exclude.ciphers", " ONE : TWO,THREE " );
204+
assertThat( config.getExcludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );
205+
206+
config.set( "ssl.exclude.ciphers", " ONE : TWO\nTHREE " );
207+
assertThat( config.getExcludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );
208+
209+
config.set( "ssl.exclude.ciphers", " ONE,TWO \n THREE " );
210+
assertThat( config.getExcludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );
211+
212+
config.set( "ssl.exclude.ciphers", " ONE,TWO \n THREE :FOUR" );
213+
assertThat( config.getExcludedSSLCiphers(), is(hasItems("ONE","TWO","THREE", "FOUR")) );
214+
215+
216+
config.set( "ssl.exclude.ciphers", " ONE,TWO,,THREE" );
217+
assertThat( config.getExcludedSSLCiphers(), is(hasItems("ONE","TWO","THREE")) );
218+
219+
config.set( "ssl.exclude.ciphers", " ONE,TWO,,THREE" );
220+
assertThat( config.getExcludedSSLCiphers(), not(hasItems("")) );
221+
}
222+
223+
@Test
224+
public void testSSLProtocols() {
225+
GatewayConfigImpl config = new GatewayConfigImpl();
226+
Set<String> list;
227+
228+
list = config.getIncludedSSLProtocols();
229+
assertThat( list, is(empty()) );
230+
231+
config.set( "ssl.include.protocols", "none" );
232+
assertThat( config.getIncludedSSLProtocols(), is(empty()) );
233+
234+
config.set( "ssl.include.protocols", "" );
235+
assertThat( config.getIncludedSSLProtocols(), is(empty()) );
236+
237+
config.set( "ssl.include.protocols", "ONE" );
238+
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE")) );
239+
240+
config.set( "ssl.include.protocols", " ONE " );
241+
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE")) );
242+
243+
config.set( "ssl.include.protocols", "ONE,TWO" );
244+
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO")) );
245+
246+
config.set( "ssl.include.protocols", "ONE,TWO,THREE" );
247+
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );
248+
249+
config.set( "ssl.include.protocols", " ONE , TWO , THREE " );
250+
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );
251+
252+
config.set( "ssl.include.protocols", " ONE:TWO:THREE " );
253+
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );
254+
255+
config.set( "ssl.include.protocols", " ONE:TWO,THREE " );
256+
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );
257+
258+
config.set( "ssl.include.protocols", " ONE : TWO,THREE " );
259+
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );
260+
261+
config.set( "ssl.include.protocols", " ONE : TWO\nTHREE " );
262+
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );
263+
264+
config.set( "ssl.include.protocols", " ONE,TWO \n THREE " );
265+
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );
266+
267+
config.set( "ssl.include.protocols", " ONE,TWO \n THREE :FOUR" );
268+
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO","THREE", "FOUR")) );
269+
270+
config.set( "ssl.include.protocols", " ONE,TWO,,THREE" );
271+
assertThat( config.getIncludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );
272+
273+
config.set( "ssl.include.protocols", " ONE,TWO,,THREE" );
274+
assertThat( config.getIncludedSSLProtocols(), not(hasItems("")) );
275+
276+
config.set( "ssl.exclude.protocols", "none" );
277+
assertThat( config.getExcludedSSLProtocols(), is(nullValue()) );
278+
279+
config.set( "ssl.exclude.protocols", "" );
280+
assertThat( config.getExcludedSSLProtocols(), is(nullValue()) );
281+
282+
config.set( "ssl.exclude.protocols", "ONE" );
283+
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE")) );
284+
285+
config.set( "ssl.exclude.protocols", " ONE " );
286+
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE")) );
287+
288+
config.set( "ssl.exclude.protocols", "ONE,TWO" );
289+
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO")) );
290+
291+
config.set( "ssl.exclude.protocols", "ONE,TWO,THREE" );
292+
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );
293+
294+
config.set( "ssl.exclude.protocols", " ONE , TWO , THREE " );
295+
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );
296+
297+
config.set( "ssl.exclude.protocols", " ONE:TWO:THREE " );
298+
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );
299+
300+
config.set( "ssl.exclude.protocols", " ONE:TWO,THREE " );
301+
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );
302+
303+
config.set( "ssl.exclude.protocols", " ONE : TWO,THREE " );
304+
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );
305+
306+
config.set( "ssl.exclude.protocols", " ONE : TWO\nTHREE " );
307+
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );
308+
309+
config.set( "ssl.exclude.protocols", " ONE,TWO \n THREE " );
310+
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );
311+
312+
config.set( "ssl.exclude.protocols", " ONE,TWO \n THREE :FOUR" );
313+
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO","THREE", "FOUR")) );
314+
315+
config.set( "ssl.exclude.protocols", " ONE,TWO,,THREE" );
316+
assertThat( config.getExcludedSSLProtocols(), is(hasItems("ONE","TWO","THREE")) );
317+
318+
config.set( "ssl.exclude.protocols", " ONE,TWO,,THREE" );
319+
assertThat( config.getExcludedSSLProtocols(), not(hasItems("")) );
169320
}
170321

171322
// KNOX-2772

‎gateway-server/src/test/java/org/apache/knox/gateway/services/security/impl/JettySSLServiceTest.java‎

Lines changed: 66 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,19 @@
2323
import static org.easymock.EasyMock.expect;
2424
import static org.easymock.EasyMock.replay;
2525
import static org.easymock.EasyMock.verify;
26+
import static org.junit.Assert.assertArrayEquals;
2627
import static org.junit.Assert.assertEquals;
2728
import static org.junit.Assert.assertFalse;
2829
import static org.junit.Assert.assertNotNull;
2930
import static org.junit.Assert.assertNull;
3031
import static org.junit.Assert.assertTrue;
3132
import static org.junit.Assert.fail;
3233

34+
import java.util.Arrays;
35+
import java.util.HashSet;
36+
import java.util.List;
37+
import java.util.Set;
38+
3339
import org.apache.knox.gateway.config.GatewayConfig;
3440
import org.apache.knox.gateway.services.security.AliasService;
3541
import org.apache.knox.gateway.services.security.AliasServiceException;
@@ -498,10 +504,65 @@ public void testExcludeTopologyFromClientAuthNoPolicy() {
498504
assertFalse(sslContextFactory.getWantClientAuth());
499505
}
500506

507+
@Test
508+
public void testBuildSslContextFactoryCiphersAndProtocols() throws Exception {
509+
String basedir = System.getProperty("basedir");
510+
if (basedir == null) {
511+
basedir = new File(".").getCanonicalPath();
512+
}
513+
514+
Path identityKeystorePath = Paths.get(basedir, "target", "test-classes", "keystores", "server-keystore.jks");
515+
String identityKeystoreType = "jks";
516+
char[] identityKeystorePassword = "horton".toCharArray();
517+
char[] identityKeyPassphrase = "horton".toCharArray();
518+
String identityKeyAlias = "server";
519+
Path truststorePath = Paths.get(basedir, "target", "test-classes", "keystores", "server-truststore.jks");
520+
String truststoreType = "jks";
521+
String truststorePasswordAlias = "trust_store_password";
522+
523+
List<String> includedCiphers = Arrays.asList("SSL_RSA_WITH_RC4_128_MD5", "SSL_RSA_WITH_RC4_128_SHA");
524+
List<String> excludedCiphers = List.of("SSL_DHE_RSA_WITH_3DES_EDE_CBC_SHA");
525+
List<String> excludedProtocols = List.of("SSLv3");
526+
Set<String> includedProtocols = new HashSet<>(Arrays.asList("TLSv1.2", "TLSv1.3"));
527+
528+
GatewayConfig config = createGatewayConfig(false, false, identityKeystorePath, identityKeystoreType, identityKeyAlias, truststorePath, truststoreType, truststorePasswordAlias,
529+
includedCiphers, excludedCiphers, includedProtocols, excludedProtocols);
530+
531+
AliasService aliasService = createMock(AliasService.class);
532+
expect(aliasService.getGatewayIdentityKeystorePassword()).andReturn(identityKeystorePassword).atLeastOnce();
533+
expect(aliasService.getGatewayIdentityPassphrase()).andReturn(identityKeyPassphrase).atLeastOnce();
534+
535+
KeystoreService keystoreService = createMock(KeystoreService.class);
536+
537+
replay(config, aliasService, keystoreService);
538+
539+
JettySSLService sslService = new JettySSLService();
540+
sslService.setAliasService(aliasService);
541+
sslService.setKeystoreService(keystoreService);
542+
543+
SslContextFactory sslContextFactory = (SslContextFactory) sslService.buildSslContextFactory(config);
544+
assertArrayEquals(excludedCiphers.toArray(), sslContextFactory.getExcludeCipherSuites());
545+
assertArrayEquals(includedCiphers.toArray(), sslContextFactory.getIncludeCipherSuites());
546+
assertArrayEquals(excludedProtocols.toArray(), sslContextFactory.getExcludeProtocols());
547+
assertArrayEquals(includedProtocols.toArray(), sslContextFactory.getIncludeProtocols());
548+
549+
550+
verify(config, aliasService, keystoreService);
551+
}
552+
501553
private GatewayConfig createGatewayConfig(boolean isClientAuthNeeded, boolean isExplicitTruststore,
502554
Path identityKeystorePath, String identityKeystoreType,
503555
String identityKeyAlias, Path truststorePath,
504556
String truststoreType, String trustStorePasswordAlias) {
557+
return createGatewayConfig(isClientAuthNeeded, isExplicitTruststore, identityKeystorePath, identityKeystoreType, identityKeyAlias, truststorePath, truststoreType, trustStorePasswordAlias, null, null, null, null);
558+
}
559+
560+
private GatewayConfig createGatewayConfig(boolean isClientAuthNeeded, boolean isExplicitTruststore,
561+
Path identityKeystorePath, String identityKeystoreType,
562+
String identityKeyAlias, Path truststorePath,
563+
String truststoreType, String trustStorePasswordAlias,
564+
List<String> includedCiphers, List<String> excludedCiphers,
565+
Set<String> includedProtocols, List<String> excludedProtocols) {
505566
GatewayConfig config = createMock(GatewayConfig.class);
506567
expect(config.getIdentityKeystorePath()).andReturn(identityKeystorePath.toString()).atLeastOnce();
507568
expect(config.getIdentityKeystoreType()).andReturn(identityKeystoreType).atLeastOnce();
@@ -523,10 +584,10 @@ private GatewayConfig createGatewayConfig(boolean isClientAuthNeeded, boolean is
523584

524585
expect(config.isClientAuthWanted()).andReturn(false).atLeastOnce();
525586
expect(config.getTrustAllCerts()).andReturn(false).atLeastOnce();
526-
expect(config.getIncludedSSLCiphers()).andReturn(null).atLeastOnce();
527-
expect(config.getExcludedSSLCiphers()).andReturn(null).atLeastOnce();
528-
expect(config.getIncludedSSLProtocols()).andReturn(null).atLeastOnce();
529-
expect(config.getExcludedSSLProtocols()).andReturn(null).atLeastOnce();
587+
expect(config.getIncludedSSLCiphers()).andReturn(includedCiphers).atLeastOnce();
588+
expect(config.getExcludedSSLCiphers()).andReturn(excludedCiphers).atLeastOnce();
589+
expect(config.getIncludedSSLProtocols()).andReturn(includedProtocols).atLeastOnce();
590+
expect(config.getExcludedSSLProtocols()).andReturn(excludedProtocols).atLeastOnce();
530591
expect(config.isSSLRenegotiationAllowed()).andReturn(true).atLeastOnce();
531592
return config;
532593
}
@@ -538,4 +599,4 @@ private GatewayConfig createGatewayConfigForExcludeTopologyTest(boolean isClient
538599
return config;
539600
}
540601

541-
}
602+
}

0 commit comments

Comments
 (0)