Restrict to proxy controllers to external servers #28439
Conversation
There was a problem hiding this comment.
currently, "code.org" is whitelisted in the xhr proxy controller, and some apps (perhaps some curriculum?) depend on applab apps being able to use this to read data from each other. Will this change break this behavior?
in security, allow-lists are typically preferred over deny lists. is there a way to assert positively that a request is going to some place outside our network rather than trying to list all of the ways that it could be going somewhere inside?
| @@ -147,4 +160,11 @@ def render_error_response(status, text) | |||
| prevent_caching | |||
| render plain: text, status: status | |||
| end | |||
|
|
|||
| # Do not permit proxying to a server on our own private network, unless it is our own dashboard IP Address (we | |||
| # sometimes proxy to ourselves, which is an internal IP address on development / continuous integration environments). | |||
There was a problem hiding this comment.
do we still need this exception allowing our own dashboard IP address, given the comments elsewhere that this is only enforced in production?
There was a problem hiding this comment.
I've eliminated the production-only logic in favor of a more narrow exception that allows proxying to a private IP address when that is the IP address that the current dashboard server is operating on.
code.org resolves to a public address, so proxying to it is allowed and not impacted by this change.
As best as I can tell, the formal definition of a public IP address, is any IP address that is not reserved for private/local use. I've tried to use a method name that is consistent with the practice of defining allow-lists ( |
| IPAddr.new('169.254.0.0/16'), | ||
| IPAddr.new('192.168.0.0/16'), | ||
| IPAddr.new('fd00::/8') | ||
| ].freeze |
There was a problem hiding this comment.
I think this check could be simplified to ip.private? || ip.link_local? || ip.loopback? || IPAddr.new('0.0.0.0/8').include?(ip), which would offload some of these details to the Ruby standard lib.
There was a problem hiding this comment.
I didn't notice those handy methods. Switched to using them!
Prevent Media Proxy and other proxy controllers from being used to communicate to internal servers.