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.

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

Deny HTTP/3 connection creation for clients missing cert when needClientAuth is true #12014

Merged
merged 11 commits into from
Jul 10, 2024
21 changes: 21 additions & 0 deletions jetty-core/jetty-http3/jetty-http3-tests/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,25 @@
</dependency>
</dependencies>

<profiles>
<profile>
<id>enable-foreign</id>
<activation>
<jdk>[22,)</jdk>
</activation>
<build>
<plugins>
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<argLine>@{argLine}
${jetty.surefire.argLine}
--enable-native-access=ALL-UNNAMED</argLine>
</configuration>
</plugin>
</plugins>
</build>
</profile>
</profiles>

</project>
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.eclipse.jetty.http3.server.AbstractHTTP3ServerConnectionFactory;
import org.eclipse.jetty.http3.server.internal.HTTP3SessionServer;
import org.eclipse.jetty.quic.client.ClientQuicSession;
import org.eclipse.jetty.quic.common.QuicErrorCode;
import org.eclipse.jetty.quic.common.QuicSession;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -640,4 +641,24 @@ public void onResponse(Stream.Client stream, HeadersFrame frame)

assertTrue(responseLatch.await(5, TimeUnit.SECONDS));
}

@Test
public void testMissingNeededClientCertificateDeniesConnection() throws Exception
{
start(new Session.Server.Listener() {});
connector.getQuicConfiguration().getSslContextFactory().setNeedClientAuth(true);

CountDownLatch latch = new CountDownLatch(1);
newSession(new Session.Client.Listener()
{
@Override
public void onDisconnect(Session session, long error, String reason)
{
assertEquals(QuicErrorCode.CONNECTION_REFUSED.code(), error);
assertEquals("missing_client_cert", reason);
latch.countDown();
}
});
assertTrue(latch.await(5, TimeUnit.SECONDS));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ protected ServerQuicConnection newConnection(EndPoint endpoint)
@Override
protected ServerQuicSession newQuicSession(SocketAddress remoteAddress, QuicheConnection quicheConnection)
{
return new ServerQuicSession(getExecutor(), getScheduler(), getByteBufferPool(), quicheConnection, this, remoteAddress, getQuicServerConnector())
return new ServerQuicSession(getExecutor(), getScheduler(), getByteBufferPool(), quicheConnection, this, remoteAddress, getQuicServerConnector(), getQuicConfiguration())
{
@Override
public int flush(long streamId, ByteBuffer buffer, boolean last) throws IOException
Expand Down
2 changes: 1 addition & 1 deletion jetty-core/jetty-quic/jetty-quic-client/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
<configuration>
<argLine>@{argLine}
${jetty.surefire.argLine}
--enable-native-access org.eclipse.jetty.quic.quiche.foreign</argLine>
--enable-native-access=ALL-UNNAMED</argLine>
</configuration>
</plugin>
</plugins>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ protected ProtocolSession createProtocolSession()
return new ClientProtocolSession(this);
}

@Override
protected boolean validateNewlyEstablishedConnection()
{
return true;
}

@Override
public Connection newConnection(QuicStreamEndPoint endPoint)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,9 @@ public Runnable process(SocketAddress remoteAddress, ByteBuffer cipherBufferIn)
ProtocolSession protocol = protocolSession;
if (protocol == null)
{
if (!validateNewlyEstablishedConnection())
return null;

protocolSession = protocol = createProtocolSession();
addManaged(protocol);
}
Expand All @@ -343,6 +346,11 @@ protected Runnable pollTask()

protected abstract ProtocolSession createProtocolSession();

/**
* @return true if the connection is valid, false otherwise.
*/
protected abstract boolean validateNewlyEstablishedConnection();

List<Long> getWritableStreamIds()
{
return quicheConnection.writableStreamIds();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ public static Path[] exportKeyPair(KeyStore keyStore, String alias, char[] keyPa
try (OutputStream os = Files.newOutputStream(paths[1]))
{
Certificate[] certChain = keyStore.getCertificateChain(alias);
if (certChain == null)
throw new IllegalArgumentException("Alias does not exist in key store: " + alias);

for (Certificate cert : certChain)
writeAsPEM(os, cert);
Files.setPosixFilePermissions(paths[1], Set.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ protected QuicSession createSession(SocketAddress remoteAddress, ByteBuffer ciph

protected ServerQuicSession newQuicSession(SocketAddress remoteAddress, QuicheConnection quicheConnection)
{
return new ServerQuicSession(getExecutor(), getScheduler(), getByteBufferPool(), quicheConnection, this, remoteAddress, getQuicServerConnector());
return new ServerQuicSession(getExecutor(), getScheduler(), getByteBufferPool(), quicheConnection, this, remoteAddress, getQuicServerConnector(), quicConfiguration);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.eclipse.jetty.io.RuntimeIOException;
import org.eclipse.jetty.quic.common.ProtocolSession;
import org.eclipse.jetty.quic.common.QuicConnection;
import org.eclipse.jetty.quic.common.QuicErrorCode;
import org.eclipse.jetty.quic.common.QuicSession;
import org.eclipse.jetty.quic.common.QuicStreamEndPoint;
import org.eclipse.jetty.quic.quiche.QuicheConnection;
Expand All @@ -44,12 +45,14 @@
public class ServerQuicSession extends QuicSession implements CyclicTimeouts.Expirable
{
private final Connector connector;
private final ServerQuicConfiguration quicConfiguration;
lorban marked this conversation as resolved.
Show resolved Hide resolved
private long expireNanoTime = Long.MAX_VALUE;

public ServerQuicSession(Executor executor, Scheduler scheduler, ByteBufferPool bufferPool, QuicheConnection quicheConnection, QuicConnection connection, SocketAddress remoteAddress, Connector connector)
public ServerQuicSession(Executor executor, Scheduler scheduler, ByteBufferPool bufferPool, QuicheConnection quicheConnection, QuicConnection connection, SocketAddress remoteAddress, Connector connector, ServerQuicConfiguration quicConfiguration)
lorban marked this conversation as resolved.
Show resolved Hide resolved
{
super(executor, scheduler, bufferPool, quicheConnection, connection, remoteAddress);
this.connector = connector;
this.quicConfiguration = quicConfiguration;
}

@Override
Expand All @@ -67,6 +70,18 @@ protected ProtocolSession createProtocolSession()
return new ServerProtocolSession(this);
}

@Override
protected boolean validateNewlyEstablishedConnection()
{
if (quicConfiguration.getSslContextFactory().getNeedClientAuth() && getPeerCertificates() == null)
{
outwardClose(QuicErrorCode.CONNECTION_REFUSED.code(), "missing_client_cert");
lorban marked this conversation as resolved.
Show resolved Hide resolved
flush();
return false;
}
return true;
}

@Override
public Connection newConnection(QuicStreamEndPoint endPoint)
{
Expand Down
Loading