mod_auth_digest: keep Authentication-Info on error responses

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]

docs/manual/mod/mod_auth_digest.xml

@@ -31,15 +31,19 @@
 
 <summary>
     <p>This module implements HTTP Digest Authentication
-    (<rfc>2617</rfc>), and
-    provides an alternative to <module>mod_auth_basic</module> where the
-    password is not transmitted as cleartext. However, this does
-    <strong>not</strong> lead to a significant security advantage over
-    basic authentication. On the other hand, the password storage on the
-    server is much less secure with digest authentication than with
-    basic authentication. Therefore, using basic auth and encrypting the
-    whole connection using <module>mod_ssl</module> is a much better
-    alternative.</p>
+    (<rfc>2617</rfc>), an alternative to the Basic authentication provided
+    by <module>mod_auth_basic</module> in which the password is not sent
+    over the network in cleartext.</p>
+
+    <p>Basic and Digest are both authentication schemes only: they
+    establish who the client is, and do nothing for the confidentiality or
+    integrity of the request and response themselves. Over an unencrypted
+    connection the contents of every request and response are exposed with
+    either scheme; Digest's one advantage is that the password is not among
+    them. Digest is therefore not a substitute for a connection secured
+    with <module>mod_ssl</module>, which protects the credentials and the
+    data alike, and that one advantage has limits of its own (see the note
+    below).</p>
 </summary>
 
 <seealso><directive module="mod_authn_core">AuthName</directive></seealso>
@@ -76,18 +80,18 @@
 
 
     <note><title>Note</title>
-    <p>Digest authentication was intended to be more secure than basic
-    authentication, but no longer fulfills that design goal. A
-    man-in-the-middle attacker can trivially force the browser to downgrade
-    to basic authentication. And even a passive eavesdropper can brute-force
-    the password using today's graphics hardware, because the hashing
-    algorithm used by digest authentication is too fast. Another problem is
-    that the storage of the passwords on the server is insecure. The contents
-    of a stolen htdigest file can be used directly for digest authentication.
-    Therefore using <module>mod_ssl</module> to encrypt the whole connection is
-    strongly recommended.</p>
-    <p><module>mod_auth_digest</module> only works properly on platforms
-      where APR supports shared memory.</p>
+    <p>Digest authentication's one advantage over Basic, keeping the
+    password off the wire, is conditional. If the client will silently fall
+    back to Basic when challenged, an active attacker can
+    strip the Digest challenge and offer Basic instead, and the browser
+    then sends the password in cleartext; a passive eavesdropper can mount
+    an offline brute-force attack on a captured response, which the fast
+    MD5 hash does little to slow; and if the <program>htdigest</program>
+    file is stolen its stored hashes can be replayed to authenticate to
+    that realm directly, without the password itself being recovered.
+    Encrypt the whole connection with <module>mod_ssl</module> instead.</p>
+    <p><module>mod_auth_digest</module> requires an APR built with shared
+    memory support, and is not built without it.</p>
     </note>
 </section>
 
@@ -117,29 +121,25 @@
 
 <directivesynopsis>
 <name>AuthDigestQop</name>
-<description>Determines the quality-of-protection to use in digest
+<description>Determines the quality-of-protection to use in Digest
 authentication</description>
-<syntax>AuthDigestQop none|auth|auth-int [auth|auth-int]</syntax>
+<syntax>AuthDigestQop auth</syntax>
 <default>AuthDigestQop auth</default>
 <contextlist><context>directory</context><context>.htaccess</context>
 </contextlist>
 <override>AuthConfig</override>
 
 <usage>
-    <p>The <directive>AuthDigestQop</directive> directive determines
-    the <dfn>quality-of-protection</dfn> to use. <code>auth</code> will
-    only do authentication (username/password); <code>auth-int</code> is
-    authentication plus integrity checking (an MD5 hash of the entity
-    is also computed and checked); <code>none</code> will cause the module
-    to use the old RFC-2069 digest algorithm (which does not include
-    integrity checking). Both <code>auth</code> and <code>auth-int</code> may
-    be specified, in which the case the browser will choose which of
-    these to use. <code>none</code> should only be used if the browser for
-    some reason does not like the challenge it receives otherwise.</p>
-
-    <note>
-      <code>auth-int</code> is not implemented yet.
-    </note>
+    <p>The <directive>AuthDigestQop</directive> directive determines the
+    <dfn>quality-of-protection</dfn> to use. Only <code>auth</code>
+    (authentication) is supported, and it is the default, so this
+    directive no longer has any effect and is retained only for
+    compatibility.</p>
+
+    <note>Earlier releases also accepted <code>none</code> (the RFC 2069
+    Digest scheme, without a quality-of-protection). It has been removed:
+    a request whose <code>qop</code> is missing or not <code>auth</code>
+    is now rejected.</note>
 </usage>
 </directivesynopsis>
 
@@ -154,26 +154,31 @@ authentication</description>
 
 <usage>
     <p>The <directive>AuthDigestNonceLifetime</directive> directive
-    controls how long the server nonce is valid. When the client
-    contacts the server using an expired nonce the server will send
-    back a 401 with <code>stale=true</code>. If <var>seconds</var> is
-    greater than 0 then it specifies the amount of time for which the
-    nonce is valid; this should probably never be set to less than 10
-    seconds. If <var>seconds</var> is less than 0 then the nonce never
-    expires. <!-- Not implemented yet: If <var>seconds</var> is 0 then
-    the nonce may be used exactly once by the client. Note that while
-    one-time-nonces provide higher security against replay attacks,
-    they also have significant performance implications, as the
-    browser cannot pipeline or multiple connections for the
-    requests. Because browsers cannot easily detect that
-    one-time-nonces are being used, this may lead to browsers trying
-    to pipeline requests and receiving 401 responses for all but the
-    first request, requiring the browser to resend the requests. Note
-    also that the protection against replay attacks only makes sense
-    for dynamically generated content and things like POST requests;
-    for static content the attacker may already have the complete
-    response, so one-time-nonces do not make sense here.  -->
-    </p>
+    controls how long the server nonce is valid. When the client contacts
+    the server using an expired nonce the server sends back a 401 with
+    <code>stale=true</code>, and an RFC-compliant client retries with a
+    fresh nonce without prompting the user.</p>
+
+    <ul>
+      <li>If <var>seconds</var> is greater than 0 then it specifies the
+      amount of time for which the nonce is valid; this should probably
+      never be set to less than 10 seconds.</li>
+
+      <li>If <var>seconds</var> is 0 then the nonce may be used exactly
+      once. The server supplies the next nonce in the
+      <code>Authentication-Info</code> header of each response so the
+      client can continue; a client which reuses a nonce is answered with
+      <code>stale=true</code> and retries. One-time nonces give stronger
+      protection against replaying a captured request, but the client
+      cannot pipeline requests, and one that does receives a 401 for all
+      but the first and has to resend them. The protection is worth this
+      cost mainly for requests that change server state or return content
+      which varies over time; replaying a request for unchanging content
+      that the eavesdropper already captured in full gains nothing.</li>
+
+      <li>If <var>seconds</var> is less than 0 then the nonce never
+      expires.</li>
+    </ul>
 </usage>
 </directivesynopsis>
 
@@ -209,8 +214,8 @@ client</description>
 <directivesynopsis>
 <name>AuthDigestAlgorithm</name>
 <description>Selects the algorithm used to calculate the challenge and
-response hashes in digest authentication</description>
-<syntax>AuthDigestAlgorithm MD5|MD5-sess</syntax>
+response hashes in Digest authentication</description>
+<syntax>AuthDigestAlgorithm MD5</syntax>
 <default>AuthDigestAlgorithm MD5</default>
 <contextlist><context>directory</context><context>.htaccess</context>
 </contextlist>
@@ -219,22 +224,13 @@ response hashes in digest authentication</description>
 <usage>
     <p>The <directive>AuthDigestAlgorithm</directive> directive
     selects the algorithm used to calculate the challenge and response
-    hashes.</p>
-
-    <note>
-      <code>MD5-sess</code> is not correctly implemented yet.
-    </note>
-    <!--
-    <p>To use <code>MD5-sess</code> you must first code up the
-    <code>get_userpw_hash()</code> function in
-    <code>mod_auth_digest.c</code>.</p>
-    -->
+    hashes. Only <code>MD5</code> is supported.</p>
 </usage>
 </directivesynopsis>
 
 <directivesynopsis>
 <name>AuthDigestDomain</name>
-<description>URIs that are in the same protection space for digest
+<description>URIs that are in the same protection space for Digest
 authentication</description>
 <syntax>AuthDigestDomain <var>URI</var> [<var>URI</var>] ...</syntax>
 <contextlist><context>directory</context><context>.htaccess</context>

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",

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"