10000 [Messenger] [AMQP] Do not leak any credentials when connection fails by ruudk · Pull Request #42707 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Messenger] [AMQP] Do not leak any credentials when connection fails #42707

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 25, 2021
Merged

[Messenger] [AMQP] Do not leak any credentials when connection fails #42707

merged 1 commit into from
Aug 25, 2021

Conversation

ruudk
Copy link
Contributor
@ruudk ruudk commented Aug 24, 2021 •
edited
Loading
Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets
Doc PR

I noticed that when the connection to AMQP fails for whatever reason all the DSN credentials are leaked.

Aug 24 15:58:40 CRITICAL: Error thrown while running command "messenger:consume async". 
Message: "Could not connect to the AMQP server. Please verify the provided DSN. 
({"host":my-hostname-on-some-server","port":my-port,"vhost":"the-real-vhost",
"login":"the-real-username","password":"********"})

Yes, the password is masked. But it still leaks the server, port, username and vhost.

I think these things should be private and not be logged to a logger server or error capture service.

@ruudk ruudk requested a review from sroze as a code owner August 24, 2021 14:50
@carsonbot carsonbot changed the title [AMQP] [Messenger] Do not leak any credentials when connection fails [Messenger] [AMQP] Do not leak any credentials when connection fails Aug 24, 2021
@ruudk @fabpot
[AMQP] [Messenger] Do not leak any credentials when connection fails
5812a49 
I noticed that when the connection to AMQP fails for whatever reason all the DSK credentials are leaked.

Yes, the password is masked. But it still leaks the server, port and username.

I think these things should be private and not be logged to a logger server or error capture service.
@fabpot fabpot changed the base branch from 5.4 to 5.3 August 25, 2021 04:45
@fabpot
Copy link
Member
fabpot commented Aug 25, 2021

Thank you @ruudk.

@fabpot fabpot merged commit d336926 into symfony:5.3 Aug 25, 2021
@fabpot fabpot mentioned this pull request Aug 30, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@OskarStark OskarStark OskarStark approved these changes

@sroze sroze Awaiting requested review from sroze

@chalasr chalasr Awaiting requested review from chalasr

@dunglas dunglas Awaiting requested review from dunglas

@jderusse jderusse Awaiting requested review from jderusse

@lyrixx lyrixx Awaiting requested review from lyrixx

@wouterj wouterj Awaiting requested review from wouterj

@xabbuh xabbuh Awaiting requested review from xabbuh

@yceruto yceruto Awaiting requested review from yceruto

Assignees
No one assigned
Labels
Bug Messenger Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ruudk @fabpot @OskarStark @carsonbot
0