From c726e3f1bb21606dfb963d8309e272f31d460209 Mon Sep 17 00:00:00 2001 From: Paul Swartz Date: Sun, 7 Jan 2024 15:44:36 -0500 Subject: [PATCH 1/7] feat: parse `mtls_endpoint_aliases` --- include/oidcc_provider_configuration.hrl | 4 ++ lib/oidcc/provider_configuration.ex | 2 + priv/test/fixtures/fapi2-metadata.json | 9 +++++ src/oidcc_decode_util.erl | 44 ++++++++++++++++++++++ src/oidcc_provider_configuration.erl | 14 ++++++- test/oidcc_provider_configuration_test.erl | 22 +++++++++++ 6 files changed, 94 insertions(+), 1 deletion(-) diff --git a/include/oidcc_provider_configuration.hrl b/include/oidcc_provider_configuration.hrl index 206dcf3..2611d1f 100644 --- a/include/oidcc_provider_configuration.hrl +++ b/include/oidcc_provider_configuration.hrl @@ -109,6 +109,10 @@ dpop_signing_alg_values_supported = undefined :: [binary()] | undefined, %% RFC 9101 The OAuth 2.0 Authorization Framework: JWT-Secured Authorization Request (JAR) require_signed_request_object = false :: boolean(), + %% RFC 8705 OAuth 2.0 Mutual-TLS Client Authentication and Certificate-Bound Access Tokens + mtls_endpoint_aliases = #{} :: #{binary() => uri_string:uri_string()}, + %% RFC 8705 OAuth 2.0 Mutual-TLS Client Authentication and Certificate-Bound Access Tokens + tls_client_certificate_bound_access_tokens = false :: boolean(), %% Unknown Fields extra_fields = #{} :: #{binary() => term()} } diff --git a/lib/oidcc/provider_configuration.ex b/lib/oidcc/provider_configuration.ex index 0d3e8d5..5a0e765 100644 --- a/lib/oidcc/provider_configuration.ex +++ b/lib/oidcc/provider_configuration.ex @@ -118,6 +118,8 @@ defmodule Oidcc.ProviderConfiguration do authorization_encryption_enc_values_supported: [String.t()] | :undefined, dpop_signing_alg_values_supported: [String.t()] | :undefined, require_signed_request_object: boolean(), + mtls_endpoint_aliases: %{binary() => :uri_string.uri_string()}, + tls_client_certificate_bound_access_tokens: boolean(), extra_fields: %{String.t() => term()} } diff --git a/priv/test/fixtures/fapi2-metadata.json b/priv/test/fixtures/fapi2-metadata.json index 8570296..4b63d9f 100644 --- a/priv/test/fixtures/fapi2-metadata.json +++ b/priv/test/fixtures/fapi2-metadata.json @@ -8,6 +8,14 @@ "userinfo_endpoint": "https://my.provider/userinfo", "revocation_endpoint": "https://my.provider/revoke", "jwks_uri": "https://my.provider/jwks", + "mtls_endpoint_aliases": { + "authorization_endpoint": "https://my.provider/tls/auth", + "registration_endpoint": "https://my.provider/tls/register", + "device_authorization_endpoint": "https://my.provider/tls/device/code", + "token_endpoint": "https://my.provider/tls/token", + "introspection_endpoint": "https://my.provider/tls/introspection", + "userinfo_endpoint": "https://my.provider/tls/userinfo" + }, "response_types_supported": [ "code", "token", @@ -44,6 +52,7 @@ "private_key_jwt", "unsupporeted_auth" ], + "tls_client_certificate_bound_access_tokens": true, "claims_supported": [ "aud", "email", diff --git a/src/oidcc_decode_util.erl b/src/oidcc_decode_util.erl index ffa7905..31bced0 100644 --- a/src/oidcc_decode_util.erl +++ b/src/oidcc_decode_util.erl @@ -13,6 +13,8 @@ -export([parse_setting_number/2]). -export([parse_setting_uri/2]). -export([parse_setting_uri_https/2]). +-export([parse_setting_uri_map/2]). +-export([parse_setting_uri_https_map/2]). -export_type([error/0]). @@ -93,6 +95,48 @@ parse_setting_uri_https(Setting, Field) when is_binary(Setting) -> parse_setting_uri_https(_Setting, Field) -> {error, {invalid_config_property, {uri_https, Field}}}. +%% @private +-spec parse_setting_uri_map(Setting :: term(), Field :: atom()) -> + {ok, #{binary() => uri_string:uri_string()}} | {error, error()}. +parse_setting_uri_map(Setting, Field) -> + do_parse_setting_uri_map(Setting, Field, fun parse_setting_uri/2). + +%% @private +-spec parse_setting_uri_https_map(Setting :: term(), Field :: atom()) -> + {ok, #{binary() => uri_string:uri_string()}} | {error, error()}. +parse_setting_uri_https_map(Setting, Field) -> + do_parse_setting_uri_map(Setting, Field, fun parse_setting_uri_https/2). + +do_parse_setting_uri_map(#{} = Setting, Field, Parser) -> + SettingList = maps:to_list(Setting), + case + lists:foldl( + fun + (_Elem, {error, Reason}) -> + {error, Reason}; + ({BinKey, Value}, {ok, Acc}) when is_binary(BinKey) -> + case Parser(Value, Field) of + {ok, SettingValue} -> + {ok, [{BinKey, SettingValue} | Acc]}; + {error, Reason} -> + {error, Reason} + end; + (_, _) -> + {error, {invalid_config_property, {uri_map, Field}}} + end, + + {ok, []}, + SettingList + ) + of + {ok, ParsedList} -> + {ok, maps:from_list(ParsedList)}; + {error, Reason} -> + {error, Reason} + end; +do_parse_setting_uri_map(_Setting, Field, _Parser) -> + {error, {invalid_config_property, {uri_map, Field}}}. + %% @private -spec parse_setting_binary(Setting :: term(), Field :: atom()) -> {ok, binary()} | {error, error()}. diff --git a/src/oidcc_provider_configuration.erl b/src/oidcc_provider_configuration.erl index 060f276..d22ef18 100644 --- a/src/oidcc_provider_configuration.erl +++ b/src/oidcc_provider_configuration.erl @@ -124,6 +124,7 @@ authorization_response_iss_parameter_supported :: boolean(), dpop_signing_alg_values_supported :: [binary()] | undefined, require_signed_request_object :: boolean(), + mtls_endpoint_aliases :: #{binary() => uri_string:uri_string()}, extra_fields :: #{binary() => term()} }. %% Record containing OpenID and OAuth 2.0 Configuration @@ -359,7 +360,9 @@ decode_configuration(Configuration0, Opts) -> authorization_response_iss_parameter_supported := AuthorizationResponseIssParameterSupported, dpop_signing_alg_values_supported := DpopSigningAlgValuesSupported, - require_signed_request_object := RequireSignedRequestObject + require_signed_request_object := RequireSignedRequestObject, + mtls_endpoint_aliases := MtlsEndpointAliases, + tls_client_certificate_bound_access_tokens := TlsClientCertificateBoundAccessTokens }, ExtraFields }} ?= @@ -470,6 +473,13 @@ decode_configuration(Configuration0, Opts) -> {optional, dpop_signing_alg_values_supported, undefined, fun parse_token_signing_alg_values_no_none/2}, {optional, require_signed_request_object, false, + fun oidcc_decode_util:parse_setting_boolean/2}, + {optional, mtls_endpoint_aliases, #{}, + case AllowUnsafeHttp of + true -> fun oidcc_decode_util:parse_setting_uri_map/2; + false -> fun oidcc_decode_util:parse_setting_uri_https_map/2 + end}, + {optional, tls_client_certificate_bound_access_tokens, false, fun oidcc_decode_util:parse_setting_boolean/2} ], #{} @@ -547,6 +557,8 @@ decode_configuration(Configuration0, Opts) -> AuthorizationResponseIssParameterSupported, dpop_signing_alg_values_supported = DpopSigningAlgValuesSupported, require_signed_request_object = RequireSignedRequestObject, + mtls_endpoint_aliases = MtlsEndpointAliases, + tls_client_certificate_bound_access_tokens = TlsClientCertificateBoundAccessTokens, extra_fields = ExtraFields }} end. diff --git a/test/oidcc_provider_configuration_test.erl b/test/oidcc_provider_configuration_test.erl index 39f77aa..4c859d8 100644 --- a/test/oidcc_provider_configuration_test.erl +++ b/test/oidcc_provider_configuration_test.erl @@ -636,6 +636,28 @@ uri_concatenation_test() -> ok. +decode_fapi2_test() -> + PrivDir = code:priv_dir(oidcc), + + {ok, Configuration} = file:read_file(PrivDir ++ "/test/fixtures/fapi2-metadata.json"), + ?assertMatch( + {ok, #oidcc_provider_configuration{ + issuer = <<"https://my.provider">>, + tls_client_certificate_bound_access_tokens = true, + mtls_endpoint_aliases = #{ + <<"authorization_endpoint">> := <<"https://my.provider/tls/auth">>, + <<"registration_endpoint">> := <<"https://my.provider/tls/register">>, + <<"device_authorization_endpoint">> := <<"https://my.provider/tls/device/code">>, + <<"token_endpoint">> := <<"https://my.provider/tls/token">>, + <<"introspection_endpoint">> := <<"https://my.provider/tls/introspection">>, + <<"userinfo_endpoint">> := <<"https://my.provider/tls/userinfo">> + } + }}, + oidcc_provider_configuration:decode_configuration(jose:decode(Configuration)) + ), + + ok. + google_merge_json(Merge) -> PrivDir = code:priv_dir(oidcc), {ok, ValidConfigString} = file:read_file(PrivDir ++ "/test/fixtures/google-metadata.json"), From 83edfdbbdc0cee615552fefa559ce119c57cbae6 Mon Sep 17 00:00:00 2001 From: Paul Swartz Date: Sun, 7 Jan 2024 15:50:44 -0500 Subject: [PATCH 2/7] feat: use `mtls_endpoint_aliases` if using `tls_client_auth` method --- src/oidcc_auth_util.erl | 46 +++++++++++++++++++++++++++++-- src/oidcc_authorization.erl | 12 ++++++-- src/oidcc_token.erl | 18 ++++++------ src/oidcc_token_introspection.erl | 12 ++++++-- src/oidcc_userinfo.erl | 16 +++++++++-- 5 files changed, 86 insertions(+), 18 deletions(-) diff --git a/src/oidcc_auth_util.erl b/src/oidcc_auth_util.erl index 264fd66..860f511 100644 --- a/src/oidcc_auth_util.erl +++ b/src/oidcc_auth_util.erl @@ -14,13 +14,19 @@ -export_type([auth_method/0, error/0]). -type auth_method() :: - none | client_secret_basic | client_secret_post | client_secret_jwt | private_key_jwt. + none + | client_secret_basic + | client_secret_post + | client_secret_jwt + | private_key_jwt + | tls_client_auth. -type error() :: no_supported_auth_method. -export([add_client_authentication/6]). -export([add_dpop_proof_header/5]). -export([add_authorization_header/6]). +-export([maybe_mtls_endpoint/4]). %% @private -spec add_client_authentication( @@ -42,6 +48,7 @@ add_client_authentication( ) -> PreferredAuthMethods = maps:get(preferred_auth_methods, Opts, [ private_key_jwt, + tls_client_auth, client_secret_jwt, client_secret_post, client_secret_basic, @@ -55,7 +62,7 @@ add_client_authentication( ) of {ok, {QueryList, Header}} -> - {ok, {QueryList, Header}}; + {ok, {QueryList, Header}, AuthMethod}; {error, _} -> add_client_authentication( QueryList0, @@ -180,6 +187,22 @@ add_authentication( else _ -> {error, auth_method_not_possible} + end; +add_authentication( + QsBodyList, + Header, + tls_client_auth, + _AllowAlgorithms, + Opts, + #oidcc_client_context{client_id = ClientId} +) -> + case Opts of + #{request_opts := #{ssl := _}} -> + %% only supported if custom SSL params are provided + NewBodyList = [{<<"client_id">>, ClientId} | QsBodyList], + {ok, {NewBodyList, Header}}; + _ -> + {error, auth_method_not_possible} end. -spec select_preferred_auth(PreferredAuthMethods, AuthMethodsSupported) -> @@ -315,6 +338,25 @@ add_authorization_header( [oidcc_http_util:bearer_auth_header(AccessToken)] end. +%% @private +-spec maybe_mtls_endpoint( + Endpoint, auth_method(), MtlsEndpointName, ClientContext +) -> Endpoint when + Endpoint :: uri_string:uri_string(), + MtlsEndpointName :: binary(), + ClientContext :: oidcc_client_context:t(). +maybe_mtls_endpoint(Endpoint, tls_client_auth, MtlsEndpointName, ClientContext) -> + case + ClientContext#oidcc_client_context.provider_configuration#oidcc_provider_configuration.mtls_endpoint_aliases + of + #{MtlsEndpointName := MtlsEndpoint} -> + MtlsEndpoint; + _ -> + Endpoint + end; +maybe_mtls_endpoint(Endpoint, _AuthMethod, _EndpointName, _ClientContext) -> + Endpoint. + -spec dpop_proof(Method, Endpoint, Claims, ClientContext) -> {ok, binary()} | error when Method :: post | get, Endpoint :: uri_string:uri_string(), diff --git a/src/oidcc_authorization.erl b/src/oidcc_authorization.erl index 6c159d9..5d095c7 100644 --- a/src/oidcc_authorization.erl +++ b/src/oidcc_authorization.erl @@ -341,7 +341,7 @@ attempt_par( issuer = Issuer, token_endpoint_auth_methods_supported = SupportedAuthMethods, token_endpoint_auth_signing_alg_values_supported = SigningAlgs, - pushed_authorization_request_endpoint = PushedAuthorizationRequestEndpoint + pushed_authorization_request_endpoint = PushedAuthorizationRequestEndpoint0 } } = ClientContext, Opts @@ -356,10 +356,10 @@ attempt_par( %% https://datatracker.ietf.org/doc/html/rfc9126#section-2 %% > To address that ambiguity, the issuer identifier URL of the authorization %% > server according to [RFC8414] SHOULD be used as the value of the audience. - AuthenticationOpts = #{audience => Issuer}, + AuthenticationOpts = maps:put(audience, Issuer, Opts), maybe - {ok, {Body0, Header}} ?= + {ok, {Body0, Header}, AuthMethod} ?= oidcc_auth_util:add_client_authentication( QueryParams, Header0, @@ -370,6 +370,12 @@ attempt_par( ), %% ensure no duplicate parameters (such as client_id) Body = lists:ukeysort(1, Body0), + PushedAuthorizationRequestEndpoint = oidcc_auth_util:maybe_mtls_endpoint( + PushedAuthorizationRequestEndpoint0, + AuthMethod, + <<"pushed_authorization_request_endpoint">>, + ClientContext + ), Request = {PushedAuthorizationRequestEndpoint, Header, "application/x-www-form-urlencoded", uri_string:compose_query(Body)}, diff --git a/src/oidcc_token.erl b/src/oidcc_token.erl index 1256958..12857a6 100644 --- a/src/oidcc_token.erl +++ b/src/oidcc_token.erl @@ -1099,7 +1099,7 @@ retrieve_a_token(QsBodyIn, PkceVerifier, ClientContext, Opts, TelemetryOpts, Aut #oidcc_client_context{provider_configuration = Configuration} = ClientContext, #oidcc_provider_configuration{ - token_endpoint = TokenEndpoint, + token_endpoint = TokenEndpoint0, token_endpoint_auth_methods_supported = SupportedAuthMethods0, token_endpoint_auth_signing_alg_values_supported = SigningAlgs } = @@ -1107,12 +1107,6 @@ retrieve_a_token(QsBodyIn, PkceVerifier, ClientContext, Opts, TelemetryOpts, Aut QueryParams = maps:get(url_extension, Opts, []), - Endpoint = - case QueryParams of - [] -> TokenEndpoint; - _ -> [TokenEndpoint, <<"?">>, uri_string:compose_query(QueryParams)] - end, - Header0 = [{"accept", "application/jwt, application/json"}], QsBody0 = QsBodyIn ++ maps:get(body_extension, Opts, []), @@ -1132,10 +1126,18 @@ retrieve_a_token(QsBodyIn, PkceVerifier, ClientContext, Opts, TelemetryOpts, Aut #{} end, maybe - {ok, {Body, Header1}} ?= + {ok, {Body, Header1}, AuthMethod} ?= oidcc_auth_util:add_client_authentication( QsBody, Header0, SupportedAuthMethods, SigningAlgs, Opts, ClientContext ), + TokenEndpoint = oidcc_auth_util:maybe_mtls_endpoint( + TokenEndpoint0, AuthMethod, <<"token_endpoint">>, ClientContext + ), + Endpoint = + case QueryParams of + [] -> TokenEndpoint; + _ -> [TokenEndpoint, <<"?">>, uri_string:compose_query(QueryParams)] + end, Header = oidcc_auth_util:add_dpop_proof_header( Header1, post, Endpoint, DpopOpts, ClientContext ), diff --git a/src/oidcc_token_introspection.erl b/src/oidcc_token_introspection.erl index 2be4985..ac0a1ab 100644 --- a/src/oidcc_token_introspection.erl +++ b/src/oidcc_token_introspection.erl @@ -114,13 +114,13 @@ introspect(AccessToken, ClientContext, Opts) -> } = ClientContext, #oidcc_provider_configuration{ - introspection_endpoint = Endpoint, + introspection_endpoint = Endpoint0, issuer = Issuer, introspection_endpoint_auth_methods_supported = SupportedAuthMethods, introspection_endpoint_auth_signing_alg_values_supported = AllowAlgorithms } = Configuration, - case Endpoint of + case Endpoint0 of undefined -> {error, introspection_not_supported}; _ -> @@ -140,10 +140,16 @@ introspect(AccessToken, ClientContext, Opts) -> #{} end, maybe - {ok, {Body, Header1}} ?= + {ok, {Body, Header1}, AuthMethod} ?= oidcc_auth_util:add_client_authentication( Body0, Header0, SupportedAuthMethods, AllowAlgorithms, Opts, ClientContext ), + Endpoint = oidcc_auth_util:maybe_mtls_endpoint( + Endpoint0, + AuthMethod, + <<"introspection_endpoint">>, + ClientContext + ), Header = oidcc_auth_util:add_dpop_proof_header( Header1, post, Endpoint, DpopOpts, ClientContext ), diff --git a/src/oidcc_userinfo.erl b/src/oidcc_userinfo.erl index 71a78c3..96fb036 100644 --- a/src/oidcc_userinfo.erl +++ b/src/oidcc_userinfo.erl @@ -134,7 +134,6 @@ retrieve(#oidcc_token_access{} = AccessTokenRecord, #oidcc_client_context{} = Cl client_id = ClientId } = ClientContext, #oidcc_provider_configuration{ - userinfo_endpoint = Endpoint, issuer = Issuer } = Configuration, #oidcc_token_access{token = AccessToken, type = AccessTokenType} = AccessTokenRecord, @@ -144,10 +143,23 @@ retrieve(#oidcc_token_access{} = AccessTokenRecord, #oidcc_client_context{} = Cl %% separate the two. %% AuthorizationOpts = Opts#{}, + Endpoint = + case Configuration of + #oidcc_provider_configuration{ + tls_client_certificate_bound_access_tokens = true, + mtls_endpoint_aliases = #{ + <<"userinfo_endpoint">> := MtlsEndpoint + } + } -> + MtlsEndpoint; + #oidcc_provider_configuration{ + userinfo_endpoint = UserinfoEndpoint + } -> + UserinfoEndpoint + end, Header = oidcc_auth_util:add_authorization_header( AccessToken, AccessTokenType, get, Endpoint, AuthorizationOpts, ClientContext ), - Request = {Endpoint, Header}, RequestOpts = maps:get(request_opts, Opts, #{}), TelemetryOpts = #{ From 6f7a895238bd15e97b168165d7749a5884454033 Mon Sep 17 00:00:00 2001 From: Paul Swartz Date: Sun, 7 Jan 2024 19:04:27 -0500 Subject: [PATCH 3/7] feat(oidcc_profile): allow tls_client_auth, enforce ciphers --- priv/test/fixtures/fapi2-metadata.json | 1 + src/oidcc_profile.erl | 36 +++++++++++++++-- test/oidcc_client_context_test.erl | 56 ++++++++++++++++++++++++-- 3 files changed, 85 insertions(+), 8 deletions(-) diff --git a/priv/test/fixtures/fapi2-metadata.json b/priv/test/fixtures/fapi2-metadata.json index 4b63d9f..568ac84 100644 --- a/priv/test/fixtures/fapi2-metadata.json +++ b/priv/test/fixtures/fapi2-metadata.json @@ -50,6 +50,7 @@ "client_secret_post", "client_secret_basic", "private_key_jwt", + "tls_client_auth", "unsupporeted_auth" ], "tls_client_certificate_bound_access_tokens": true, diff --git a/src/oidcc_profile.erl b/src/oidcc_profile.erl index 70a4bc9..412b768 100644 --- a/src/oidcc_profile.erl +++ b/src/oidcc_profile.erl @@ -22,12 +22,14 @@ profiles => [profile()], require_pkce => boolean(), trusted_audiences => [binary()] | any, - preferred_auth_methods => [oidcc_auth_util:auth_method()] + preferred_auth_methods => [oidcc_auth_util:auth_method()], + request_opts => oidcc_http_util:request_opts() }. -type opts_no_profiles() :: #{ require_pkce => boolean(), trusted_audiences => [binary()] | any, - preferred_auth_methods => [oidcc_auth_util:auth_method()] + preferred_auth_methods => [oidcc_auth_util:auth_method()], + request_opts => oidcc_http_util:request_opts() }. -type error() :: {unknown_profile, atom()}. @@ -60,8 +62,17 @@ apply_profiles( ), Opts2 = Opts1#{profiles => RestProfiles}, Opts3 = map_put_new(trusted_audiences, [], Opts2), - %% TODO include tls_client_auth">> here when it's supported by the library. - Opts = map_put_new(preferred_auth_methods, [private_key_jwt], Opts3), + Opts4 = map_put_new(preferred_auth_methods, [private_key_jwt, tls_client_auth], Opts3), + Opts5 = put_tls_defaults(Opts4), + Opts = limit_tls_ciphers( + [ + "TLS_DHE_RSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + "TLS_DHE_RSA_WITH_AES_256_GCM_SHA384", + "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384" + ], + Opts5 + ), apply_profiles(ClientContext, Opts); apply_profiles( #oidcc_client_context{} = ClientContext0, @@ -182,6 +193,23 @@ limit_signing_alg_values(AlgSupported, ClientContext0) -> }, ClientContext. +put_tls_defaults(Opts) -> + RequestOpts0 = maps:get(request_opts, Opts, #{}), + SslOpts0 = maps:get(ssl, RequestOpts0, []), + SslOpts1 = SslOpts0 ++ httpc:ssl_verify_host_options(true), + SslOpts = lists:ukeysort(1, SslOpts1), + RequestOpts = RequestOpts0#{ssl => SslOpts}, + Opts#{request_opts => RequestOpts}. + +limit_tls_ciphers(SupportedCipherStrs, Opts) -> + RequestOpts0 = maps:get(request_opts, Opts, #{}), + SslOpts0 = maps:get(ssl, RequestOpts0, []), + SupportedCiphers = lists:map(fun ssl:str_to_suite/1, SupportedCipherStrs), + SslOpts1 = [{ciphers, SupportedCiphers} | SslOpts0], + SslOpts = lists:ukeysort(1, SslOpts1), + RequestOpts = RequestOpts0#{ssl => SslOpts}, + Opts#{request_opts => RequestOpts}. + limit_values(_Limit, undefined) -> undefined; limit_values(Limit, Values) -> diff --git a/test/oidcc_client_context_test.erl b/test/oidcc_client_context_test.erl index ac277bd..d2c8d45 100644 --- a/test/oidcc_client_context_test.erl +++ b/test/oidcc_client_context_test.erl @@ -57,13 +57,58 @@ apply_profiles_fapi2_security_profile_test() -> ?assertMatch( #{ - preferred_auth_methods := [private_key_jwt], + preferred_auth_methods := [private_key_jwt, tls_client_auth], require_pkce := true, - trusted_audiences := [] + trusted_audiences := [], + request_opts := #{ + ssl := _ + } }, Opts ), + #{request_opts := #{ssl := SslOpts}} = Opts, + + ?assertEqual( + {ciphers, [ + #{ + cipher => 'aes_128_gcm', + mac => aead, + key_exchange => dhe_rsa, + prf => sha256 + }, + #{ + cipher => 'aes_128_gcm', + mac => aead, + key_exchange => ecdhe_rsa, + prf => sha256 + }, + #{ + cipher => 'aes_256_gcm', + mac => aead, + key_exchange => dhe_rsa, + prf => sha384 + }, + #{ + cipher => 'aes_256_gcm', + mac => aead, + key_exchange => ecdhe_rsa, + prf => sha384 + } + ]}, + lists:keyfind(ciphers, 1, SslOpts) + ), + + ?assertEqual( + {verify, verify_peer}, + lists:keyfind(verify, 1, SslOpts) + ), + + ?assertMatch( + {cacerts, _}, + lists:keyfind(cacerts, 1, SslOpts) + ), + ok. apply_profiles_fapi2_message_signing_test() -> @@ -106,9 +151,12 @@ apply_profiles_fapi2_message_signing_test() -> ?assertMatch( #{ - preferred_auth_methods := [private_key_jwt], + preferred_auth_methods := [private_key_jwt, tls_client_auth], require_pkce := true, - trusted_audiences := [] + trusted_audiences := [], + request_opts := #{ + ssl := _ + } }, Opts ), From 014e374c58c9fd6a6101cf5cee7c2f8912f9e4dc Mon Sep 17 00:00:00 2001 From: Paul Swartz Date: Sun, 7 Jan 2024 19:21:55 -0500 Subject: [PATCH 4/7] feat: create an httpc profile which disables keep-alive/pipelining As noted in https://github.com/erlef/oidcc/issues/318#issuecomment-1880065555, `httpc` by default will re-use existing connections. This is great if you're using normal HTTPS, but if you're using client authentication then you need to make sure that every time `httpc` connects to a host, it's using the client authentication, which is impossible in practice. This works around that, by creating a new profile which disables that functionality. Using that profile for requests which provide SSL overrides will ensure that each of those requests will use the client certificate. --- mix.exs | 6 +- priv/test/fixtures/README.md | 6 ++ priv/test/fixtures/jwk.csr | 16 ++++ priv/test/fixtures/jwk_cert.pem | 19 ++++ src/oidcc.app.src | 1 + src/oidcc_app.erl | 61 ++++++++++++ src/oidcc_auth_util.erl | 2 +- src/oidcc_http_util.erl | 9 +- test/oidcc_authorization_test.erl | 12 ++- test/oidcc_client_registration_test.erl | 6 +- test/oidcc_http_util_SUITE.erl | 106 +++++++++++++++++++++ test/oidcc_provider_configuration_test.erl | 2 +- test/oidcc_token_test.erl | 39 +++++--- test/oidcc_userinfo_test.erl | 14 +-- 14 files changed, 266 insertions(+), 33 deletions(-) create mode 100644 priv/test/fixtures/README.md create mode 100644 priv/test/fixtures/jwk.csr create mode 100644 priv/test/fixtures/jwk_cert.pem create mode 100644 src/oidcc_app.erl create mode 100644 test/oidcc_http_util_SUITE.erl diff --git a/mix.exs b/mix.exs index 1e63564..7016ac1 100644 --- a/mix.exs +++ b/mix.exs @@ -23,7 +23,11 @@ defmodule Oidcc.Mixfile do ] end - def application, do: [extra_applications: extra_applications(Mix.env())] + def application, + do: [ + mod: {:oidcc_app, []}, + extra_applications: extra_applications(Mix.env()) + ] defp extra_applications(env) defp extra_applications(:dev), do: [:inets, :ssl, :edoc, :xmerl] diff --git a/priv/test/fixtures/README.md b/priv/test/fixtures/README.md new file mode 100644 index 0000000..4184cb8 --- /dev/null +++ b/priv/test/fixtures/README.md @@ -0,0 +1,6 @@ +# Regenerating `jwk_cert.pem` + +``` bash +openssl x509 -signkey jwk.pem -in jwk.csr -req -days 3650 -out jwk_cert.pem +``` + diff --git a/priv/test/fixtures/jwk.csr b/priv/test/fixtures/jwk.csr new file mode 100644 index 0000000..948444b --- /dev/null +++ b/priv/test/fixtures/jwk.csr @@ -0,0 +1,16 @@ +-----BEGIN CERTIFICATE REQUEST----- +MIICezCCAWMCAQAwNjEkMCIGA1UECgwbRXJsYW5nIEVjb3N5c3RlbSBGb3VuZGF0 +aW9uMQ4wDAYDVQQDDAVPaWRjYzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoC +ggEBAKIBNjF96IT2TkwDlkXJ/uneGbYfg/5YqwOZtzscwSDKRGmevVQPiD+8kTG9 +0j8ie7CryjjHJTxtxLq93H6gg74OWmVCffTf2pA0dMGizg3Ua0QPPXmwtHZfmKbJ +cKelCSPTDngQQkkomn+2ROs4xXtDmxeyjKovk/ECOEOV005KTfv0Nh0ZqZlxgmHI +Ot0XBFD4II1pESeiL3l8RE4RLDPq10V3jlWnfNORnNNAY0HgbryuggZGVifcxpnB +DAcRL5BPGaw5lCZn5Yul4ts8JoLpqLcglHbWVoTJnSUxlSKEI/kteOvMiQqwoUPG +KnuG1sktCEm3Wv+hUeq/1B3S7J8CAwEAAaAAMA0GCSqGSIb3DQEBCwUAA4IBAQBY +WZ6HCP6Yrws9/jOWWYS3JOEilIjqLfxgtEM7tOz8zID225DLV0m75UFkl7JIwwxY +Tx4U2FhoDqfVLbarrw31kZ2tbMRELdt9zLZbTv4b9QsB1Q+fXLn5x8W5m6qXK7kh +WIfMfbpUwmuIlcUMxwWuEN3a5XSuHbOqsaY7V9H0c4YSVdyE2C5M2VP0oUECCPjC +p3D6c47qHRkWYY2ssutK2U9cW5IusEUrcjyVIoOcW14pUjkcd3e+lr9S/59onAY1 +Pkb2wd8CsEvdsr+P58uXleWwuHBxwybwAySp5GRvkuEPuuI1YUoDuwkgOeY8Y+te +6LBUBw2DW+Z0QBSleoqs +-----END CERTIFICATE REQUEST----- diff --git a/priv/test/fixtures/jwk_cert.pem b/priv/test/fixtures/jwk_cert.pem new file mode 100644 index 0000000..e0bd43c --- /dev/null +++ b/priv/test/fixtures/jwk_cert.pem @@ -0,0 +1,19 @@ +-----BEGIN CERTIFICATE----- +MIIDGzCCAgOgAwIBAgIUGnShYZbN8W/ZJ5no7hh/WRLKougwDQYJKoZIhvcNAQEL +BQAwNjEkMCIGA1UECgwbRXJsYW5nIEVjb3N5c3RlbSBGb3VuZGF0aW9uMQ4wDAYD +VQQDDAVPaWRjYzAeFw0yNDAxMDcxNjQwMTBaFw0zNDAxMDQxNjQwMTBaMDYxJDAi +BgNVBAoMG0VybGFuZyBFY29zeXN0ZW0gRm91bmRhdGlvbjEOMAwGA1UEAwwFT2lk +Y2MwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCiATYxfeiE9k5MA5ZF +yf7p3hm2H4P+WKsDmbc7HMEgykRpnr1UD4g/vJExvdI/Inuwq8o4xyU8bcS6vdx+ +oIO+DlplQn3039qQNHTBos4N1GtEDz15sLR2X5imyXCnpQkj0w54EEJJKJp/tkTr +OMV7Q5sXsoyqL5PxAjhDldNOSk379DYdGamZcYJhyDrdFwRQ+CCNaREnoi95fERO +ESwz6tdFd45Vp3zTkZzTQGNB4G68roIGRlYn3MaZwQwHES+QTxmsOZQmZ+WLpeLb +PCaC6ai3IJR21laEyZ0lMZUihCP5LXjrzIkKsKFDxip7htbJLQhJt1r/oVHqv9Qd +0uyfAgMBAAGjITAfMB0GA1UdDgQWBBQJXpMge7QiKlfQFkpIx9ailJL21TANBgkq +hkiG9w0BAQsFAAOCAQEAfRspbVWaRIC0ZQv8Y3TrmqzxKcmyHi/ixVn3fW9Ygeq2 +Uasq6r0XE52gnU+Lb/3X8J0n0ENE1ovPjczjxAtrXwdM1l59C1YR7trVZJfRzNGy +2ItO7efI3fCLYPxk4OkTeSubvuxklvyVALSo5dgsZg/7PLy3Vgkzz7XPfJPtFKQ+ +xAOmul26zaJPNz49KT+m/2z77WoJHEyhEleJDo1DUABUwplI6BNecUW6VU+1BiCo +x0Oc3CF+DkU5cKBHulRm5XP+8KvAW8Az52ZNpUGe4YkFKLsyipgFiqiE182QYtVA +vWrEMdmPNr9xbPb5GGg3lropINwy4T8w/WKEdjPttg== +-----END CERTIFICATE----- diff --git a/src/oidcc.app.src b/src/oidcc.app.src index 889531b..d57ac1f 100644 --- a/src/oidcc.app.src +++ b/src/oidcc.app.src @@ -3,6 +3,7 @@ {vsn, "3.1.2-beta.1"}, {registered, []}, {applications, [kernel, stdlib, inets, ssl, public_key, telemetry, jose]}, + {mod, {oidcc_app, []}}, {env, []}, {modules, []}, {licenses, ["Apache-2.0"]}, diff --git a/src/oidcc_app.erl b/src/oidcc_app.erl new file mode 100644 index 0000000..8300de5 --- /dev/null +++ b/src/oidcc_app.erl @@ -0,0 +1,61 @@ +-module(oidcc_app). + +-export([start/2]). +-export([stop/1]). +-export([init/1]). +-export([handle_call/3]). +-export([handle_cast/2]). +-export([handle_info/2]). +-export([terminate/2]). +-export([httpc_profile/0]). + +-behaviour(application). +-behaviour(gen_server). + +%% @private +httpc_profile() -> + oidcc. + +%% Application Callbacks + +%% @private +start(_StartType, StartArgs) -> + gen_server:start_link(oidcc_app, StartArgs, []). + +%% @private +stop(_State) -> + ok. + +%% GenServer Callbacks +%% @private +init(_Args) -> + try + inets:start(httpc, [{profile, httpc_profile()}]) + catch + error:{already_started, _} -> ok + end, + + % disable keep-alive + httpc:set_options( + [ + {pipeline_timeout, 0}, + {keep_alive_timeout, 0}, + {max_sessions, 1} + ], + httpc_profile() + ), + + {ok, [], hibernate}. + +handle_call(_Call, _From, State) -> + {stop, unexpected_call, State}. + +handle_cast(_Call, State) -> + {stop, unexpected_cast, State}. + +handle_info(_Call, State) -> + {stop, unexpected_info, State}. + +terminate(_Reason, _State) -> + inets:stop(httpc, httpc_profile()), + ok. diff --git a/src/oidcc_auth_util.erl b/src/oidcc_auth_util.erl index 860f511..0f6f8a3 100644 --- a/src/oidcc_auth_util.erl +++ b/src/oidcc_auth_util.erl @@ -32,7 +32,7 @@ -spec add_client_authentication( QueryList, Header, SupportedAuthMethods, AllowAlgorithms, Opts, ClientContext ) -> - {ok, {oidcc_http_util:query_params(), [oidcc_http_util:http_header()]}} + {ok, {oidcc_http_util:query_params(), [oidcc_http_util:http_header()]}, auth_method()} | {error, error()} when QueryList :: oidcc_http_util:query_params(), diff --git a/src/oidcc_http_util.erl b/src/oidcc_http_util.erl index 55d44b9..adb5072 100644 --- a/src/oidcc_http_util.erl +++ b/src/oidcc_http_util.erl @@ -92,10 +92,10 @@ request(Method, Request, TelemetryOpts, RequestOpts) -> SslOpts = maps:get(ssl, RequestOpts, undefined), HttpOpts0 = [{timeout, Timeout}], - HttpOpts = + {HttpOpts, HttpProfile} = case SslOpts of - undefined -> HttpOpts0; - _Opts -> [{ssl, SslOpts} | HttpOpts0] + undefined -> {HttpOpts0, default}; + _Opts -> {[{ssl, SslOpts} | HttpOpts0], oidcc_app:httpc_profile()} end, telemetry:span( @@ -108,7 +108,8 @@ request(Method, Request, TelemetryOpts, RequestOpts) -> Method, Request, HttpOpts, - [{body_format, binary}] + [{body_format, binary}], + HttpProfile ), {ok, BodyAndFormat} ?= extract_successful_response(Response), {{ok, {BodyAndFormat, Headers}}, TelemetryExtraMeta} diff --git a/test/oidcc_authorization_test.erl b/test/oidcc_authorization_test.erl index 518bc2a..b11444f 100644 --- a/test/oidcc_authorization_test.erl +++ b/test/oidcc_authorization_test.erl @@ -739,7 +739,8 @@ create_redirect_url_with_par_url_test() -> post, {ReqParEndpoint, Header, "application/x-www-form-urlencoded", Body}, _HttpOpts, - _Opts + _Opts, + _Profile ) -> ?assertMatch(<<"https://my.server/par">>, ReqParEndpoint), ?assertMatch(none, proplists:lookup("authorization", Header)), @@ -838,7 +839,8 @@ create_redirect_url_with_par_error_when_required_test() -> post, {_Endpoint, _Header, _ContentType, _Body}, _HttpOpts, - _Opts + _Opts, + _Profile ) -> {ok, {{"HTTP/1.1", 400, "OK"}, [{"content-type", "application/json"}], ParResponseData}} end, @@ -892,7 +894,8 @@ create_redirect_url_with_par_invalid_response_test() -> post, {_Endpoint, _Header, _ContentType, _Body}, _HttpOpts, - _Opts + _Opts, + _Profile ) -> {ok, {{"HTTP/1.1", 201, "OK"}, [{"content-type", "application/json"}], ParResponseData}} end, @@ -957,7 +960,8 @@ create_redirect_url_with_par_client_secret_jwt_request_object_test() -> post, {_Endpoint, _Header, "application/x-www-form-urlencoded", Body}, _HttpOpts, - _Opts + _Opts, + _Profile ) -> BodyParsed = uri_string:dissect_query(Body), BodyMap = maps:from_list(BodyParsed), diff --git a/test/oidcc_client_registration_test.erl b/test/oidcc_client_registration_test.erl index c397ebd..077861a 100644 --- a/test/oidcc_client_registration_test.erl +++ b/test/oidcc_client_registration_test.erl @@ -46,7 +46,8 @@ register_test() -> post, {ReqEndpoint, _Header, "application/json", Body}, _HttpOpts, - _Opts + _Opts, + _Profile ) -> RegistrationEndpoint = ReqEndpoint, @@ -164,7 +165,8 @@ registration_invalid_response_test() -> post, {ReqEndpoint, _Header, "application/json", Body}, _HttpOpts, - _Opts + _Opts, + _Profile ) -> RegistrationEndpoint = ReqEndpoint, diff --git a/test/oidcc_http_util_SUITE.erl b/test/oidcc_http_util_SUITE.erl new file mode 100644 index 0000000..09d331b --- /dev/null +++ b/test/oidcc_http_util_SUITE.erl @@ -0,0 +1,106 @@ +-module(oidcc_http_util_SUITE). + +-export([all/0]). +-export([init_per_suite/1]). +-export([end_per_suite/1]). +-export([bad_ssl/1]). +-export([client_cert/1]). + +-include_lib("common_test/include/ct.hrl"). +-include_lib("stdlib/include/assert.hrl"). + +all() -> + [ + bad_ssl, + client_cert + ]. + +init_per_suite(_Config) -> + {ok, _} = application:ensure_all_started(oidcc), + []. + +end_per_suite(_Config) -> + ok = application:stop(oidcc). + +telemetry_opts() -> + #{ + topic => [oidcc, oidcc_http_util_SUITE] + }. + +bad_ssl(_Config) -> + ?assertMatch( + {error, {failed_connect, _}}, + oidcc_http_util:request(get, {"https://expired.badssl.com/", []}, telemetry_opts(), #{}) + ), + + ?assertMatch( + {error, {failed_connect, _}}, + oidcc_http_util:request(get, {"https://wrong.host.badssl.com/", []}, telemetry_opts(), #{}) + ), + + ?assertMatch( + {error, {failed_connect, _}}, + oidcc_http_util:request(get, {"https://self-signed.badssl.com/", []}, telemetry_opts(), #{}) + ), + + ?assertMatch( + {error, {failed_connect, _}}, + oidcc_http_util:request( + get, {"https://untrusted-root.badssl.com/", []}, telemetry_opts(), #{} + ) + ), + + ?assertMatch( + {error, {failed_connect, _}}, + oidcc_http_util:request( + get, {"https://tls-v1-1.badssl.com:1011/", []}, telemetry_opts(), #{} + ) + ), + + ok. + +client_cert(_Config) -> + PrivDir = code:priv_dir(oidcc), + KeyFile = + PrivDir ++ + "/test/fixtures/jwk.pem", + CertFile = + PrivDir ++ + "/test/fixtures/jwk_cert.pem", + CertsKeys = [ + #{ + certfile => CertFile, + keyfile => KeyFile + } + ], + ?assertMatch( + {ok, { + {json, #{ + <<"SSL_CLIENT_I_DN">> := <<"CN=Oidcc,O=Erlang Ecosystem Foundation">> + }}, + _ + }}, + oidcc_http_util:request( + get, {"https://certauth.idrix.fr/json/", []}, telemetry_opts(), #{ + ssl => [ + {verify, verify_peer}, + {cacerts, public_key:cacerts_get()}, + {certs_keys, CertsKeys} + ] + } + ) + ), + + ?assertMatch( + {error, {http_error, 403, <<"">>}}, + oidcc_http_util:request( + get, {"https://certauth.idrix.fr/json/", []}, telemetry_opts(), #{ + ssl => [ + {verify, verify_peer}, + {cacerts, public_key:cacerts_get()} + ] + } + ) + ), + + ok. diff --git a/test/oidcc_provider_configuration_test.erl b/test/oidcc_provider_configuration_test.erl index 4c859d8..fb95b0f 100644 --- a/test/oidcc_provider_configuration_test.erl +++ b/test/oidcc_provider_configuration_test.erl @@ -593,7 +593,7 @@ document_overrides_quirk_test() -> uri_concatenation_test() -> ok = meck:new(httpc, [no_link]), HttpFun = - fun(get, {ReqEndpoint, _Header}, _HttpOpts, _Opts) -> + fun(get, {ReqEndpoint, _Header}, _HttpOpts, _Opts, _Profile) -> self() ! {req, ReqEndpoint}, {ok, {{"HTTP/1.1", 501, "Not Implemented"}, [], ""}} diff --git a/test/oidcc_token_test.erl b/test/oidcc_token_test.erl index f4084a4..e38dc8c 100644 --- a/test/oidcc_token_test.erl +++ b/test/oidcc_token_test.erl @@ -76,7 +76,8 @@ retrieve_none_test() -> post, {ReqTokenEndpoint, Header, "application/x-www-form-urlencoded", Body}, _HttpOpts, - _Opts + _Opts, + _Profile ) -> ?assertEqual(<>, iolist_to_binary(ReqTokenEndpoint)), ?assertMatch({"authorization", _}, proplists:lookup("authorization", Header)), @@ -206,7 +207,8 @@ retrieve_rs256_with_rotation_test() -> post, {ReqTokenEndpoint, Header, "application/x-www-form-urlencoded", Body}, _HttpOpts, - _Opts + _Opts, + _Profile ) -> TokenEndpoint = ReqTokenEndpoint, ?assertMatch(none, proplists:lookup("authorization", Header)), @@ -308,7 +310,8 @@ retrieve_hs256_test() -> post, {ReqTokenEndpoint, _Header, "application/x-www-form-urlencoded", _Body}, _HttpOpts, - _Opts + _Opts, + _Profile ) -> TokenEndpoint = ReqTokenEndpoint, {ok, {{"HTTP/1.1", 200, "OK"}, [{"content-type", "application/json"}], TokenData}} @@ -390,7 +393,8 @@ retrieve_hs256_with_max_clock_skew_test() -> post, {ReqTokenEndpoint, _Header, "application/x-www-form-urlencoded", _Body}, _HttpOpts, - _Opts + _Opts, + _Profile ) -> TokenEndpoint = ReqTokenEndpoint, {ok, {{"HTTP/1.1", 200, "OK"}, [{"content-type", "application/json"}], TokenData}} @@ -490,7 +494,8 @@ auth_method_client_secret_jwt_test() -> post, {ReqTokenEndpoint, Header, "application/x-www-form-urlencoded", Body}, _HttpOpts, - _Opts + _Opts, + _Profile ) -> TokenEndpoint = ReqTokenEndpoint, ?assertMatch(none, proplists:lookup("authorization", Header)), @@ -615,7 +620,8 @@ auth_method_client_secret_jwt_with_max_clock_skew_test() -> post, {ReqTokenEndpoint, _, "application/x-www-form-urlencoded", Body}, _HttpOpts, - _Opts + _Opts, + _Profile ) -> TokenEndpoint = ReqTokenEndpoint, BodyMap = maps:from_list(uri_string:dissect_query(Body)), @@ -714,7 +720,8 @@ auth_method_private_key_jwt_no_supported_alg_test() -> post, {ReqTokenEndpoint, Header, "application/x-www-form-urlencoded", Body}, _HttpOpts, - _Opts + _Opts, + _Profile ) -> TokenEndpoint = ReqTokenEndpoint, @@ -819,7 +826,8 @@ auth_method_private_key_jwt_test() -> post, {ReqTokenEndpoint, Header, "application/x-www-form-urlencoded", Body}, _HttpOpts, - _Opts + _Opts, + _Profile ) -> TokenEndpoint = ReqTokenEndpoint, ?assertMatch(none, proplists:lookup("authorization", Header)), @@ -960,7 +968,8 @@ auth_method_private_key_jwt_with_dpop_test() -> post, {ReqTokenEndpoint, Header, "application/x-www-form-urlencoded", Body}, _HttpOpts, - _Opts + _Opts, + _Profile ) -> TokenEndpoint = ReqTokenEndpoint, ?assertMatch(none, proplists:lookup("authorization", Header)), @@ -1147,7 +1156,8 @@ auth_method_private_key_jwt_with_dpop_and_nonce_test() -> post, {ReqTokenEndpoint, Header, "application/x-www-form-urlencoded", Body}, _HttpOpts, - _Opts + _Opts, + _Profile ) -> TokenEndpoint = ReqTokenEndpoint, ?assertMatch(none, proplists:lookup("authorization", Header)), @@ -1320,7 +1330,8 @@ auth_method_private_key_jwt_with_invalid_dpop_nonce_test() -> post, {_Endpoint, _Header, "application/x-www-form-urlencoded", _Body}, _HttpOpts, - _Opts + _Opts, + _Profile ) -> {ok, { {"HTTP/1.1", 400, "OK"}, @@ -1445,7 +1456,8 @@ preferred_auth_methods_test() -> post, {ReqTokenEndpoint, Header, "application/x-www-form-urlencoded", Body}, _HttpOpts, - _Opts + _Opts, + _Profile ) -> TokenEndpoint = ReqTokenEndpoint, ?assertMatch({"authorization", _}, proplists:lookup("authorization", Header)), @@ -1646,7 +1658,8 @@ trusted_audiences_test() -> post, {_TokenEndpoint, _Header, "application/x-www-form-urlencoded", _Body}, _HttpOpts, - _Opts + _Opts, + _Profile ) -> {ok, {{"HTTP/1.1", 200, "OK"}, [{"content-type", "application/json"}], TokenData}} end, diff --git a/test/oidcc_userinfo_test.erl b/test/oidcc_userinfo_test.erl index 1892c1f..e0c0ffd 100644 --- a/test/oidcc_userinfo_test.erl +++ b/test/oidcc_userinfo_test.erl @@ -26,7 +26,7 @@ json_test() -> BadSub = <<"123789">>, HttpFun = - fun(get, {Url, _Header}, _HttpOpts, _Opts) -> + fun(get, {Url, _Header}, _HttpOpts, _Opts, _Profile) -> Url = UserInfoEndpoint, {ok, {{"HTTP/1.1", 200, "OK"}, [{"content-type", "application/json"}], HttpBody}} end, @@ -141,7 +141,7 @@ jwt_test() -> ), HttpFun = - fun(get, {Url, _Header}, _HttpOpts, _Opts) -> + fun(get, {Url, _Header}, _HttpOpts, _Opts, _Profile) -> Url = UserInfoEndpoint, {ok, {{"HTTP/1.1", 200, "OK"}, [{"content-type", "application/jwt"}], HttpBody}} end, @@ -247,7 +247,7 @@ jwt_encrypted_not_signed_test() -> ), HttpFun = - fun(get, {_Url, _Header}, _HttpOpts, _Opts) -> + fun(get, {_Url, _Header}, _HttpOpts, _Opts, _Profile) -> {ok, {{"HTTP/1.1", 200, "OK"}, [{"content-type", "application/jwt"}], HttpBody}} end, ok = meck:new(httpc), @@ -641,7 +641,7 @@ dpop_proof_test() -> ), HttpFun = - fun(get, {Url, Header}, _HttpOpts, _Opts) -> + fun(get, {Url, Header}, _HttpOpts, _Opts, _Profile) -> Url = UserInfoEndpoint, {_, Authorization} = proplists:lookup("authorization", Header), @@ -742,7 +742,7 @@ dpop_proof_case_insensitive_token_type_test() -> AccessToken = <<"opensesame">>, HttpFun = - fun(get, {Url, Header}, _HttpOpts, _Opts) -> + fun(get, {Url, Header}, _HttpOpts, _Opts, _Profile) -> Url = UserInfoEndpoint, {_, Authorization} = proplists:lookup("authorization", Header), @@ -816,7 +816,7 @@ dpop_proof_with_nonce_test() -> }), HttpFun = - fun(get, {Url, Header}, _HttpOpts, _Opts) -> + fun(get, {Url, Header}, _HttpOpts, _Opts, _Profile) -> Url = UserInfoEndpoint, {_, Authorization} = proplists:lookup("authorization", Header), @@ -933,7 +933,7 @@ dpop_proof_with_invalid_nonce_test() -> }), HttpFun = - fun(get, _UrlHeader, _HttpOpts, _Opts) -> + fun(get, _UrlHeader, _HttpOpts, _Opts, _Profile) -> {ok, { {"HTTP/1.1", 400, "Bad Request"}, [{"content-type", "application/json"}, {"dpop-nonce", DpopNonce}], From a23417def960581d535e7438f4db10616249f567 Mon Sep 17 00:00:00 2001 From: Paul Swartz Date: Sun, 7 Jan 2024 22:32:55 -0500 Subject: [PATCH 5/7] feat: tell `httpc` to close the connection if we're using client certs In addition to the separate profile, this header serves as a note to both the remote server and `httpc` that we're not keeping the connection open after the request.` --- src/oidcc_http_util.erl | 41 ++++++++++++++++++++++++++++++---- test/oidcc_http_util_SUITE.erl | 13 +++++++++++ 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/oidcc_http_util.erl b/src/oidcc_http_util.erl index adb5072..2b2d6a0 100644 --- a/src/oidcc_http_util.erl +++ b/src/oidcc_http_util.erl @@ -85,17 +85,32 @@ when Accumulator :: term()}, TelemetryOpts :: telemetry_opts(), RequestOpts :: request_opts(). -request(Method, Request, TelemetryOpts, RequestOpts) -> +request(Method, Request0, TelemetryOpts, RequestOpts) -> TelemetryTopic = maps:get(topic, TelemetryOpts), TelemetryExtraMeta = maps:get(extra_meta, TelemetryOpts, #{}), Timeout = maps:get(timeout, RequestOpts, timer:minutes(1)), SslOpts = maps:get(ssl, RequestOpts, undefined), HttpOpts0 = [{timeout, Timeout}], - {HttpOpts, HttpProfile} = + HttpOpts = case SslOpts of - undefined -> {HttpOpts0, default}; - _Opts -> {[{ssl, SslOpts} | HttpOpts0], oidcc_app:httpc_profile()} + undefined -> HttpOpts0; + _Opts -> [{ssl, SslOpts} | HttpOpts0] + end, + + %% if we're using special SSL opts, always close the connection after we're + %% done. we do this to work around an issue with httpc where it doesn't take + %% the client certificates into account when pipelining, so if the HTTP + %% implementation changes we should think about undoing this. + {Request, HttpProfile} = + case using_client_certificate(SslOpts) of + false -> + {Request0, default}; + true -> + ReqHeaders0 = element(2, Request0), + ReqHeaders1 = [{"connection", "close"} | ReqHeaders0], + ReqHeaders = lists:ukeysort(1, ReqHeaders1), + {setelement(2, Request0, ReqHeaders), oidcc_app:httpc_profile()} end, telemetry:span( @@ -166,3 +181,21 @@ fetch_content_type(Headers) -> _Other -> unknown end. + +-spec using_client_certificate([ssl:tls_client_option()] | undefined) -> boolean(). +using_client_certificate(undefined) -> + false; +using_client_certificate(SslOpts) -> + lists:foldl( + fun + (_, true) -> true; + ({key, _}, _) -> true; + ({keyfile, _}, _) -> true; + ({cert, _}, _) -> true; + ({certfile, _}, _) -> true; + ({certs_keys, _}, _) -> true; + (_, Acc) -> Acc + end, + false, + SslOpts + ). diff --git a/test/oidcc_http_util_SUITE.erl b/test/oidcc_http_util_SUITE.erl index 09d331b..58bdd96 100644 --- a/test/oidcc_http_util_SUITE.erl +++ b/test/oidcc_http_util_SUITE.erl @@ -73,6 +73,19 @@ client_cert(_Config) -> keyfile => KeyFile } ], + + ?assertMatch( + {error, {http_error, 403, <<"">>}}, + oidcc_http_util:request( + get, {"https://certauth.idrix.fr/json/", []}, telemetry_opts(), #{ + ssl => [ + {verify, verify_peer}, + {cacerts, public_key:cacerts_get()} + ] + } + ) + ), + ?assertMatch( {ok, { {json, #{ From 462ce3b65a70166f2950b070c90f448e6e487236 Mon Sep 17 00:00:00 2001 From: Paul Swartz Date: Wed, 10 Jan 2024 09:05:29 -0500 Subject: [PATCH 6/7] feat: `httpc_profile` opt and have the client manage the HTTPC profile --- src/oidcc_http_util.erl | 39 ++++------------------------------ test/oidcc_http_util_SUITE.erl | 15 +++---------- 2 files changed, 7 insertions(+), 47 deletions(-) diff --git a/src/oidcc_http_util.erl b/src/oidcc_http_util.erl index 2b2d6a0..1d23078 100644 --- a/src/oidcc_http_util.erl +++ b/src/oidcc_http_util.erl @@ -28,7 +28,8 @@ -type request_opts() :: #{ timeout => timeout(), - ssl => [ssl:tls_option()] + ssl => [ssl:tls_option()], + httpc_profile => atom() | pid() }. %% See {@link httpc:request/5} %% @@ -85,11 +86,12 @@ when Accumulator :: term()}, TelemetryOpts :: telemetry_opts(), RequestOpts :: request_opts(). -request(Method, Request0, TelemetryOpts, RequestOpts) -> +request(Method, Request, TelemetryOpts, RequestOpts) -> TelemetryTopic = maps:get(topic, TelemetryOpts), TelemetryExtraMeta = maps:get(extra_meta, TelemetryOpts, #{}), Timeout = maps:get(timeout, RequestOpts, timer:minutes(1)), SslOpts = maps:get(ssl, RequestOpts, undefined), + HttpProfile = maps:get(httpc_profile, RequestOpts, default), HttpOpts0 = [{timeout, Timeout}], HttpOpts = @@ -98,21 +100,6 @@ request(Method, Request0, TelemetryOpts, RequestOpts) -> _Opts -> [{ssl, SslOpts} | HttpOpts0] end, - %% if we're using special SSL opts, always close the connection after we're - %% done. we do this to work around an issue with httpc where it doesn't take - %% the client certificates into account when pipelining, so if the HTTP - %% implementation changes we should think about undoing this. - {Request, HttpProfile} = - case using_client_certificate(SslOpts) of - false -> - {Request0, default}; - true -> - ReqHeaders0 = element(2, Request0), - ReqHeaders1 = [{"connection", "close"} | ReqHeaders0], - ReqHeaders = lists:ukeysort(1, ReqHeaders1), - {setelement(2, Request0, ReqHeaders), oidcc_app:httpc_profile()} - end, - telemetry:span( TelemetryTopic, TelemetryExtraMeta, @@ -181,21 +168,3 @@ fetch_content_type(Headers) -> _Other -> unknown end. - --spec using_client_certificate([ssl:tls_client_option()] | undefined) -> boolean(). -using_client_certificate(undefined) -> - false; -using_client_certificate(SslOpts) -> - lists:foldl( - fun - (_, true) -> true; - ({key, _}, _) -> true; - ({keyfile, _}, _) -> true; - ({cert, _}, _) -> true; - ({certfile, _}, _) -> true; - ({certs_keys, _}, _) -> true; - (_, Acc) -> Acc - end, - false, - SslOpts - ). diff --git a/test/oidcc_http_util_SUITE.erl b/test/oidcc_http_util_SUITE.erl index 58bdd96..98a6268 100644 --- a/test/oidcc_http_util_SUITE.erl +++ b/test/oidcc_http_util_SUITE.erl @@ -86,6 +86,8 @@ client_cert(_Config) -> ) ), + inets:start(httpc, [{profile, idrix_fr}]), + ?assertMatch( {ok, { {json, #{ @@ -95,6 +97,7 @@ client_cert(_Config) -> }}, oidcc_http_util:request( get, {"https://certauth.idrix.fr/json/", []}, telemetry_opts(), #{ + httpc_profile => idrix_fr, ssl => [ {verify, verify_peer}, {cacerts, public_key:cacerts_get()}, @@ -104,16 +107,4 @@ client_cert(_Config) -> ) ), - ?assertMatch( - {error, {http_error, 403, <<"">>}}, - oidcc_http_util:request( - get, {"https://certauth.idrix.fr/json/", []}, telemetry_opts(), #{ - ssl => [ - {verify, verify_peer}, - {cacerts, public_key:cacerts_get()} - ] - } - ) - ), - ok. From b4bafcbaa14d2eda7b32b2596fbdef380f5f90b8 Mon Sep 17 00:00:00 2001 From: Paul Swartz Date: Wed, 10 Jan 2024 10:04:27 -0500 Subject: [PATCH 7/7] fixup! feat: `httpc_profile` opt and have the client manage the HTTPC profile --- mix.exs | 1 - src/oidcc.app.src | 1 - src/oidcc_app.erl | 61 ----------------------------------------------- 3 files changed, 63 deletions(-) delete mode 100644 src/oidcc_app.erl diff --git a/mix.exs b/mix.exs index 7016ac1..ef6a820 100644 --- a/mix.exs +++ b/mix.exs @@ -25,7 +25,6 @@ defmodule Oidcc.Mixfile do def application, do: [ - mod: {:oidcc_app, []}, extra_applications: extra_applications(Mix.env()) ] diff --git a/src/oidcc.app.src b/src/oidcc.app.src index d57ac1f..889531b 100644 --- a/src/oidcc.app.src +++ b/src/oidcc.app.src @@ -3,7 +3,6 @@ {vsn, "3.1.2-beta.1"}, {registered, []}, {applications, [kernel, stdlib, inets, ssl, public_key, telemetry, jose]}, - {mod, {oidcc_app, []}}, {env, []}, {modules, []}, {licenses, ["Apache-2.0"]}, diff --git a/src/oidcc_app.erl b/src/oidcc_app.erl deleted file mode 100644 index 8300de5..0000000 --- a/src/oidcc_app.erl +++ /dev/null @@ -1,61 +0,0 @@ --module(oidcc_app). - --export([start/2]). --export([stop/1]). --export([init/1]). --export([handle_call/3]). --export([handle_cast/2]). --export([handle_info/2]). --export([terminate/2]). --export([httpc_profile/0]). - --behaviour(application). --behaviour(gen_server). - -%% @private -httpc_profile() -> - oidcc. - -%% Application Callbacks - -%% @private -start(_StartType, StartArgs) -> - gen_server:start_link(oidcc_app, StartArgs, []). - -%% @private -stop(_State) -> - ok. - -%% GenServer Callbacks -%% @private -init(_Args) -> - try - inets:start(httpc, [{profile, httpc_profile()}]) - catch - error:{already_started, _} -> ok - end, - - % disable keep-alive - httpc:set_options( - [ - {pipeline_timeout, 0}, - {keep_alive_timeout, 0}, - {max_sessions, 1} - ], - httpc_profile() - ), - - {ok, [], hibernate}. - -handle_call(_Call, _From, State) -> - {stop, unexpected_call, State}. - -handle_cast(_Call, State) -> - {stop, unexpected_cast, State}. - -handle_info(_Call, State) -> - {stop, unexpected_info, State}. - -terminate(_Reason, _State) -> - inets:stop(httpc, httpc_profile()), - ok.