fail SCRAM exchange on invalid ServerSignature

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;
     }
 }

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<Response> 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<String, ServerSession> 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));