mod_auth_digest: cleanup IV

test/modules/aaa/conftest.py

@@ -94,6 +94,26 @@ def env(pytestconfig) -> AAATestEnv:
         # AuthDigestProvider intentionally omitted: falls back to "file".
         f'AuthUserFile "{pwfile}"',
     ]))
+    # A request which fails header validation (e.g. no Host on HTTP/1.1)
+    # is answered before the Digest module's post_read_request runs, so it
+    # has no per-request Digest record; an ErrorDocument redirecting it into
+    # a Digest-protected location must not then dereference that record.
+    conf.add('ErrorDocument 400 /digest/default/secret.txt')
+    # A reverse-proxied location which authenticates with Digest at the
+    # proxy (so the backend is never reached and need not be reachable).
+    # The challenge is an origin-style WWW-Authenticate, so AuthDigestDomain
+    # applies to it just as for a non-proxied location.
+    conf.add([
+        'ProxyPass "/pxdomain/" "http://127.0.0.1:1/unused/"',
+        '<Location "/pxdomain/">',
+        '    AuthType Digest',
+        f'    AuthName "{AAATestEnv.REALM}"',
+        '    AuthDigestProvider file',
+        f'    AuthUserFile "{pwfile}"',
+        '    AuthDigestDomain "/pxdomain/"',
+        '    Require valid-user',
+        '</Location>',
+    ])
     conf.install()
     assert env.apache_restart() == 0
     return env

test/modules/aaa/env.py

@@ -16,7 +16,7 @@ def __init__(self, env: 'HttpdTestEnv'):
         super().__init__(env=env)
         self.add_source_dir(os.path.dirname(inspect.getfile(AAATestSetup)))
         self.add_modules(["auth_digest", "authn_file", "authn_core",
-                          "authz_core", "authz_user"])
+                          "authz_core", "authz_user", "proxy", "proxy_http"])
 
 
 class AAATestEnv(HttpdTestEnv):

test/modules/aaa/test_001_challenge_response.py

@@ -167,7 +167,7 @@ def test_digest_013_invalid_opaque(self, env):
             opaque="not-a-hex-number")
         r = env.curl_get(self.url(env), options=["-H", f"Authorization: {auth}"])
         assert r.response["status"] == 401
-        env.httpd_error_log.ignore_recent(lognos=["AH01787"])
+        env.httpd_error_log.ignore_recent(lognos=["AH01782", "AH01787"])
 
     def test_digest_014_tampered_response_hash(self, env):
         challenge = self.challenge(env)
@@ -178,3 +178,20 @@ def test_digest_014_tampered_response_hash(self, env):
         r = env.curl_get(self.url(env), options=["-H", f"Authorization: {auth}"])
         assert r.response["status"] == 401
         env.httpd_error_log.ignore_recent(lognos=["AH01794"])
+
+    def test_digest_015_uri_bad_escaping_rejected(self, env):
+        # The Authorization uri= is compared against the request-target after
+        # %-decoding both. A uri= that fails to decode -- an encoded NUL or
+        # slash -- must be rejected, not silently truncated at the bad octet
+        # so that its prefix matches the request-target.
+        challenge = self.challenge(env)
+        for bad_uri in ["/digest/default/secret.txt%00ignored",
+                        "/digest/default%2fsecret.txt"]:
+            auth = dc.build_authorization(
+                AAATestEnv.DIGEST_USER, challenge, AAATestEnv.DIGEST_PASSWORD,
+                method="GET", uri=bad_uri)
+            r = env.curl_get(self.url(env),
+                             options=["-H", f"Authorization: {auth}"])
+            assert r.response["status"] == 400, \
+                f"uri={bad_uri!r} was not rejected"
+        env.httpd_error_log.ignore_recent(lognos=["AH01783"])

test/modules/aaa/test_003_nccheck.py

@@ -116,28 +116,62 @@ def test_digest_034_no_nccheck_allows_replay(self, env):
         assert r2.response["status"] == 200
 
     def test_digest_035_out_of_range_opaque_is_not_truncated(self, env):
-        # The opaque is a 32-bit client id. A value which would truncate onto
-        # a live id must not select that client. This is observable in the
-        # challenge which comes back: a client the server still knows is
-        # re-challenged with its own opaque, whereas an unknown one is given a
-        # freshly minted opaque and stale=true.
+        # The opaque is a 32- or 64-bit client id. A value which would
+        # truncate onto a live id must not select that client. This is
+        # observable in the challenge which comes back: a client the server
+        # still knows is re-challenged with its own opaque, whereas an unknown
+        # one is given a freshly minted opaque and stale=true.
         challenge = self.challenge(env, "nccheck")
         assert self.authenticate(env, "nccheck", challenge,
                                  nc="00000001").response["status"] == 200
 
-        # 2^32 + the live id, which truncates to the live id in 32 bits
-        crafted = "%x" % ((1 << 32) + int(challenge.opaque, 16))
-        auth = dc.build_authorization(
-            AAATestEnv.DIGEST_USER, challenge, AAATestEnv.DIGEST_PASSWORD,
-            method="GET", uri="/digest/nccheck/secret.txt", nc="00000002",
-            cnonce="trunc-cnonce", opaque=crafted)
-        r = env.curl_get(env.mkurl("http", "aaa", "/digest/nccheck/secret.txt"),
-                         options=["-H", f"Authorization: {auth}"])
-        # AH01787 with the range check in place; AH01776 (nonce hash) if the
-        # opaque were truncated onto the live client instead
-        env.httpd_error_log.ignore_recent(lognos=["AH01787", "AH01776"])
-        assert r.response["status"] == 401
-        new_challenge = dc.DigestChallenge.parse(
-            r.response["header"]["www-authenticate"])
-        assert new_challenge.opaque != crafted, \
-            "an out-of-range opaque was truncated onto a live client id"
+        # 2^bits + the live id, which truncates to the live id in that many
+        # bits; one of the two is out of range whichever width is in use
+        for bits in (32, 64):
+            crafted = "%x" % ((1 << bits) + int(challenge.opaque, 16))
+            auth = dc.build_authorization(
+                AAATestEnv.DIGEST_USER, challenge, AAATestEnv.DIGEST_PASSWORD,
+                method="GET", uri="/digest/nccheck/secret.txt", nc="00000002",
+                cnonce="trunc-cnonce", opaque=crafted)
+            r = env.curl_get(env.mkurl("http", "aaa", "/digest/nccheck/secret.txt"),
+                             options=["-H", f"Authorization: {auth}"])
+            # AH01782 (AH01787 before 64-bit ids) for an opaque out of range,
+            # AH01776 (nonce hash) for one in range but not the opaque the
+            # nonce was issued with
+            env.httpd_error_log.ignore_recent(lognos=["AH01782", "AH01787",
+                                                      "AH01776"])
+            assert r.response["status"] == 401
+            new_challenge = dc.DigestChallenge.parse(
+                r.response["header"]["www-authenticate"])
+            assert new_challenge.opaque != crafted, \
+                f"an opaque 2^{bits} above a live client id was truncated onto it"
+
+    def test_digest_036_malformed_nc_rejected(self, env):
+        # The nonce-count is 8 hex digits. A value which is not -- a negative
+        # number, non-hex, or the wrong length -- must be rejected rather
+        # than parsed leniently into a wrapped count.
+        challenge = self.challenge(env, "nccheck")
+        for bad in ["-0000001", "zzzzzzzz", "000000001", "1", "0000 001"]:
+            r = self.authenticate(env, "nccheck", challenge, nc=bad,
+                                  cnonce=f"nc-{len(bad)}")
+            assert r.response["status"] == 401, f"nc={bad!r} was accepted"
+        env.httpd_error_log.ignore_recent(lognos=["AH01782"])
+
+    def test_digest_037_malformed_nc_does_not_poison_client(self, env):
+        # A correctly-signed request carrying nc="-0000001" must not be
+        # accepted: before the count was validated it was read as a huge
+        # unsigned value and stored, locking the client out of every later
+        # (smaller) in-sequence count.
+        challenge = self.challenge(env, "nccheck")
+        assert self.authenticate(env, "nccheck", challenge,
+                                 nc="00000001").response["status"] == 200
+
+        r = self.authenticate(env, "nccheck", challenge, nc="-0000001",
+                              cnonce="poison-cnonce")
+        assert r.response["status"] == 401, "a negative nc was accepted"
+
+        # the legitimate client carries on with its next in-sequence count
+        assert self.authenticate(env, "nccheck", challenge,
+                                 nc="00000002").response["status"] == 200, \
+            "a malformed nc locked the client out"
+        env.httpd_error_log.ignore_recent(lognos=["AH01782"])

test/modules/aaa/test_006_config_errors.py

@@ -84,3 +84,21 @@ def test_digest_066_shmemsize_valid_accepted(self, env):
     def test_digest_067_shmemsize_units_accepted(self, env):
         r = env.configtest([], extra_top_lines=["AuthDigestShmemSize 64K"])
         assert r.exit_code == 0
+
+    def test_digest_068_shmemsize_trailing_junk_rejected(self, env):
+        # Junk after the unit character must be rejected, not silently
+        # ignored (which read "64Kfoo" as 64K).
+        r = env.configtest([], extra_top_lines=["AuthDigestShmemSize 64Kfoo"])
+        assert r.exit_code != 0
+        assert "AuthDigestShmemSize" in r.stderr
+
+    def test_digest_069_shmemsize_no_room_for_entry_rejected(self, env):
+        # A size which would hold the table and an entry by their bare
+        # struct sizes, but not once the per-allocation rmm overhead is
+        # counted, must be rejected: otherwise every request needing an
+        # entry gets a 503 the config check did not warn about. 120 bytes is
+        # above those struct sizes but below the overhead-aware minimum on
+        # both 32- and 64-bit.
+        r = env.configtest([], extra_top_lines=["AuthDigestShmemSize 120"])
+        assert r.exit_code != 0
+        assert "AuthDigestShmemSize" in r.stderr

test/modules/aaa/test_012_errordoc.py

@@ -0,0 +1,37 @@
+"""An ErrorDocument redirecting into a Digest-protected location.
+
+A request which fails header validation -- e.g. an HTTP/1.1 request with no
+Host header -- is rejected in ap_check_request_header, before
+ap_post_read_request runs. The Digest module sets up its per-request record
+in a post_read_request hook, so that record was never created for such a
+request.
+
+If ErrorDocument then redirects the failure into a Digest-protected
+location, the auth hook runs for the redirect and walks back along ->prev
+to the original request to find the shared record -- which is NULL here.
+Dereferencing it crashes the child, so an unauthenticated client can take
+down a worker with a single malformed request.
+"""
+
+from .env import AAATestEnv
+
+
+class TestDigestErrorDocument:
+
+    def test_digest_125_errordoc_into_digest_without_prr(self, env):
+        # Send an HTTP/1.1 request with the Host header removed; the server
+        # answers 400 before the Digest post_read_request hook runs, and the
+        # ErrorDocument points into a Digest-protected location.
+        url = env.mkurl("http", "aaa", "/anything")
+        r = env.curl_get(url, options=["--http1.1", "-H", "Host:"])
+
+        # The request must get an HTTP response, not a reset connection from
+        # a crashed child.
+        assert r.exit_code == 0, \
+            f"no HTTP response (curl exit {r.exit_code}): the worker likely " \
+            f"crashed handling the ErrorDocument"
+        assert r.response["status"] in (400, 401)
+
+        # The server is still serving afterwards.
+        r = env.curl_get(env.mkurl("http", "aaa", "/digest/default/secret.txt"))
+        assert r.response["status"] == 401

test/modules/aaa/test_013_proxy_domain.py

@@ -0,0 +1,25 @@
+"""AuthDigestDomain on a reverse-proxied location.
+
+Digest auth can run at a reverse proxy, for a location mapped by ProxyPass.
+Such a request carries PROXYREQ_REVERSE, but its 401 is an ordinary
+origin-style WWW-Authenticate (the client authenticates to this server, not
+through it), so the configured AuthDigestDomain belongs in the challenge
+just as for a non-proxied location.
+
+The suppression test treated any proxy request alike and dropped domain=
+for the reverse-proxied case, so clients re-authenticated per URI prefix
+and sent the Authorization header outside the intended protection space.
+"""
+
+from . import digest_client as dc
+
+
+class TestDigestProxyDomain:
+
+    def test_digest_130_domain_sent_for_reverse_proxy(self, env):
+        r = env.curl_get(env.mkurl("http", "aaa", "/pxdomain/secret.txt"))
+        assert r.response["status"] == 401
+        challenge = dc.DigestChallenge.parse(
+            r.response["header"]["www-authenticate"])
+        assert "/pxdomain/" in challenge.domain_list(), \
+            "AuthDigestDomain was dropped from a reverse-proxied challenge"