Skip to content

Commit a4d3b16

Browse files
rmartincfl4via
rmartinc
authored and
fl4via
committed
[UNDERTOW-2212] CVE-2022-4492 Server identity in https connection is not checked by the undertow client
Signed-off-by: Flavia Rainone <[email protected]>
1 parent fdaaa3b commit a4d3b16

File tree

7 files changed

+119
-14
lines changed

7 files changed

+119
-14
lines changed

‎core/src/main/java/io/undertow/UndertowOptions.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ public class UndertowOptions {
358358
public static final Option<Integer> SHUTDOWN_TIMEOUT = Option.simple(UndertowOptions.class, "SHUTDOWN_TIMEOUT", Integer.class);
359359

360360
/**
361-
* The endpoint identification algorithm.
361+
* The endpoint identification algorithm, the empty string can be used to set none.
362362
*
363363
* @see javax.net.ssl.SSLParameters#setEndpointIdentificationAlgorithm(String)
364364
*/

‎core/src/main/java/io/undertow/client/http/HttpClientProvider.java‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@
3939
import java.io.IOException;
4040
import java.net.InetSocketAddress;
4141
import java.net.URI;
42+
import java.security.AccessController;
43+
import java.security.PrivilegedAction;
4244
import java.util.ArrayList;
4345
import java.util.Arrays;
4446
import java.util.HashSet;
@@ -50,6 +52,21 @@
5052
*/
5153
public class HttpClientProvider implements ClientProvider {
5254

55+
public static final String DISABLE_HTTPS_ENDPOINT_IDENTIFICATION_PROPERTY = "io.undertow.client.https.disableEndpointIdentification";
56+
public static final boolean DISABLE_HTTPS_ENDPOINT_IDENTIFICATION;
57+
58+
static {
59+
String disable = System.getSecurityManager() == null
60+
? System.getProperty(DISABLE_HTTPS_ENDPOINT_IDENTIFICATION_PROPERTY)
61+
: AccessController.doPrivileged(new PrivilegedAction<String>() {
62+
@Override
63+
public String run() {
64+
return System.getProperty(DISABLE_HTTPS_ENDPOINT_IDENTIFICATION_PROPERTY);
65+
}
66+
});
67+
DISABLE_HTTPS_ENDPOINT_IDENTIFICATION = disable != null && (disable.isEmpty() || Boolean.parseBoolean(disable));
68+
}
69+
5370
@Override
5471
public Set<String> handlesSchemes() {
5572
return new HashSet<>(Arrays.asList(new String[]{"http", "https"}));
@@ -72,7 +89,11 @@ public void connect(ClientCallback<ClientConnection> listener, InetSocketAddress
7289
listener.failed(UndertowMessages.MESSAGES.sslWasNull());
7390
return;
7491
}
75-
OptionMap tlsOptions = OptionMap.builder().addAll(options).set(Options.SSL_STARTTLS, true).getMap();
92+
OptionMap tlsOptions = OptionMap.builder()
93+
.set(UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM, DISABLE_HTTPS_ENDPOINT_IDENTIFICATION? "" : "HTTPS")
94+
.addAll(options)
95+
.set(Options.SSL_STARTTLS, true)
96+
.getMap();
7697
if (bindAddress == null) {
7798
ssl.openSslConnection(worker, new InetSocketAddress(uri.getHost(), uri.getPort() == -1 ? 443 : uri.getPort()), createOpenListener(listener, bufferPool, tlsOptions, uri), tlsOptions).addNotifier(createNotifier(listener), null);
7899
} else {
@@ -94,7 +115,11 @@ public void connect(ClientCallback<ClientConnection> listener, InetSocketAddress
94115
listener.failed(UndertowMessages.MESSAGES.sslWasNull());
95116
return;
96117
}
97-
OptionMap tlsOptions = OptionMap.builder().addAll(options).set(Options.SSL_STARTTLS, true).getMap();
118+
OptionMap tlsOptions = OptionMap.builder()
119+
.set(UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM, DISABLE_HTTPS_ENDPOINT_IDENTIFICATION? "" : "HTTPS")
120+
.addAll(options)
121+
.set(Options.SSL_STARTTLS, true)
122+
.getMap();
98123
if (bindAddress == null) {
99124
ssl.openSslConnection(ioThread, new InetSocketAddress(uri.getHost(), uri.getPort() == -1 ? 443 : uri.getPort()), createOpenListener(listener, bufferPool, tlsOptions, uri), tlsOptions).addNotifier(createNotifier(listener), null);
100125
} else {

‎core/src/main/java/io/undertow/client/http2/Http2ClientProvider.java‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import io.undertow.client.ClientConnection;
2727
import io.undertow.client.ClientProvider;
2828
import io.undertow.client.ClientStatistics;
29+
import io.undertow.client.http.HttpClientProvider;
2930
import io.undertow.conduits.ByteActivityCallback;
3031
import io.undertow.conduits.BytesReceivedStreamSourceConduit;
3132
import io.undertow.conduits.BytesSentStreamSinkConduit;
@@ -78,7 +79,7 @@ public void connect(final ClientCallback<ClientConnection> listener, final URI u
7879

7980
@Override
8081
public Set<String> handlesSchemes() {
81-
return new HashSet<>(Arrays.asList(new String[]{"h2"}));
82+
return new HashSet<>(Arrays.asList(new String[]{HTTP2}));
8283
}
8384

8485
@Override
@@ -87,7 +88,11 @@ public void connect(final ClientCallback<ClientConnection> listener, InetSocketA
8788
listener.failed(UndertowMessages.MESSAGES.sslWasNull());
8889
return;
8990
}
90-
OptionMap tlsOptions = OptionMap.builder().addAll(options).set(Options.SSL_STARTTLS, true).getMap();
91+
OptionMap tlsOptions = OptionMap.builder()
92+
.set(UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM, HttpClientProvider.DISABLE_HTTPS_ENDPOINT_IDENTIFICATION? "" : "HTTPS")
93+
.addAll(options)
94+
.set(Options.SSL_STARTTLS, true)
95+
.getMap();
9196
if(bindAddress == null) {
9297
ssl.openSslConnection(worker, new InetSocketAddress(uri.getHost(), uri.getPort() == -1 ? 443 : uri.getPort()), createOpenListener(listener, uri, ssl, bufferPool, tlsOptions), tlsOptions).addNotifier(createNotifier(listener), null);
9398
} else {
@@ -102,11 +107,15 @@ public void connect(final ClientCallback<ClientConnection> listener, InetSocketA
102107
listener.failed(UndertowMessages.MESSAGES.sslWasNull());
103108
return;
104109
}
110+
OptionMap tlsOptions = OptionMap.builder()
111+
.set(UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM, HttpClientProvider.DISABLE_HTTPS_ENDPOINT_IDENTIFICATION? "" : "HTTPS")
112+
.addAll(options)
113+
.set(Options.SSL_STARTTLS, true)
114+
.getMap();
105115
if(bindAddress == null) {
106-
OptionMap tlsOptions = OptionMap.builder().addAll(options).set(Options.SSL_STARTTLS, true).getMap();
107-
ssl.openSslConnection(ioThread, new InetSocketAddress(uri.getHost(), uri.getPort() == -1 ? 443 : uri.getPort()), createOpenListener(listener, uri, ssl, bufferPool, tlsOptions), options).addNotifier(createNotifier(listener), null);
116+
ssl.openSslConnection(ioThread, new InetSocketAddress(uri.getHost(), uri.getPort() == -1 ? 443 : uri.getPort()), createOpenListener(listener, uri, ssl, bufferPool, tlsOptions), tlsOptions).addNotifier(createNotifier(listener), null);
108117
} else {
109-
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);
118+
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);
110119
}
111120

112121
}

‎core/src/main/java/io/undertow/protocols/ssl/UndertowXnioSsl.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ private static SSLEngine createSSLEngine(SSLContext sslContext, OptionMap option
323323
sslParameters.setUseCipherSuitesOrder(true);
324324
engine.setSSLParameters(sslParameters);
325325
}
326-
final String endpointIdentificationAlgorithm = optionMap.get(UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM, null);
326+
final String endpointIdentificationAlgorithm = optionMap.get(UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM);
327327
if (endpointIdentificationAlgorithm != null) {
328328
SSLParameters sslParameters = engine.getSSLParameters();
329329
sslParameters.setEndpointIdentificationAlgorithm(endpointIdentificationAlgorithm);
@@ -484,7 +484,7 @@ public void handleEvent(final StreamConnection connection) {
484484
SSLEngine sslEngine = JsseSslUtils.createSSLEngine(sslContext, optionMap, destination);
485485
SSLParameters params = sslEngine.getSSLParameters();
486486
setSNIHostName(destination, optionMap, params);
487-
final String endpointIdentificationAlgorithm = optionMap.get(UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM, null);
487+
final String endpointIdentificationAlgorithm = optionMap.get(UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM);
488488
if (endpointIdentificationAlgorithm != null) {
489489
params.setEndpointIdentificationAlgorithm(endpointIdentificationAlgorithm);
490490
}

‎core/src/test/java/io/undertow/client/http/Http2ClientTestCase.java‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,11 @@
1616
package io.undertow.client.http;
1717

1818
import java.io.IOException;
19+
import java.net.Inet6Address;
20+
import java.net.InetAddress;
1921
import java.net.URI;
2022
import java.net.URISyntaxException;
23+
import java.nio.channels.ClosedChannelException;
2124
import java.util.List;
2225
import java.util.concurrent.CopyOnWriteArrayList;
2326
import java.util.concurrent.CountDownLatch;
@@ -80,6 +83,8 @@ public class Http2ClientTestCase {
8083

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

86+
private IOException exception;
87+
8388
static {
8489
final OptionMap.Builder builder = OptionMap.builder()
8590
.set(Options.WORKER_IO_THREADS, 8)
@@ -195,6 +200,32 @@ public void run() {
195200
}
196201
}
197202

203+
@Test
204+
public void testH2ServerIdentity() throws Exception {
205+
final UndertowClient client = createClient();
206+
exception = null;
207+
208+
final List<ClientResponse> responses = new CopyOnWriteArrayList<>();
209+
final CountDownLatch latch = new CountDownLatch(1);
210+
InetAddress address = InetAddress.getByName(ADDRESS.getHost());
211+
String hostname = address instanceof Inet6Address? "[" + address.getHostAddress() + "]" : address.getHostAddress();
212+
URI uri = new URI("h2", ADDRESS.getUserInfo(), hostname, ADDRESS.getPort(), ADDRESS.getPath(), ADDRESS.getQuery(), ADDRESS.getFragment());
213+
final ClientConnection connection = client.connect(uri, worker, new UndertowXnioSsl(worker.getXnio(), OptionMap.EMPTY, DefaultServer.getClientSSLContext()), DefaultServer.getBufferPool(), OptionMap.create(UndertowOptions.ENABLE_HTTP2, true)).get();
214+
try {
215+
connection.getIoThread().execute(() -> {
216+
final ClientRequest request = new ClientRequest().setMethod(Methods.GET).setPath(MESSAGE);
217+
request.getRequestHeaders().put(Headers.HOST, DefaultServer.getHostAddress());
218+
connection.sendRequest(request, createClientCallback(responses, latch));
219+
});
220+
221+
latch.await(10, TimeUnit.SECONDS);
222+
223+
Assert.assertEquals(0, responses.size());
224+
Assert.assertTrue(exception instanceof ClosedChannelException);
225+
} finally {
226+
IoUtils.safeClose(connection);
227+
}
228+
}
198229

199230
@Test
200231
public void testHeadRequest() throws Exception {
@@ -317,7 +348,7 @@ protected void stringDone(String string) {
317348
@Override
318349
protected void error(IOException e) {
319350
e.printStackTrace();
320-
351+
exception = e;
321352
latch.countDown();
322353
}
323354
}.setup(result.getResponseChannel());
@@ -326,7 +357,7 @@ protected void error(IOException e) {
326357
@Override
327358
public void failed(IOException e) {
328359
e.printStackTrace();
329-
360+
exception = e;
330361
latch.countDown();
331362
}
332363
});
@@ -338,13 +369,15 @@ public void failed(IOException e) {
338369
}
339370
} catch (IOException e) {
340371
e.printStackTrace();
372+
exception = e;
341373
latch.countDown();
342374
}
343375
}
344376

345377
@Override
346378
public void failed(IOException e) {
347379
e.printStackTrace();
380+
exception = e;
348381
latch.countDown();
349382
}
350383
};

‎core/src/test/java/io/undertow/client/http/HttpClientSNITestCase.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ public void testNoSNIWhenIP() throws Exception {
183183
// connect using the IP, no SNI expected
184184
final ClientConnection connection = client.connect(new URI("https://" + hostname + ":" + ADDRESS.getPort()), worker,
185185
new UndertowXnioSsl(worker.getXnio(), OptionMap.EMPTY, DefaultServer.createClientSslContext()),
186-
DefaultServer.getBufferPool(), OptionMap.EMPTY).get();
186+
DefaultServer.getBufferPool(), OptionMap.create(UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM, "")).get();
187187
try {
188188
connection.getIoThread().execute(() -> {
189189
final ClientRequest request = new ClientRequest().setMethod(Methods.GET).setPath(SNI);
@@ -244,7 +244,7 @@ public void testForcingSNIForHostname() throws Exception {
244244
// connect using hostname but add option to another hostname, SNI expected to the forced one
245245
final ClientConnection connection = client.connect(new URI("https://" + address.getHostName() + ":" + ADDRESS.getPort()), worker,
246246
new UndertowXnioSsl(worker.getXnio(), OptionMap.EMPTY, DefaultServer.createClientSslContext()),
247-
DefaultServer.getBufferPool(), OptionMap.create(UndertowOptions.SSL_SNI_HOSTNAME, "server")).get();
247+
DefaultServer.getBufferPool(), OptionMap.create(UndertowOptions.SSL_SNI_HOSTNAME, "server", UndertowOptions.ENDPOINT_IDENTIFICATION_ALGORITHM, "")).get();
248248
try {
249249
connection.getIoThread().execute(() -> {
250250
final ClientRequest request = new ClientRequest().setMethod(Methods.GET).setPath(SNI);

‎core/src/test/java/io/undertow/client/http/HttpClientTestCase.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,12 @@
1919
package io.undertow.client.http;
2020

2121
import java.io.IOException;
22+
import java.net.Inet6Address;
23+
import java.net.InetAddress;
2224
import java.net.URI;
2325
import java.net.URISyntaxException;
2426
import java.nio.ByteBuffer;
27+
import java.nio.channels.ClosedChannelException;
2528
import java.util.List;
2629
import java.util.concurrent.CopyOnWriteArrayList;
2730
import java.util.concurrent.CountDownLatch;
@@ -330,6 +333,41 @@ public void run() {
330333
}
331334
}
332335

336+
@Test
337+
public void testSslServerIdentity() throws Exception {
338+
final UndertowClient client = createClient();
339+
exception = null;
340+
341+
final List<ClientResponse> responses = new CopyOnWriteArrayList<>();
342+
final CountDownLatch latch = new CountDownLatch(1);
343+
DefaultServer.startSSLServer();
344+
SSLContext context = DefaultServer.getClientSSLContext();
345+
XnioSsl ssl = new UndertowXnioSsl(DefaultServer.getWorker().getXnio(), OptionMap.EMPTY, DefaultServer.SSL_BUFFER_POOL, context);
346+
347+
// change the URI to use the IP instead the "localhost" name set in the certificate
348+
URI uri = new URI(DefaultServer.getDefaultServerSSLAddress());
349+
InetAddress address = InetAddress.getByName(uri.getHost());
350+
String hostname = address instanceof Inet6Address? "[" + address.getHostAddress() + "]" : address.getHostAddress();
351+
uri = new URI(uri.getScheme(), uri.getUserInfo(), hostname, uri.getPort(), uri.getPath(), uri.getQuery(), uri.getFragment());
352+
353+
// this should fail as IP alternative name is not set in the certificate
354+
final ClientConnection connection = client.connect(uri, worker, ssl, DefaultServer.getBufferPool(), OptionMap.EMPTY).get();
355+
try {
356+
connection.getIoThread().execute(() -> {
357+
final ClientRequest request = new ClientRequest().setMethod(Methods.GET).setPath(MESSAGE);
358+
request.getRequestHeaders().put(Headers.HOST, DefaultServer.getHostAddress());
359+
connection.sendRequest(request, createClientCallback(responses, latch));
360+
});
361+
362+
latch.await(10, TimeUnit.SECONDS);
363+
364+
Assert.assertEquals(0, responses.size());
365+
Assert.assertTrue(exception instanceof ClosedChannelException);
366+
} finally {
367+
connection.getIoThread().execute(() -> IoUtils.safeClose(connection));
368+
DefaultServer.stopSSLServer();
369+
}
370+
}
333371

334372
@Test
335373
public void testConnectionClose() throws Exception {

0 commit comments

Comments
 (0)