diff options
| author | Luke Bakken <luke@bakken.io> | 2023-05-10 06:57:59 -0700 |
|---|---|---|
| committer | Luke Bakken <luke@bakken.io> | 2023-05-10 07:13:47 -0700 |
| commit | 494d171ff2ceabee9770217ee24462f6c45b451d (patch) | |
| tree | c81f59a285ebb6991fe82f6dc40954b774beb6e1 | |
| parent | f5c780325e1af2bebd570745a343c1a3cb960097 (diff) | |
| download | rabbitmq-server-git-494d171ff2ceabee9770217ee24462f6c45b451d.tar.gz | |
Correctly use AMQP URI query parameter `password`
Fixes #8129
The query parameter `password` in an AMQP URI should only be used to set
a certificate password, *not* the login password. The login password is
set via the `amqp_authority` section as defined here -
https://www.rabbitmq.com/uri-spec.html
* Add test that demonstrates issue in #8129
* Modify code to fix test
Modify amqp_uri so that test passes
| -rw-r--r-- | deps/amqp_client/src/amqp_uri.erl | 20 | ||||
| -rw-r--r-- | deps/amqp_client/test/unit_SUITE.erl | 10 |
2 files changed, 19 insertions, 11 deletions
diff --git a/deps/amqp_client/src/amqp_uri.erl b/deps/amqp_client/src/amqp_uri.erl index d15c1607e4..7ca723d1e1 100644 --- a/deps/amqp_client/src/amqp_uri.erl +++ b/deps/amqp_client/src/amqp_uri.erl @@ -204,12 +204,15 @@ broker_add_query(Params, ParsedUri, Fields) -> return({ParamsN, Pos1}); Value -> try - ValueParsed = parse_amqp_param(Field, Value), - return( - {setelement(Pos, ParamsN, ValueParsed), Pos1}) + case parse_amqp_param(Field, Value) of + ignore -> + return({ParamsN, Pos1}); + ValueParsed -> + return({setelement(Pos, ParamsN, ValueParsed), Pos1}) + end catch throw:Reason -> - fail({invalid_amqp_params_parameter, - Field, Value, Query, Reason}) + fail({invalid_amqp_params_parameter, + Field, Value, Query, Reason}) end end end || Field <- Fields], {Params, 2}), @@ -221,8 +224,11 @@ parse_amqp_param(Field, String) when Field =:= channel_max orelse Field =:= connection_timeout orelse Field =:= depth -> find_integer_parameter(String); -parse_amqp_param(Field, String) when Field =:= password -> - find_identity_parameter(String); +parse_amqp_param(Field, _String) when Field =:= password -> + %% https://github.com/rabbitmq/rabbitmq-server/issues/8129 + %% Ignore `password` here since the parameter is used for setting a + %% certificate password, NOT an AMQP login password + return(ignore); parse_amqp_param(Field, String) -> fail({parameter_unconfigurable_in_query, Field, String}). diff --git a/deps/amqp_client/test/unit_SUITE.erl b/deps/amqp_client/test/unit_SUITE.erl index af4bc688e6..aeee31ec9a 100644 --- a/deps/amqp_client/test/unit_SUITE.erl +++ b/deps/amqp_client/test/unit_SUITE.erl @@ -147,8 +147,9 @@ amqp_uri_parsing(_Config) -> {server_name_indication,"host3"}], ?assertEqual(lists:usort(Exp3), lists:usort(TLSOpts3)), - {ok, #amqp_params_network{host = "host4", ssl_options = TLSOpts4}} = - amqp_uri:parse("amqps://host4/%2f?cacertfile=/path/to/cacertfile.pem" + {ok, #amqp_params_network{username = <<"user">>, password = <<"pass">>, + host = "host4", ssl_options = TLSOpts4}} = + amqp_uri:parse("amqps://user:pass@host4/%2f?cacertfile=/path/to/cacertfile.pem" "&certfile=/path/to/certfile.pem" "&password=topsecret" "&depth=5"), @@ -171,8 +172,9 @@ amqp_uri_parsing(_Config) -> {verify, verify_none}]), lists:usort(TLSOpts8)), - {ok, #amqp_params_network{host = "127.0.0.1", ssl_options = TLSOpts9}} = - amqp_uri:parse("amqps://127.0.0.1/%2f?cacertfile=/path/to/cacertfile.pem" + {ok, #amqp_params_network{username = <<"user">>, password = <<"pass">>, + host = "127.0.0.1", ssl_options = TLSOpts9}} = + amqp_uri:parse("amqps://user:pass@127.0.0.1/%2f?cacertfile=/path/to/cacertfile.pem" "&certfile=/path/to/certfile.pem" "&password=topsecret" "&depth=5"), |