OAuth 2 plugin improvements

[anhanhnguyen]

Hi,

We implemented some changes to the Oauth plugin which affect how RabbitMQ fetches the JWKey Sets from a remote host, restrict the list of valid algorithms in the token, and some improvements to the config file.

These changes are the following:

• Currently, when the keys are fetched from a remote service, the certificate of the remote is not validated. For this we implemented the configuration options:
  • auth_oauth2.https.peer_verification = verify_peer
  • auth_oauth2.https.cacertfile = /usr/local/etc/ca-certificates/cert.pem
  • auth_oauth2.https.depth = 10
• Added configuration option in the ini style config for a jwks URL.
  • auth_oauth2.jwks_url = https://URL
  • According to the JWK spec the URL must be HTTPS, so we added validation for that.
• The list of algorithms usable by the tokens is restrictable.
  • We wanted to enforce certain algorithms (ie, symmetric keys should not be used), therefore we implemented a configuration option, for example:
    • auth_oauth2.algorithms.0 = HS256
    • auth_oauth2.algorithms.1 = RS256
• Changes the HTTPC timeout to 60 seconds.
• Updated README and tests.

Care was taken to keep the current behaviour, however for example this caused that verify_none needs to be the default for the key fetching.

This PR was created as part of a security audit made on behalf of LKAB.

[pjk25]

Hi @anhanhnguyen I can see that there are some test failures for //deps/rabbitmq_auth_backend_oauth2:jwks_SUITE on your fork in actions. They don't show here, but I have made a change in master such that if you rebase your PR, the checks should run here.

[michaelklishin]

There are warnings and that will break compilation from scratch with the flags we use by default:

jwks_SUITE.erl:365:6: Warning: variable '_Algo' is already bound. If you mean to ignore this value, use '_' or a different underscore-prefixed name
jwks_SUITE.erl:426:6: Warning: variable '_Algo' is already bound. If you mean to ignore this value, use '_' or a different underscore-prefixed name

[michaelklishin]

gmake tests fails for me locally with

jwks_SUITE > no_peer_verification > happy_path > test_successful_connection_with_a_full_permission_token_and_all_defaults
    #1. {error,
         {{badmatch,
           {error,
            {{function_clause,
              [{gen,do_for_proc,
                [{error,
                  {auth_failure,
                   "ACCESS_REFUSED - Login was refused using authentication mechanism PLAIN. For details see the broker logfile."}},
                 #Fun<gen.0.49329122>],
                [{file,"gen.erl"},{line,363}]},
               {gen_server,call,3,[{file,"gen_server.erl"},{line,243}]},
               {amqp_gen_connection,open_channel,3,
                [{file,"amqp_gen_connection.erl"},{line,52}]},
               {jwks_SUITE,
                test_successful_connection_with_a_full_permission_token_and_all_defaults,
                1,
                [{file,"jwks_SUITE.erl"},{line,272}]},
               {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1783}]},
               {test_server,run_test_case_eval1,6,
                [{file,"test_server.erl"},{line,1292}]},
               {test_server,run_test_case_eval,9,
                [{file,"test_server.erl"},{line,1224}]}]},
             {gen_server,call,
              [{error,
                {auth_failure,
                 "ACCESS_REFUSED - Login was refused using authentication mechanism PLAIN. For details see the broker logfile."}},
               {command,{open_channel,none,{amqp_selective_consumer,[]}}},
               infinity]}}}},
          [{jwks_SUITE,
            test_successful_connection_with_a_full_permission_token_and_all_defaults,
            1,
            [{file,"jwks_SUITE.erl"},{line,272}]},
           {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1783}]},
           {test_server,run_test_case_eval1,6,
            [{file,"test_server.erl"},{line,1292}]},
           {test_server,run_test_case_eval,9,
            [{file,"test_server.erl"},{line,1224}]}]}}

jwks_SUITE > no_peer_verification > happy_path > test_successful_connection_with_algorithm_restriction
    #1. {error,
         {{badmatch,
           {error,
            {{function_clause,
              [{gen,do_for_proc,
                [{error,
                  {auth_failure,
                   "ACCESS_REFUSED - Login was refused using authentication mechanism PLAIN. For details see the broker logfile."}},
                 #Fun<gen.0.49329122>],
                [{file,"gen.erl"},{line,363}]},
               {gen_server,call,3,[{file,"gen_server.erl"},{line,243}]},
               {amqp_gen_connection,open_channel,3,
                [{file,"amqp_gen_connection.erl"},{line,52}]},
               {jwks_SUITE,
                test_successful_connection_with_algorithm_restriction,1,
                [{file,"jwks_SUITE.erl"},{line,384}]},
               {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1783}]},
               {test_server,run_test_case_eval1,6,
                [{file,"test_server.erl"},{line,1292}]},
               {test_server,run_test_case_eval,9,
                [{file,"test_server.erl"},{line,1224}]}]},
             {gen_server,call,
              [{error,
                {auth_failure,
                 "ACCESS_REFUSED - Login was refused using authentication mechanism PLAIN. For details see the broker logfile."}},
               {command,{open_channel,none,{amqp_selective_consumer,[]}}},
               infinity]}}}},
          [{jwks_SUITE,test_successful_connection_with_algorithm_restriction,
            1,
            [{file,"jwks_SUITE.erl"},{line,384}]},
           {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1783}]},
           {test_server,run_test_case_eval1,6,
            [{file,"test_server.erl"},{line,1292}]},
           {test_server,run_test_case_eval,9,
            [{file,"test_server.erl"},{line,1224}]}]}}

[michaelklishin]

The documentation section reinvents a lot of docs and uses non-standard (at least for us) terminology. Instead it should reference existing RabbitMQ TLS docs and use the same terms used there as much as possible.

[anhanhnguyen]

Thank you for your comments. I updated the README and tests. Please let me know what you think.

[michaelklishin]

Thank you!