Issue #4325 - X509ExtendedKeyManager exceptions on non-Server SSL

Signed-off-by: Joakim Erdfelt <[email protected]>
jetty · Nov 18, 2019 · 55ad107 · 55ad107
1 parent bf2482a
commit 55ad107
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 1 deletion.
diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SniX509ExtendedKeyManager.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SniX509ExtendedKeyManager.java
@@ -50,6 +50,10 @@ public class SniX509ExtendedKeyManager extends X509ExtendedKeyManager
     private final X509ExtendedKeyManager _delegate;
     private final SslContextFactory.Server _sslContextFactory;
 
+    /**
+     * @deprecated not supported, you must have a {@link SslContextFactory.Server} for this to work.
+     */
+    @Deprecated
     public SniX509ExtendedKeyManager(X509ExtendedKeyManager keyManager)
     {
         this(keyManager, null);
@@ -58,7 +62,7 @@ public SniX509ExtendedKeyManager(X509ExtendedKeyManager keyManager)
     public SniX509ExtendedKeyManager(X509ExtendedKeyManager keyManager, SslContextFactory.Server sslContextFactory)
     {
         _delegate = keyManager;
-        _sslContextFactory = sslContextFactory;
+        _sslContextFactory = Objects.requireNonNull(sslContextFactory, "SslContextFactory.Server must be provided");
     }
 
     @Override

diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java
@@ -1264,8 +1264,13 @@ protected KeyManager[] getKeyManagers(KeyStore keyStore) throws Exception
         return managers;
     }
 
+    /**
+     * @deprecated use {@link SslContextFactory.Server#newSniX509ExtendedKeyManager(X509ExtendedKeyManager)} instead
+     */
+    @Deprecated
     protected X509ExtendedKeyManager newSniX509ExtendedKeyManager(X509ExtendedKeyManager keyManager)
     {
+        // Will throw a NPE.
         return new SniX509ExtendedKeyManager(keyManager);
     }
 
@@ -2174,6 +2179,16 @@ protected void checkConfiguration()
             checkEndPointIdentificationAlgorithm();
             super.checkConfiguration();
         }
+
+        /**
+         * @deprecated Not supported on Client, only {@link SslContextFactory.Server}
+         */
+        @Deprecated
+        @Override
+        protected X509ExtendedKeyManager newSniX509ExtendedKeyManager(X509ExtendedKeyManager keyManager)
+        {
+            throw new RuntimeException("X509ExtendedKeyManager not supported on Client");
+        }
     }
 
     @ManagedObject

diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/ssl/X509Test.java b/jetty-util/src/test/java/org/eclipse/jetty/util/ssl/X509Test.java
@@ -19,11 +19,17 @@
 package org.eclipse.jetty.util.ssl;
 
 import java.security.cert.X509Certificate;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.X509ExtendedKeyManager;
 
+import org.eclipse.jetty.util.resource.Resource;
 import org.junit.jupiter.api.Test;
 
 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
 {
@@ -123,4 +129,56 @@ public boolean[] getKeyUsage()
 
         assertThat("Normal X509", X509.isCertSign(bogusX509), is(false));
     }
+
+    private X509ExtendedKeyManager getX509ExtendedKeyManager(SslContextFactory sslContextFactory) throws Exception
+    {
+        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;
+    }
+
+    @Test
+    public void testSniX509ExtendedKeyManager_BaseClass() throws Exception
+    {
+        SslContextFactory base = new SslContextFactory();
+        X509ExtendedKeyManager x509ExtendedKeyManager = getX509ExtendedKeyManager(base);
+        NullPointerException npe = assertThrows(NullPointerException.class, () -> base.newSniX509ExtendedKeyManager(x509ExtendedKeyManager));
+        assertThat("NullPointerException.message", npe.getMessage(), containsString("SslContextFactory.Server"));
+    }
+
+    @Test
+    public void testSniX509ExtendedKeyManager_ClientClass() throws Exception
+    {
+        SslContextFactory base = new SslContextFactory.Client();
+        X509ExtendedKeyManager x509ExtendedKeyManager = getX509ExtendedKeyManager(base);
+        RuntimeException re = assertThrows(RuntimeException.class, () -> base.newSniX509ExtendedKeyManager(x509ExtendedKeyManager));
+        assertThat("RuntimeException.message", re.getMessage(), containsString("X509ExtendedKeyManager not supported on Client"));
+    }
+
+    @Test
+    public void testSniX509ExtendedKeyManager_ServerClass() throws Exception
+    {
+        SslContextFactory base = new SslContextFactory.Server();
+        X509ExtendedKeyManager x509ExtendedKeyManager = getX509ExtendedKeyManager(base);
+        base.newSniX509ExtendedKeyManager(x509ExtendedKeyManager);
+    }
 }