Closed Bug 1263823 Opened 4 years ago Closed 4 years ago

Some http:// livemarks fail to load

Categories

(Toolkit :: Places, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla48
Tracking Status
firefox47 --- unaffected
firefox48 + fixed

People

(Reporter: markh, Assigned: ckerschb)

References

Details

(Keywords: regression)

Attachments

(1 file)

A number of my livemarks recently started failing to load.

mozregression pointed at https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9766c66da7717e0ca88962f1b6621a455bb57635&tochange=0c90d5d3f78dfd7d98a4a4c75c70f3d1ae4908c0, which includes bug 1242857 - it starts working when I manually revert that change.

One example is a livemark to hacker news - the URL for the feed is http://news.ycombinator.com/rss. Notably, trying to connect to this URL in the browser automagically upgrades to https:// - presumably via the upgrade-insecure-requests mechanism?

The console shows:
> NS_ERROR_NOT_AVAILABLE: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIChannel.contentType]FeedProcessor.js:1292
> Livemark Service: feed processor received an invalid channel for http://news.ycombinator.com/rss   nsLivemarkService.js:831

The best STR I can come up with is:
* Visit http://news.ycombinator.com/rss - note that the request is actually made to https://news.ycombinator.com/rss - and hit "subscribe".
* Close Firefox then use "sqlite manager" or similar to change the moz_annos table so the URL is actually the http:// version (ie, replace the https:// with https://) - there may be another way to achieve this - the key is to ensure the livemark url is http, not https.
* Restart the browser - the livemark will fail to load and the above messages will appear in the console.

(Obviously I didn't do this for my existing livemarks - the livemarks were added before this automagic https:// upgrade happened).
(In reply to Mark Hammond [:markh] from comment #0)
> change the moz_annos table so the URL is actually the http:// version (ie, replace
> the https:// with https://)

Doh! Obviously this should read "(ie, replace the https:// with http://)"
sounds like maybe SEC_REQUIRE_SAME_ORIGIN_DATA_IS_BLOCKED is too strict?
Flags: needinfo?(ckerschb)
[Tracking Requested - why for this release]:

This is a regression that may cause people's long-standing Livemark (ie, RSS bookmarks) to stop working.
I can reproduce the issue; here is what happens. We create a new channel within the livemarkservice, the security checks within asyncOpen2() pass. I don't think that SEC_REQUIRE_SAME_ORIGIN_DATA_IS_BLOCKED is too restrictive, because we create a codebasePrincipal from the feedURI, hence the principal uri and the feedURI match exactly.
Here are the important arguments when calling securitychecks within asyncOpen2():

> doContentSecurityCheck {
>  channelURI: http://news.ycombinator.com/rss
>  loadingPrincipal: http://news.ycombinator.com/rss
>  triggeringPrincipal: http://news.ycombinator.com/rss
>  contentPolicyType: 33
>  securityMode: SEC_REQUIRE_SAME_ORIGIN_DATA_IS_BLOCKED, 
>  initalSecurityChecksDone: no
>  enforceSecurity: no
> }

Right after that the channel gets redirected; most likely due to sending upgrade-insecure-requests header and then the channel gets redirected:

> AsyncOnChannelRedirect {
>  oldURI: http://news.ycombinator.com/rss
>  newURI: https://news.ycombinator.com/rss
> }

And we receive the following error:
> Security Error: Content at http://news.ycombinator.com/rss may not load data from https://news.ycombinator.com/rss.
> NS_ERROR_NOT_AVAILABLE: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE)[nsIChannel.contentType]FeedProcessor.js:1292

Probably we have to implement some special logic to allow upgrading from http to https. I am slightly surprised that we haven't encountered that problem anywhere else until now.

This definitely needs more investigation - thanks for reporting.

Jonas, I will investigate further tomorrow; any ideas how we could handle that? I would imagine this problem is not fixed by just losening the securityFlag, because we might face that problem elsewhere as well where we pass SEC_REQUIRE_SAME_ORIGIN, but want to allow upgrading from http to https.
Assignee: nobody → ckerschb
Status: NEW → ASSIGNED
Flags: needinfo?(jonas)
Flags: needinfo?(ckerschb)
What is the exact feature where we're now using a same-origin load?

I.e. what part of the livemark implementation is it that's starting this load, and what principal are we using as the loadingPrincipal?
Flags: needinfo?(jonas)
Ah, comment 0 has good STR, sorry I missed that.

So sounds like the problem here is something like:
1. The user browse to http://news.ycombinator.com/rss
2. The user create a livemark
3. Firefox store "http://news.ycombinator.com/rss" in some database (the bookmark database?)
4. At some point later in time the server decides to start redirecting http://news.ycombinator.com/rss
   to https://news.ycombinator.com/rss. Possibly using HSTS, possibly using a normal server-side 302
   redirect, possibly through other means.
5. The firefox/gecko code for loading a livemark create a principal using the livemark URL and use
   that as loadingPrincipal, and also use a SAME_ORIGIN policy.
6. The load fails due to the redirect.

Can someone confirm that my guesses for Firefox/Gecko behavior in steps 3 and 5 are actually correct?

If so, I think we should just remove the SAME_ORIGIN policy. I see little reason to not allow cross-origin redirects.
(In reply to Jonas Sicking (:sicking) from comment #5)
> I.e. what part of the livemark implementation is it that's starting this
> load, and what principal are we using as the loadingPrincipal?

See comment 4 which states all the important arguments like loadingPrincipal, channelURI, etc.
(In reply to Jonas Sicking (:sicking) from comment #6)
> Can someone confirm that my guesses for Firefox/Gecko behavior in steps 3
> and 5 are actually correct?

This is also my understanding of what happens here.
 
> If so, I think we should just remove the SAME_ORIGIN policy. I see little
> reason to not allow cross-origin redirects.

Agreed, here we go.
Attachment #8741297 - Flags: review?(jonas)
> (In reply to Jonas Sicking (:sicking) from comment #6)
> > Can someone confirm that my guesses for Firefox/Gecko behavior in steps 3
> > and 5 are actually correct?

Mark, just to make sure we are not missing anything here. Can you confirm that our assumptions from comment 6 are what you are experiencing?
Flags: needinfo?(markh)
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #9)
> Mark, just to make sure we are not missing anything here. Can you confirm
> that our assumptions from comment 6 are what you are experiencing?

Yep, that's correct - although I don't know enough about that code to know that point 5 is correct (although I suspect it is)
Flags: needinfo?(markh)
https://hg.mozilla.org/mozilla-central/rev/382498551ec0
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Works great, thanks!
Status: RESOLVED → VERIFIED
Recent regression, tracking in case it reopens.
Version: unspecified → Trunk
You need to log in before you can comment on or make changes to this bug.