Support option to use other HTTP clients
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.
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?
@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.
@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.
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.
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.