Oauth2 plugin improvements

- Validate JWKS server when getting keys
- Restrict usable algorithms

(cherry picked from commit dd685f11798effdb36fdfb07f13b9d7c08992fff)

6 files changed, 156 insertions, 23 deletions

diff --git a/deps/rabbitmq_auth_backend_oauth2/priv/schema/rabbitmq_auth_backend_oauth2.schema b/deps/rabbitmq_auth_backend_oauth2/priv/schema/rabbitmq_auth_backend_oauth2.schema
index 0feb73d9aa..014aec2d6d 100644
--- a/deps/rabbitmq_auth_backend_oauth2/priv/schema/rabbitmq_auth_backend_oauth2.schema
+++ b/deps/rabbitmq_auth_backend_oauth2/priv/schema/rabbitmq_auth_backend_oauth2.schema
@@ -77,3 +77,32 @@
                   end, Settings),
     maps:from_list(SigningKeys)
  end}.
+
+{mapping,
+ "auth_oauth2.jwks_url",
+ "rabbitmq_auth_backend_oauth2.key_config.jwks_url",
+ [{datatype, string}, {validators, ["uri", "https_uri"]}]}.
+
+{mapping,
+ "auth_oauth2.strict",
+ "rabbitmq_auth_backend_oauth2.key_config.strict",
+ [{datatype, {enum, [true, false]}}]}.
+
+{mapping,
+ "auth_oauth2.cacertfile",
+ "rabbitmq_auth_backend_oauth2.key_config.cacertfile",
+ [{datatype, file}, {validators, ["file_accessible"]}]}.
+
+{validator, "https_uri", "invalid https uri",
+ fun(Uri) -> string:nth_lexeme(Uri, 1, "://") == "https" end}.
+
+{mapping,
+ "auth_oauth2.algorithms.$algorithm",
+ "rabbitmq_auth_backend_oauth2.key_config.algorithms",
+ [{datatype, string}]}.
+
+{translation, "rabbitmq_auth_backend_oauth2.key_config.algorithms",
+ fun(Conf) ->
+     Settings = cuttlefish_variable:filter_by_prefix("auth_oauth2.algorithms", Conf),
+     [list_to_binary(V) || {_, V} <- Settings]
+ end}.
diff --git a/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl b/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl
index c004c6070c..11d184b478 100644
--- a/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl
+++ b/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl
@@ -58,7 +58,7 @@ update_jwks_signing_keys() ->
         undefined ->
             {error, no_jwks_url};
         JwksUrl ->
-            case httpc:request(JwksUrl) of
+            case fetch_keys(JwksUrl) of
                 {ok, {_, _, JwksBody}} ->
                     KeyList = maps:get(<<"keys">>, jose:decode(erlang:iolist_to_binary(JwksBody)), []),
                     Keys = maps:from_list(lists:map(fun(Key) -> {maps:get(<<"kid">>, Key, undefined), {json, Key}} end, KeyList)),
@@ -68,6 +68,18 @@ update_jwks_signing_keys() ->
             end
     end.
 
+-spec fetch_keys(binary() | list()) -> {ok, term()} | {error, term()}.
+fetch_keys(JwksUrl) ->
+    UaaEnv = application:get_env(?APP, key_config, []),
+    case proplists:get_value(strict, UaaEnv, true) of
+        false ->
+            httpc:request(JwksUrl);
+        true ->
+            CaCertFile = proplists:get_value(cacertfile, UaaEnv),
+            SslOpts = [{verify, verify_peer}, {cacertfile, CaCertFile}, {fail_if_no_peer_cert, true}],
+            httpc:request(get, {JwksUrl, []}, [{ssl, SslOpts}], [])
+    end.
+
 -spec decode_and_verify(binary()) -> {boolean(), map()} | {error, term()}.
 decode_and_verify(Token) ->
     case uaa_jwt_jwt:get_key_id(Token) of
diff --git a/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt_jwt.erl b/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt_jwt.erl
index bb73cf9ae4..aa1cdd8241 100644
--- a/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt_jwt.erl
+++ b/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt_jwt.erl
@@ -24,7 +24,15 @@ decode(Token) ->
     end.
 
 decode_and_verify(Jwk, Token) ->
-    case jose_jwt:verify(Jwk, Token) of
+    UaaEnv = application:get_env(rabbitmq_auth_backend_oauth2, key_config, []),
+    Verify =
+        case proplists:get_value(algorithms, UaaEnv) of
+            undefined ->
+                jose_jwt:verify(Jwk, Token);
+            Algs ->
+                jose_jwt:verify_strict(Jwk, Algs, Token)
+        end,
+    case Verify of
         {true, #jose_jwt{fields = Fields}, _}  -> {true, Fields};
         {false, #jose_jwt{fields = Fields}, _} -> {false, Fields}
     end.
diff --git a/deps/rabbitmq_auth_backend_oauth2/test/config_schema_SUITE_data/rabbitmq_auth_backend_oauth2.snippets b/deps/rabbitmq_auth_backend_oauth2/test/config_schema_SUITE_data/rabbitmq_auth_backend_oauth2.snippets
index 2b7018fdd8..3391628267 100644
--- a/deps/rabbitmq_auth_backend_oauth2/test/config_schema_SUITE_data/rabbitmq_auth_backend_oauth2.snippets
+++ b/deps/rabbitmq_auth_backend_oauth2/test/config_schema_SUITE_data/rabbitmq_auth_backend_oauth2.snippets
@@ -4,7 +4,12 @@
         auth_oauth2.additional_scopes_key = my_custom_scope_key
         auth_oauth2.default_key = id1
         auth_oauth2.signing_keys.id1 = test/config_schema_SUITE_data/certs/key.pem
-        auth_oauth2.signing_keys.id2 = test/config_schema_SUITE_data/certs/cert.pem",
+        auth_oauth2.signing_keys.id2 = test/config_schema_SUITE_data/certs/cert.pem
+        auth_oauth2.jwks_url = https://my-jwt-issuer/jwks.json
+        auth_oauth2.cacertfile = test/config_schema_SUITE_data/certs/cacert.pem
+        auth_oauth2.strict = false
+        auth_oauth2.algorithms.1 = HS256
+        auth_oauth2.algorithms.2 = RS256",
         [
         {rabbitmq_auth_backend_oauth2, [
             {resource_server_id,<<"new_resource_server_id">>},
@@ -16,7 +21,11 @@
                     <<"id1">> => {pem, <<"I'm not a certificate">>},
                     <<"id2">> => {pem, <<"I'm not a certificate">>}
                 }
-            }
+            },
+            {jwks_url, "https://my-jwt-issuer/jwks.json"},
+            {cacertfile, "test/config_schema_SUITE_data/certs/cacert.pem"},
+            {strict, false},
+            {algorithms, [<<"HS256">>, <<"RS256">>]}
             ]
             }
         ]}
diff --git a/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl b/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl
index 4823a68e27..85e82072b7 100644
--- a/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl
+++ b/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl
@@ -21,7 +21,9 @@
 all() ->
     [
      {group, happy_path},
-     {group, unhappy_path}
+     {group, unhappy_path},
+     {group, unvalidated_jwks_server},
+     {group, non_strict_mode}
     ].
 
 groups() ->
@@ -34,6 +36,7 @@ groups() ->
                        test_successful_connection_with_complex_claim_as_a_list,
                        test_successful_connection_with_complex_claim_as_a_binary,
                        test_successful_connection_with_keycloak_token,
+                       test_successful_connection_with_algorithm_restriction,
                        test_successful_token_refresh
                       ]},
      {unhappy_path, [], [
@@ -41,9 +44,12 @@ groups() ->
                          test_failed_connection_with_a_non_token,
                          test_failed_connection_with_a_token_with_insufficient_vhost_permission,
                          test_failed_connection_with_a_token_with_insufficient_resource_permission,
+                         test_failed_connection_with_algorithm_restriction,
                          test_failed_token_refresh_case1,
                          test_failed_token_refresh_case2
-                        ]}
+                        ]},
+     {unvalidated_jwks_server, [], [test_failed_connection_with_unvalidated_jwks_server]},
+     {non_strict_mode, [], [{group, happy_path}, {group, unhappy_path}]}
     ].
 
 %%
@@ -69,23 +75,35 @@ end_per_suite(Config) ->
         fun stop_jwks_server/1
       ] ++ rabbit_ct_broker_helpers:teardown_steps()).
 
+init_per_group(non_strict_mode, Config) ->
+    add_vhosts(Config),
+    KeyConfig = rabbit_ct_helpers:set_config(?config(key_config, Config), [{jwks_url, ?config(non_strict_jwks_url, Config)}, {strict, false}]),
+    ok = rabbit_ct_broker_helpers:rpc(Config, 0, application, set_env, [rabbitmq_auth_backend_oauth2, key_config, KeyConfig]),
+    rabbit_ct_helpers:set_config(Config, {key_config, KeyConfig});
 
 init_per_group(_Group, Config) ->
-    %% The broker is managed by {init,end}_per_testcase().
-    lists:foreach(fun(Value) ->
-                          rabbit_ct_broker_helpers:add_vhost(Config, Value)
-                  end,
-                  [<<"vhost1">>, <<"vhost2">>, <<"vhost3">>, <<"vhost4">>]),
+    add_vhosts(Config),
     Config.
 
+end_per_group(non_strict_mode, Config) ->
+    delete_vhosts(Config),
+    KeyConfig = rabbit_ct_helpers:set_config(?config(key_config, Config), [{jwks_url, ?config(strict_jwks_url, Config)}, {strict, true}]),
+    ok = rabbit_ct_broker_helpers:rpc(Config, 0, application, set_env, [rabbitmq_auth_backend_oauth2, key_config, KeyConfig]),
+    rabbit_ct_helpers:set_config(Config, {key_config, KeyConfig});
+
 end_per_group(_Group, Config) ->
-    %% The broker is managed by {init,end}_per_testcase().
-    lists:foreach(fun(Value) ->
-                          rabbit_ct_broker_helpers:delete_vhost(Config, Value)
-                  end,
-                  [<<"vhost1">>, <<"vhost2">>, <<"vhost3">>, <<"vhost4">>]),
+    delete_vhosts(Config),
     Config.
 
+add_vhosts(Config) ->
+    %% The broker is managed by {init,end}_per_testcase().
+    lists:foreach(fun(Value) -> rabbit_ct_broker_helpers:add_vhost(Config, Value) end,
+                  [<<"vhost1">>, <<"vhost2">>, <<"vhost3">>, <<"vhost4">>]).
+
+delete_vhosts(Config) ->
+    %% The broker is managed by {init,end}_per_testcase().
+    lists:foreach(fun(Value) -> rabbit_ct_broker_helpers:delete_vhost(Config, Value) end,
+                  [<<"vhost1">>, <<"vhost2">>, <<"vhost3">>, <<"vhost4">>]).
 
 init_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection_with_a_full_permission_token_and_explicitly_configured_vhost orelse
                                          Testcase =:= test_successful_token_refresh ->
@@ -107,6 +125,24 @@ init_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection
   rabbit_ct_helpers:testcase_started(Config, Testcase),
   Config;
 
+init_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection_with_algorithm_restriction ->
+    KeyConfig = ?config(key_config, Config),
+    ok = rabbit_ct_broker_helpers:rpc(Config, 0, application, set_env, [rabbitmq_auth_backend_oauth2, key_config, [{algorithms, [<<"HS256">>]} | KeyConfig]]),
+    rabbit_ct_helpers:testcase_started(Config, Testcase),
+    Config;
+
+init_per_testcase(Testcase, Config) when Testcase =:= test_failed_connection_with_algorithm_restriction ->
+    KeyConfig = ?config(key_config, Config),
+    ok = rabbit_ct_broker_helpers:rpc(Config, 0, application, set_env, [rabbitmq_auth_backend_oauth2, key_config, [{algorithms, [<<"RS256">>]} | KeyConfig]]),
+    rabbit_ct_helpers:testcase_started(Config, Testcase),
+    Config;
+
+init_per_testcase(Testcase, Config) when Testcase =:= test_failed_connection_with_unvalidated_jwks_server ->
+    KeyConfig = rabbit_ct_helpers:set_config(?config(key_config, Config), {jwks_url, ?config(non_strict_jwks_url, Config)}),
+    ok = rabbit_ct_broker_helpers:rpc(Config, 0, application, set_env, [rabbitmq_auth_backend_oauth2, key_config, KeyConfig]),
+    rabbit_ct_helpers:testcase_started(Config, Testcase),
+    Config;
+
 init_per_testcase(Testcase, Config) ->
     rabbit_ct_helpers:testcase_started(Config, Testcase),
     Config.
@@ -126,6 +162,14 @@ end_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection_
   rabbit_ct_helpers:testcase_started(Config, Testcase),
   Config;
 
+end_per_testcase(Testcase, Config) when Testcase =:= test_successful_connection_with_algorithm_restriction orelse
+                                        Testcase =:= test_failed_connection_with_algorithm_restriction orelse
+                                        Testcase =:= test_failed_connection_with_unvalidated_jwks_server ->
+    rabbit_ct_broker_helpers:delete_vhost(Config, <<"vhost1">>),
+    ok = rabbit_ct_broker_helpers:rpc(Config, 0, application, set_env, [rabbitmq_auth_backend_oauth2, key_config, ?config(key_config, Config)]),
+    rabbit_ct_helpers:testcase_finished(Config, Testcase),
+    Config;
+
 end_per_testcase(Testcase, Config) ->
     rabbit_ct_broker_helpers:delete_vhost(Config, <<"vhost1">>),
     rabbit_ct_helpers:testcase_finished(Config, Testcase),
@@ -143,13 +187,25 @@ start_jwks_server(Config) ->
     %% Assume we don't have more than 100 ports allocated for tests
     PortBase = rabbit_ct_broker_helpers:get_node_config(Config, 0, tcp_ports_base),
     JwksServerPort = PortBase + 100,
+
+    %% Both URLs direct to the same JWKS server
+    %% The NonStrictJwksUrl identity cannot be validated while StrictJwksUrl identity can be validated
+    NonStrictJwksUrl = "https://127.0.0.1:" ++ integer_to_list(JwksServerPort) ++ "/jwks",
+    StrictJwksUrl = "https://localhost:" ++ integer_to_list(JwksServerPort) ++ "/jwks",
+
     ok = application:set_env(jwks_http, keys, [Jwk]),
     {ok, _} = application:ensure_all_started(cowboy),
-    ok = jwks_http_app:start(JwksServerPort),
-    KeyConfig = [{jwks_url, "http://127.0.0.1:" ++ integer_to_list(JwksServerPort) ++ "/jwks"}],
+    CertsDir = ?config(rmq_certsdir, Config),
+    ok = jwks_http_app:start(JwksServerPort, CertsDir),
+    KeyConfig = [{jwks_url, StrictJwksUrl},
+                 {cacertfile, filename:join([CertsDir, "testca", "cacert.pem"])}],
     ok = rabbit_ct_broker_helpers:rpc(Config, 0, application, set_env,
                                       [rabbitmq_auth_backend_oauth2, key_config, KeyConfig]),
-    rabbit_ct_helpers:set_config(Config, {fixture_jwk, Jwk}).
+    rabbit_ct_helpers:set_config(Config,
+                                 [{non_strict_jwks_url, NonStrictJwksUrl},
+                                  {strict_jwks_url, StrictJwksUrl},
+                                  {key_config, KeyConfig},
+                                  {fixture_jwk, Jwk}]).
 
 stop_jwks_server(Config) ->
     ok = jwks_http_app:stop(),
@@ -321,6 +377,13 @@ test_successful_token_refresh(Config) ->
     amqp_channel:close(Ch2),
     close_connection_and_channel(Conn, Ch).
 
+test_successful_connection_with_algorithm_restriction(Config) ->
+    {_Algo, Token} = rabbit_ct_helpers:get_config(Config, fixture_jwt),
+    Conn = open_unmanaged_connection(Config, 0, <<"username">>, Token),
+    {ok, Ch} = amqp_connection:open_channel(Conn),
+    #'queue.declare_ok'{queue = _} =
+        amqp_channel:call(Ch, #'queue.declare'{exclusive = true}),
+    close_connection_and_channel(Conn, Ch).
 
 test_failed_connection_with_expired_token(Config) ->
     {_Algo, Token} = generate_expired_token(Config, [<<"rabbitmq.configure:vhost1/*">>,
@@ -387,3 +450,13 @@ test_failed_token_refresh_case2(Config) ->
        amqp_connection:open_channel(Conn)),
 
     close_connection(Conn).
+
+test_failed_connection_with_algorithm_restriction(Config) ->
+    {_Algo, Token} = rabbit_ct_helpers:get_config(Config, fixture_jwt),
+    ?assertMatch({error, {auth_failure, _}},
+                 open_unmanaged_connection(Config, 0, <<"username">>, Token)).
+
+test_failed_connection_with_unvalidated_jwks_server(Config) ->
+    {_Algo, Token} = rabbit_ct_helpers:get_config(Config, fixture_jwt),
+    ?assertMatch({error, {auth_failure, _}},
+                 open_unmanaged_connection(Config, 0, <<"username">>, Token)).
diff --git a/deps/rabbitmq_auth_backend_oauth2/test/jwks_http_app.erl b/deps/rabbitmq_auth_backend_oauth2/test/jwks_http_app.erl
index 16353e34f4..c745e436f6 100644
--- a/deps/rabbitmq_auth_backend_oauth2/test/jwks_http_app.erl
+++ b/deps/rabbitmq_auth_backend_oauth2/test/jwks_http_app.erl
@@ -1,8 +1,8 @@
 -module(jwks_http_app).
 
--export([start/1, stop/0]).
+-export([start/2, stop/0]).
 
-start(Port) ->
+start(Port, CertsDir) ->
     Dispatch =
         cowboy_router:compile(
           [
@@ -11,8 +11,10 @@ start(Port) ->
                  ]}
           ]
          ),
-    {ok, _} = cowboy:start_clear(jwks_http_listener,
-                      [{port, Port}],
+    {ok, _} = cowboy:start_tls(jwks_http_listener,
+                      [{port, Port},
+                       {certfile, filename:join([CertsDir, "server", "cert.pem"])},
+                       {keyfile, filename:join([CertsDir, "server", "key.pem"])}],
                       #{env => #{dispatch => Dispatch}}),
     ok.