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

Jump to bottom

Do not reuse connection if proxy credentials changed #4835

Closed
wants to merge 5 commits into from
Closed

Do not reuse connection if proxy credentials changed #4835

wants to merge 5 commits into from

Conversation

peterpiekarski
Copy link
Contributor

Connections are reused even if the user credentials have changed.
This is a security issue. A user could get access to an already existing connection to a server via a proxy even though the user is not allowed to use the proxy for that connection in the first place.

@bagder
Copy link
Member

bagder commented Jan 20, 2020

proxy_info_matches() is called for both socks proxies and HTTP(S) proxies, and I'm pretty sure the HTTP(S) case doesn't have this problem (as they are typically authenticated in every request). I suggest you split that into two separate functions so that your improvements only apply for the socks ones.

@bagder bagder self-assigned this Jan 20, 2020
@peterpiekarski
Copy link
Contributor Author

Agreed. I split the method into proxy_info_matches (which is now unchanged) and added socks_proxy_info_matches and use it at the right place.

@bagder bagder closed this in 34e6bc4 Jan 24, 2020
@bagder
Copy link
Member

bagder commented Jan 24, 2020

Thanks!

@lock lock bot locked as resolved and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@bagder bagder

Labels
connecting & proxies
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@peterpiekarski @bagder