From 5f3abf6ccd33f8f2175846e32fd2c1c92df39f4d Mon Sep 17 00:00:00 2001 From: Hans Zandbelt Date: Fri, 27 Sep 2024 09:07:17 +0200 Subject: [PATCH] use compact encoding and preserve order for most calls to json_dumps correct usage of free() for json_dumps return values instead of cjose_get_dealloc()() Signed-off-by: Hans Zandbelt --- ChangeLog | 4 ++++ src/http.c | 2 +- src/jose.c | 12 ++++++------ src/oauth.c | 2 +- src/proto/request.c | 5 +++-- src/session.c | 6 ++++-- src/util.c | 20 +++++++++----------- 7 files changed, 28 insertions(+), 23 deletions(-) diff --git a/ChangeLog b/ChangeLog index a15ea98c..b73ed077 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +09/27/2024 +- correct usage of free() for json_dumps return values instead of cjose_get_dealloc()() +- use compact encoding and preserve order where appropriate for most calls to json_dumps + 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 diff --git a/src/http.c b/src/http.c index 3d0a7b35..8d5b752a 100644 --- a/src/http.c +++ b/src/http.c @@ -926,7 +926,7 @@ apr_byte_t oidc_http_post_json(request_rec *r, const char *url, json_t *json, co long *response_code, apr_hash_t *response_hdrs, oidc_http_timeout_t *http_timeout, const oidc_http_outgoing_proxy_t *outgoing_proxy, const apr_array_header_t *pass_cookies, const char *ssl_cert, const char *ssl_key, const char *ssl_key_pwd) { - char *data = json != NULL ? oidc_util_encode_json_object(r, json, JSON_COMPACT) : NULL; + char *data = json != NULL ? oidc_util_encode_json_object(r, json, JSON_PRESERVE_ORDER | JSON_COMPACT) : NULL; return oidc_http_request(r, url, data, OIDC_HTTP_CONTENT_TYPE_JSON, basic_auth, access_token, dpop, ssl_validate_server, response, response_code, response_hdrs, http_timeout, outgoing_proxy, pass_cookies, ssl_cert, ssl_key, ssl_key_pwd); diff --git a/src/jose.c b/src/jose.c index 129c873e..af8f8a4f 100644 --- a/src/jose.c +++ b/src/jose.c @@ -202,7 +202,7 @@ char *oidc_jwt_serialize(apr_pool_t *pool, oidc_jwt_t *jwt, oidc_jose_error_t *e cser = apr_pstrmemdup(pool, out, out_len); cjose_get_dealloc()(out); - cjose_get_dealloc()(s_payload); + free(s_payload); cser = apr_psprintf(pool, "%s.%s.", OIDC_JOSE_HDR_ALG_NONE, cser); } @@ -340,7 +340,7 @@ oidc_jwk_t *oidc_jwk_parse(apr_pool_t *pool, json_t *json, oidc_jose_error_t *er char *use = NULL; json_t *v = NULL; - char *s_json = json_dumps(json, 0); + char *s_json = json_dumps(json, JSON_PRESERVE_ORDER | JSON_COMPACT); if (s_json == NULL) { oidc_jose_error(err, "could not serialize JWK"); goto end; @@ -523,7 +523,7 @@ apr_byte_t oidc_jwk_to_json(apr_pool_t *pool, const oidc_jwk_t *jwk, char **s_js if (s == NULL) return FALSE; *s_json = apr_pstrdup(pool, s); - cjose_get_dealloc()(s); + free(s); return TRUE; } @@ -1059,7 +1059,7 @@ apr_byte_t oidc_jwt_parse(apr_pool_t *pool, const char *input_json, oidc_jwt_t * jwt->header.value.json = json_deep_copy((json_t *)hdr); char *str = json_dumps(jwt->header.value.json, JSON_PRESERVE_ORDER | JSON_COMPACT); jwt->header.value.str = apr_pstrdup(pool, str); - cjose_get_dealloc()(str); + free(str); jwt->header.alg = apr_pstrdup(pool, cjose_header_get(hdr, CJOSE_HDR_ALG, &cjose_err)); jwt->header.enc = apr_pstrdup(pool, cjose_header_get(hdr, CJOSE_HDR_ENC, &cjose_err)); @@ -1146,7 +1146,7 @@ apr_byte_t oidc_jwt_sign(apr_pool_t *pool, oidc_jwt_t *jwt, oidc_jwk_t *jwk, apr if (compress == TRUE) { if (oidc_jose_compress(pool, (char *)plaintext, _oidc_strlen(plaintext), &s_payload, &payload_len, err) == FALSE) { - cjose_get_dealloc()(plaintext); + free(plaintext); return FALSE; } } else { @@ -1156,7 +1156,7 @@ apr_byte_t oidc_jwt_sign(apr_pool_t *pool, oidc_jwt_t *jwt, oidc_jwk_t *jwk, apr } jwt->cjose_jws = cjose_jws_sign(jwk->cjose_jwk, hdr, (const uint8_t *)s_payload, payload_len, &cjose_err); - cjose_get_dealloc()(plaintext); + free(plaintext); if (jwt->cjose_jws == NULL) { oidc_jose_error(err, "cjose_jws_sign failed: %s", oidc_cjose_e2s(pool, cjose_err)); diff --git a/src/oauth.c b/src/oauth.c index cea93932..becb74b4 100644 --- a/src/oauth.c +++ b/src/oauth.c @@ -535,7 +535,7 @@ static apr_byte_t oidc_oauth_resolve_access_token(request_rec *r, oidc_cfg_t *c, } /* stringify the response */ - *response = oidc_util_encode_json_object(r, *token, JSON_COMPACT); + *response = oidc_util_encode_json_object(r, *token, JSON_PRESERVE_ORDER | JSON_COMPACT); return TRUE; } diff --git a/src/proto/request.c b/src/proto/request.c index 3aac38a7..89bd34a7 100644 --- a/src/proto/request.c +++ b/src/proto/request.c @@ -361,8 +361,9 @@ static char *oidc_request_uri_request_object(request_rec *r, struct oidc_provide apr_table_do(oidc_request_uri_delete_from_request, &data, delete_from_query_params, NULL); /* debug logging */ - oidc_debug(r, "request object: %s", - oidc_util_encode_json_object(r, request_object->payload.value.json, JSON_COMPACT)); + oidc_debug( + r, "request object: %s", + oidc_util_encode_json_object(r, request_object->payload.value.json, JSON_PRESERVE_ORDER | JSON_COMPACT)); char *serialized_request_object = NULL; oidc_jose_error_t err; diff --git a/src/session.c b/src/session.c index d261066b..f99dc51b 100644 --- a/src/session.c +++ b/src/session.c @@ -545,7 +545,9 @@ void oidc_session_set_filtered_claims(request_rec *r, oidc_session_t *z, const c } if (is_allowed == TRUE) { - s = value ? oidc_util_encode_json_object(r, value, JSON_COMPACT | JSON_ENCODE_ANY) : ""; + s = value ? oidc_util_encode_json_object(r, value, + JSON_PRESERVE_ORDER | JSON_COMPACT | JSON_ENCODE_ANY) + : ""; if (_oidc_strlen(s) > warn_claim_size) oidc_warn(r, "(encoded) value size of [%s] claim \"%s\" is larger than %d; consider " @@ -558,7 +560,7 @@ void oidc_session_set_filtered_claims(request_rec *r, oidc_session_t *z, const c iter = json_object_iter_next(src, iter); } - const char *filtered_claims = oidc_util_encode_json_object(r, dst, JSON_COMPACT); + const char *filtered_claims = oidc_util_encode_json_object(r, dst, JSON_PRESERVE_ORDER | JSON_COMPACT); filtered_claims = oidc_util_jq_filter(r, filtered_claims, oidc_util_apr_expr_exec(r, oidc_cfg_filter_claims_expr_get(c), TRUE)); json_decref(dst); diff --git a/src/util.c b/src/util.c index 1cdc43ee..bda08ead 100644 --- a/src/util.c +++ b/src/util.c @@ -945,8 +945,9 @@ apr_byte_t oidc_util_request_parameter_get(request_rec *r, char *name, char **va static apr_byte_t oidc_util_json_string_print(request_rec *r, json_t *result, const char *key, const char *log) { json_t *value = json_object_get(result, key); if (value != NULL && !json_is_null(value)) { - oidc_error(r, "%s: response contained an \"%s\" entry with value: \"%s\"", log, key, - oidc_util_encode_json_object(r, value, JSON_ENCODE_ANY)); + oidc_error( + r, "%s: response contained an \"%s\" entry with value: \"%s\"", log, key, + oidc_util_encode_json_object(r, value, JSON_PRESERVE_ORDER | JSON_COMPACT | JSON_ENCODE_ANY)); return TRUE; } return FALSE; @@ -1540,10 +1541,6 @@ void oidc_util_set_app_infos(request_rec *r, json_t *j_attrs, const char *claim_ s_key = json_object_iter_key(iter); j_value = json_object_iter_value(iter); - // char *s_value= json_dumps(j_value, JSON_ENCODE_ANY); - // oidc_util_set_app_info(r, s_key, s_value, claim_prefix); - // free(s_value); - /* check if it is a single value string */ if (json_is_string(j_value)) { @@ -1576,8 +1573,9 @@ void oidc_util_set_app_infos(request_rec *r, json_t *j_attrs, const char *claim_ } else if (json_is_object(j_value)) { /* set json value in the application header whose name is based on the key and the prefix */ - oidc_util_set_app_info(r, s_key, oidc_util_encode_json_object(r, j_value, 0), claim_prefix, - pass_in, encoding); + oidc_util_set_app_info( + r, s_key, oidc_util_encode_json_object(r, j_value, JSON_PRESERVE_ORDER | JSON_COMPACT), + claim_prefix, pass_in, encoding); /* check if it is a multi-value string */ } else if (json_is_array(j_value)) { @@ -1749,8 +1747,8 @@ apr_byte_t oidc_util_json_merge(request_rec *r, json_t *src, json_t *dst) { if ((src == NULL) || (dst == NULL)) return FALSE; - oidc_debug(r, "src=%s, dst=%s", oidc_util_encode_json_object(r, src, JSON_COMPACT), - oidc_util_encode_json_object(r, dst, JSON_COMPACT)); + oidc_debug(r, "src=%s, dst=%s", oidc_util_encode_json_object(r, src, JSON_PRESERVE_ORDER | JSON_COMPACT), + oidc_util_encode_json_object(r, dst, JSON_PRESERVE_ORDER | JSON_COMPACT)); iter = json_object_iter(src); while (iter) { @@ -1760,7 +1758,7 @@ apr_byte_t oidc_util_json_merge(request_rec *r, json_t *src, json_t *dst) { iter = json_object_iter_next(src, iter); } - oidc_debug(r, "result dst=%s", oidc_util_encode_json_object(r, dst, JSON_COMPACT)); + oidc_debug(r, "result dst=%s", oidc_util_encode_json_object(r, dst, JSON_PRESERVE_ORDER | JSON_COMPACT)); return TRUE; }