Block FTP requests if "Block all unencrypted requests" is on #11738
Conversation
|
This should fix #2311 |
|
@Hainish can we ship this in the next release? I think this is a quite important feature. |
ghost
mentioned this pull request
|
@J0WI Add this to "Work in Progress" in your kanban I guess. |
| !/\.onion$/.test(uri.hostname) && | ||
| !/^localhost$/.test(uri.hostname) && | ||
| !/^127(\.[0-9]{1,3}){3}$/.test(uri.hostname) && | ||
| !/^0\.0\.0\.0$/.test(uri.hostname) |
There was a problem hiding this comment.
@Hainish 0.0.0.0 is not a valid IPv4 address. Not sure why we have it here.
There was a problem hiding this comment.
I also changed the regexp for loopback IP addresses just now.
There was a problem hiding this comment.
It is a valid IP address. See https://en.wikipedia.org/wiki/0.0.0.0
In the Internet Protocol Version 4, the address 0.0.0.0 is a non-routable meta-address used to designate an invalid, unknown or non-applicable target. To give a special meaning to an otherwise invalid piece of data is an application of in-band signaling.
In the context of servers, 0.0.0.0 means "all IPv4 addresses on the local machine". If a host has two IP addresses, 192.168.1.1 and 10.1.2.1, and a server running on the host listens on 0.0.0.0, it will be reachable at both of those IPs.
For instance, setting up a simple local webserver with python is by default accessible over 0.0.0.0:
user@https-everywhere ~/blah $ python -m SimpleHTTPServer &
[1] 16779
user@https-everywhere ~/blah $ Serving HTTP on 0.0.0.0 port 8000 ...
user@https-everywhere ~/blah $ curl 0.0.0.0:8000
127.0.0.1 - - [15/Aug/2017 11:27:48] "GET / HTTP/1.1" 200 -
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 3.2 Final//EN"><html>
<title>Directory listing for /</title>
<body>
<h2>Directory listing for /</h2>
<hr>
<ul>
<li><a href="default.rulesets.gz.base64">default.rulesets.gz.base64</a>
<li><a href="rulesets-signature.sha256.base64">rulesets-signature.sha256.base64</a>
<li><a href="rulesets-timestamp">rulesets-timestamp</a>
</ul>
<hr>
</body>
</html>
user@https-everywhere ~/blah $There was a problem hiding this comment.
@Hainish Thanks for useful information!
There was a problem hiding this comment.
@Hainish Chrome returns ERR_ADDRESS_INVALID if you attempt to visit http://0.0.0.0/ though.
| @@ -568,7 +566,7 @@ function onBeforeRedirect(details) { | |||
|
|
|||
| // Registers the handler for requests | |||
| // See: https://github.com/EFForg/https-everywhere/issues/10039 | |||
| wr.onBeforeRequest.addListener(onBeforeRequest, {urls: ["*://*/*"]}, ["blocking"]); | |||
| wr.onBeforeRequest.addListener(onBeforeRequest, {urls: ["http://*/*", "https://*/*", "ftp://*/*"]}, ["blocking"]); | |||
There was a problem hiding this comment.
Why change this back to http://*/* and https://*/* rather than keep *://*/*? Is this just for readability?
|
This seems not to be blocking unencrypted |
|
For either Chromium or Firefox. It also doesn't work when I switch the match pattern to |
|
This is probably a bug in the WebExtensions APIs. This would be a good feature to have, let's open bugs the respective implementations. |
|
@Hainish I found what caused the problem: sloppy code in |
|
@Hainish Done. Test and if there are no problems, merge. |
|
@Hainish Can you figure out why the tests fail? My testing can't find any issues. |
|
Re-running seems to have resolved the issue |
|
Testing again with ftp://mirror.fra10.de.leaseweb.net/centos/ I see the same behavior - this request is not blocked. It is a problem with WebExtensions. Add the following lines to the top of console.log('GOT HERE');
console.log(details);With ftp URLs, this is never triggered. |
|
@Hainish Try with an extension that cancels all requests. |
|
The hook itself doesn't work. |
|
@Hainish File a bug on Chromium and Firefox. |
|
Blocked by WebExtensions flaw. |
ghost
closed this
No description provided.