Skip to content

Commit

Permalink
Issue #4385 - Reverting WARN log in favor of IllegalStateException
Browse files Browse the repository at this point in the history 
+ Plus fleshing out the testcases more for Base / Client / Server
  with and without certificates that will trigger SNI requirement
  and ISE.

Signed-off-by: Joakim Erdfelt <[email protected]>
  • Loading branch information
@joakime
joakime committed Dec 6, 2019
1 parent a5e31dc commit 53073ca
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 54 deletions.
24 changes: 15 additions & 9 deletions jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1249,18 +1249,13 @@ protected KeyManager[] getKeyManagers(KeyStore keyStore) throws Exception
// Is SNI needed to select a certificate?
if (!_certWilds.isEmpty() || _certHosts.size() > 1 || (_certHosts.size() == 1 && _aliasX509.size() > 1))
{
if (this instanceof SslContextFactory.Server)
for (int idx = 0; idx < managers.length; idx++)
{
for (int idx = 0; idx < managers.length; idx++)
if (managers[idx] instanceof X509ExtendedKeyManager)
{
if (managers[idx] instanceof X509ExtendedKeyManager)
managers[idx] = newSniX509ExtendedKeyManager((X509ExtendedKeyManager)managers[idx]);
managers[idx] = newSniX509ExtendedKeyManager((X509ExtendedKeyManager)managers[idx]);
}
}
else
{
LOG.warn("Unable to support SNI on {} (expecting {})", this.getClass().getName(), SslContextFactory.Server.class.getName());
}
}
}
}
Expand All @@ -1277,7 +1272,11 @@ protected KeyManager[] getKeyManagers(KeyStore keyStore) throws Exception
@Deprecated
protected X509ExtendedKeyManager newSniX509ExtendedKeyManager(X509ExtendedKeyManager keyManager)
{
throw new UnsupportedOperationException("X509ExtendedKeyManager only supported on " + SslContextFactory.Server.class.getName());
throw new IllegalStateException(String.format(
"KeyStores with multiple certificates are not supported on the base class %s. (Use %s or %s instead)",
SslContextFactory.class.getName(),
Server.class.getName(),
Client.class.getName()));
}

protected TrustManager[] getTrustManagers(KeyStore trustStore, Collection<? extends CRL> crls) throws Exception
Expand Down Expand Up @@ -2185,6 +2184,13 @@ protected void checkConfiguration()
checkEndPointIdentificationAlgorithm();
super.checkConfiguration();
}

@Override
protected X509ExtendedKeyManager newSniX509ExtendedKeyManager(X509ExtendedKeyManager keyManager)
{
// Client has no SNI functionality.
return keyManager;
}
}

@ManagedObject
Expand Down
91 changes: 46 additions & 45 deletions jetty-util/src/test/java/org/eclipse/jetty/util/ssl/X509Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@

import java.nio.file.Path;
import java.security.cert.X509Certificate;
import javax.net.ssl.KeyManager;
import javax.net.ssl.X509ExtendedKeyManager;

import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.util.resource.PathResource;
Expand All @@ -31,7 +29,6 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.junit.jupiter.api.Assertions.assertThrows;

public class X509Test
Expand Down Expand Up @@ -133,66 +130,70 @@ public boolean[] getKeyUsage()
assertThat("Normal X509", X509.isCertSign(bogusX509), is(false));
}

private X509ExtendedKeyManager getX509ExtendedKeyManager(SslContextFactory sslContextFactory) throws Exception
@Test
public void testBaseClass_WithSni()
{
Resource keystoreResource = Resource.newSystemResource("keystore");
Resource truststoreResource = Resource.newSystemResource("keystore");
sslContextFactory.setKeyStoreResource(keystoreResource);
sslContextFactory.setTrustStoreResource(truststoreResource);
sslContextFactory.setKeyStorePassword("storepwd");
sslContextFactory.setKeyManagerPassword("keypwd");
sslContextFactory.setTrustStorePassword("storepwd");
sslContextFactory.start();

KeyManager[] keyManagers = sslContextFactory.getKeyManagers(sslContextFactory.getKeyStore());
X509ExtendedKeyManager x509ExtendedKeyManager = null;

for (KeyManager keyManager : keyManagers)
{
if (keyManager instanceof X509ExtendedKeyManager)
{
x509ExtendedKeyManager = (X509ExtendedKeyManager)keyManager;
break;
}
}
assertThat("Found X509ExtendedKeyManager", x509ExtendedKeyManager, is(notNullValue()));
return x509ExtendedKeyManager;
SslContextFactory baseSsl = new SslContextFactory();
Path keystorePath = MavenTestingUtils.getTestResourcePathFile("keystore_sni.p12");
baseSsl.setKeyStoreResource(new PathResource(keystorePath));
baseSsl.setKeyStorePassword("OBF:1vny1zlo1x8e1vnw1vn61x8g1zlu1vn4");
baseSsl.setKeyManagerPassword("OBF:1u2u1wml1z7s1z7a1wnl1u2g");
IllegalStateException ex = assertThrows(IllegalStateException.class, baseSsl::start);
assertThat("IllegalStateException.message", ex.getMessage(), containsString("KeyStores with multiple certificates are not supported on the base class"));
}

@Test
public void testSniX509ExtendedKeyManager_BaseClass() throws Exception
public void testServerClass_WithSni() throws Exception
{
SslContextFactory baseSsl = new SslContextFactory();
X509ExtendedKeyManager x509ExtendedKeyManager = getX509ExtendedKeyManager(baseSsl);
UnsupportedOperationException ex = assertThrows(UnsupportedOperationException.class, () -> baseSsl.newSniX509ExtendedKeyManager(x509ExtendedKeyManager));
assertThat("UnsupportedOperationException.message", ex.getMessage(), containsString("X509ExtendedKeyManager only supported on " + SslContextFactory.Server.class.getName()));
SslContextFactory serverSsl = new SslContextFactory.Server();
Path keystorePath = MavenTestingUtils.getTestResourcePathFile("keystore_sni.p12");
serverSsl.setKeyStoreResource(new PathResource(keystorePath));
serverSsl.setKeyStorePassword("OBF:1vny1zlo1x8e1vnw1vn61x8g1zlu1vn4");
serverSsl.setKeyManagerPassword("OBF:1u2u1wml1z7s1z7a1wnl1u2g");
serverSsl.start();
}

@Test
public void testSniX509ExtendedKeyManager_BaseClass_Start() throws Exception
public void testClientClass_WithSni() throws Exception
{
SslContextFactory baseSsl = new SslContextFactory();
SslContextFactory clientSsl = new SslContextFactory.Client();
Path keystorePath = MavenTestingUtils.getTestResourcePathFile("keystore_sni.p12");
baseSsl.setKeyStoreResource(new PathResource(keystorePath));
baseSsl.setKeyStorePassword("OBF:1vny1zlo1x8e1vnw1vn61x8g1zlu1vn4");
baseSsl.setKeyManagerPassword("OBF:1u2u1wml1z7s1z7a1wnl1u2g");
baseSsl.start(); // should not throw an exception
clientSsl.setKeyStoreResource(new PathResource(keystorePath));
clientSsl.setKeyStorePassword("OBF:1vny1zlo1x8e1vnw1vn61x8g1zlu1vn4");
clientSsl.setKeyManagerPassword("OBF:1u2u1wml1z7s1z7a1wnl1u2g");
clientSsl.start();
}

@Test
public void testSniX509ExtendedKeyManager_ClientClass() throws Exception
public void testBaseClass_WithoutSni() throws Exception
{
SslContextFactory clientSsl = new SslContextFactory.Client();
X509ExtendedKeyManager x509ExtendedKeyManager = getX509ExtendedKeyManager(clientSsl);
UnsupportedOperationException ex = assertThrows(UnsupportedOperationException.class, () -> clientSsl.newSniX509ExtendedKeyManager(x509ExtendedKeyManager));
assertThat("SNI X509 ExtendedKeyManager is unsupported in Client mode", ex.getMessage(), containsString("X509ExtendedKeyManager only supported on " + SslContextFactory.Server.class.getName()));
SslContextFactory baseSsl = new SslContextFactory();
Resource keystoreResource = Resource.newSystemResource("keystore");
baseSsl.setKeyStoreResource(keystoreResource);
baseSsl.setKeyStorePassword("storepwd");
baseSsl.setKeyManagerPassword("keypwd");
baseSsl.start();
}

@Test
public void testSniX509ExtendedKeyManager_ServerClass() throws Exception
public void testServerClass_WithoutSni() throws Exception
{
SslContextFactory serverSsl = new SslContextFactory.Server();
X509ExtendedKeyManager x509ExtendedKeyManager = getX509ExtendedKeyManager(serverSsl);
serverSsl.newSniX509ExtendedKeyManager(x509ExtendedKeyManager);
Resource keystoreResource = Resource.newSystemResource("keystore");
serverSsl.setKeyStoreResource(keystoreResource);
serverSsl.setKeyStorePassword("storepwd");
serverSsl.setKeyManagerPassword("keypwd");
serverSsl.start();
}

@Test
public void testClientClass_WithoutSni() throws Exception
{
SslContextFactory clientSsl = new SslContextFactory.Client();
Resource keystoreResource = Resource.newSystemResource("keystore");
clientSsl.setKeyStoreResource(keystoreResource);
clientSsl.setKeyStorePassword("storepwd");
clientSsl.setKeyManagerPassword("keypwd");
clientSsl.start();
}
}

0 comments on commit 53073ca

Please sign in to comment.