Some http:// livemarks fail to load

VERIFIED FIXED in Firefox 48

Status

()

Toolkit
Places
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: markh, Assigned: ckerschb)

Tracking

({regression})

Trunk
mozilla48
regression
Points:
---

Firefox Tracking Flags

(firefox47 unaffected, firefox48+ fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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).
(Reporter)

Comment 1

2 years ago
(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)
(Reporter)

Comment 3

2 years ago
[Tracking Requested - why for this release]:

This is a regression that may cause people's long-standing Livemark (ie, RSS bookmarks) to stop working.
status-firefox47: --- → unaffected
status-firefox48: --- → affected
tracking-firefox48: --- → ?
(Assignee)

Comment 4

2 years ago
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)
(Assignee)

Updated

2 years ago
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.
(Assignee)

Comment 7

2 years ago
(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.
(Assignee)

Comment 8

2 years ago
Created attachment 8741297 [details] [diff] [review]
bug_1263823_livemarks_securityflag_update.patch

(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)
(Assignee)

Comment 9

2 years ago
> (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)
(Reporter)

Comment 10

2 years ago
(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)
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/382498551ec0
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox48: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
(Reporter)

Comment 13

2 years ago
Works great, thanks!
Status: RESOLVED → VERIFIED
Recent regression, tracking in case it reopens.
tracking-firefox48: ? → +
Version: unspecified → Trunk
You need to log in before you can comment on or make changes to this bug.