Skip to content

Commit

Permalink
[UNDERTOW-2212] CVE-2022-4492 Server identity in https connection is …
Browse files Browse the repository at this point in the history 
…not checked by the undertow client

Signed-off-by: Flavia Rainone <frainone@redhat.com>
  • Loading branch information
@rmartinc @fl4via
rmartinc authored and fl4via committed Mar 26, 2023
1 parent fdaaa3b commit a4d3b16
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 14 deletions.
2 changes: 1 addition & 1 deletion core/src/main/java/io/undertow/UndertowOptions.java
View file
Expand Up @@ -358,7 +358,7 @@ public class UndertowOptions {
public static final Option<Integer> 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)
*/
Expand Down
29 changes: 27 additions & 2 deletions core/src/main/java/io/undertow/client/http/HttpClientProvider.java
View file
Expand Up @@ -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;
Expand All @@ -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<String>() {
@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<String> handlesSchemes() {
return new HashSet<>(Arrays.asList(new String[]{"http", "https"}));
Expand All @@ -72,7 +89,11 @@ public void connect(ClientCallback<ClientConnection> 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 {
Expand All @@ -94,7 +115,11 @@ public void connect(ClientCallback<ClientConnection> 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 {
Expand Down
19 changes: 14 additions & 5 deletions core/src/main/java/io/undertow/client/http2/Http2ClientProvider.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -78,7 +79,7 @@ public void connect(final ClientCallback<ClientConnection> listener, final URI u

@Override
public Set<String> handlesSchemes() {
return new HashSet<>(Arrays.asList(new String[]{"h2"}));
return new HashSet<>(Arrays.asList(new String[]{HTTP2}));
}

@Override
Expand All @@ -87,7 +88,11 @@ public void connect(final ClientCallback<ClientConnection> 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 {
Expand All @@ -102,11 +107,15 @@ public void connect(final ClientCallback<ClientConnection> 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);
}

}
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/io/undertow/protocols/ssl/UndertowXnioSsl.java
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down
37 changes: 35 additions & 2 deletions core/src/test/java/io/undertow/client/http/Http2ClientTestCase.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -80,6 +83,8 @@ public class Http2ClientTestCase {

private static final AttachmentKey<String> RESPONSE_BODY = AttachmentKey.create(String.class);

private IOException exception;

static {
final OptionMap.Builder builder = OptionMap.builder()
.set(Options.WORKER_IO_THREADS, 8)
Expand Down Expand Up @@ -195,6 +200,32 @@ public void run() {
}
}

@Test
public void testH2ServerIdentity() throws Exception {
final UndertowClient client = createClient();
exception = null;

final List<ClientResponse> 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 {
Expand Down Expand Up @@ -317,7 +348,7 @@ protected void stringDone(String string) {
@Override
protected void error(IOException e) {
e.printStackTrace();

exception = e;
latch.countDown();
}
}.setup(result.getResponseChannel());
Expand All @@ -326,7 +357,7 @@ protected void error(IOException e) {
@Override
public void failed(IOException e) {
e.printStackTrace();

exception = e;
latch.countDown();
}
});
Expand All @@ -338,13 +369,15 @@ public void failed(IOException e) {
}
} catch (IOException e) {
e.printStackTrace();
exception = e;
latch.countDown();
}
}

@Override
public void failed(IOException e) {
e.printStackTrace();
exception = e;
latch.countDown();
}
};
Expand Down
4 changes: 2 additions & 2 deletions core/src/test/java/io/undertow/client/http/HttpClientSNITestCase.java
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
38 changes: 38 additions & 0 deletions core/src/test/java/io/undertow/client/http/HttpClientTestCase.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -330,6 +333,41 @@ public void run() {
}
}

@Test
public void testSslServerIdentity() throws Exception {
final UndertowClient client = createClient();
exception = null;

final List<ClientResponse> 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 {
Expand Down

0 comments on commit a4d3b16

Please sign in to comment.