mod_auth_digest: Keep Authentication-Info on error responses. HEADtrunk

* modules/aaa/mod_auth_digest.c (add_auth_info): Add the
  (Proxy-)Authentication-Info header to err_headers_out, so a one-time
  nonce client gets its nextnonce after an error response rather than a
  stale re-challenge.
  (note_digest_auth_failure): Drop any (Proxy-)Authentication-Info when
  issuing a challenge, so a post-authentication 401 carries only the
  fresh nonce.

* test/modules/aaa/test_011_authinfo_errors.py: New tests.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
GitHub: closes PR #739


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1937888 13f79535-47bb-0310-9956-ffa450edef68

3 files changed, 102 insertions, 1 deletions

diff --git a/changes-entries/digest-authinfo-errors.txt b/changes-entries/digest-authinfo-errors.txt
new file mode 100644
index 0000000000..7d5ba783ea
--- /dev/null
+++ b/changes-entries/digest-authinfo-errors.txt
@@ -0,0 +1,3 @@
+  *) mod_auth_digest: With one-time nonces (AuthDigestNonceLifetime 0),
+     stop needlessly re-challenging a client after an error response.
+     [Joe Orton]
diff --git a/modules/aaa/mod_auth_digest.c b/modules/aaa/mod_auth_digest.c
index 2e59b7a397..c08ffff403 100644
--- a/modules/aaa/mod_auth_digest.c
+++ b/modules/aaa/mod_auth_digest.c
@@ -1297,6 +1297,14 @@ static int note_digest_auth_failure(request_rec *r,
         domain = conf->uri_list;
     }
 
+    /* A challenge supersedes any Authentication-Info added at fixups
+     * for this request, e.g. when a handler notes an auth failure
+     * after the user was authenticated: the client must not be given
+     * both a nextnonce and a new nonce. */
+    apr_table_unset(r->err_headers_out,
+                    (PROXYREQ_PROXY == r->proxyreq)
+                        ? "Proxy-Authentication-Info" : "Authentication-Info");
+
     apr_table_mergen(r->err_headers_out,
                      (PROXYREQ_PROXY == r->proxyreq)
                          ? "Proxy-Authenticate" : "WWW-Authenticate",
@@ -1878,7 +1886,10 @@ static int add_auth_info(request_rec *r)
     }
 
     if (ai && ai[0]) {
-        apr_table_mergen(r->headers_out,
+        /* This must use ->err_headers_out so it survives an error
+         * response, else a one-time-nonce client loses the nextnonce
+         * and is stale-challenged on its next request. */
+        apr_table_mergen(r->err_headers_out,
                          (PROXYREQ_PROXY == r->proxyreq)
                              ? "Proxy-Authentication-Info"
                              : "Authentication-Info",
diff --git a/test/modules/aaa/test_011_authinfo_errors.py b/test/modules/aaa/test_011_authinfo_errors.py
new file mode 100644
index 0000000000..64361f53ba
--- /dev/null
+++ b/test/modules/aaa/test_011_authinfo_errors.py
@@ -0,0 +1,87 @@
+"""Authentication-Info survives an error response.
+
+The header is added at fixups, so unless it goes into err_headers_out it
+is dropped when the response turns into an error, costing a one-time-nonce
+client the nextnonce and forcing a stale re-challenge.  A 404 for a missing
+file inside the protected area reaches the error path while fully
+authenticated, so it exercises this without a handler like mod_dav.
+"""
+
+from . import digest_client as dc
+from .env import AAATestEnv
+
+
+class TestAuthInfoOnError:
+
+    def url(self, env, location, path):
+        return env.mkurl("http", "aaa", f"/digest/{location}/{path}")
+
+    def uri(self, location, path):
+        return f"/digest/{location}/{path}"
+
+    def challenge(self, env, location, path="secret.txt"):
+        r = env.curl_get(self.url(env, location, path))
+        assert r.response["status"] == 401
+        return dc.DigestChallenge.parse(r.response["header"]["www-authenticate"])
+
+    def auth(self, env, location, path, challenge, nc="00000001"):
+        return dc.build_authorization(
+            AAATestEnv.DIGEST_USER, challenge, AAATestEnv.DIGEST_PASSWORD,
+            method="GET", uri=self.uri(location, path), nc=nc,
+            cnonce="authinfo-cnonce")
+
+    def get(self, env, location, path, challenge, nc="00000001"):
+        return env.curl_get(self.url(env, location, path), options=[
+            "-H", f"Authorization: {self.auth(env, location, path, challenge, nc)}"])
+
+    def test_digest_110_error_response_carries_authentication_info(self, env):
+        # A 404 for an authenticated request still confirms the response to
+        # the client: Authentication-Info with a valid rspauth.
+        location = "default"
+        challenge = self.challenge(env, location)
+        r = self.get(env, location, "no-such-file.txt", challenge)
+        assert r.response["status"] == 404
+        assert "authentication-info" in r.response["header"], \
+            "the error response dropped Authentication-Info"
+        ai = dc.parse_params(r.response["header"]["authentication-info"])
+        expect = dc.rspauth_digest(
+            dc.ha1(AAATestEnv.DIGEST_USER, AAATestEnv.REALM,
+                   AAATestEnv.DIGEST_PASSWORD),
+            challenge.nonce, "00000001", "authinfo-cnonce", "auth",
+            self.uri(location, "no-such-file.txt"))
+        assert ai.get("rspauth") == expect, "rspauth wrong on the error response"
+
+    def test_digest_111_onetime_client_continues_after_an_error(self, env):
+        # Under one-time nonces the error must still hand back a nextnonce,
+        # or the client's next request is stale-challenged.  Prove the client
+        # can carry straight on to a real request with what the 404 gave it.
+        location = "onetime"
+        challenge = self.challenge(env, location)
+        r = self.get(env, location, "no-such-file.txt", challenge)
+        assert r.response["status"] == 404
+        assert "authentication-info" in r.response["header"], \
+            "the one-time-nonce error response dropped Authentication-Info"
+        ai = dc.parse_params(r.response["header"]["authentication-info"])
+        assert "nextnonce" in ai, "no nextnonce to continue with after the error"
+
+        challenge.nonce = ai["nextnonce"]
+        ok = self.get(env, location, "secret.txt", challenge)
+        assert ok.response["status"] == 200, \
+            "the nextnonce from the error response was not usable"
+
+    def test_digest_112_challenge_has_no_authentication_info(self, env):
+        # When the error is itself a 401, the challenge stands alone: the
+        # client is not handed a nextnonce it could not use beside a fresh
+        # nonce.  A wrong password reaches note_digest_auth_failure.
+        location = "onetime"
+        challenge = self.challenge(env, location)
+        bad = dc.build_authorization(
+            AAATestEnv.DIGEST_USER, challenge, "wrong-password",
+            method="GET", uri=self.uri(location, "secret.txt"),
+            cnonce="authinfo-cnonce")
+        r = env.curl_get(self.url(env, location, "secret.txt"),
+                         options=["-H", f"Authorization: {bad}"])
+        env.httpd_error_log.ignore_recent(lognos=["AH01794"])  # password mismatch
+        assert r.response["status"] == 401
+        assert "authentication-info" not in r.response["header"], \
+            "a 401 challenge carried an Authentication-Info header"