diff --git a/ChangeLog b/ChangeLog index ff4406aa..0a2bea4b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,6 @@ +08/08/2024 +- support DPoP nonces to the userinfo endpoint + 06/07/2024 - return response headers from outgoing HTTP requests to callers - support DPoP nonces to the token endpoint diff --git a/src/handle/dpop.c b/src/handle/dpop.c index c1a837c7..1c8b2e27 100644 --- a/src/handle/dpop.c +++ b/src/handle/dpop.c @@ -119,8 +119,8 @@ int oidc_dpop_request(request_rec *r, oidc_cfg_t *c, oidc_session_t *session) { } /* create the DPoP header value */ - s_dpop = oidc_proto_dpop_create(r, c, s_url, s_method, s_access_token, s_nonce); - if (s_dpop == NULL) { + if ((oidc_proto_dpop_create(r, c, s_url, s_method, s_access_token, s_nonce, &s_dpop) == FALSE) || + (s_dpop == NULL)) { oidc_error(r, "creating the DPoP proof value failed"); rc = HTTP_INTERNAL_SERVER_ERROR; goto end; diff --git a/src/proto/dpop.c b/src/proto/dpop.c index 6571b8ea..34817ca5 100644 --- a/src/proto/dpop.c +++ b/src/proto/dpop.c @@ -46,19 +46,43 @@ #define OIDC_PROTO_DPOP_JWT_TYP "dpop+jwt" +apr_byte_t oidc_proto_dpop_use_nonce(request_rec *r, oidc_cfg_t *cfg, json_t *j_result, apr_hash_t *response_hdrs, + const char *url, const char *method, const char *access_token, char **dpop) { + apr_byte_t rv = FALSE; + char *dpop_nonce = NULL; + + json_t *j_error = json_object_get(j_result, OIDC_PROTO_ERROR); + if ((j_error == NULL) || (!json_is_string(j_error)) || + (_oidc_strcmp(json_string_value(j_error), OIDC_PROTO_DPOP_USE_NONCE) != 0)) + goto end; + + /* try again with a DPoP nonce provided by the server */ + dpop_nonce = (char *)apr_hash_get(response_hdrs, OIDC_HTTP_HDR_DPOP_NONCE, APR_HASH_KEY_STRING); + if (dpop_nonce == NULL) { + oidc_error(r, "error is \"%s\" but no \"%s\" header found", OIDC_PROTO_DPOP_USE_NONCE, + OIDC_HTTP_HDR_DPOP_NONCE); + goto end; + } + + rv = oidc_proto_dpop_create(r, cfg, url, method, access_token, dpop_nonce, dpop); + +end: + + return rv; +} + /* * generate a DPoP proof for the specified URL/method/access_token */ -char *oidc_proto_dpop_create(request_rec *r, oidc_cfg_t *cfg, const char *url, const char *method, - const char *access_token, const char *nonce) { - // TODO: share with create_userinfo_jwt +apr_byte_t oidc_proto_dpop_create(request_rec *r, oidc_cfg_t *cfg, const char *url, const char *method, + const char *access_token, const char *nonce, char **dpop) { + apr_byte_t rv = FALSE; oidc_jwt_t *jwt = NULL; oidc_jwk_t *jwk = NULL; oidc_jose_error_t err; char *jti = NULL; cjose_err cjose_err; char *s_jwk = NULL; - char *cser = NULL; char *ath = NULL; oidc_debug(r, "enter"); @@ -88,9 +112,11 @@ char *oidc_proto_dpop_create(request_rec *r, oidc_cfg_t *cfg, const char *url, c if (nonce != NULL) json_object_set_new(jwt->payload.value.json, OIDC_CLAIM_NONCE, json_string(nonce)); - if (oidc_proto_jwt_sign_and_serialize(r, jwk, jwt, &cser) == FALSE) + if (oidc_proto_jwt_sign_and_serialize(r, jwk, jwt, dpop) == FALSE) goto end; + rv = TRUE; + end: if (s_jwk) @@ -99,5 +125,5 @@ char *oidc_proto_dpop_create(request_rec *r, oidc_cfg_t *cfg, const char *url, c if (jwt) oidc_jwt_destroy(jwt); - return cser; + return rv; } diff --git a/src/proto/proto.h b/src/proto/proto.h index 2d514b9a..ad7a3a74 100644 --- a/src/proto/proto.h +++ b/src/proto/proto.h @@ -133,8 +133,10 @@ apr_byte_t oidc_proto_discovery_account_based(request_rec *r, oidc_cfg_t *cfg, c apr_byte_t oidc_proto_discovery_url_based(request_rec *r, oidc_cfg_t *cfg, const char *url, char **issuer); // dpop.c -char *oidc_proto_dpop_create(request_rec *r, oidc_cfg_t *cfg, const char *url, const char *method, - const char *access_token, const char *nonce); +apr_byte_t oidc_proto_dpop_create(request_rec *r, oidc_cfg_t *cfg, const char *url, const char *method, + const char *access_token, const char *nonce, char **dpop); +apr_byte_t oidc_proto_dpop_use_nonce(request_rec *r, oidc_cfg_t *cfg, json_t *j_result, apr_hash_t *response_hdrs, + const char *url, const char *method, const char *access_token, char **dpop); // id_token.c apr_byte_t oidc_proto_idtoken_parse(request_rec *r, oidc_cfg_t *cfg, oidc_provider_t *provider, const char *id_token, diff --git a/src/proto/token.c b/src/proto/token.c index 2a422387..77cd6319 100644 --- a/src/proto/token.c +++ b/src/proto/token.c @@ -62,6 +62,26 @@ static apr_byte_t oidc_proto_validate_token_type(request_rec *r, oidc_provider_t return TRUE; } +/* + * send the request to the token endpoint + */ +static apr_byte_t oidc_proto_token_endpoint_call(request_rec *r, oidc_cfg_t *cfg, oidc_provider_t *provider, + apr_table_t *params, const char *basic_auth, const char *bearer_auth, + const char *dpop, char **response, apr_hash_t *response_hdrs) { + if (oidc_http_post_form(r, oidc_cfg_provider_token_endpoint_url_get(provider), params, basic_auth, bearer_auth, + dpop, oidc_cfg_provider_ssl_validate_server_get(provider), response, NULL, + response_hdrs, oidc_cfg_http_timeout_long_get(cfg), oidc_cfg_outgoing_proxy_get(cfg), + oidc_cfg_dir_pass_cookies_get(r), + oidc_cfg_provider_token_endpoint_tls_client_cert_get(provider), + oidc_cfg_provider_token_endpoint_tls_client_key_get(provider), + oidc_cfg_provider_token_endpoint_tls_client_key_pwd_get(provider)) == FALSE) { + oidc_error(r, "error when calling the token endpoint (%s)", + oidc_cfg_provider_token_endpoint_url_get(provider)); + return FALSE; + } + return TRUE; +} + /* * send a code/refresh request to the token endpoint and return the parsed contents */ @@ -70,13 +90,12 @@ apr_byte_t oidc_proto_token_endpoint_request(request_rec *r, oidc_cfg_t *cfg, oi char **token_type, int *expires_in, char **refresh_token) { apr_byte_t rv = FALSE; - char *response = NULL; char *basic_auth = NULL; char *bearer_auth = NULL; + char *response = NULL; char *dpop = NULL; - char *dpop_nonce = NULL; apr_hash_t *response_hdrs = NULL; - json_t *j_result = NULL, *j_expires_in = NULL, *j_error = NULL; + json_t *j_result = NULL, *j_expires_in = NULL; /* add the token endpoint authentication credentials */ if (oidc_proto_token_endpoint_auth( @@ -96,55 +115,30 @@ apr_byte_t oidc_proto_token_endpoint_request(request_rec *r, oidc_cfg_t *cfg, oi apr_hash_set(response_hdrs, OIDC_HTTP_HDR_DPOP_NONCE, APR_HASH_KEY_STRING, ""); apr_hash_set(response_hdrs, OIDC_HTTP_HDR_CONTENT_TYPE, APR_HASH_KEY_STRING, ""); - dpop = oidc_proto_dpop_create(r, cfg, oidc_cfg_provider_token_endpoint_url_get(provider), "POST", NULL, - NULL); + if ((oidc_proto_dpop_create(r, cfg, oidc_cfg_provider_token_endpoint_url_get(provider), "POST", NULL, + NULL, &dpop) == FALSE) && + (oidc_cfg_provider_dpop_mode_get(provider) == OIDC_DPOP_MODE_REQUIRED)) + goto end; } /* send the request to the token endpoint */ - if (oidc_http_post_form(r, oidc_cfg_provider_token_endpoint_url_get(provider), params, basic_auth, bearer_auth, - dpop, oidc_cfg_provider_ssl_validate_server_get(provider), &response, NULL, - response_hdrs, oidc_cfg_http_timeout_long_get(cfg), oidc_cfg_outgoing_proxy_get(cfg), - oidc_cfg_dir_pass_cookies_get(r), - oidc_cfg_provider_token_endpoint_tls_client_cert_get(provider), - oidc_cfg_provider_token_endpoint_tls_client_key_get(provider), - oidc_cfg_provider_token_endpoint_tls_client_key_pwd_get(provider)) == FALSE) { - oidc_error(r, "error when calling the token endpoint (%s)", - oidc_cfg_provider_token_endpoint_url_get(provider)); + if (oidc_proto_token_endpoint_call(r, cfg, provider, params, basic_auth, bearer_auth, dpop, &response, + response_hdrs) == FALSE) goto end; - } /* check for errors, the response itself will have been logged already */ if (oidc_util_decode_json_and_check_error(r, response, &j_result) == FALSE) { - j_error = json_object_get(j_result, OIDC_PROTO_ERROR); - if ((j_error == NULL) || (!json_is_string(j_error)) || - (_oidc_strcmp(json_string_value(j_error), OIDC_PROTO_DPOP_USE_NONCE) != 0)) + if (oidc_proto_dpop_use_nonce(r, cfg, j_result, response_hdrs, + oidc_cfg_provider_token_endpoint_url_get(provider), "POST", NULL, + &dpop) == FALSE) goto end; - json_decref(j_result); - - /* try again with a DPoP nonce provided by the server */ - dpop_nonce = (char *)apr_hash_get(response_hdrs, OIDC_HTTP_HDR_DPOP_NONCE, APR_HASH_KEY_STRING); - if (dpop_nonce == NULL) { - oidc_error(r, "error is \"%s\" but no \"%s\" header found", OIDC_PROTO_DPOP_USE_NONCE, - OIDC_HTTP_HDR_DPOP_NONCE); + if (oidc_proto_token_endpoint_call(r, cfg, provider, params, basic_auth, bearer_auth, dpop, &response, + response_hdrs) == FALSE) goto end; - } - dpop = oidc_proto_dpop_create(r, cfg, oidc_cfg_provider_token_endpoint_url_get(provider), "POST", NULL, - dpop_nonce); - - if (oidc_http_post_form(r, oidc_cfg_provider_token_endpoint_url_get(provider), params, basic_auth, - bearer_auth, dpop, oidc_cfg_provider_ssl_validate_server_get(provider), - &response, NULL, NULL, oidc_cfg_http_timeout_long_get(cfg), - oidc_cfg_outgoing_proxy_get(cfg), oidc_cfg_dir_pass_cookies_get(r), - oidc_cfg_provider_token_endpoint_tls_client_cert_get(provider), - oidc_cfg_provider_token_endpoint_tls_client_key_get(provider), - oidc_cfg_provider_token_endpoint_tls_client_key_pwd_get(provider)) == FALSE) { - oidc_error(r, "error when calling the token endpoint (%s)", - oidc_cfg_provider_token_endpoint_url_get(provider)); - goto end; - } + json_decref(j_result); if (oidc_util_decode_json_and_check_error(r, response, &j_result) == FALSE) goto end; diff --git a/src/proto/userinfo.c b/src/proto/userinfo.c index 702734ca..53128272 100644 --- a/src/proto/userinfo.c +++ b/src/proto/userinfo.c @@ -46,11 +46,17 @@ #include "util.h" /* - * validate the response from the userinfo endpoint: + * parse a JWT response from the userinfo endpoint: at this point the response is not a JSON object * if the response is an encrypted and/or signed JWT, decrypt/verify it before validating it */ -static apr_byte_t oidc_proto_userinfo_response_validate(request_rec *r, oidc_cfg_t *cfg, oidc_provider_t *provider, - char **response, json_t **claims, char **userinfo_jwt) { +static apr_byte_t oidc_proto_userinfo_response_jwt_parse(request_rec *r, oidc_cfg_t *cfg, oidc_provider_t *provider, + char **response, json_t **claims, char **userinfo_jwt) { + apr_byte_t rv = FALSE; + oidc_jose_error_t err; + oidc_jwk_t *jwk = NULL; + oidc_jwt_t *jwt = NULL; + char *alg = NULL; + char *payload = NULL; oidc_debug(r, "enter: userinfo_signed_response_alg=%s, userinfo_encrypted_response_alg=%s, " @@ -59,29 +65,22 @@ static apr_byte_t oidc_proto_userinfo_response_validate(request_rec *r, oidc_cfg oidc_cfg_provider_userinfo_encrypted_response_alg_get(provider), oidc_cfg_provider_userinfo_encrypted_response_enc_get(provider)); - char *alg = NULL; if ((oidc_cfg_provider_userinfo_signed_response_alg_get(provider) != NULL) || (oidc_cfg_provider_userinfo_encrypted_response_alg_get(provider) != NULL) || (oidc_cfg_provider_userinfo_encrypted_response_enc_get(provider) != NULL)) { oidc_debug(r, "JWT header=%s", oidc_proto_jwt_header_peek(r, *response, &alg, NULL, NULL)); } - oidc_jose_error_t err; - oidc_jwk_t *jwk = NULL; - oidc_jwt_t *jwt = NULL; - char *payload = NULL; - if (oidc_util_create_symmetric_key(r, oidc_cfg_provider_client_secret_get(provider), oidc_alg2keysize(alg), OIDC_JOSE_ALG_SHA256, TRUE, &jwk) == FALSE) - return FALSE; + goto end; if (oidc_cfg_provider_userinfo_encrypted_response_alg_get(provider) != NULL) { if (oidc_jwe_decrypt(r->pool, *response, oidc_util_merge_symmetric_key(r->pool, oidc_cfg_private_keys_get(cfg), jwk), &payload, NULL, &err, TRUE) == FALSE) { oidc_error(r, "oidc_jwe_decrypt failed: %s", oidc_jose_e2s(r->pool, err)); - oidc_jwk_destroy(jwk); - return FALSE; + goto end; } else { oidc_debug(r, "successfully decrypted JWE returned from userinfo endpoint: %s", payload); *response = payload; @@ -93,19 +92,18 @@ static apr_byte_t oidc_proto_userinfo_response_validate(request_rec *r, oidc_cfg oidc_util_merge_symmetric_key(r->pool, oidc_cfg_private_keys_get(cfg), jwk), FALSE, &err) == FALSE) { oidc_error(r, "oidc_jwt_parse failed: %s", oidc_jose_e2s(r->pool, err)); - oidc_jwt_destroy(jwt); - oidc_jwk_destroy(jwk); - return FALSE; + goto end; } oidc_debug(r, "successfully parsed JWT with header=%s, and payload=%s", jwt->header.value.str, jwt->payload.value.str); + // disard the encryption key and load the signing key oidc_jwk_destroy(jwk); - jwk = NULL; + if (oidc_util_create_symmetric_key(r, oidc_cfg_provider_client_secret_get(provider), 0, NULL, TRUE, &jwk) == FALSE) - return FALSE; + goto end; if (oidc_proto_jwt_verify(r, cfg, jwt, oidc_cfg_provider_jwks_uri_get(provider), oidc_cfg_provider_ssl_validate_server_get(provider), @@ -113,25 +111,26 @@ static apr_byte_t oidc_proto_userinfo_response_validate(request_rec *r, oidc_cfg oidc_cfg_provider_userinfo_signed_response_alg_get(provider)) == FALSE) { oidc_error(r, "JWT signature could not be validated, aborting"); - oidc_jwt_destroy(jwt); - oidc_jwk_destroy(jwk); - return FALSE; + goto end; } - oidc_jwk_destroy(jwk); oidc_debug(r, "successfully verified signed JWT returned from userinfo endpoint: %s", jwt->payload.value.str); *userinfo_jwt = apr_pstrdup(r->pool, *response); *claims = json_deep_copy(jwt->payload.value.json); *response = apr_pstrdup(r->pool, jwt->payload.value.str); - oidc_jwt_destroy(jwt); - return TRUE; + rv = TRUE; } - oidc_jwk_destroy(jwk); +end: + + if (jwt) + oidc_jwt_destroy(jwt); + if (jwk) + oidc_jwk_destroy(jwk); - return oidc_util_decode_json_and_check_error(r, *response, claims); + return rv; } #define OIDC_COMPOSITE_CLAIM_NAMES "_claim_names" @@ -236,27 +235,16 @@ static apr_byte_t oidc_proto_userinfo_request_composite_claims(request_rec *r, o } /* - * get claims from the OP UserInfo endpoint using the provided access_token + * send the request to the userinfo endpoint */ -apr_byte_t oidc_proto_userinfo_request(request_rec *r, oidc_cfg_t *cfg, oidc_provider_t *provider, - const char *id_token_sub, const char *access_token, - const char *access_token_type, char **response, char **userinfo_jwt, - long *response_code) { - char *dpop = NULL; - - oidc_debug(r, "enter, endpoint=%s, access_token=%s, token_type=%s", - oidc_cfg_provider_userinfo_endpoint_url_get(provider), access_token, access_token_type); - - OIDC_METRICS_TIMING_START(r, cfg); - +static apr_byte_t oidc_proto_userinfo_endpoint_call(request_rec *r, oidc_cfg_t *cfg, oidc_provider_t *provider, + const char *access_token, const char *dpop, char **response, + long *response_code, apr_hash_t *response_hdrs) { /* get the JSON response */ if (oidc_cfg_provider_userinfo_token_method_get(provider) == OIDC_USER_INFO_TOKEN_METHOD_HEADER) { - if (_oidc_strnatcasecmp(access_token_type, OIDC_PROTO_DPOP) == 0) - dpop = oidc_proto_dpop_create(r, cfg, oidc_cfg_provider_userinfo_endpoint_url_get(provider), - "GET", access_token, NULL); if (oidc_http_get(r, oidc_cfg_provider_userinfo_endpoint_url_get(provider), NULL, NULL, access_token, dpop, oidc_cfg_provider_ssl_validate_server_get(provider), response, response_code, - NULL, oidc_cfg_http_timeout_long_get(cfg), oidc_cfg_outgoing_proxy_get(cfg), + response_hdrs, oidc_cfg_http_timeout_long_get(cfg), oidc_cfg_outgoing_proxy_get(cfg), oidc_cfg_dir_pass_cookies_get(r), NULL, NULL, NULL) == FALSE) { OIDC_METRICS_COUNTER_INC(r, cfg, OM_PROVIDER_USERINFO_ERROR); return FALSE; @@ -264,12 +252,9 @@ apr_byte_t oidc_proto_userinfo_request(request_rec *r, oidc_cfg_t *cfg, oidc_pro } else if (oidc_cfg_provider_userinfo_token_method_get(provider) == OIDC_USER_INFO_TOKEN_METHOD_POST) { apr_table_t *params = apr_table_make(r->pool, 4); apr_table_setn(params, OIDC_PROTO_ACCESS_TOKEN, access_token); - if (_oidc_strnatcasecmp(access_token_type, OIDC_PROTO_DPOP) == 0) - dpop = oidc_proto_dpop_create(r, cfg, oidc_cfg_provider_userinfo_endpoint_url_get(provider), - "POST", access_token, NULL); if (oidc_http_post_form(r, oidc_cfg_provider_userinfo_endpoint_url_get(provider), params, NULL, NULL, dpop, oidc_cfg_provider_ssl_validate_server_get(provider), response, - response_code, NULL, oidc_cfg_http_timeout_long_get(cfg), + response_code, response_hdrs, oidc_cfg_http_timeout_long_get(cfg), oidc_cfg_outgoing_proxy_get(cfg), oidc_cfg_dir_pass_cookies_get(r), NULL, NULL, NULL) == FALSE) { OIDC_METRICS_COUNTER_INC(r, cfg, OM_PROVIDER_USERINFO_ERROR); @@ -280,18 +265,82 @@ apr_byte_t oidc_proto_userinfo_request(request_rec *r, oidc_cfg_t *cfg, oidc_pro oidc_cfg_provider_userinfo_token_method_get(provider)); return FALSE; } + return TRUE; +} + +/* + * get claims from the OP UserInfo endpoint using the provided access_token + */ +apr_byte_t oidc_proto_userinfo_request(request_rec *r, oidc_cfg_t *cfg, oidc_provider_t *provider, + const char *id_token_sub, const char *access_token, + const char *access_token_type, char **response, char **userinfo_jwt, + long *response_code) { + apr_byte_t rv = FALSE; + char *dpop = NULL; + apr_hash_t *response_hdrs = NULL; + json_t *j_result = NULL; + const char *method = + oidc_cfg_provider_userinfo_token_method_get(provider) == OIDC_USER_INFO_TOKEN_METHOD_POST ? "POST" : "GET"; + + oidc_debug(r, "enter, endpoint=%s, access_token=%s, token_type=%s", + oidc_cfg_provider_userinfo_endpoint_url_get(provider), access_token, access_token_type); + + OIDC_METRICS_TIMING_START(r, cfg); + + if (_oidc_strnatcasecmp(access_token_type, OIDC_PROTO_DPOP) == 0) { + response_hdrs = apr_hash_make(r->pool); + apr_hash_set(response_hdrs, OIDC_HTTP_HDR_AUTHORIZATION, APR_HASH_KEY_STRING, ""); + apr_hash_set(response_hdrs, OIDC_HTTP_HDR_DPOP_NONCE, APR_HASH_KEY_STRING, ""); + apr_hash_set(response_hdrs, OIDC_HTTP_HDR_CONTENT_TYPE, APR_HASH_KEY_STRING, ""); + if (oidc_proto_dpop_create(r, cfg, oidc_cfg_provider_userinfo_endpoint_url_get(provider), method, + access_token, NULL, &dpop) == FALSE) + goto end; + } + + if (oidc_proto_userinfo_endpoint_call(r, cfg, provider, access_token, dpop, response, response_code, + response_hdrs) == FALSE) + goto end; OIDC_METRICS_TIMING_ADD(r, cfg, OM_PROVIDER_USERINFO); - json_t *claims = NULL; - if (oidc_proto_userinfo_response_validate(r, cfg, provider, response, &claims, userinfo_jwt) == FALSE) - return FALSE; + if (oidc_util_decode_json_object_err(r, *response, &j_result, FALSE) == FALSE) { + + // must be a JWT + if (oidc_proto_userinfo_response_jwt_parse(r, cfg, provider, response, &j_result, userinfo_jwt) == + FALSE) + goto end; + + } else if (oidc_util_check_json_error(r, j_result) == TRUE) { + + if (oidc_proto_dpop_use_nonce(r, cfg, j_result, response_hdrs, + oidc_cfg_provider_userinfo_endpoint_url_get(provider), method, + access_token, &dpop) == FALSE) + // a regular error response + goto end; - if (oidc_proto_userinfo_request_composite_claims(r, cfg, claims) == TRUE) - *response = oidc_util_encode_json_object(r, claims, JSON_PRESERVE_ORDER | JSON_COMPACT); + if (oidc_proto_userinfo_endpoint_call(r, cfg, provider, access_token, dpop, response, response_code, + response_hdrs) == FALSE) + goto end; + + json_decref(j_result); + + if (oidc_util_decode_json_object_err(r, *response, &j_result, FALSE) == FALSE) { + + // must be a JWT + if (oidc_proto_userinfo_response_jwt_parse(r, cfg, provider, response, &j_result, + userinfo_jwt) == FALSE) + goto end; + } + + if (oidc_util_check_json_error(r, j_result) == TRUE) + goto end; + } + + if (oidc_proto_userinfo_request_composite_claims(r, cfg, j_result) == TRUE) + *response = oidc_util_encode_json_object(r, j_result, JSON_PRESERVE_ORDER | JSON_COMPACT); char *user_info_sub = NULL; - oidc_jose_get_string(r->pool, claims, OIDC_CLAIM_SUB, FALSE, &user_info_sub, NULL); + oidc_jose_get_string(r->pool, j_result, OIDC_CLAIM_SUB, FALSE, &user_info_sub, NULL); oidc_debug(r, "id_token_sub=%s, user_info_sub=%s", id_token_sub, user_info_sub); @@ -300,8 +349,7 @@ apr_byte_t oidc_proto_userinfo_request(request_rec *r, oidc_cfg_t *cfg, oidc_pro "mandatory claim (\"%s\") was not returned from userinfo endpoint " "(https://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse)", OIDC_CLAIM_SUB); - json_decref(claims); - return FALSE; + goto end; } if ((id_token_sub != NULL) && (user_info_sub != NULL)) { @@ -310,12 +358,16 @@ apr_byte_t oidc_proto_userinfo_request(request_rec *r, oidc_cfg_t *cfg, oidc_pro "\"%s\" claim (\"%s\") returned from userinfo endpoint does not match the one in " "the id_token (\"%s\")", OIDC_CLAIM_SUB, user_info_sub, id_token_sub); - json_decref(claims); - return FALSE; + goto end; } } - json_decref(claims); + rv = TRUE; - return TRUE; +end: + + if (j_result) + json_decref(j_result); + + return rv; } diff --git a/src/util.c b/src/util.c index 35c93800..4386f8b6 100644 --- a/src/util.c +++ b/src/util.c @@ -955,7 +955,7 @@ static apr_byte_t oidc_util_json_string_print(request_rec *r, json_t *result, co /* * check a JSON object for "error" results and printout */ -static apr_byte_t oidc_util_check_json_error(request_rec *r, json_t *json) { +apr_byte_t oidc_util_check_json_error(request_rec *r, json_t *json) { if (oidc_util_json_string_print(r, json, OIDC_PROTO_ERROR, "oidc_util_check_json_error") == TRUE) { oidc_util_json_string_print(r, json, OIDC_PROTO_ERROR_DESCRIPTION, "oidc_util_check_json_error"); return TRUE; @@ -968,8 +968,7 @@ static apr_byte_t oidc_util_check_json_error(request_rec *r, json_t *json) { /* * parse a JSON object */ -apr_byte_t oidc_util_decode_json_object(request_rec *r, const char *str, json_t **json) { - +apr_byte_t oidc_util_decode_json_object_err(request_rec *r, const char *str, json_t **json, apr_byte_t log_err) { if (str == NULL) return FALSE; @@ -978,31 +977,41 @@ apr_byte_t oidc_util_decode_json_object(request_rec *r, const char *str, json_t /* decode the JSON contents of the buffer */ if (*json == NULL) { - /* something went wrong */ + if (log_err) { + /* something went wrong */ #if JANSSON_VERSION_HEX >= 0x020B00 - if (json_error_code(&json_error) == json_error_null_character) { - oidc_error(r, "JSON parsing returned an error: %s", json_error.text); - } else { + if (json_error_code(&json_error) == json_error_null_character) { + oidc_error(r, "JSON parsing returned an error: %s", json_error.text); + } else { #endif - oidc_error(r, "JSON parsing returned an error: %s (%s)", json_error.text, - apr_pstrndup(r->pool, str, OIDC_JSON_MAX_ERROR_STR)); + oidc_error(r, "JSON parsing returned an error: %s (%s)", json_error.text, + apr_pstrndup(r->pool, str, OIDC_JSON_MAX_ERROR_STR)); #if JANSSON_VERSION_HEX >= 0x020B00 - } + } #endif + } return FALSE; } if (!json_is_object(*json)) { /* oops, no JSON */ - oidc_error(r, "parsed JSON did not contain a JSON object"); - json_decref(*json); - *json = NULL; - return FALSE; + if (log_err) { + oidc_error(r, "parsed JSON did not contain a JSON object"); + json_decref(*json); + *json = NULL; + return FALSE; + } + + return TRUE; } return TRUE; } +apr_byte_t oidc_util_decode_json_object(request_rec *r, const char *str, json_t **json) { + return oidc_util_decode_json_object_err(r, str, json, TRUE); +} + /* * encode a JSON object */ diff --git a/src/util.h b/src/util.h index c940d9ac..a3565904 100644 --- a/src/util.h +++ b/src/util.h @@ -57,7 +57,9 @@ apr_byte_t oidc_util_hash_string_and_base64url_encode(request_rec *r, const char apr_byte_t oidc_util_create_symmetric_key(request_rec *r, const char *client_secret, unsigned int r_key_len, const char *hash_algo, apr_byte_t set_kid, oidc_jwk_t **jwk); char *oidc_util_encode_json_object(request_rec *r, json_t *json, size_t flags); +apr_byte_t oidc_util_decode_json_object_err(request_rec *r, const char *str, json_t **json, apr_byte_t log_err); apr_byte_t oidc_util_decode_json_object(request_rec *r, const char *str, json_t **json); +apr_byte_t oidc_util_check_json_error(request_rec *r, json_t *json); apr_byte_t oidc_util_random_bytes(unsigned char *buf, apr_size_t length); apr_byte_t oidc_util_generate_random_bytes(request_rec *r, unsigned char *buf, apr_size_t length); apr_byte_t oidc_util_generate_random_hex_string(request_rec *r, char **hex_str, int byte_len);