diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SniSslConnectionFactoryTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SniSslConnectionFactoryTest.java index 5ca56f4a60ce..8e11b802bc23 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SniSslConnectionFactoryTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SniSslConnectionFactoryTest.java @@ -44,6 +44,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.io.Connection; @@ -123,21 +124,23 @@ public void before() protected void start(String keystorePath) throws Exception { - start(ssl -> ssl.setKeyStorePath(keystorePath)); + start(ssl -> + { + ssl.setKeyStorePath(keystorePath); + ssl.setKeyManagerPassword("keypwd"); + }); } protected void start(Consumer sslConfig) throws Exception { SslContextFactory.Server sslContextFactory = new SslContextFactory.Server(); + sslContextFactory.setKeyStorePassword("storepwd"); sslConfig.accept(sslContextFactory); File keystoreFile = sslContextFactory.getKeyStoreResource().getFile(); if (!keystoreFile.exists()) throw new FileNotFoundException(keystoreFile.getAbsolutePath()); - sslContextFactory.setKeyStorePassword("OBF:1vny1zlo1x8e1vnw1vn61x8g1zlu1vn4"); - sslContextFactory.setKeyManagerPassword("OBF:1u2u1wml1z7s1z7a1wnl1u2g"); - ServerConnector https = _connector = new ServerConnector(_server, new SslConnectionFactory(sslContextFactory, HttpVersion.HTTP_1_1.asString()), new HttpConnectionFactory(_httpsConfiguration)); @@ -194,6 +197,7 @@ public void testSNIConnect() throws Exception start(ssl -> { ssl.setKeyStorePath("src/test/resources/keystore_sni.p12"); + ssl.setKeyManagerPassword("OBF:1u2u1wml1z7s1z7a1wnl1u2g"); ssl.setSNISelector((keyType, issuers, session, sniHost, certificates) -> { // Make sure the *.domain.com comes before sub.domain.com @@ -264,6 +268,7 @@ public void testWrongSNIRejectedConnection() throws Exception start(ssl -> { ssl.setKeyStorePath("src/test/resources/keystore_sni.p12"); + ssl.setKeyManagerPassword("OBF:1u2u1wml1z7s1z7a1wnl1u2g"); // Do not allow unmatched SNI. ssl.setSniRequired(true); }); @@ -281,6 +286,7 @@ public void testWrongSNIRejectedBadRequest() throws Exception start(ssl -> { ssl.setKeyStorePath("src/test/resources/keystore_sni.p12"); + ssl.setKeyManagerPassword("OBF:1u2u1wml1z7s1z7a1wnl1u2g"); // Do not allow unmatched SNI. ssl.setSniRequired(false); _httpsConfiguration.getCustomizers().stream() @@ -307,6 +313,7 @@ public void testWrongSNIRejectedFunction() throws Exception start(ssl -> { ssl.setKeyStorePath("src/test/resources/keystore_sni.p12"); + ssl.setKeyManagerPassword("OBF:1u2u1wml1z7s1z7a1wnl1u2g"); // Do not allow unmatched SNI. ssl.setSniRequired(true); ssl.setSNISelector((keyType, issuers, session, sniHost, certificates) -> @@ -338,6 +345,7 @@ public void testWrongSNIRejectedConnectionWithNonSNIKeystore() throws Exception { // Keystore has only one certificate, but we want to enforce SNI. ssl.setKeyStorePath("src/test/resources/keystore"); + ssl.setKeyManagerPassword("OBF:1u2u1wml1z7s1z7a1wnl1u2g"); ssl.setSniRequired(true); }); @@ -519,6 +527,21 @@ protected void customize(Socket socket, Class connection, assertEquals(0, history.size()); } + @Test + public void testSNIWithDifferentKeyTypes() throws Exception + { + // This KeyStore contains 2 certificates, one with keyAlg=EC, one with keyAlg=RSA. + start(ssl -> ssl.setKeyStorePath("src/test/resources/keystore_sni_key_types.p12")); + + // Make a request with SNI = rsa.domain.com, the RSA certificate should be chosen. + HttpTester.Response response1 = HttpTester.parseResponse(getResponse("rsa.domain.com", "rsa.domain.com")); + assertEquals(HttpStatus.OK_200, response1.getStatus()); + + // Make a request with SNI = ec.domain.com, the EC certificate should be chosen. + HttpTester.Response response2 = HttpTester.parseResponse(getResponse("ec.domain.com", "ec.domain.com")); + assertEquals(HttpStatus.OK_200, response2.getStatus()); + } + private String getResponse(String host, String cn) throws Exception { String response = getResponse(host, host, cn); diff --git a/jetty-server/src/test/resources/keystore_sni_key_types.p12 b/jetty-server/src/test/resources/keystore_sni_key_types.p12 new file mode 100644 index 000000000000..b000fbeff573 Binary files /dev/null and b/jetty-server/src/test/resources/keystore_sni_key_types.p12 differ 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 index 72cce1f6dc5d..c178ad1b6ba4 100644 --- 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 @@ -245,7 +245,7 @@ public interface SniSelector *

Selects a certificate based on SNI information.

*

This method may be invoked multiple times during the TLS handshake, with different parameters. * For example, the {@code keyType} could be different, and subsequently the collection of certificates - * (because they need to match the {@code keyType}.

+ * (because they need to match the {@code keyType}).

* * @param keyType the key algorithm type name * @param issuers the list of acceptable CA issuer subject names or null if it does not matter which issuers are used 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 index 209d69d39889..0da182e7cbfb 100644 --- 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 @@ -479,6 +479,11 @@ private void unload() _certWilds.clear(); } + Map aliasCerts() + { + return _aliasX509; + } + @ManagedAttribute(value = "The selected TLS protocol versions", readonly = true) public String[] getSelectedProtocols() { @@ -2118,7 +2123,7 @@ public String toString() _trustStoreResource); } - class Factory + private static class Factory { private final KeyStore _keyStore; private final KeyStore _trustStore; @@ -2133,7 +2138,7 @@ class Factory } } - class AliasSNIMatcher extends SNIMatcher + static class AliasSNIMatcher extends SNIMatcher { private String _host; @@ -2235,11 +2240,17 @@ public void setNeedClientAuth(boolean needClientAuth) } /** - * Does the default {@link #sniSelect(String, Principal[], SSLSession, String, Collection)} implementation - * require an SNI match? Note that if a non SNI handshake is accepted, requests may still be rejected - * at the HTTP level for incorrect SNI (see SecureRequestCustomizer). - * @return true if no SNI match is handled as no certificate match, false if no SNI match is handled by - * delegation to the non SNI matching methods. + *

Returns whether an SNI match is required when choosing the alias that + * identifies the certificate to send to the client.

+ *

The exact logic to choose an alias given the SNI is configurable via + * {@link #setSNISelector(SniX509ExtendedKeyManager.SniSelector)}.

+ *

The default implementation is {@link #sniSelect(String, Principal[], SSLSession, String, Collection)} + * and if SNI is not required it will delegate the TLS implementation to + * choose an alias (typically the first alias in the KeyStore).

+ *

Note that if a non SNI handshake is accepted, requests may still be rejected + * at the HTTP level for incorrect SNI (see SecureRequestCustomizer).

+ * + * @return whether an SNI match is required when choosing the alias that identifies the certificate */ @ManagedAttribute("Whether the TLS handshake is rejected if there is no SNI host match") public boolean isSniRequired() @@ -2248,13 +2259,12 @@ public boolean isSniRequired() } /** - * Set if the default {@link #sniSelect(String, Principal[], SSLSession, String, Collection)} implementation - * require an SNI match? Note that if a non SNI handshake is accepted, requests may still be rejected - * at the HTTP level for incorrect SNI (see SecureRequestCustomizer). - * This setting may have no effect if {@link #sniSelect(String, Principal[], SSLSession, String, Collection)} is - * overridden or a non null function is passed to {@link #setSNISelector(SniX509ExtendedKeyManager.SniSelector)}. - * @param sniRequired true if no SNI match is handled as no certificate match, false if no SNI match is handled by - * delegation to the non SNI matching methods. + *

Sets whether an SNI match is required when choosing the alias that + * identifies the certificate to send to the client.

+ *

This setting may have no effect if {@link #sniSelect(String, Principal[], SSLSession, String, Collection)} is + * overridden or a custom function is passed to {@link #setSNISelector(SniX509ExtendedKeyManager.SniSelector)}.

+ * + * @param sniRequired whether an SNI match is required when choosing the alias that identifies the certificate */ public void setSniRequired(boolean sniRequired) { @@ -2297,7 +2307,7 @@ public String sniSelect(String keyType, Principal[] issuers, SSLSession session, if (sniHost == null) { // No SNI, so reject or delegate. - return _sniRequired ? null : SniX509ExtendedKeyManager.SniSelector.DELEGATE; + return isSniRequired() ? null : SniX509ExtendedKeyManager.SniSelector.DELEGATE; } else { @@ -2306,9 +2316,15 @@ public String sniSelect(String keyType, Principal[] issuers, SSLSession session, .filter(x509 -> x509.matches(sniHost)) .collect(Collectors.toList()); - // No match, let the JDK decide unless unmatched SNIs are rejected. if (matching.isEmpty()) - return isSniRequired() ? null : SniX509ExtendedKeyManager.SniSelector.DELEGATE; + { + // There is no match for this SNI among the certificates valid for + // this keyType; check if there is any certificate that matches this + // SNI, as we will likely be called again with a different keyType. + boolean anyMatching = aliasCerts().values().stream() + .anyMatch(x509 -> x509.matches(sniHost)); + return isSniRequired() || anyMatching ? null : SniX509ExtendedKeyManager.SniSelector.DELEGATE; + } String alias = matching.get(0).getAlias(); if (matching.size() == 1)