From 6e8fd34a2858ad392ef6e155de468d21eb8f9aa3 Mon Sep 17 00:00:00 2001 From: mohammed adib Date: Tue, 7 Jul 2026 11:22:31 +0530 Subject: [PATCH] fail scram exchange on invalid server signature --- .../netty/handler/intercept/Interceptors.java | 46 +++++++++++++------ .../org/asynchttpclient/ScramAuthTest.java | 35 +++++++++++++- 2 files changed, 66 insertions(+), 15 deletions(-) diff --git a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Interceptors.java b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Interceptors.java index f974f88b0..e4b7c1419 100644 --- a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Interceptors.java +++ b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Interceptors.java @@ -40,6 +40,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.Base64; @@ -64,11 +65,13 @@ public class Interceptors { private final boolean hasResponseFilters; private final ClientCookieDecoder cookieDecoder; private final NonceCounter nonceCounter; + private final NettyRequestSender requestSender; public Interceptors(AsyncHttpClientConfig config, ChannelManager channelManager, NettyRequestSender requestSender) { this.config = config; + this.requestSender = requestSender; nonceCounter = new NonceCounter(); unauthorized401Interceptor = new Unauthorized401Interceptor(channelManager, requestSender, nonceCounter); proxyUnauthorized407Interceptor = new ProxyUnauthorized407Interceptor(channelManager, requestSender, nonceCounter); @@ -135,11 +138,13 @@ public boolean exitAfterIntercept(Channel channel, NettyResponseFuture future } // Process SCRAM Authentication-Info (RFC 7804 §5) - if (realm != null && realm.getScheme() == Realm.AuthScheme.SCRAM_SHA_256) { - processScramAuthenticationInfo(future, responseHeaders, "Authentication-Info"); + if (realm != null && realm.getScheme() == Realm.AuthScheme.SCRAM_SHA_256 + && processScramAuthenticationInfo(channel, future, responseHeaders, "Authentication-Info")) { + return true; } - if (proxyRealm != null && proxyRealm.getScheme() == Realm.AuthScheme.SCRAM_SHA_256) { - processScramAuthenticationInfo(future, responseHeaders, "Proxy-Authentication-Info"); + if (proxyRealm != null && proxyRealm.getScheme() == Realm.AuthScheme.SCRAM_SHA_256 + && processScramAuthenticationInfo(channel, future, responseHeaders, "Proxy-Authentication-Info")) { + return true; } return false; @@ -181,25 +186,30 @@ private void processAuthenticationInfo(NettyResponseFuture future, HttpHeader } } - private void processScramAuthenticationInfo(NettyResponseFuture future, HttpHeaders responseHeaders, - String headerName) { + /** + * @return true if the exchange failed server verification and the request has been aborted, in which + * case the caller must stop delivering the response as a success. + */ + private boolean processScramAuthenticationInfo(Channel channel, NettyResponseFuture future, HttpHeaders responseHeaders, + String headerName) { ScramContext ctx = future.getScramContext(); if (ctx == null || ctx.getState() != ScramState.CLIENT_FINAL_SENT) { - return; + return false; } String authInfo = responseHeaders.get(headerName); if (authInfo == null) { - // RFC 7804 §6: may be in chunked trailers (not supported by AHC) + // RFC 7804 §6: may be in chunked trailers (not supported by AHC). We can't tell an honest + // server that used trailers from a stripped header, so leave the response untouched here. LOGGER.warn("SCRAM: response without {} header; " + "ServerSignature cannot be verified (may be in chunked trailers)", headerName); - return; + return false; } String data = Realm.Builder.matchParam(authInfo, "data"); if (data == null) { LOGGER.warn("SCRAM: Authentication-Info header missing data attribute"); - return; + return false; } String serverFinalMsg; @@ -208,15 +218,23 @@ private void processScramAuthenticationInfo(NettyResponseFuture future, HttpH } catch (IllegalArgumentException e) { LOGGER.warn("SCRAM: invalid base64 in {} data attribute: {}", headerName, e.getMessage()); ctx.setState(ScramState.FAILED); - return; + return abortScram(channel, future, "SCRAM: unparseable ServerSignature in " + headerName); } // verifyServerFinal sets state to AUTHENTICATED or FAILED internally if (ctx.verifyServerFinal(serverFinalMsg)) { LOGGER.debug("SCRAM ServerSignature verified successfully"); - } else { - LOGGER.warn("SCRAM ServerSignature verification failed — authentication unsuccessful " - + "(RFC 7804 §5: MUST consider unsuccessful)"); + return false; } + // RFC 7804 §5: a present-but-invalid ServerSignature means the peer failed to prove knowledge of + // the shared secret, so the client MUST consider the exchange unsuccessful rather than hand the + // caller a response from an unauthenticated peer. + return abortScram(channel, future, "SCRAM ServerSignature verification failed"); + } + + private boolean abortScram(Channel channel, NettyResponseFuture future, String message) { + LOGGER.warn("{} — aborting request (RFC 7804 §5: MUST consider unsuccessful)", message); + requestSender.abort(channel, future, new IOException(message)); + return true; } } diff --git a/client/src/test/java/org/asynchttpclient/ScramAuthTest.java b/client/src/test/java/org/asynchttpclient/ScramAuthTest.java index 65810161f..8e4c60afc 100644 --- a/client/src/test/java/org/asynchttpclient/ScramAuthTest.java +++ b/client/src/test/java/org/asynchttpclient/ScramAuthTest.java @@ -32,6 +32,7 @@ import java.util.Base64; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; @@ -43,6 +44,8 @@ import static org.asynchttpclient.test.TestUtils.addHttpConnector; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; public class ScramAuthTest extends AbstractBasicTest { @@ -145,6 +148,27 @@ public void testScramSha256_malformedBase64InData() throws Exception { } } + @RepeatedIfExceptionsTest(repeats = 5) + public void testScramSha256_invalidServerSignatureIsRejected() throws Exception { + // Server completes the handshake but returns a ServerSignature it could not have computed without + // the shared secret. RFC 7804 §5 requires the client to consider the exchange unsuccessful. + server.stop(); + server = new Server(); + ServerConnector connector = addHttpConnector(server); + server.setHandler(new ScramAuthHandler(true, true)); + server.start(); + port1 = connector.getLocalPort(); + + try (AsyncHttpClient client = asyncHttpClient()) { + Future f = client.prepareGet("http://localhost:" + port1 + '/') + .setRealm(scramSha256AuthRealm(USER, ADMIN).setRealmName("ScramRealm").build()) + .execute(); + ExecutionException ex = assertThrows(ExecutionException.class, () -> f.get(20, TimeUnit.SECONDS)); + assertNotNull(ex.getCause()); + assertTrue(ex.getCause().getMessage().contains("ServerSignature"), ex.getCause().getMessage()); + } + } + @RepeatedIfExceptionsTest(repeats = 5) public void testScramSha256_quotedDataAttribute() throws Exception { try (AsyncHttpClient client = asyncHttpClient()) { @@ -196,16 +220,22 @@ private static class ScramAuthHandler extends AbstractHandler { private static final int ITERATIONS = 4096; private final boolean sendAuthInfo; + private final boolean corruptServerSignature; // Session tracking: sid → ServerSession private final ConcurrentHashMap sessions = new ConcurrentHashMap<>(); ScramAuthHandler() { - this(true); + this(true, false); } ScramAuthHandler(boolean sendAuthInfo) { + this(sendAuthInfo, false); + } + + ScramAuthHandler(boolean sendAuthInfo, boolean corruptServerSignature) { this.sendAuthInfo = sendAuthInfo; + this.corruptServerSignature = corruptServerSignature; } static class ServerSession { @@ -361,6 +391,9 @@ public void handle(String s, Request r, HttpServletRequest request, HttpServletR // Compute ServerSignature for Authentication-Info byte[] serverSignature = ScramEngine.computeServerSignature(serverKey, authMessage); + if (corruptServerSignature) { + serverSignature[0] ^= 0xFF; // simulate a peer that can't prove knowledge of the secret + } String serverFinal = "v=" + Base64.getEncoder().encodeToString(serverSignature); String base64ServerFinal = Base64.getEncoder().encodeToString(serverFinal.getBytes(StandardCharsets.UTF_8));