mod_autht_jwt: Fix authentication bypass via claim-name prefix matching (BZ 69946)

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]

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;

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"], []),

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

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 <IfModule mod_autht_jwt.c> 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

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
+# <IfModule>) 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.
+<IfModule mod_autht_jwt.c>
+  <IfModule mod_auth_bearer.c>
+    <IfModule mod_autht_core.c>
+      <IfDirective AuthtJwtVerify>
+        # 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@
+
+        <Location /autht-jwt-test>
+          AuthType bearer
+          AuthName autht-jwt-test
+          AuthBearerProvider jwt
+          AuthtJwtVerify hs256 file "@SERVERROOT@/autht_jwt.secret"
+          Require valid-user
+        </Location>
+      </IfDirective>
+    </IfModule>
+  </IfModule>
+</IfModule>

test/pytest_suite/t/htdocs/autht-jwt-test/index.html

@@ -0,0 +1 @@
+jwt-ok

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 <IfDirective AuthtJwtVerify> 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})"
+    )