From 966ff37165a77375a0373c2058d46e9c750c1d21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20M=C3=A4nnchen?= Date: Tue, 30 Apr 2024 13:20:13 +0200 Subject: [PATCH 1/2] Fix DPoP with JWK Set --- src/oidcc_auth_util.erl | 12 +----------- src/oidcc_authorization.erl | 13 ++++++++----- src/oidcc_jwt_util.erl | 27 +++++++++++++++++++++++++++ test/oidcc_authorization_test.erl | 6 +++++- test/oidcc_userinfo_test.erl | 5 +++-- 5 files changed, 44 insertions(+), 19 deletions(-) diff --git a/src/oidcc_auth_util.erl b/src/oidcc_auth_util.erl index 0f6f8a3..460b6e9 100644 --- a/src/oidcc_auth_util.erl +++ b/src/oidcc_auth_util.erl @@ -383,18 +383,8 @@ dpop_proof(Method, Endpoint, Claims0, #oidcc_client_context{ <<"nbf">> => os:system_time(seconds) - MaxClockSkew }, Jwt = jose_jwt:from(Claims), - {_, PublicJwk} = jose_jwk:to_public_map(ClientJwks), - case - oidcc_jwt_util:sign(Jwt, ClientJwks, SigningAlgSupported, #{ - <<"typ">> => <<"dpop+jwt">>, <<"jwk">> => PublicJwk - }) - of - {ok, SignedRequestObject} -> - {ok, SignedRequestObject}; - {error, no_supported_alg_or_key} -> - error - end; + oidcc_jwt_util:sign_dpop(Jwt, ClientJwks, SigningAlgSupported); dpop_proof(_Method, _Endpoint, _Claims, _ClientContext) -> error. diff --git a/src/oidcc_authorization.erl b/src/oidcc_authorization.erl index 08d8847..ad593de 100644 --- a/src/oidcc_authorization.erl +++ b/src/oidcc_authorization.erl @@ -237,15 +237,18 @@ when maybe_append_dpop_jkt( QueryParams, #oidcc_client_context{ - client_jwks = #jose_jwk{}, + client_jwks = #jose_jwk{} = ClientJwks, provider_configuration = #oidcc_provider_configuration{ dpop_signing_alg_values_supported = [_ | _] } - } = ClientContext + } ) -> - #oidcc_client_context{client_jwks = ClientJwks} = ClientContext, - Thumbprint = jose_jwk:thumbprint(ClientJwks), - [{"dpop_jkt", Thumbprint} | QueryParams]; + case oidcc_jwt_util:thumbprint(ClientJwks) of + {ok, Thumbprint} -> + [{<<"dpop_jkt">>, Thumbprint} | QueryParams]; + error -> + QueryParams + end; maybe_append_dpop_jkt(QueryParams, _ClientContext) -> QueryParams. diff --git a/src/oidcc_jwt_util.erl b/src/oidcc_jwt_util.erl index 1fd074f..b2d8a64 100644 --- a/src/oidcc_jwt_util.erl +++ b/src/oidcc_jwt_util.erl @@ -21,6 +21,8 @@ -export([refresh_jwks_fun/1]). -export([sign/3]). -export([sign/4]). +-export([sign_dpop/3]). +-export([thumbprint/1]). -export([verify_claims/2]). -export([verify_not_none_alg/1]). -export([verify_signature/3]). @@ -376,6 +378,7 @@ verify_decrypted_token(Jwt, SigningAlgs, Jwe, Jwks) -> {error, Reason} -> {error, Reason} end. + %% @private -spec encrypt( Jwt :: binary(), @@ -432,6 +435,30 @@ encrypt(Jwt, Jwk, [Algorithm | _RestAlgorithms] = SupportedAlgorithms, Supported error -> encrypt(Jwt, Jwk, SupportedAlgorithms, SupportedEncValues, RestEncValues) end. +%% @private +-spec thumbprint(Jwk :: jose_jwk:key()) -> {ok, binary()} | error. +thumbprint(Jwk) -> + evaluate_for_all_keys(Jwk, fun + (#jose_jwk{fields = #{<<"use">> := <<"sig">>}} = Key) -> + {ok, jose_jwk:thumbprint(Key)}; + (_Key) -> + error + end). + +%% @private +-spec sign_dpop(Jwt :: #jose_jwt{}, Jwk :: jose_jwk:key(), SigningAlgSupported :: [binary()]) -> + {ok, binary()} | {error, no_supported_alg_or_key}. +sign_dpop(Jwt, Jwk, SigningAlgSupported) -> + evaluate_for_all_keys(Jwk, fun + (#jose_jwk{fields = #{<<"use">> := <<"sig">>}} = Key) -> + {_, PublicJwk} = jose_jwk:to_public_map(Key), + sign(Jwt, Key, SigningAlgSupported, #{ + <<"typ">> => <<"dpop+jwt">>, <<"jwk">> => PublicJwk + }); + (_Key) -> + error + end). + %% @private -spec evaluate_for_all_keys(Jwk :: jose_jwk:key(), fun((jose_jwk:key()) -> {ok, Result} | error)) -> {ok, Result} | error diff --git a/test/oidcc_authorization_test.erl b/test/oidcc_authorization_test.erl index 8eea1db..9d7c70a 100644 --- a/test/oidcc_authorization_test.erl +++ b/test/oidcc_authorization_test.erl @@ -1137,7 +1137,11 @@ private_key_jwt_fixture() -> dpop_signing_alg_values_supported = [<<"RS256">>] }, - Jwks = jose_jwk:from_pem_file(PrivDir ++ "/test/fixtures/jwk.pem"), + Jwks0 = jose_jwk:from_pem_file(PrivDir ++ "/test/fixtures/jwk.pem"), + + Jwks = Jwks0#jose_jwk{ + fields = #{<<"kid">> => <<"private_kid">>, <<"use">> => <<"sig">>} + }, ClientId = <<"client_id">>, diff --git a/test/oidcc_userinfo_test.erl b/test/oidcc_userinfo_test.erl index 87fc12b..1195d41 100644 --- a/test/oidcc_userinfo_test.erl +++ b/test/oidcc_userinfo_test.erl @@ -620,16 +620,17 @@ dpop_proof_test() -> dpop_signing_alg_values_supported = [<<"RS256">>] }, Jwks = jose_jwk:from_pem_file(PrivDir ++ "/test/fixtures/jwk.pem"), - ClientJwk = Jwks#jose_jwk{ fields = #{<<"kid">> => <<"private_kid">>, <<"use">> => <<"sig">>} }, + ClientJwks = #jose_jwk{keys = {jose_jwk_set, [ClientJwk]}}, + ClientId = <<"client_id">>, ClientSecret = <<"client_secret">>, ClientContext = oidcc_client_context:from_manual( - Configuration, Jwks, ClientId, ClientSecret, #{client_jwks => ClientJwk} + Configuration, Jwks, ClientId, ClientSecret, #{client_jwks => ClientJwks} ), HttpBody = <<"{\"name\":\"joe\", \"sub\":\"123456\"}">>, From f3e0a721917a5731ce5dbe3f50bfd714ee8dfdf6 Mon Sep 17 00:00:00 2001 From: Paul Swartz Date: Tue, 30 Apr 2024 10:13:54 -0400 Subject: [PATCH 2/2] fix(dpop): support keys without "use" header (#347) --- src/oidcc_jwt_util.erl | 16 ++++++++-------- test/oidcc_authorization_test.erl | 6 +----- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/oidcc_jwt_util.erl b/src/oidcc_jwt_util.erl index b2d8a64..4223d41 100644 --- a/src/oidcc_jwt_util.erl +++ b/src/oidcc_jwt_util.erl @@ -439,10 +439,10 @@ encrypt(Jwt, Jwk, [Algorithm | _RestAlgorithms] = SupportedAlgorithms, Supported -spec thumbprint(Jwk :: jose_jwk:key()) -> {ok, binary()} | error. thumbprint(Jwk) -> evaluate_for_all_keys(Jwk, fun - (#jose_jwk{fields = #{<<"use">> := <<"sig">>}} = Key) -> - {ok, jose_jwk:thumbprint(Key)}; - (_Key) -> - error + (#jose_jwk{fields = #{<<"use">> := Use}}) when Use =/= <<"sig">> -> + error; + (Key) -> + {ok, jose_jwk:thumbprint(Key)} end). %% @private @@ -450,13 +450,13 @@ thumbprint(Jwk) -> {ok, binary()} | {error, no_supported_alg_or_key}. sign_dpop(Jwt, Jwk, SigningAlgSupported) -> evaluate_for_all_keys(Jwk, fun - (#jose_jwk{fields = #{<<"use">> := <<"sig">>}} = Key) -> + (#jose_jwk{fields = #{<<"use">> := Use}}) when Use =/= <<"sig">> -> + error; + (Key) -> {_, PublicJwk} = jose_jwk:to_public_map(Key), sign(Jwt, Key, SigningAlgSupported, #{ <<"typ">> => <<"dpop+jwt">>, <<"jwk">> => PublicJwk - }); - (_Key) -> - error + }) end). %% @private diff --git a/test/oidcc_authorization_test.erl b/test/oidcc_authorization_test.erl index 9d7c70a..8eea1db 100644 --- a/test/oidcc_authorization_test.erl +++ b/test/oidcc_authorization_test.erl @@ -1137,11 +1137,7 @@ private_key_jwt_fixture() -> dpop_signing_alg_values_supported = [<<"RS256">>] }, - Jwks0 = jose_jwk:from_pem_file(PrivDir ++ "/test/fixtures/jwk.pem"), - - Jwks = Jwks0#jose_jwk{ - fields = #{<<"kid">> => <<"private_kid">>, <<"use">> => <<"sig">>} - }, + Jwks = jose_jwk:from_pem_file(PrivDir ++ "/test/fixtures/jwk.pem"), ClientId = <<"client_id">>,