From a4d3b167126a803cc4f7fb740dd9a6ecabf59342 Mon Sep 17 00:00:00 2001 From: rmartinc Date: Wed, 14 Dec 2022 09:27:20 +0100 Subject: [PATCH] [UNDERTOW-2212] CVE-2022-4492 Server identity in https connection is not checked by the undertow client Signed-off-by: Flavia Rainone --- .../java/io/undertow/UndertowOptions.java | 2 +- .../client/http/HttpClientProvider.java | 29 +++++++++++++- .../client/http2/Http2ClientProvider.java | 19 +++++++--- .../protocols/ssl/UndertowXnioSsl.java | 4 +- .../client/http/Http2ClientTestCase.java | 37 +++++++++++++++++- .../client/http/HttpClientSNITestCase.java | 4 +- .../client/http/HttpClientTestCase.java | 38 +++++++++++++++++++ 7 files changed, 119 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/io/undertow/UndertowOptions.java b/core/src/main/java/io/undertow/UndertowOptions.java index e1cf5ea5c6..57f051350a 100644 --- a/core/src/main/java/io/undertow/UndertowOptions.java +++ b/core/src/main/java/io/undertow/UndertowOptions.java @@ -358,7 +358,7 @@ public class UndertowOptions { public static final Option SHUTDOWN_TIMEOUT = Option.simple(UndertowOptions.class, "SHUTDOWN_TIMEOUT", Integer.class); /** - * The endpoint identification algorithm. + * The endpoint identification algorithm, the empty string can be used to set none. * * @see javax.net.ssl.SSLParameters#setEndpointIdentificationAlgorithm(String) */ diff --git a/core/src/main/java/io/undertow/client/http/HttpClientProvider.java b/core/src/main/java/io/undertow/client/http/HttpClientProvider.java index b9dffabcf4..8699b2bde4 100644 --- a/core/src/main/java/io/undertow/client/http/HttpClientProvider.java +++ b/core/src/main/java/io/undertow/client/http/HttpClientProvider.java @@ -39,6 +39,8 @@ import java.io.IOException; import java.net.InetSocketAddress; import java.net.URI; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; @@ -50,6 +52,21 @@ */ public class HttpClientProvider implements ClientProvider { + public static final String DISABLE_HTTPS_ENDPOINT_IDENTIFICATION_PROPERTY = "io.undertow.client.https.disableEndpointIdentification"; + public static final boolean DISABLE_HTTPS_ENDPOINT_IDENTIFICATION; + + static { + String disable = System.getSecurityManager() == null + ? System.getProperty(DISABLE_HTTPS_ENDPOINT_IDENTIFICATION_PROPERTY) + : AccessController.doPrivileged(new PrivilegedAction() { + @Override + public String run() { + return System.getProperty(DISABLE_HTTPS_ENDPOINT_IDENTIFICATION_PROPERTY); + } + }); + DISABLE_HTTPS_ENDPOINT_IDENTIFICATION = disable != null && (disable.isEmpty() || Boolean.parseBoolean(disable)); + } + @Override public Set handlesSchemes() { return new HashSet<>(Arrays.asList(new String[]{"http", "https"})); @@ -72,7 +89,11 @@ public void connect(ClientCallback listener, InetSocketAddress listener.failed(UndertowMessages.MESSAGES.sslWasNull()); return; } - OptionMap tlsOptions = OptionMap.builder().addAll(options).set(Options.SSL_STARTTLS, true).getMap(); + OptionMap tlsOptions = OptionMap.builder() + .set(UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM, DISABLE_HTTPS_ENDPOINT_IDENTIFICATION? "" : "HTTPS") + .addAll(options) + .set(Options.SSL_STARTTLS, true) + .getMap(); if (bindAddress == null) { ssl.openSslConnection(worker, new InetSocketAddress(uri.getHost(), uri.getPort() == -1 ? 443 : uri.getPort()), createOpenListener(listener, bufferPool, tlsOptions, uri), tlsOptions).addNotifier(createNotifier(listener), null); } else { @@ -94,7 +115,11 @@ public void connect(ClientCallback listener, InetSocketAddress listener.failed(UndertowMessages.MESSAGES.sslWasNull()); return; } - OptionMap tlsOptions = OptionMap.builder().addAll(options).set(Options.SSL_STARTTLS, true).getMap(); + OptionMap tlsOptions = OptionMap.builder() + .set(UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM, DISABLE_HTTPS_ENDPOINT_IDENTIFICATION? "" : "HTTPS") + .addAll(options) + .set(Options.SSL_STARTTLS, true) + .getMap(); if (bindAddress == null) { ssl.openSslConnection(ioThread, new InetSocketAddress(uri.getHost(), uri.getPort() == -1 ? 443 : uri.getPort()), createOpenListener(listener, bufferPool, tlsOptions, uri), tlsOptions).addNotifier(createNotifier(listener), null); } else { diff --git a/core/src/main/java/io/undertow/client/http2/Http2ClientProvider.java b/core/src/main/java/io/undertow/client/http2/Http2ClientProvider.java index dd9dac43c5..11dc7eb81d 100644 --- a/core/src/main/java/io/undertow/client/http2/Http2ClientProvider.java +++ b/core/src/main/java/io/undertow/client/http2/Http2ClientProvider.java @@ -26,6 +26,7 @@ import io.undertow.client.ClientConnection; import io.undertow.client.ClientProvider; import io.undertow.client.ClientStatistics; +import io.undertow.client.http.HttpClientProvider; import io.undertow.conduits.ByteActivityCallback; import io.undertow.conduits.BytesReceivedStreamSourceConduit; import io.undertow.conduits.BytesSentStreamSinkConduit; @@ -78,7 +79,7 @@ public void connect(final ClientCallback listener, final URI u @Override public Set handlesSchemes() { - return new HashSet<>(Arrays.asList(new String[]{"h2"})); + return new HashSet<>(Arrays.asList(new String[]{HTTP2})); } @Override @@ -87,7 +88,11 @@ public void connect(final ClientCallback listener, InetSocketA listener.failed(UndertowMessages.MESSAGES.sslWasNull()); return; } - OptionMap tlsOptions = OptionMap.builder().addAll(options).set(Options.SSL_STARTTLS, true).getMap(); + OptionMap tlsOptions = OptionMap.builder() + .set(UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM, HttpClientProvider.DISABLE_HTTPS_ENDPOINT_IDENTIFICATION? "" : "HTTPS") + .addAll(options) + .set(Options.SSL_STARTTLS, true) + .getMap(); if(bindAddress == null) { ssl.openSslConnection(worker, new InetSocketAddress(uri.getHost(), uri.getPort() == -1 ? 443 : uri.getPort()), createOpenListener(listener, uri, ssl, bufferPool, tlsOptions), tlsOptions).addNotifier(createNotifier(listener), null); } else { @@ -102,11 +107,15 @@ public void connect(final ClientCallback listener, InetSocketA listener.failed(UndertowMessages.MESSAGES.sslWasNull()); return; } + OptionMap tlsOptions = OptionMap.builder() + .set(UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM, HttpClientProvider.DISABLE_HTTPS_ENDPOINT_IDENTIFICATION? "" : "HTTPS") + .addAll(options) + .set(Options.SSL_STARTTLS, true) + .getMap(); if(bindAddress == null) { - OptionMap tlsOptions = OptionMap.builder().addAll(options).set(Options.SSL_STARTTLS, true).getMap(); - ssl.openSslConnection(ioThread, new InetSocketAddress(uri.getHost(), uri.getPort() == -1 ? 443 : uri.getPort()), createOpenListener(listener, uri, ssl, bufferPool, tlsOptions), options).addNotifier(createNotifier(listener), null); + ssl.openSslConnection(ioThread, new InetSocketAddress(uri.getHost(), uri.getPort() == -1 ? 443 : uri.getPort()), createOpenListener(listener, uri, ssl, bufferPool, tlsOptions), tlsOptions).addNotifier(createNotifier(listener), null); } else { - ssl.openSslConnection(ioThread, bindAddress, new InetSocketAddress(uri.getHost(), uri.getPort() == -1 ? 443 : uri.getPort()), createOpenListener(listener, uri, ssl, bufferPool, options), options).addNotifier(createNotifier(listener), null); + ssl.openSslConnection(ioThread, bindAddress, new InetSocketAddress(uri.getHost(), uri.getPort() == -1 ? 443 : uri.getPort()), createOpenListener(listener, uri, ssl, bufferPool, tlsOptions), tlsOptions).addNotifier(createNotifier(listener), null); } } diff --git a/core/src/main/java/io/undertow/protocols/ssl/UndertowXnioSsl.java b/core/src/main/java/io/undertow/protocols/ssl/UndertowXnioSsl.java index 7bef394d46..c03645219a 100644 --- a/core/src/main/java/io/undertow/protocols/ssl/UndertowXnioSsl.java +++ b/core/src/main/java/io/undertow/protocols/ssl/UndertowXnioSsl.java @@ -323,7 +323,7 @@ private static SSLEngine createSSLEngine(SSLContext sslContext, OptionMap option sslParameters.setUseCipherSuitesOrder(true); engine.setSSLParameters(sslParameters); } - final String endpointIdentificationAlgorithm = optionMap.get(UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM, null); + final String endpointIdentificationAlgorithm = optionMap.get(UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM); if (endpointIdentificationAlgorithm != null) { SSLParameters sslParameters = engine.getSSLParameters(); sslParameters.setEndpointIdentificationAlgorithm(endpointIdentificationAlgorithm); @@ -484,7 +484,7 @@ public void handleEvent(final StreamConnection connection) { SSLEngine sslEngine = JsseSslUtils.createSSLEngine(sslContext, optionMap, destination); SSLParameters params = sslEngine.getSSLParameters(); setSNIHostName(destination, optionMap, params); - final String endpointIdentificationAlgorithm = optionMap.get(UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM, null); + final String endpointIdentificationAlgorithm = optionMap.get(UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM); if (endpointIdentificationAlgorithm != null) { params.setEndpointIdentificationAlgorithm(endpointIdentificationAlgorithm); } diff --git a/core/src/test/java/io/undertow/client/http/Http2ClientTestCase.java b/core/src/test/java/io/undertow/client/http/Http2ClientTestCase.java index 2f740b32ab..e26da3be11 100644 --- a/core/src/test/java/io/undertow/client/http/Http2ClientTestCase.java +++ b/core/src/test/java/io/undertow/client/http/Http2ClientTestCase.java @@ -16,8 +16,11 @@ package io.undertow.client.http; import java.io.IOException; +import java.net.Inet6Address; +import java.net.InetAddress; import java.net.URI; import java.net.URISyntaxException; +import java.nio.channels.ClosedChannelException; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; @@ -80,6 +83,8 @@ public class Http2ClientTestCase { private static final AttachmentKey RESPONSE_BODY = AttachmentKey.create(String.class); + private IOException exception; + static { final OptionMap.Builder builder = OptionMap.builder() .set(Options.WORKER_IO_THREADS, 8) @@ -195,6 +200,32 @@ public void run() { } } + @Test + public void testH2ServerIdentity() throws Exception { + final UndertowClient client = createClient(); + exception = null; + + final List responses = new CopyOnWriteArrayList<>(); + final CountDownLatch latch = new CountDownLatch(1); + InetAddress address = InetAddress.getByName(ADDRESS.getHost()); + String hostname = address instanceof Inet6Address? "[" + address.getHostAddress() + "]" : address.getHostAddress(); + URI uri = new URI("h2", ADDRESS.getUserInfo(), hostname, ADDRESS.getPort(), ADDRESS.getPath(), ADDRESS.getQuery(), ADDRESS.getFragment()); + final ClientConnection connection = client.connect(uri, worker, new UndertowXnioSsl(worker.getXnio(), OptionMap.EMPTY, DefaultServer.getClientSSLContext()), DefaultServer.getBufferPool(), OptionMap.create(UndertowOptions.ENABLE_HTTP2, true)).get(); + try { + connection.getIoThread().execute(() -> { + final ClientRequest request = new ClientRequest().setMethod(Methods.GET).setPath(MESSAGE); + request.getRequestHeaders().put(Headers.HOST, DefaultServer.getHostAddress()); + connection.sendRequest(request, createClientCallback(responses, latch)); + }); + + latch.await(10, TimeUnit.SECONDS); + + Assert.assertEquals(0, responses.size()); + Assert.assertTrue(exception instanceof ClosedChannelException); + } finally { + IoUtils.safeClose(connection); + } + } @Test public void testHeadRequest() throws Exception { @@ -317,7 +348,7 @@ protected void stringDone(String string) { @Override protected void error(IOException e) { e.printStackTrace(); - + exception = e; latch.countDown(); } }.setup(result.getResponseChannel()); @@ -326,7 +357,7 @@ protected void error(IOException e) { @Override public void failed(IOException e) { e.printStackTrace(); - + exception = e; latch.countDown(); } }); @@ -338,6 +369,7 @@ public void failed(IOException e) { } } catch (IOException e) { e.printStackTrace(); + exception = e; latch.countDown(); } } @@ -345,6 +377,7 @@ public void failed(IOException e) { @Override public void failed(IOException e) { e.printStackTrace(); + exception = e; latch.countDown(); } }; diff --git a/core/src/test/java/io/undertow/client/http/HttpClientSNITestCase.java b/core/src/test/java/io/undertow/client/http/HttpClientSNITestCase.java index dcd9e0b91c..41aec2e043 100644 --- a/core/src/test/java/io/undertow/client/http/HttpClientSNITestCase.java +++ b/core/src/test/java/io/undertow/client/http/HttpClientSNITestCase.java @@ -183,7 +183,7 @@ public void testNoSNIWhenIP() throws Exception { // connect using the IP, no SNI expected final ClientConnection connection = client.connect(new URI("https://" + hostname + ":" + ADDRESS.getPort()), worker, new UndertowXnioSsl(worker.getXnio(), OptionMap.EMPTY, DefaultServer.createClientSslContext()), - DefaultServer.getBufferPool(), OptionMap.EMPTY).get(); + DefaultServer.getBufferPool(), OptionMap.create(UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM, "")).get(); try { connection.getIoThread().execute(() -> { final ClientRequest request = new ClientRequest().setMethod(Methods.GET).setPath(SNI); @@ -244,7 +244,7 @@ public void testForcingSNIForHostname() throws Exception { // connect using hostname but add option to another hostname, SNI expected to the forced one final ClientConnection connection = client.connect(new URI("https://" + address.getHostName() + ":" + ADDRESS.getPort()), worker, new UndertowXnioSsl(worker.getXnio(), OptionMap.EMPTY, DefaultServer.createClientSslContext()), - DefaultServer.getBufferPool(), OptionMap.create(UndertowOptions.SSL_SNI_HOSTNAME, "server")).get(); + DefaultServer.getBufferPool(), OptionMap.create(UndertowOptions.SSL_SNI_HOSTNAME, "server", UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM, "")).get(); try { connection.getIoThread().execute(() -> { final ClientRequest request = new ClientRequest().setMethod(Methods.GET).setPath(SNI); diff --git a/core/src/test/java/io/undertow/client/http/HttpClientTestCase.java b/core/src/test/java/io/undertow/client/http/HttpClientTestCase.java index 05ab3bbc49..c3cec40753 100644 --- a/core/src/test/java/io/undertow/client/http/HttpClientTestCase.java +++ b/core/src/test/java/io/undertow/client/http/HttpClientTestCase.java @@ -19,9 +19,12 @@ package io.undertow.client.http; import java.io.IOException; +import java.net.Inet6Address; +import java.net.InetAddress; import java.net.URI; import java.net.URISyntaxException; import java.nio.ByteBuffer; +import java.nio.channels.ClosedChannelException; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; @@ -330,6 +333,41 @@ public void run() { } } + @Test + public void testSslServerIdentity() throws Exception { + final UndertowClient client = createClient(); + exception = null; + + final List responses = new CopyOnWriteArrayList<>(); + final CountDownLatch latch = new CountDownLatch(1); + DefaultServer.startSSLServer(); + SSLContext context = DefaultServer.getClientSSLContext(); + XnioSsl ssl = new UndertowXnioSsl(DefaultServer.getWorker().getXnio(), OptionMap.EMPTY, DefaultServer.SSL_BUFFER_POOL, context); + + // change the URI to use the IP instead the "localhost" name set in the certificate + URI uri = new URI(DefaultServer.getDefaultServerSSLAddress()); + InetAddress address = InetAddress.getByName(uri.getHost()); + String hostname = address instanceof Inet6Address? "[" + address.getHostAddress() + "]" : address.getHostAddress(); + uri = new URI(uri.getScheme(), uri.getUserInfo(), hostname, uri.getPort(), uri.getPath(), uri.getQuery(), uri.getFragment()); + + // this should fail as IP alternative name is not set in the certificate + final ClientConnection connection = client.connect(uri, worker, ssl, DefaultServer.getBufferPool(), OptionMap.EMPTY).get(); + try { + connection.getIoThread().execute(() -> { + final ClientRequest request = new ClientRequest().setMethod(Methods.GET).setPath(MESSAGE); + request.getRequestHeaders().put(Headers.HOST, DefaultServer.getHostAddress()); + connection.sendRequest(request, createClientCallback(responses, latch)); + }); + + latch.await(10, TimeUnit.SECONDS); + + Assert.assertEquals(0, responses.size()); + Assert.assertTrue(exception instanceof ClosedChannelException); + } finally { + connection.getIoThread().execute(() -> IoUtils.safeClose(connection)); + DefaultServer.stopSSLServer(); + } + } @Test public void testConnectionClose() throws Exception {