From 0da954539352d74b04bf8e51abcf6cdf492d4227 Mon Sep 17 00:00:00 2001 From: Hans Zandbelt Date: Thu, 26 Sep 2024 22:26:40 +0200 Subject: [PATCH] 2.4.16.4rc2: fix oidc_jwk_copy wrt. "x5t" which broke private_key_jwt authentication to Azure AD since 2.4.13; see #1269; thanks @uoe-pjackson Signed-off-by: Hans Zandbelt --- ChangeLog | 5 +++++ configure.ac | 2 +- src/jose.c | 51 ++++++++++++++++++++++++++++++++++--------------- src/jose.h | 2 +- test/test-cmd.c | 14 +++++++++++--- test/test.c | 42 +++++++++++++++++++++++++++++++++++++++- 6 files changed, 95 insertions(+), 21 deletions(-) diff --git a/ChangeLog b/ChangeLog index f96db47c..a15ea98c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +09/26/2024 +- fix oidc_jwk_copy wrt. "x5t", which broke private_key_jwt authentication to Azure AD since 2.4.13 + see #1269; thanks @uoe-pjackson +- bump to 2.4.16.4rc2 + 09/21/2024 - refactor state and userinfo diff --git a/configure.ac b/configure.ac index 8a824d89..5167b6bb 100644 --- a/configure.ac +++ b/configure.ac @@ -1,4 +1,4 @@ -AC_INIT([mod_auth_openidc],[2.4.16.4rc1],[hans.zandbelt@openidc.com]) +AC_INIT([mod_auth_openidc],[2.4.16.4rc2],[hans.zandbelt@openidc.com]) AC_SUBST(NAMEVER, AC_PACKAGE_TARNAME()-AC_PACKAGE_VERSION()) diff --git a/src/jose.c b/src/jose.c index c8348441..129c873e 100644 --- a/src/jose.c +++ b/src/jose.c @@ -332,17 +332,17 @@ static oidc_jwk_t *oidc_jwk_from_cjose(apr_pool_t *pool, cjose_jwk_t *cjose_jwk, /* * parse a JSON string to a JWK struct */ -oidc_jwk_t *oidc_jwk_parse(apr_pool_t *pool, const char *s_json, oidc_jose_error_t *err) { +oidc_jwk_t *oidc_jwk_parse(apr_pool_t *pool, json_t *json, oidc_jose_error_t *err) { oidc_jwk_t *result = NULL; cjose_jwk_t *cjose_jwk = NULL; cjose_err cjose_err; - json_error_t json_error; oidc_jose_error_t x5c_err; char *use = NULL; + json_t *v = NULL; - json_t *json = json_loads(s_json, 0, &json_error); - if (json == NULL) { - oidc_jose_error(err, "could not parse JWK: %s (%s)", json_error.text, s_json); + char *s_json = json_dumps(json, 0); + if (s_json == NULL) { + oidc_jose_error(err, "could not serialize JWK"); goto end; } @@ -362,10 +362,22 @@ oidc_jwk_t *oidc_jwk_parse(apr_pool_t *pool, const char *s_json, oidc_jose_error result = oidc_jwk_from_cjose(pool, cjose_jwk, use); + // TODO: parse the optional x5c array + + // set x5t#256 + v = json_object_get(json, OIDC_JOSE_JWK_X5T256_STR); + if (v) + result->x5t_S256 = apr_pstrdup(pool, json_string_value(v)); + + // set x5t + v = json_object_get(json, OIDC_JOSE_JWK_X5T_STR); + if (v) + result->x5t = apr_pstrdup(pool, json_string_value(v)); + end: - if (json) - json_decref(json); + if (s_json) + free(s_json); return result; } @@ -374,11 +386,22 @@ oidc_jwk_t *oidc_jwk_parse(apr_pool_t *pool, const char *s_json, oidc_jose_error * copy a JWK by converting oidc_jwk_t to JSON and parsing it back */ oidc_jwk_t *oidc_jwk_copy(apr_pool_t *pool, const oidc_jwk_t *src) { - char *s_json = NULL; - oidc_jose_error_t err; - if (oidc_jwk_to_json(pool, src, &s_json, &err) == FALSE) - return NULL; - return oidc_jwk_parse(pool, s_json, &err); + int i = 0; + cjose_err err; + oidc_jwk_t *dst = oidc_jwk_new(pool); + dst->cjose_jwk = cjose_jwk_retain(src->cjose_jwk, &err); + dst->kid = apr_pstrdup(pool, src->kid); + dst->kty = src->kty; + dst->use = apr_pstrdup(pool, src->use); + dst->x5c = NULL; + if (src->x5c) { + dst->x5c = apr_array_make(pool, src->x5c->nelts, sizeof(char *)); + for (i = 0; i < src->x5c->nelts; i++) + APR_ARRAY_PUSH(dst->x5c, char *) = APR_ARRAY_IDX(src->x5c, i, char *); + } + dst->x5t = apr_pstrdup(pool, src->x5t); + dst->x5t_S256 = apr_pstrdup(pool, src->x5t_S256); + return dst; } /* @@ -443,9 +466,7 @@ void oidc_jwk_list_destroy(apr_array_header_t *keys_list) { * parse a JSON object in to a JWK struct */ apr_byte_t oidc_jwk_parse_json(apr_pool_t *pool, json_t *json, oidc_jwk_t **jwk, oidc_jose_error_t *err) { - char *s_json = json_dumps(json, 0); - *jwk = oidc_jwk_parse(pool, s_json, err); - cjose_get_dealloc()(s_json); + *jwk = oidc_jwk_parse(pool, json, err); return (*jwk != NULL); } diff --git a/src/jose.h b/src/jose.h index d246e654..4802c29a 100644 --- a/src/jose.h +++ b/src/jose.h @@ -174,7 +174,7 @@ typedef struct oidc_jwk_t { apr_byte_t oidc_jwe_decrypt(apr_pool_t *pool, const char *input_json, apr_hash_t *keys, char **plaintext, int *plaintext_len, oidc_jose_error_t *err, apr_byte_t import_must_succeed); /* parse a JSON string (JWK) to a JWK struct */ -oidc_jwk_t *oidc_jwk_parse(apr_pool_t *pool, const char *s_json, oidc_jose_error_t *err); +oidc_jwk_t *oidc_jwk_parse(apr_pool_t *pool, json_t *json, oidc_jose_error_t *err); oidc_jwk_t *oidc_jwk_copy(apr_pool_t *pool, const oidc_jwk_t *jwk); /* parse a JSON object (JWK) in to a JWK struct */ apr_byte_t oidc_jwk_parse_json(apr_pool_t *pool, json_t *json, oidc_jwk_t **jwk, oidc_jose_error_t *err); diff --git a/test/test-cmd.c b/test/test-cmd.c index 58ae08ff..a0ffaf65 100644 --- a/test/test-cmd.c +++ b/test/test-cmd.c @@ -163,8 +163,11 @@ int verify(int argc, char **argv, apr_pool_t *pool) { return -1; } + json_error_t json_error; + json_t *json = json_loads(s_jwk, 0, &json_error); + oidc_jose_error_t oidc_err; - oidc_jwk_t *jwk = oidc_jwk_parse(pool, s_jwk, &oidc_err); + oidc_jwk_t *jwk = oidc_jwk_parse(pool, json, &oidc_err); if (jwk == NULL) { fprintf(stderr, "could not import JWK: %s [file: %s, function: %s, line: %d]\n", oidc_err.text, oidc_err.source, oidc_err.function, oidc_err.line); @@ -189,6 +192,7 @@ int verify(int argc, char **argv, apr_pool_t *pool) { cjose_jws_release(jws); oidc_jwk_destroy(jwk); + json_decref(json); return 0; } @@ -206,9 +210,12 @@ int decrypt(int argc, char **argv, apr_pool_t *pool) { return -1; apr_hash_t *keys = apr_hash_make(pool); - oidc_jose_error_t oidc_err; - oidc_jwk_t *jwk = oidc_jwk_parse(pool, s_jwk, &oidc_err); + json_error_t json_error; + json_t *json = json_loads(s_jwk, 0, &json_error); + + oidc_jose_error_t oidc_err; + oidc_jwk_t *jwk = oidc_jwk_parse(pool, json, &oidc_err); if (jwk == NULL) { fprintf(stderr, "could not import JWK: %s [file: %s, function: %s, line: %d]\n", oidc_err.text, oidc_err.source, oidc_err.function, oidc_err.line); @@ -226,6 +233,7 @@ int decrypt(int argc, char **argv, apr_pool_t *pool) { fprintf(stdout, "%s", plaintext); oidc_jwk_destroy(jwk); + json_decref(json); return 0; } diff --git a/test/test.c b/test/test.c index 68dc4fb8..da8821a1 100644 --- a/test/test.c +++ b/test/test.c @@ -119,9 +119,12 @@ static int TST_RC; return message; static char *_jwk_parse(apr_pool_t *pool, const char *s, oidc_jwk_t **jwk, oidc_jose_error_t *err) { - oidc_jwk_t *k = oidc_jwk_parse(pool, s, err); + json_error_t json_err; + json_t *json = json_loads(s, 0, &json_err); + oidc_jwk_t *k = oidc_jwk_parse(pool, json, err); TST_ASSERT_ERR("oidc_jwk_parse", k != NULL, pool, (*err)); *jwk = k; + json_decref(json); return 0; } @@ -777,6 +780,42 @@ static char *test_jwk_parse_json(apr_pool_t *pool) { return 0; } +static char *test_jwk_copy(apr_pool_t *pool) { + oidc_jose_error_t err; + char *s = NULL; + oidc_jwk_t *jwk1 = NULL; + oidc_jwk_t *jwk2 = NULL; + + s = "{" + "\"kty\": \"RSA\"," + "\"kid\": \"bilbo.baggins@hobbiton.example\"," + "\"use\": \"sig\"," + "\"n\": \"n4EPtAOCc9AlkeQHPzHStgAbgs7bTZLwUBZdR8_KuKPEHLd4rHVTeT" + "-O-XV2jRojdNhxJWTDvNd7nqQ0VEiZQHz_AJmSCpMaJMRBSFKrKb2wqV" + "wGU_NsYOYL-QtiWN2lbzcEe6XC0dApr5ydQLrHqkHHig3RBordaZ6Aj-" + "oBHqFEHYpPe7Tpe-OfVfHd1E6cS6M1FZcD1NNLYD5lFHpPI9bTwJlsde" + "3uhGqC0ZCuEHg8lhzwOHrtIQbS0FVbb9k3-tVTU4fg_3L_vniUFAKwuC" + "LqKnS2BYwdq_mzSnbLY7h_qixoR7jig3__kRhuaxwUkRz5iaiQkqgc5g" + "HdrNP5zw\"," + "\"e\": \"AQAB\"," + "\"x5t\": \"myx5t\"," + "\"x5t#S256\": \"myx5t#S256\"" + "}"; + + jwk1 = NULL; + TST_ASSERT_ERR("oidc_jwk_parse", _jwk_parse(pool, s, &jwk1, &err) == 0, pool, err); + jwk2 = oidc_jwk_copy(pool, jwk1); + + TST_ASSERT_STR("oidc_jwk_parse (x5t)", jwk1->x5t, "myx5t"); + TST_ASSERT_STR("oidc_jwk_parse (x5t#S256)", jwk1->x5t_S256, "myx5t#S256"); + TST_ASSERT_STR("oidc_jwk_copy (x5t)", jwk2->x5t, "myx5t"); + TST_ASSERT_STR("oidc_jwk_copy (x5t#S256)", jwk2->x5t_S256, "myx5t#S256"); + + oidc_jwk_destroy(jwk2); + oidc_jwk_destroy(jwk1); + + return 0; +} static char *test_plaintext_decrypt_symmetric(apr_pool_t *pool) { oidc_jose_error_t err; apr_hash_t *keys = apr_hash_make(pool); @@ -1703,6 +1742,7 @@ static char *all_tests(apr_pool_t *pool, request_rec *r) { TST_RUN(test_jwt_get_string, pool); TST_RUN(test_jwk_parse_json, pool); + TST_RUN(test_jwk_copy, pool); TST_RUN(test_plaintext_decrypt_symmetric, pool); #if (OPENSSL_VERSION_NUMBER >= 0x1000100f)