Closed claustromaniac closed 5 years ago
@practik
Thanks for reporting that! It turns out that site redirects from https to http in a non-standard way, and this extension doesn't have any decent/reliable way to figure out that doing so is the server's intention. Sites that redirect to http like that should be fairly rare. Nevertheless, I would like for this extension to handle those appropriately too, but after researching and thinking a good while, I've concluded that the code for that would get very convoluted, and I'm not even sure I can make a reliable implementation that doesn't cause other issues of its own. Moreover, a decent solution would very likely require more permissions. That's not to say I already gave up on it though.
The page includes an inline script that looks like this: ```html ``` This reloads the tab over `http`, but does not trigger the `webRequest.onBeforeRedirect` event. It *does* trigger the `webRequest.onBeforeRequest` event, but the extension has no way to tell who or what started that request and why, so it treats it like any other request, and ends up redirecting it again to https. Most servers instead return a `301` status code header to redirect from https to http, and the extension handles those correctly.
"Redirection loop," that's the phrase I was looking for :-) Glad I could find a puzzle for you to work on.
When it happened to me I had no clue what was going on, but since it was an unfamiliar problem, and I had just installed HTTPZ the day before, I tried disabling it first. So then I was thinking that maybe if the issue wasn't fixable there might at least be a way to let users know that HTTPZ was involved β¦ but if HTTPZ doesn't actually know it's caught in a loop, then I'm guessing that an error message wouldn't be any easier to implement than a fix.
I managed to pull off a fix without requiring more permissions. It's not exactly elegant, but it gets the job done, and it didn't take too many more lines of code!
I didn't test it exhaustively yet. You can try 0.8.0b4
to see if it works for you, if you want.
Edit: damn, a regression. I broke this in 86e1cf1... Don't bother testing 0.8.0b4
:sweat_smile:
Edit2: ok, fixed again in 0.8.0b5
. In case you're curious, what happened was that I missed a single character.
Edit3: I noticed that, when the automatic mode is off, this is treating the server's redirection as an error, which is not right, since the extension is supposed to respect the server's preference to downgrade to HTTP. That's a human bug. I'll fix that one later.
Edit4: OK, fixed in 0.8.0b6
Editx10999... π π π
Editx10999... π π π
Username checks out βΒ at least the "maniac" part :-) Just tested & 0.8.0b6
works fine for me β thanks!
Username checks out
Indeed. π
Thank you again for reporting this, and for testing!
Now for the bad news:
After a much needed rest and further analysis, I've come to the late realization that my approach (and any other approach I can think of) carries a security risk.
Back to my initial analysis, the gist of this problem is that there is no way to tell who or what changes the value of location.href
. Even third-party scripts can change it, causing the client to load another page (that doesn't necessarily have to be the same one). I initially tried to use the DOMContentLoaded
of the window
interface to implement this, but that didn't work, so I had to resort to using the load
event, that fires when the page first fully loads. This means that a Man in the Middle could theoretically rewrite a third-party script served over HTTP, and cause the main document (served over HTTPS) to redirect to HTTP or simply to some other site like www.imsoevil.net
.
Unfortunately, I'm going to have to roll these changes back. Sorry about that. I got carried away trying to solve the puzzle, because I love challenges like this one that test my creativity (a maniac through and through), and didn't think it through the first time around. I wish I had the tools for doing something about this (something like a particular event that fires when location.href
changes).
The good thing is this "fix" didn't reach the stable release. This is one of the reasons I do so many beta releases between stable releases, so I can have some freedom to experiment and occasionally make wrong decisions without that necessarily affecting the userbase.
my approach (and any other approach I can think of) carries a security risk.
Actually, it's doesn't have anything to do with the approach per se... it's a risk that exists even without this extension.
Hm... I'm having a hard time making up my mind. If I don't do anything about this, the extension will break some sites, and if I do something I expose users to a risk they were already exposed to in the first place (without this extension)...
If I don't do anything about this, the extension will break some sites, and if I do something I expose users to a risk they were already exposed to in the first place (without this extension)...
Well, would it be true to say that if you don't do anything about it, the extension will protect users from that risk? (Probably not, I'm guessing, but I'm not sure.) Because that might be an argument in favor of wontfix
.
Other than that I don't think I can offer much in the way of feedback βΒ you clearly have a much better handle on the implications than I do β except to say no worries about the back-and-forth; I'd much rather have you roll back bad fixes than keep them!
would it be true to say that if you don't do anything about it, the extension will protect users from that risk?
Indeed. The redirection loop happens because this extension intercepts the page's attempt at reloading itself over HTTP and redirects it again to HTTPS. Since the page never gets to actually load over HTTP, the extension effectively protects against that (albeit annoyingly).
Technically, the security risk comes from allowing mixed content to load, and my implementation of the "fix" for the redirection loop doesn't aggravate it in the least.
Users can (and should) block mixed content via about:config or using uMatrix. Blocking third-party scripts by default (with a content blocker like uMatrix or uBlock Origin) is another way to protect oneself. So, it's not like there aren't any ways to eliminate or mitigate said risk.
Thinking that way, I should just leave the fix in. I'm going to leave this issue open in the meantime anyway.
Edit: I forgot that security.mixed_content.block_active_content
defaults to true
, which means users should be protected by default. :tada:
Originally posted by @practik in https://github.com/claustromaniac/httpz/issues/2#issuecomment-497835835