From aae688df300eaa84dde4b8998fc07f67da87855c Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Thu, 5 Nov 2026 06:40:00 +0100 Subject: [PATCH] mod_autht_jwt: Fix authentication bypass via claim-name prefix matching (BZ 69946) check_token() identified the 'aud', 'sub', 'exp' and 'nbf' claims with: strncmp("aud", kv->k->value.string.p, kv->k->value.string.len) strncmp(a, b, n) only compares the first n bytes, and n here is the actual claim-name length, not the length of the literal. A claim whose name is a proper prefix of "aud" (e.g. "a" or "au") therefore satisfies this check and is treated as the real 'aud' claim. A JWT that omits 'aud' entirely but carries such a short claim set to the expected AuthName value then passes the audience check that exists specifically to confirm the token was issued for this location -- an authentication bypass. The same pattern affected 'sub', 'exp' and 'nbf'. Fix: require the claim-name length to match the literal's length before comparing, so only an exact match counts. Confirmed with a standalone C harness mirroring the exact comparison (strncmp("aud", "au", 2) == 0 before the fix; correctly rejected after). Confirmed via direct code reading that the bug has been present since the module's creation in d8246a15d7 (April 2023) and no later commit touches these lines. Verified against a real, running httpd + mod_autht_jwt (built apr-util 1.7 from source with OpenSSL crypto support, since the module needs apr_jose which requires apr-util >= 1.7). With the fix applied, a JWT with no 'aud' claim but an 'au' claim set to the AuthName value (the bypass) is rejected with the same AH10444 'aud value was missing' message as a token with no aud-like claim at all; same result for a single-character 'a' claim. Added a pytest regression test (test_mod_autht_jwt.py) building real HS256 JWTs by hand. Two test-infrastructure bugs found and fixed along the way, independent of the production fix above: - AuthtJwtVerify reads its secret file while parsing the config, before httpd starts; writing it from inside each test function (as the first version of this test did) runs too late. Fixed by writing it once, centrally, the same way TestConfig already generates the SSL CA (apache_pytest/config.py). - modules/aaa/mod_autht_jwt.c can be present as a loadable module (satisfying ) while still being a non-functional stub that registers neither AuthtJwtVerify nor the 'jwt' Autht provider, on any build whose apr-util lacks apr_jose (a separate, pre-existing bug, BZ 70276/70277). Without a guard, the new autht_jwt.conf.in fragment broke httpd startup -- and the entire shared pytest_suite session, not just these four tests -- on exactly such a CI leg (confirmed via the actual failing GitHub Actions log for PR #950, both 'Python pytest_suite, motorz MPM' and the 64-bit ARM job). Fixed by also guarding on , which I verified directly with httpd -t against both a stub build (skips the block, Syntax OK) and a real apr-util-1.7-linked build (still applies the block). Claude-Session: https://claude.ai/code/session_01BrsHTcwUro4KV5Fp5hv8XM --- changes-entries/69946.txt | 3 + modules/aaa/mod_autht_jwt.c | 12 +- test/pyhttpd/classic_errorlog.py | 1 + test/pytest_suite/.gitignore | 1 + test/pytest_suite/apache_pytest/config.py | 36 ++++++ test/pytest_suite/t/conf/autht_jwt.conf.in | 31 +++++ .../t/htdocs/autht-jwt-test/index.html | 1 + .../tests/t/modules/test_mod_autht_jwt.py | 117 ++++++++++++++++++ 8 files changed, 198 insertions(+), 4 deletions(-) create mode 100644 changes-entries/69946.txt create mode 100644 test/pytest_suite/t/conf/autht_jwt.conf.in create mode 100644 test/pytest_suite/t/htdocs/autht-jwt-test/index.html create mode 100644 test/pytest_suite/tests/t/modules/test_mod_autht_jwt.py diff --git a/changes-entries/69946.txt b/changes-entries/69946.txt new file mode 100644 index 00000000000..94221f40710 --- /dev/null +++ b/changes-entries/69946.txt @@ -0,0 +1,3 @@ + *) mod_autht_jwt: Fix an authentication bypass where a JWT claim whose + name was a prefix of "aud", "sub", "exp" or "nbf" (e.g. "a" or "au") + could be mistaken for that claim. PR 69946 [Arturo Bernal] diff --git a/modules/aaa/mod_autht_jwt.c b/modules/aaa/mod_autht_jwt.c index 617344dfdd7..dea0606586a 100644 --- a/modules/aaa/mod_autht_jwt.c +++ b/modules/aaa/mod_autht_jwt.c @@ -966,28 +966,32 @@ static autht_status check_token(request_rec *r, const char *type, continue; } - if (!strncmp("aud", kv->k->value.string.p, kv->k->value.string.len)) { + if (kv->k->value.string.len == (sizeof("aud") - 1) + && !strncmp("aud", kv->k->value.string.p, kv->k->value.string.len)) { if (kv->v->type == APR_JSON_STRING) { aud = apr_pstrndup(r->pool, kv->v->value.string.p, kv->v->value.string.len); } } - if (!strncmp("sub", kv->k->value.string.p, kv->k->value.string.len)) { + if (kv->k->value.string.len == (sizeof("sub") - 1) + && !strncmp("sub", kv->k->value.string.p, kv->k->value.string.len)) { if (kv->v->type == APR_JSON_STRING) { sub = apr_pstrndup(r->pool, kv->v->value.string.p, kv->v->value.string.len); } } - if (!strncmp("exp", kv->k->value.string.p, kv->k->value.string.len)) { + if (kv->k->value.string.len == (sizeof("exp") - 1) + && !strncmp("exp", kv->k->value.string.p, kv->k->value.string.len)) { if (kv->v->type == APR_JSON_LONG) { exp = kv->v->value.lnumber; exp_set = 1; } } - if (!strncmp("nbf", kv->k->value.string.p, kv->k->value.string.len)) { + if (kv->k->value.string.len == (sizeof("nbf") - 1) + && !strncmp("nbf", kv->k->value.string.p, kv->k->value.string.len)) { if (kv->v->type == APR_JSON_LONG) { nbf = kv->v->value.lnumber; nbf_set = 1; diff --git a/test/pyhttpd/classic_errorlog.py b/test/pyhttpd/classic_errorlog.py index 8e9c88c01d3..9c8b6e6f7a2 100644 --- a/test/pyhttpd/classic_errorlog.py +++ b/test/pyhttpd/classic_errorlog.py @@ -79,6 +79,7 @@ r".*\(20023\)The given path was above the root path:.*unable to include file", r".*\[include:error\].* unable to include \""]), "test_ldap.py": (["AH01617", "AH01631"], []), + "test_mod_autht_jwt.py": (["AH10428"], []), "test_mergeslashes.py": (["AH01630"], []), "test_negotiation.py": (["AH00690"], []), "test_ocsp.py": (["AH03239", "AH10373"], []), diff --git a/test/pytest_suite/.gitignore b/test/pytest_suite/.gitignore index d81d4779ab7..e13e9b1b778 100644 --- a/test/pytest_suite/.gitignore +++ b/test/pytest_suite/.gitignore @@ -31,6 +31,7 @@ t/conf/ssl/ca t/conf/cacheroot/ t/logs/ t/state/ +t/autht_jwt.secret # Generated htpasswd / htdigest files t/basic1 diff --git a/test/pytest_suite/apache_pytest/config.py b/test/pytest_suite/apache_pytest/config.py index d6b4afed71c..74426e7ea8a 100644 --- a/test/pytest_suite/apache_pytest/config.py +++ b/test/pytest_suite/apache_pytest/config.py @@ -670,6 +670,38 @@ def _maybe_generate_sslca(self) -> None: SSLCA(Path(self.vars["sslca"]), self.vars["servername"]).generate() + # Shared with test/pytest_suite/tests/t/modules/test_mod_autht_jwt.py, + # which signs its JWTs with this same value. + AUTHT_JWT_SECRET = b"bz69946-test-secret-do-not-use-in-prod" + + def _maybe_generate_autht_jwt_secret(self) -> None: + """Write the HS256 secret AuthtJwtVerify reads while parsing its + config (modules/aaa/mod_autht_jwt.c:set_jwt_verify), so the file is + already in place before httpd starts -- a missing file is a fatal + config error, and writing it from a test function runs too late. + + Also sets the @jwt_crypto_driver@ token: AuthtJwtVerify's HS256 path + needs an apr_crypto driver, which mod_autht_jwt only initializes when + AuthtJwtDriver names one explicitly (modules/aaa/mod_autht_jwt.c: + auth_bearer_init() skips apr_crypto_get_driver() entirely unless + library_set is true). Without it every token fails HMAC verification + before the 'aud' claim is even inspected. The driver name mirrors + apr_crypto.h's APU_CRYPTO_RECOMMENDED_DRIVER per platform. + + The token is set unconditionally, before the has_module() check below: + process_conf_in() expands every @token@ in autht_jwt.conf.in textually, + regardless of the guard around its use (that + guard is only meaningful to httpd at runtime, not to this expansion + pass) -- on a build without mod_autht_jwt, returning early here would + leave the token unset and crash config generation with a KeyError. + """ + self.vars["jwt_crypto_driver"] = "commoncrypto" if sys.platform == "darwin" else "openssl" + if not self.info.has_module("mod_autht_jwt"): + return + secret = Path(self.vars["serverroot"]) / "autht_jwt.secret" + if not secret.exists(): + secret.write_bytes(self.AUTHT_JWT_SECRET) + def _emit_inherited_loadmodules(self) -> None: """Re-emit inherited (active) LoadModule lines (inherit_load_module). @@ -829,6 +861,10 @@ def generate(self, cmodule_loads: list[tuple[str, Path]] | None = None) -> Path: # dir exists (mirrors sslca_can/sslca_generate, TestConfig.pm:1524-1554). self._maybe_generate_sslca() + # Same reasoning as the SSL CA: AuthtJwtVerify reads its secret file + # while parsing the config, so it must exist before server.start(). + self._maybe_generate_autht_jwt_secret() + # Re-emit inherited LoadModule directives so the test server actually # loads modules (the generated conf does not Include the install conf). # Mirrors inherit_load_module (TestConfigParse.pm:213): wrap each in diff --git a/test/pytest_suite/t/conf/autht_jwt.conf.in b/test/pytest_suite/t/conf/autht_jwt.conf.in new file mode 100644 index 00000000000..6110dd74778 --- /dev/null +++ b/test/pytest_suite/t/conf/autht_jwt.conf.in @@ -0,0 +1,31 @@ +# +# Test config for mod_autht_jwt (see BZ 69946) +# +# mod_autht_jwt.c may be present as a loadable module (satisfying +# ) but still be a non-functional stub that registers neither +# the AuthtJwtVerify directive nor the "jwt" Autht provider, on any build +# whose apr-util lacks apr_jose support (BZ 70276/70277). Guard on the +# directive itself, not just the module, so this fragment never breaks +# httpd startup -- and the whole pytest_suite session -- on such a build. + + + + + # AuthtJwtVerify's HS256 path needs an apr_crypto driver, which + # mod_autht_jwt only initializes when a library is explicitly named + # (its own default library field is never marked "set" on its own). + # Without this, every token fails HMAC verification before the + # 'aud' claim is even inspected, masking the BZ 69946 fix either way. + AuthtJwtDriver @jwt_crypto_driver@ + + + AuthType bearer + AuthName autht-jwt-test + AuthBearerProvider jwt + AuthtJwtVerify hs256 file "@SERVERROOT@/autht_jwt.secret" + Require valid-user + + + + + diff --git a/test/pytest_suite/t/htdocs/autht-jwt-test/index.html b/test/pytest_suite/t/htdocs/autht-jwt-test/index.html new file mode 100644 index 00000000000..9a4b59d9b01 --- /dev/null +++ b/test/pytest_suite/t/htdocs/autht-jwt-test/index.html @@ -0,0 +1 @@ +jwt-ok diff --git a/test/pytest_suite/tests/t/modules/test_mod_autht_jwt.py b/test/pytest_suite/tests/t/modules/test_mod_autht_jwt.py new file mode 100644 index 00000000000..d87c2b046d2 --- /dev/null +++ b/test/pytest_suite/tests/t/modules/test_mod_autht_jwt.py @@ -0,0 +1,117 @@ +r"""Regression test for BZ 69946 -- mod_autht_jwt authentication bypass via +claim-name prefix matching. + +``check_token()`` in modules/aaa/mod_autht_jwt.c identified the ``aud``, +``sub``, ``exp`` and ``nbf`` claims with: + + strncmp("aud", kv->k->value.string.p, kv->k->value.string.len) + +``strncmp(a, b, n)`` only compares the first ``n`` bytes. Since ``n`` here is +the *actual* claim-name length (not the length of the literal), a claim +whose name is a proper prefix of "aud" (e.g. "a" or "au") satisfies this +check and is treated as if it were the real "aud" claim. A JWT that omits +"aud" entirely but carries such a short claim with the expected AuthName +value then passes the audience check that is supposed to confirm the token +was issued for this specific location. + +This test builds real HS256 JWTs (no external `jwt` package -- the standard +library's hmac/hashlib/base64 are enough) and checks that a token exploiting +the prefix match is rejected, while a token using the real "aud" claim name +still works. +""" + +import base64 +import hashlib +import hmac +import json + +import pytest + +from apache_pytest import need_module + +# Kept in sync with apache_pytest.config.TestConfig.AUTHT_JWT_SECRET, which +# writes this value to the file AuthtJwtVerify loads before httpd starts. +SECRET = b"bz69946-test-secret-do-not-use-in-prod" + + +def _b64url(data: bytes) -> str: + return base64.urlsafe_b64encode(data).rstrip(b"=").decode("ascii") + + +def _make_jwt(claims: dict) -> str: + header = _b64url(json.dumps({"alg": "HS256", "typ": "JWT"}).encode()) + payload = _b64url(json.dumps(claims).encode()) + signing_input = f"{header}.{payload}".encode() + sig = hmac.new(SECRET, signing_input, hashlib.sha256).digest() + return f"{header}.{payload}.{_b64url(sig)}" + + +def _skip_unless_functional(http): + """need_module() only checks that mod_autht_jwt.so is loaded, not that it + was built with apr_jose/JWT support (BZ 70276/70277): a stub build + registers the same module name but neither the AuthtJwtVerify directive + nor the "jwt" Autht provider, so in + autht_jwt.conf.in omits the /autht-jwt-test/ Location entirely. On such a + build this request hits a plain, unauthenticated static file (200); on a + functional build AuthType bearer with no Authorization header is + rejected (401). Skip explicitly rather than failing on the stub build's + 404/200, and never silently pass tests that never actually exercised the + module.""" + r = http.GET("/autht-jwt-test/") + if r.status_code != 401: + pytest.skip( + "mod_autht_jwt built without JOSE/JWT support " + f"(stub build, BZ 70276/70277; got {r.status_code} with no Authorization header)" + ) + + +@need_module("autht_jwt", "auth_bearer", "autht_core") +def test_jwt_aud_exact_match_is_accepted(http): + """Sanity check: a token with the real 'aud' claim is accepted.""" + _skip_unless_functional(http) + token = _make_jwt({"aud": "autht-jwt-test", "sub": "alice"}) + r = http.GET("/autht-jwt-test/", headers={"Authorization": f"Bearer {token}"}) + assert r.status_code == 200, ( + f"expected 200 for a valid 'aud' claim, got {r.status_code}" + ) + + +@need_module("autht_jwt", "auth_bearer", "autht_core") +def test_jwt_missing_aud_is_rejected(http): + """Sanity check: a token with no audience-like claim at all is rejected.""" + _skip_unless_functional(http) + token = _make_jwt({"foo": "bar", "sub": "alice"}) + r = http.GET("/autht-jwt-test/", headers={"Authorization": f"Bearer {token}"}) + assert r.status_code == 401, ( + f"expected 401 with no 'aud' claim present, got {r.status_code}" + ) + + +@need_module("autht_jwt", "auth_bearer", "autht_core") +def test_jwt_aud_prefix_bypass_is_rejected(http): + """BZ 69946: a claim whose name is a prefix of 'aud' ('au') must NOT be + treated as the 'aud' claim. Before the fix this token is wrongly + AUTHT_GRANTED (200); after the fix it must be AUTHT_MISMATCH (401). + + 'sub' is required here: Require valid-user needs r->user, which + check_token() only sets when 'sub' is present. Without it this test + would get 401 regardless of the aud bug and prove nothing.""" + _skip_unless_functional(http) + token = _make_jwt({"au": "autht-jwt-test", "sub": "alice"}) + r = http.GET("/autht-jwt-test/", headers={"Authorization": f"Bearer {token}"}) + assert r.status_code == 401, ( + "authentication bypass: a claim named 'au' (a prefix of 'aud') with " + f"the correct AuthName value was accepted as 'aud' (got {r.status_code})" + ) + + +@need_module("autht_jwt", "auth_bearer", "autht_core") +def test_jwt_aud_single_char_prefix_bypass_is_rejected(http): + """Same bypass with the shortest possible prefix, 'a'.""" + _skip_unless_functional(http) + token = _make_jwt({"a": "autht-jwt-test", "sub": "alice"}) + r = http.GET("/autht-jwt-test/", headers={"Authorization": f"Bearer {token}"}) + assert r.status_code == 401, ( + "authentication bypass: a claim named 'a' (a prefix of 'aud') with " + f"the correct AuthName value was accepted as 'aud' (got {r.status_code})" + )