Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

password AMQP URI parameter is parsed incorrectly #8129

Closed
lukebakken opened this issue May 10, 2023 · 1 comment · Fixed by #8130
Closed

password AMQP URI parameter is parsed incorrectly #8129

lukebakken opened this issue May 10, 2023 · 1 comment · Fixed by #8130
Assignees
Labels
Milestone

Comments

@lukebakken
Copy link
Collaborator

Describe the bug

In my testing, I found that the query parameter &password=xxx overrides the password in "amqps://user:pass@rabbitmq-host:5671", i.e. in this link it will use user:xxx to log into RabbitMQ instead of user:pass:

amqps://user:pass@rabbitmq-host:5671?cacertfile=/etc/rabbitmq/ssl/ca_certificate.pem&certfile=/etc/rabbitmq/ssl/client_certificate.pem&keyfile=/etc/rabbitmq/ssl/client_key.pem&verify=verify_peer&password=xxx

Reproduction steps

To see how this is parsed, start up RabbitMQ with RABBITMQ_ALLOW_INPUT=true and enter the following into the Erlang shell:

(rabbit@shostakovich)3> amqp_uri:parse(<<"amqps://user:pass@rabbitmq-host:5671?cacertfile=/etc/rabbitmq/ssl/ca_certificate.pem&certfile=/etc/rabbitmq/ssl/client_certificate.pem&keyfile=/etc/rabbitmq/ssl/client_key.pem&verify=verify_peer&password=xxx">>).

{ok,{amqp_params_network,<<"user">>,"xxx",<<"/">>,
                         "rabbitmq-host",5671,2047,0,10,60000,
                         [{server_name_indication,"rabbitmq-host"},
                          {password,"xxx"},
                          {verify,verify_peer},
                          {keyfile,"/etc/rabbitmq/ssl/client_key.pem"},
                          {certfile,"/etc/rabbitmq/ssl/client_certificate.pem"},
                          {cacertfile,"/etc/rabbitmq/ssl/ca_certificate.pem"}],
                         [#Fun<amqp_uri.9.95926583>,#Fun<amqp_uri.9.95926583>],
                         [],[]}}

Note that xxx is the AMQP login password as well, whoops.

Expected behavior

The AMQP password is not overridden.

Additional context

No response

@lukebakken
Copy link
Collaborator Author

b3abf24

lukebakken added a commit that referenced this issue May 10, 2023
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
lukebakken added a commit that referenced this issue May 10, 2023
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
@michaelklishin michaelklishin added this to the 3.11.16 milestone May 10, 2023
mergify bot pushed a commit that referenced this issue May 10, 2023
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

(cherry picked from commit 494d171)
mergify bot pushed a commit that referenced this issue May 10, 2023
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

(cherry picked from commit 494d171)
(cherry picked from commit 90549cd)
mergify bot pushed a commit that referenced this issue May 10, 2023
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

(cherry picked from commit 494d171)
(cherry picked from commit 90549cd)
(cherry picked from commit 5eda3d2)
dcorbacho pushed a commit that referenced this issue May 11, 2023
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
dcorbacho pushed a commit that referenced this issue May 11, 2023
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants