Merge pull request #340 from artgins/exact-alg-pin-verify
jwt: pin the exact algorithm in both verify paths (RFC 8725 3.1)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 7bd3a6f..fc7646a 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -446,7 +446,7 @@
list (APPEND UNIT_TESTS jwt_claims jwt_cnf)
# Security
- list (APPEND UNIT_TESTS jwt_security)
+ list (APPEND UNIT_TESTS jwt_security jwt_alg_downgrade)
foreach (TEST ${UNIT_TESTS})
jwt_add_test(NAME ${TEST})
diff --git a/libjwt/jwt-verify.c b/libjwt/jwt-verify.c
index c3c4e72..6db7b50 100644
--- a/libjwt/jwt-verify.c
+++ b/libjwt/jwt-verify.c
@@ -630,6 +630,27 @@
// LCOV_EXCL_STOP
}
+ /* @rfc{8725,3.1} Pin to the EXACT algorithm, not just the key family.
+ * The alg-vs-alg checks above compare config and key to each OTHER, but
+ * on the common pinned path -- both config->alg and config->key->alg set
+ * and equal -- none of them compares the token's own alg (jwt->alg). The
+ * jwt_alg_required_kty() backstop below is only family-granular (every
+ * RSn/PSn alg maps to JWK_KEY_TYPE_RSA, every ESn to EC), so without this
+ * a token presenting e.g. "RS512" verifies against a key the caller
+ * pinned as "RS256": same family, so the kty guard passes. Not a forgery
+ * (the signature must still be valid), but a pinned verifier accepting an
+ * algorithm it never pinned is a policy/conformance gap. Require exact
+ * agreement with whichever alg the caller pinned. Purely additive: no
+ * legitimately-pinned token is newly rejected. */
+ if (config->alg != JWT_ALG_NONE && jwt->alg != config->alg) {
+ jwt_write_error(jwt, "JWT alg does not match pinned config alg");
+ return 1;
+ } else if (config->key->alg != JWT_ALG_NONE &&
+ jwt->alg != config->key->alg) {
+ jwt_write_error(jwt, "JWT alg does not match pinned key alg");
+ return 1;
+ }
+
/* Algorithm is now bound (jwt->alg). Defensively confirm that the
* JWK's actual key type can carry it. This blocks algorithm
* confusion (GHSA-q843-6q5f-w55g) even if a malformed JWK has an
@@ -794,6 +815,16 @@
if (jwt_alg_required_kty(s->alg) != jwks_item_kty(key))
return;
+ /* @rfc{8725,3.1} Exact-alg pin: if the candidate key declares an "alg",
+ * the signature's alg must match it exactly, not merely its family (the
+ * kty gate above is family-granular -- every RSn/PSn maps to RSA). This
+ * is the JSON-path mirror of the Compact pinned-alg check and the keyring
+ * scan's own "kalg != s->alg" skip, applied at the single choke point so
+ * the explicit-key and "kid"-matched branches are covered too: without
+ * it a key declaring "RS256" would verify an "RS512" signature. */
+ if (jwks_item_alg(key) != JWT_ALG_NONE && jwks_item_alg(key) != s->alg)
+ return;
+
jwt->alg = s->alg;
jwt->key = key;
jwt->error = 0;
diff --git a/tests/jwt_alg_downgrade.c b/tests/jwt_alg_downgrade.c
new file mode 100644
index 0000000..95d5891
--- /dev/null
+++ b/tests/jwt_alg_downgrade.c
@@ -0,0 +1,125 @@
+/* Public domain, no copyright. Use at your own risk. */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "jwt_tests.h"
+
+/* Same-family algorithm downgrade in the Compact verify path.
+ *
+ * On the pinned path of __verify_config_post() -- both config->alg and
+ * config->key->alg set and equal -- the token's own alg (jwt->alg) was never
+ * compared against the pinned alg, and the jwt_alg_required_kty() backstop is
+ * only family-granular (every RSn/PSn alg maps to RSA). So a token presenting
+ * "RS512" verified against a key pinned as "RS256" -- same family, valid
+ * signature -- was accepted, i.e. a pinned verifier accepted an algorithm it
+ * never pinned. Not a forgery; a policy/conformance gap. These tests pin it
+ * shut and guard the legitimate (matching) cases against over-rejection.
+ *
+ * Runs under each crypto backend (SET_OPS); RSA only, so JSON-backend-agnostic.
+ */
+
+static jwk_set_t *load(const char *file)
+{
+ char *path;
+ jwk_set_t *set;
+ int ret;
+
+ ret = asprintf(&path, KEYDIR "/%s", file);
+ ck_assert_int_gt(ret, 0);
+ set = jwks_create_fromfile(path);
+ free(path);
+ ck_assert_ptr_nonnull(set);
+
+ return set;
+}
+
+/* Sign a token with the given alg and serialization using a key that carries no
+ * "alg" hint (so the builder lets us choose any RSA alg for the same modulus). */
+static char *sign(const jwk_item_t *key, jwt_alg_t alg, jwt_serialization_t fmt)
+{
+ jwt_builder_auto_t *b = jwt_builder_new();
+
+ ck_assert_int_eq(jwt_builder_setkey(b, alg, key), 0);
+ ck_assert_int_eq(jwt_builder_set_format(b, fmt), 0);
+
+ return jwt_builder_generate(b);
+}
+
+/* The same downgrade check on both serializations: the Compact path runs
+ * through __verify_config_post(), the JSON path through verify_entry() /
+ * try_candidate(). jwt_checker_verify() auto-detects the format. */
+START_TEST(test_same_family_downgrade)
+{
+ jwk_set_t *sks, *vks;
+ const jwk_item_t *skey, *vkey;
+ jwt_serialization_t fmts[] = { JWT_FORMAT_COMPACT, JWT_FORMAT_JSON_FLAT };
+ size_t i;
+
+ SET_OPS();
+
+ /* Same 2048-bit RSA modulus, two views: one with no "alg" (to sign any
+ * RSA alg), one pinned to RS256 (to verify). */
+ sks = load("rsa_key_2048_no_alg.json");
+ skey = jwks_item_get(sks, 0);
+ ck_assert_ptr_nonnull(skey);
+
+ vks = load("rsa_key_2048.json"); /* carries "alg":"RS256" */
+ vkey = jwks_item_get(vks, 0);
+ ck_assert_ptr_nonnull(vkey);
+
+ for (i = 0; i < ARRAY_SIZE(fmts); i++) {
+ char_auto *rs512 = NULL, *rs256 = NULL;
+ jwt_checker_auto_t *c1 = NULL, *c2 = NULL, *c3 = NULL;
+
+ /* A genuinely RS512-signed token (valid signature on this key). */
+ rs512 = sign(skey, JWT_ALG_RS512, fmts[i]);
+ ck_assert_ptr_nonnull(rs512);
+
+ /* THE GAP: pin RS256, present RS512. The signature is otherwise
+ * valid, so without the alg pin this verifies; it must be rejected. */
+ c1 = jwt_checker_new();
+ ck_assert_int_eq(jwt_checker_setkey(c1, JWT_ALG_RS256, vkey), 0);
+ ck_assert_int_ne(jwt_checker_verify(c1, rs512), 0);
+
+ /* Positive control 1: pin RS256, present RS256 -> accepted. */
+ rs256 = sign(skey, JWT_ALG_RS256, fmts[i]);
+ ck_assert_ptr_nonnull(rs256);
+ c2 = jwt_checker_new();
+ ck_assert_int_eq(jwt_checker_setkey(c2, JWT_ALG_RS256, vkey), 0);
+ ck_assert_int_eq(jwt_checker_verify(c2, rs256), 0);
+
+ /* Positive control 2: legitimately accept RS512 (via the no-alg key,
+ * which pins no alg) -> the pin does not over-reject. */
+ c3 = jwt_checker_new();
+ ck_assert_int_eq(jwt_checker_setkey(c3, JWT_ALG_RS512, skey), 0);
+ ck_assert_int_eq(jwt_checker_verify(c3, rs512), 0);
+ }
+
+ jwks_free(sks);
+ jwks_free(vks);
+}
+END_TEST
+
+static Suite *libjwt_suite(const char *title)
+{
+ Suite *s;
+ TCase *tc_core;
+ int i = ARRAY_SIZE(jwt_test_ops);
+
+ s = suite_create(title);
+ tc_core = tcase_create("jwt_alg_downgrade");
+
+ tcase_add_loop_test(tc_core, test_same_family_downgrade, 0, i);
+
+ tcase_set_timeout(tc_core, 30);
+ suite_add_tcase(s, tc_core);
+
+ return s;
+}
+
+int main(void)
+{
+ JWT_TEST_MAIN("LibJWT same-family algorithm downgrade in verify");
+}
diff --git a/tests/keys/rsa_key_2048_no_alg.json b/tests/keys/rsa_key_2048_no_alg.json
new file mode 100644
index 0000000..24f1198
--- /dev/null
+++ b/tests/keys/rsa_key_2048_no_alg.json
@@ -0,0 +1,17 @@
+ {
+ "use": "sig",
+ "key_ops": [
+ "verify",
+ "sign"
+ ],
+ "kid": "28c243a4-a9ac-473f-ab61-d3a68f408279",
+ "kty": "RSA",
+ "n": "wtpMAM4l1H995oqlqdMhuqNuffp4-4aUCwuFE9B5s9MJr63gyf8jW0oDr7Mb1Xb8y9iGkWfhouZqNJbMFry-iBs-z2TtJF06vbHQZzajDsdux3XVfXv9v6dDIImyU24MsGNkpNt0GISaaiqv51NMZQX0miOXXWdkQvWTZFXhmsFCmJLE67oQFSar4hzfAaCulaMD-b3Mcsjlh0yvSq7g6swiIasEU3qNLKaJAZEzfywroVYr3BwM1IiVbQeKgIkyPS_85M4Y6Ss_T-OWi1OeK49NdYBvFP-hNVEoeZzJz5K_nd6C35IX0t2bN5CVXchUFmaUMYk2iPdhXdsC720tBw",
+ "e": "AQAB",
+ "d": "D8dTnkETSSjlzhRuI9loAtAXM3Zj86JLPLW7GgaoxEoTn7lJ2bGicFMHB2ROnbOb9vnas82gtOtJsGaBslmoaCckp_C5T1eJWTEb-i-vdpPpwZcmKZovyyRFSE4-NYlU17fEv6DRvuaGBpDcW7QgHJIl45F8QWEM-msee2KE-V4Gz_9vAQ-sOlvsb4mJP1tJIBx9Lb5loVREwCRy2Ha9tnWdDNar8EYkOn8si4snPT-E3ZCy8mlcZyUkZeiS_HdtydxZfoiwrSRYamd1diQpPhWCeRteQ802a7ds0Y2YzgfFUaYjNuRQm7zA__hwbXS7ELPyNMU15N00bajlG0tUOQ",
+ "p": "5y8tNZdtDp3lugNnCAyA8mIcuTu7rpbGhLSUXlJXIxAzJIp_G--u6tlWR2__92R_oqZ0WAclFMmFIXTrNg2ERhngHm5x_MKU0-BQmMVLCFKQ3we0cH3QTjYGinsEyc_xT7MMtWHtG-R9nOr1YpthWd9x0HKGd8LxRashlZzM6Ts",
+ "q": "18S_ecqMVBimvLzM3BNwp9NbM4dHRatLKnBaTPh1i1a_OAiLbhvR-i9CmkrRBCDoibMTlaXF3xRYbCbzhoTAOA0E8B-Frl5l6dT6FTA5VeuT2Kr4c8C-PkIb2Ttm7KxLpyE1zplOwiOpIpZwDdy7j-B26mYKF_ZajyN9VWMy7qU",
+ "dp": "YRhyT3DSz_HHG1H0gu_ldGd6kt2gnNocdH33VooUqNhT8oPskMog1-gCEazbf4cJCEIK2THfBBUDQiL96szQgjS56W4Pl84NfdNXZmJuegdbayCsSxa8VyzfoGe8ghpAym1z5_ZCBJX5n98awphp0bpD7f07tq78cHtIdrLNaSM",
+ "dq": "yC0XOyWn1Old32IFaPN8I6cZSI_rln4ZaRD9JcWoP5JGKvT6bjfPMZ2g28Ynbf4d3opN1BsMnS6h7gyhB56nOhkSCLgl7KRVRn-5V-j6eHTrICtV_wXFObtZXMsYbOBX-4D7C2X9xG0TICyTXrj3Jb8oc8Qg_yQl1gAl6g7zFKU",
+ "qi": "ERMrkFR7KGYZG1eFNRdVmJMq-Ibxyw8ks_CbiI-n3yUyk1U8962ol2Q0T4qjBmb26L5rrhNQhneM4e8mo9FXLlQapYkPvkdrqW0Bp72A_UNAvcGTmN7z5OCJGMUutx2hmEAlrYmpLKS8pM_p9zpKtEOtzsP5GMDYVlEp1jYSjzQ"
+ }
