Stop negotiating a qop whose mutual authentication cannot be verified

Under auth-int the rspauth signs a response body that has not been read when the header is
processed, so verification was skipped and mutual authentication was off for the exchange. A peer
could select that by offering qop="auth-int" alone.

Do not negotiate it. A server offering auth-int and nothing else can no longer be authenticated
against; failing is better than authenticating unverifiably.

Author: hyperxpro

client/src/main/java/org/asynchttpclient/Realm.java

@@ -495,19 +495,17 @@ public Builder setMaxIterationCount(int maxIterationCount) {
                 serverSupportedQops[i] = rawServerSupportedQops[i].trim();
             }
 
-            // prefer auth over auth-int
             for (String rawServerSupportedQop : serverSupportedQops) {
                 if ("auth".equals(rawServerSupportedQop)) {
                     return rawServerSupportedQop;
                 }
             }
 
-            for (String rawServerSupportedQop : serverSupportedQops) {
-                if ("auth-int".equals(rawServerSupportedQop)) {
-                    return rawServerSupportedQop;
-                }
-            }
-
+            // auth-int is deliberately not selected. Its rspauth signs the response entity-body, which has
+            // not been read when the Authentication-Info header is processed, so the value the server signed
+            // cannot be derived there and mutual authentication is skipped for the whole exchange. A peer
+            // that chooses the challenge could therefore switch mutual authentication off simply by offering
+            // auth-int on its own. Declining to negotiate a mode we cannot verify keeps that decision ours.
             return null;
         }
 

client/src/test/java/org/asynchttpclient/DigestMutualAuthTest.java

@@ -150,17 +150,21 @@ public void invalidRspAuthIsRejected() throws Exception {
     }
 
     /**
-     * RFC 7616 §3.5 defines {@code A2} for {@code qop=auth-int} as {@code ":" request-uri ":" H(entity-body)}
-     * over the <em>response</em> entity-body. That body has not been received when the {@code
-     * Authentication-Info} header is processed, so the expected value simply cannot be derived there. The
-     * client must therefore fall back to warn-and-deliver for auth-int instead of enforcing the auth-only
-     * formula, which would abort against a perfectly conformant server.
+     * RFC 7616 Section 3.5 defines {@code A2} for {@code qop=auth-int} as {@code ":" request-uri ":"
+     * H(entity-body)} over the <em>response</em> entity-body. That body has not arrived when the {@code
+     * Authentication-Info} header is processed, so the expected rspauth cannot be derived there at all.
      * <p>
-     * The server arm here computes rspauth <em>from the RFC</em> rather than by mirroring the client, which is
-     * what makes this test able to catch the mismatch at all.
+     * The client used to negotiate auth-int anyway and then report "cannot verify", which delivered the
+     * response with mutual authentication silently skipped for the whole exchange. Since the peer chooses
+     * the challenge, offering {@code qop="auth-int"} on its own was a one-word way to switch mutual
+     * authentication off. The client now declines to negotiate a mode whose rspauth it cannot check.
+     * <p>
+     * Interoperability cost, deliberately accepted: a server offering auth-int and nothing else can no
+     * longer be authenticated against. {@code auth,auth-int} is unaffected, because auth is preferred and
+     * its rspauth is verified.
      */
     @RepeatedIfExceptionsTest(repeats = 5)
-    public void authIntRspAuthIsNotEnforcedAgainstAConformantServer() throws Exception {
+    public void anAuthIntOnlyChallengeIsNotAnsweredWithAuthInt() throws Exception {
         restartServer(new AuthIntRspAuthHandler());
 
         try (AsyncHttpClient client = asyncHttpClient()) {
@@ -169,12 +173,12 @@ public void authIntRspAuthIsNotEnforcedAgainstAConformantServer() throws Excepti
                     .execute();
             Response resp = f.get(60, TimeUnit.SECONDS);
             assertNotNull(resp);
-            assertEquals(HttpServletResponse.SC_OK, resp.getStatusCode());
-            assertEquals(AuthIntRspAuthHandler.BODY, resp.getResponseBody());
-            // The exchange really did negotiate auth-int; the client did not quietly fall back to auth.
-            assertNotNull(resp.getHeader("X-Auth"));
-            assertTrue(resp.getHeader("X-Auth").contains("qop=auth-int"),
-                    "expected the request to have been sent with qop=auth-int but got: " + resp.getHeader("X-Auth"));
+            // The server only accepts qop=auth-int, so declining it means the exchange is not authenticated.
+            assertEquals(HttpServletResponse.SC_UNAUTHORIZED, resp.getStatusCode(),
+                    "an auth-int-only challenge must not be answered, since its rspauth cannot be verified");
+            String sent = resp.getHeader("X-Auth");
+            assertTrue(sent == null || !sent.contains("qop=auth-int"),
+                    "the client must not negotiate auth-int: " + sent);
         }
     }
 

client/src/test/java/org/asynchttpclient/RealmTest.java

@@ -322,6 +322,31 @@ public void anUnescapedBackslashInARealmIsKept() {
         assertEquals("abc123", realm.getNonce());
     }
 
+    /**
+     * A peer that offers only auth-int must not get auth-int negotiated. The rspauth of an auth-int
+     * exchange signs the response entity-body, which has not been read when Authentication-Info is
+     * processed, so the expected value cannot be derived and mutual authentication ends up skipped for the
+     * whole exchange. Offering auth-int alone would therefore let the peer choosing the challenge switch
+     * mutual authentication off. Declining to negotiate it keeps that decision on our side.
+     */
+    @Test
+    public void aQopOfAuthIntAloneIsNotNegotiated() {
+        Realm realm = new Realm.Builder("user", "pass")
+                .parseWWWAuthenticateHeader("Digest realm=\"protected\", nonce=\"N\", qop=\"auth-int\"")
+                .build();
+
+        assertNull(realm.getQop(), "auth-int must not be negotiated: its rspauth cannot be verified");
+    }
+
+    @Test
+    public void authIsStillPreferredWhenBothAreOffered() {
+        Realm realm = new Realm.Builder("user", "pass")
+                .parseWWWAuthenticateHeader("Digest realm=\"protected\", nonce=\"N\", qop=\"auth,auth-int\"")
+                .build();
+
+        assertEquals("auth", realm.getQop());
+    }
+
     private String getMd5(String what) throws Exception {
         MessageDigest md = MessageDigest.getInstance("MD5");
         md.update(what.getBytes(StandardCharsets.ISO_8859_1));