Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[UNDERTOW-2207][UNDERTOW-2212][UNDERTOW-2239][UNDERTOW-2252] 2.2.x backport bug fixes #1457

Merged
merged 5 commits into from
Mar 30, 2023
Merged

[UNDERTOW-2207][UNDERTOW-2212][UNDERTOW-2239][UNDERTOW-2252] 2.2.x backport bug fixes #1457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fdaaa3b
UNDERTOW-2207: UndertowOutputStream allows flush after writes
carterkozak Nov 17, 2022
a4d3b16
[UNDERTOW-2212] CVE-2022-4492 Server identity in https connection is …
rmartinc Dec 14, 2022
a511cb5
[UNDERTOW-2212] At the HttpClientTestCase.testSslServerIdentity test,…
fl4via Mar 24, 2023
1302c8c
[UNDERTOW-2239] CVE-2023-1108 At SslConduit.wrapAndFlip, do not attem…
fl4via Feb 28, 2023
f0a0f24
[UNDERTOW-2252] Potential fix for the SslConduit.dataToUnwrap buffer …
fl4via Mar 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/src/main/java/io/undertow/UndertowOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
13 changes: 10 additions & 3 deletions core/src/main/java/io/undertow/io/UndertowOutputStream.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,11 +272,18 @@ void updateWritten(final long len) throws IOException {
* {@inheritDoc}
*/
public void flush() throws IOException {
if (anyAreSet(state, FLAG_CLOSED)) {
throw UndertowMessages.MESSAGES.streamIsClosed();
}
boolean closed = anyAreSet(state, FLAG_CLOSED);
if (buffer != null && buffer.position() != 0) {
if (closed) {
throw UndertowMessages.MESSAGES.streamIsClosed();
}
writeBufferBlocking(false);
} else if (closed) {
// No-op if flush is called with no buffered data on a closed channel.
// This stream closes itself when a Content-Length is provided, once sufficient
// bytes have been written -- a flush following the last write is entirely
// reasonable.
return;
}
if (channel == null) {
channel = exchange.getResponseChannel();
Expand Down
19 changes: 11 additions & 8 deletions core/src/main/java/io/undertow/protocols/ssl/SslConduit.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -999,7 +999,8 @@ private synchronized long doWrap(ByteBuffer[] userBuffers, int off, int len) thr

private SSLEngineResult wrapAndFlip(ByteBuffer[] userBuffers, int off, int len) throws IOException {
SSLEngineResult result = null;
while (result == null || (result.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NEED_WRAP && result.getStatus() != SSLEngineResult.Status.BUFFER_OVERFLOW)) {
while (result == null || (result.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NEED_WRAP
&& result.getStatus() != SSLEngineResult.Status.BUFFER_OVERFLOW && !engine.isInboundDone())) {
if (userBuffers == null) {
result = engine.wrap(EMPTY_BUFFER, wrappedData.getBuffer());
} else {
Expand Down Expand Up @@ -1275,14 +1276,16 @@ public void readReady() {
}
if(anyAreSet(state, FLAG_READS_RESUMED) && (unwrappedData != null || anyAreSet(state, FLAG_DATA_TO_UNWRAP))) {
if(anyAreSet(state, FLAG_READ_CLOSED)) {
if(unwrappedData != null) {
unwrappedData.close();
}
if(dataToUnwrap != null) {
dataToUnwrap.close();
synchronized (SslConduit.this) {
if (unwrappedData != null) {
unwrappedData.close();
}
if (dataToUnwrap != null) {
dataToUnwrap.close();
}
unwrappedData = null;
dataToUnwrap = null;
}
unwrappedData = null;
dataToUnwrap = null;
} else {
//there is data in the buffers so we do a wakeup
//as we may not get an actual read notification
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/io/undertow/protocols/ssl/UndertowXnioSsl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
59 changes: 48 additions & 11 deletions core/src/test/java/io/undertow/client/http/HttpClientTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,6 @@

package io.undertow.client.http;

import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.ByteBuffer;
import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;

import javax.net.ssl.SSLContext;

import io.undertow.client.ClientCallback;
import io.undertow.client.ClientConnection;
import io.undertow.client.ClientExchange;
Expand Down Expand Up @@ -64,6 +53,18 @@
import org.xnio.channels.StreamSinkChannel;
import org.xnio.ssl.XnioSsl;

import javax.net.ssl.SSLContext;
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.util.List;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;

import static io.undertow.testutils.StopServerWithExternalWorkerUtils.stopWorker;

/**
Expand Down Expand Up @@ -330,6 +331,42 @@ 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());
// see UNDERTOW-2249: assert exception instanceof ClosedChannelException
Assert.assertNotNull(exception);
} finally {
connection.getIoThread().execute(() -> IoUtils.safeClose(connection));
DefaultServer.stopSSLServer();
}
}

@Test
public void testConnectionClose() throws Exception {
Expand Down
Loading