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

Support option to use other HTTP clients #5

Open
tgxworld opened this issue May 25, 2023 · 5 comments
Open

Support option to use other HTTP clients #5

tgxworld opened this issue May 25, 2023 · 5 comments

Comments

@tgxworld
Copy link

Hi, I'll like to propose adding an option that allows users to switch out the Net::HTTP client for another HTTP client of their choice. By default, the Net::HTTP client does not provide any protection against SSRF or DNS rebinding attacks so using this gem can easily lead to SSRF or DNS rebinding attacks if no endpoint validation is done by the application before sending out the payload. At https://github.com/discourse/discourse, we're using a patched version of Net::HTTP to protect us against SSRF and DNS rebinding attacks and we would like an easy way to use another HTTP client for this gem without having to monkey patch. I was wondering if a PR for such a change will be welcomed.

@collimarco
Copy link
Contributor

Thanks for opening this issue, I am always happy to review any proposals / pull requests for this gem.

In this specific case both your attacks can be prevented if you connect only to trusted domain names: for Pushpad we first check that the Web Push endpoint matches an official push service.

This is the up-to-date list of push services that we use:
https://github.com/pushpad/known-push-services

Basically you can simply check with a regex if the endpoint is valid before sending the Web Push message with this gem.

Does this solve your issue?

@tgxworld
Copy link
Author

@collimarco Sorry for the late response as I totally missed the notification email. Unfortunately for us, our security requirements are quite strict in the sense that checking for trusted domain names is insufficient for us as we want to assume the worst case scenario where even the trusted domains can also be vulnerable.

@collimarco
Copy link
Contributor

@tgxworld I see. However please consider the following:

  • the domains are from Google, Mozilla, Microsoft, Apple, so they are pretty trustworthy
  • even if a domain is not safe, the HTTP request is generated by this gem (it's not an arbitrary request set by the user)
  • the payload is encrypted, so it would be extremely hard to forge arbitrary HTTP requests (even if you can set the payload data)

Given the above points I see extremely difficult to perform any kind of attacks, even in the worst case scenario.

@tgxworld
Copy link
Author

even if a domain is not safe, the HTTP request is generated by this gem (it's not an arbitrary request set by the user)

While the request is generated by the gem, the request is still being executed on the server where its operating environment may give it access to internal services via internal IPs.

the payload is encrypted, so it would be extremely hard to forge arbitrary HTTP requests (even if you can set the payload data)

In certain cases for us, the payload is not a factor because we have internal service which will carry out an action as long as a request is received.

@collimarco
Copy link
Contributor

collimarco commented Jul 20, 2023

While the request is generated by the gem, the request is still being executed on the server where its operating environment may give it access to internal services via internal IPs.

Ok, but that's like any normal call to an external HTTP REST API.

You would have the exact same problem for any request made to an external API (create an invoice, email, blog post, anything...).

This means that if there is a security bug, that should be fixed in Net::HTTP, it's not something specific to Web Push.

Maybe you can reference this case and open a pull request directly in Ruby Net::HTTP? They could add an option for higher security for example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants