Intent to deprecate (and then remove) AppCache on insecure origins

13 971 vues
Accéder directement au premier message non lu

Joel Weinberger

non lue,
8 janv. 2016, 17:29:5208/01/2016
à blink-dev
Firefox recently announced an intent to remove AppCache entirely: https://www.fxsitecompat.com/en-US/docs/2016/application-cache-support-will-be-removed/

While it seems awfully aggressive to straight up remove AppCache immediately, it has been a long source of consternation to the security team that AppCache is allowed over insecure origins. This allows for potentially persistent, online and offline, XSS of sites, which is a pretty serious privilege escalation from regular XSS.

In the spirit of Deprecating Powerful Features on Insecure Origins, we are proposing to deprecate and then remove AppCache on insecure origins. I'd like to add a deprecation message ASAP, and then in a release or two, do the actual disabling.

Unfortunately, we don't have perfect numbers for usage at the moment, since there isn't a proper WebCore.FeatureObserver entry for AppCache. However, doing some rough math using WebCore.FeatureObserver.PageVisits and AppCache.MainPageLoad entries, it appears that around 1.9% of all page loads use include an AppCache main page load event, but only 0.05% do that over an insecure origin. While not quite at the 0.03% level we'd like it to be, I'd also point out that this is a really, really rough estimate since the PageVisits and MainPageLoad events are not from the same measurement positions in the Chrome stack.

Thoughts?
--Joel

Joel Weinberger

non lue,
8 janv. 2016, 17:33:5908/01/2016
à blink-dev,Mike West,Joshua Bell
Also, if you'd like to know the precise numbers and how I calculated it (and you're privileged to that info), feel free to contact me offline.
--Joel

Rick Byers

non lue,
8 janv. 2016, 19:11:1308/01/2016
à Joel Weinberger,Mike West,Joshua Bell,blink-dev

Are you proposing that the window.applicationCache API won't exist for documents loaded from an insecure origin? Or just that the manifest wont be fetched and the API would be neutered somehow?  Other than not working offline, what failure modes would you expect?

Rick

Joel Weinberger

non lue,
15 janv. 2016, 17:35:0615/01/2016
à Rick Byers,Mike West,Joshua Bell,blink-dev
Sorry for the delayed response! My proposal is to return an undefined object when window.applicationCache is accessed on a non-secure context (plus give a console message) and to throw an error when the manifest attribute is used in a document in a non-secure context. This would mimic, as closely as possible, what we've done with getUserMedia() and geolocation. I'm certainly open to other suggestions, though.

There is an open question about explicitly clearing out already existing AppCaches in insecure origins as well. I would lean towards doing that just to be explicit about it, but that's certainly up for debate.

Thus, the failure modes I expect would be:
   * Failure to register a manifest in a non-secure context, plus a thrown error.
   * An undefined reference returned when window.applicationCache is accessed on an insecure origin, plus a console message.
   * Clear out existing AppCaches on insecure origins, plus a console message that this is being done.
--Joel

PhistucK

non lue,
15 janv. 2016, 17:38:1815/01/2016
à Joel Weinberger,Rick Byers,Mike West,Joshua Bell,blink-dev
Will feature detection work?
"applicationCache" in window // should return false


PhistucK

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

Joel Weinberger

non lue,
15 janv. 2016, 17:39:4315/01/2016
à PhistucK,Rick Byers,Mike West,Joshua Bell,blink-dev
Great point. Yes, it should be removed from the window object completely, not just marked as undefined.

Joshua Bell

non lue,
15 janv. 2016, 17:56:4715/01/2016
à Joel Weinberger,Rick Byers,Mike West,blink-dev
By "throw an error when the manifest attribute is used in a document in a non-secure context" do you mean log a deprecation error to the console? This wouldn't in itself cause an actual exception to be thrown in any script context, correct?

Joel Weinberger

non lue,
15 janv. 2016, 17:58:2315/01/2016
à Joshua Bell,Rick Byers,Mike West,blink-dev
Well, I was imagining throwing an exception, but I guess I don't know what's standard in similar cases. If it's more appropriate to just fail and to give a deprecation message in the console, then I'm fine with that. I guess that's equivalent to the "feature not present" case, so that would make sense as the way to go.

Dominic Cooney

non lue,
17 janv. 2016, 21:03:0917/01/2016
à Joel Weinberger,Joshua Bell,Rick Byers,Mike West,blink-dev
I think authors using appcache will typically write the manifest attribute in their HTML, like this:

<html manifest="foo.appcache">

as opposed to manipulating the manifest attribute via script. (I believe "manifest" is only effective when it is processed by the parser?) There's a wrinkle with throwing an exception in this case: because there's no JavaScript executing when the parser processes the manifest attribute, there's nowhere to throw the exception *to*. We could invoke the page's onerror handlers, but it is unlikely any have been set up yet.

When you make this change, you might need to disable the appcache, preload scanner integration. That should be < one line in HTMLPreloadScanner.cpp.

Rick Byers

non lue,
18 janv. 2016, 17:46:5918/01/2016
à Joel Weinberger,PhistucK,Mike West,Joshua Bell,blink-dev

On Sat, Jan 16, 2016 at 12:34 AM, Joel Weinberger <j...@chromium.org> wrote:
Sorry for the delayed response! My proposal is to return an undefined object when window.applicationCache is accessed on a non-secure context (plus give a console message) and to throw an error when the manifest attribute is used in a document in a non-secure context. This would mimic, as closely as possible, what we've done with getUserMedia() and geolocation. I'm certainly open to other suggestions, though.

There is an open question about explicitly clearing out already existing AppCaches in insecure origins as well. I would lean towards doing that just to be explicit about it, but that's certainly up for debate.

Thus, the failure modes I expect would be:
   * Failure to register a manifest in a non-secure context, plus a thrown error.
   * An undefined reference returned when window.applicationCache is accessed on an insecure origin, plus a console message.
 
Today window.applicationCache is always an ApplicationCache instance on Chrome, right?  This change seems like it could be pretty breaking - throwing an exception and breaking page functionality in arbitrary ways (eg. what if it's common to have a bunch of app-cache code under a Chrome UA check).  This design certainly seems the cleanest, so I'm happy to see you give it a shot, but you'd need to collect some compat data that indicates this isn't likely to impact many page views.  We could brainstorm separately about how to do that (eg. you could use cluster telemetry to load the top 10k websites with and without this feature and look for new exceptions).

I was expecting that we'd probably have to do something uglier but more pragmatic to avoid serious compat impact.  Eg. could you provide a "neutered" ApplicationCache instance that would observably behave just like a real object but not actually do any persistent caching?

It would be nice to know what Firefox was thinking here.  They've got some more app compat testing infrastructure than we do, perhaps they've already done some work to understand what forms of removal are possibly web compatible?

On Fri, Jan 15, 2016 at 2:38 PM PhistucK <phis...@gmail.com> wrote:
Will feature detection work?
"applicationCache" in window // should return false

On Fri, Jan 15, 2016 at 5:39 PM, Joel Weinberger <j...@chromium.org> wrote:
Great point. Yes, it should be removed from the window object completely, not just marked as undefined.
 
I agree this is preferable to the value 'undefined'.  But from other discussions we've had with the bindings team, I think that may be hard to implement (the window prototype is baked into the V8 snapshot IIRC).
 

Elliott Sprehn

non lue,
18 janv. 2016, 19:32:5818/01/2016
à Rick Byers,Joel Weinberger,Mike West,PhistucK,blink-dev,Joshua Bell

Indeed, I'd rather we made it just expire the contents of the cache immediately and log a warning to the console.

Making the whole API missing doesn't seem developer or platform friendly.

andrea.g...@gmail.com

non lue,
19 janv. 2016, 01:47:1319/01/2016
à blink-dev,j...@google.com
> Making the whole API missing doesn't seem developer or platform friendly.

Agreed, plus it's not about "preferable" ... if the accessor will return undefined but `"applicationCache" in window` will return true we'll be in clown-town.

Please do not repeat mistakes like these [1]

Thank you

Mike West

non lue,
19 janv. 2016, 03:38:0719/01/2016
à Elliott Sprehn,Rick Byers,Joel Weinberger,PhistucK,blink-dev,Joshua Bell
On Tue, Jan 19, 2016 at 1:32 AM, Elliott Sprehn <esp...@chromium.org> wrote:

Indeed, I'd rather we made it just expire the contents of the cache immediately and log a warning to the console.

Making the whole API missing doesn't seem developer or platform friendly.

The specific example of appcache to the side, I've been talking with folks about the general behavior we'd like APIs to specify for themselves: https://github.com/heycam/webidl/pull/65 captures most of that conversation. The tendency at the moment is towards hiding the bits and pieces that are locked to secure contexts; if you're unhappy with that resolution, then please do weigh in. :)

-mike

Philip Jägenstedt

non lue,
19 janv. 2016, 11:11:5919/01/2016
à Mike West,Elliott Sprehn,Rick Byers,Joel Weinberger,PhistucK,blink-dev,Joshua Bell
As Anne points out in that issue, it's not certain that adding [SecureContext] to existing APIs will be web compatible, but if it could be shown to be safe for window.applicationCache, especially given that Firefox will attempt full removal.

What are the other alternatives? Expiring the whole cache and then never updating it again sounds like it carries lower risk, but there's still some risk, as presumably sites can depend on the many states and events on ApplicationCache, and faking all of that isn't an option.

P.S. I manually added this to https://bit.ly/blinkintents, the script is picky about the title format

Philip Jägenstedt

non lue,
2 févr. 2016, 06:03:1802/02/2016
à Mike West,Elliott Sprehn,Rick Byers,Joel Weinberger,PhistucK,blink-dev,Joshua Bell
So, perhaps we don't need to block the deprecation on knowing the exact method of removal, at least if there's a separate Intent to Remove down the line.

However, it would be nice to decide on the timeframe, so that the deprecation message can include it. Is AppCache blocking other useful work, or would it be fine to leave it deprecated for 3-6 months? Migrating to Service Worker could be a lot of work, so giving plenty of time would be nice. The Firefox documentation says only "will be removed in the future", do you know anything about their timeline?

Philip

Joel Weinberger

non lue,
2 févr. 2016, 14:58:3702/02/2016
à Philip Jägenstedt,Mike West,Elliott Sprehn,Rick Byers,PhistucK,blink-dev,Joshua Bell
Hi everyone. I met with the API owners today, and here are a few updates.

As a clarification to the numbers I cited in the original email, the AppCache.MainPageLoad event is counted for all response retrievals from the AppCache. So this includes all resources loaded via AppCache instead of hitting the network. It does not measure the use of any of the AppCache JS APIs, but that's OK since all resource loads are done implicitly via the cache. This, I believe, is another indicator that the numbers I cited originally are a large overcount since it's taking resource loads over page loads (rather than "this page has 1 or more resources loaded via AppCache" over page loads).

The conclusion from the meeting was two fold:
  • Add a deprecation message immediately for the insecure use of AppCache.
  • In 2 or 3 releases, we will proceed with disabling AppCache on insecure origins. We will do so by simply not loading resources from the AppCache when the origin is insecure. We will *not* disable the APIs themselves since that has the potential to more greatly break applications, and in theory, sites should be prepared to handle a case where the AppCache is empty anyway. We will additionally add a message to the console to tell developers what is going on.
As a side note, unfortunately, I do not know about the actual Firefox timeline. I will ping Mozilla security folks and try to find out.
--Joel

Chris Harrelson

non lue,
2 févr. 2016, 15:51:0602/02/2016
à Joel Weinberger,Philip Jägenstedt,Mike West,Elliott Sprehn,Rick Byers,PhistucK,blink-dev,Joshua Bell
This plan LGTM1.

--

Dimitri Glazkov

non lue,
2 févr. 2016, 15:52:3002/02/2016
à Chris Harrelson,Joel Weinberger,Philip Jägenstedt,Mike West,Elliott Sprehn,Rick Byers,PhistucK,blink-dev,Joshua Bell
LGTM2.

:DG<

Michael Nordman

non lue,
2 févr. 2016, 15:53:0002/02/2016
à Joel Weinberger,Philip Jägenstedt,Mike West,Elliott Sprehn,Rick Byers,PhistucK,blink-dev,Joshua Bell
A point of clarification, there is only one AppCache.MainPageLoad event per document load. The main document and each nested iframe add to the count, things like scripts, css, images do not add to the count. I don't know if that changes any decision making, just making the clarification.

--

Rick Byers

non lue,
2 févr. 2016, 15:57:5902/02/2016
à Michael Nordman,Joel Weinberger,Philip Jägenstedt,Mike West,Elliott Sprehn,PhistucK,blink-dev,Joshua Bell
Given that this change shouldn't break anything (other than offline support not working), I don't think the exact usage is too relevant.
LGTM3

Joel Weinberger

non lue,
2 févr. 2016, 17:14:1702/02/2016
à Rick Byers,Michael Nordman,Philip Jägenstedt,Mike West,Elliott Sprehn,PhistucK,blink-dev,Joshua Bell
Ah, thanks Michael! That is a useful clarification and means that the original numbers I cited are probably not that far off from reality.
--Joel 

Joel Weinberger

non lue,
2 févr. 2016, 17:20:1002/02/2016
à Rick Byers,Michael Nordman,Philip Jägenstedt,Mike West,Elliott Sprehn,PhistucK,blink-dev,Joshua Bell
Also, one further clarification: Looking at the Firefox bug for removing AppCache (https://bugzilla.mozilla.org/show_bug.cgi?id=1237782), it appears that although they do not have an explicit deadline for removing AppCache, it is mostly blocked on shipping Service Workers. Once that happens, they plan on putting together a more definite timeline.
--Joel

Matt Falkenhagen

non lue,
2 févr. 2016, 22:28:2602/02/2016
à Joel Weinberger,Rick Byers,Michael Nordman,Philip Jägenstedt,Mike West,Elliott Sprehn,PhistucK,blink-dev,Joshua Bell
2016-02-03 7:19 GMT+09:00 Joel Weinberger <j...@chromium.org>:
Also, one further clarification: Looking at the Firefox bug for removing AppCache (https://bugzilla.mozilla.org/show_bug.cgi?id=1237782), it appears that although they do not have an explicit deadline for removing AppCache, it is mostly blocked on shipping Service Workers. Once that happens, they plan on putting together a more definite timeline.
--Joel

Service workers shipped in Firefox 44 <https://developer.mozilla.org/en-US/Firefox/Releases/44>, and a deprecation notice for AppCache was added <https://bugzilla.mozilla.org/show_bug.cgi?id=1204581>.

Joe Medley

non lue,
3 févr. 2016, 11:08:4303/02/2016
à Matt Falkenhagen,Joel Weinberger,Rick Byers,Michael Nordman,Philip Jägenstedt,Mike West,Elliott Sprehn,PhistucK,blink-dev,Joshua Bell,Dru Knox
I see 3 LGTMs. Can someone please provide a tracking bug and status entry.

Thank you,


Joe Medley | Technical Writer, DevPlat | jme...@google.com | 816-678-7195

Joel Weinberger

non lue,
3 févr. 2016, 12:20:0503/02/2016
à Joe Medley,Matt Falkenhagen,Rick Byers,Michael Nordman,Philip Jägenstedt,Mike West,Elliott Sprehn,PhistucK,blink-dev,Joshua Bell,Dru Knox

Currently mid transit, but will do once I land.

Joe Medley

non lue,
22 févr. 2016, 13:04:0422/02/2016
à Joel Weinberger,Matt Falkenhagen,Rick Byers,Michael Nordman,Philip Jägenstedt,Mike West,Elliott Sprehn,PhistucK,blink-dev,Joshua Bell,Dru Knox
Joel,

Did you remember to do this?

Joe

Joe Medley | Technical Writer, DevPlat | jme...@google.com | 816-678-7195

Joel Weinberger

non lue,
22 févr. 2016, 13:06:4822/02/2016
à Joe Medley,Matt Falkenhagen,Rick Byers,Michael Nordman,Philip Jägenstedt,Mike West,Elliott Sprehn,PhistucK,blink-dev,Joshua Bell,Dru Knox
It is not done yet. I got confirmation last week from the WebView folks that they don't expect any breakage on their end, so I will put together a CL this week to do it.
--Joel 

Joel Weinberger

non lue,
22 févr. 2016, 20:04:4922/02/2016
à Joe Medley,Matt Falkenhagen,Rick Byers,Michael Nordman,Philip Jägenstedt,Mike West,Elliott Sprehn,PhistucK,blink-dev,Joshua Bell,Dru Knox
Ah, Joe, perhaps I misunderstood you. Were you referring to your earlier request specifically for a tracking bug and status entry? The tracking bug is now filed as https://crbug.com/588931. For the other insecure origin deprecations and removals, we've been using the single https://www.chromestatus.com/features/6021277022158848 feature, but perhaps we should file an explicit one for this?
--Joel 

Joe Medley

non lue,
23 févr. 2016, 11:56:1023/02/2016
à Joel Weinberger,Matt Falkenhagen,Rick Byers,Michael Nordman,Philip Jägenstedt,Mike West,Elliott Sprehn,PhistucK,blink-dev,Joshua Bell,Dru Knox,Paul Kinlan
Joel,

Thank you for the tracking bug. 

I'm neither the expert nor the arbiter or launch review procedures. My personal opinion is that this needs it's own dashboard item to give developers a proper and courteous heads-up.

Joe

Joe Medley | Technical Writer, Chrome DevRel | jme...@google.com | 816-678-7195
If an API's not documented it doesn't exist.

Rick Byers

non lue,
23 févr. 2016, 11:58:5723/02/2016
à Joe Medley,Joel Weinberger,Matt Falkenhagen,Michael Nordman,Philip Jägenstedt,Mike West,Elliott Sprehn,PhistucK,blink-dev,Joshua Bell,Dru Knox,Paul Kinlan
Yeah it's generally best to err on the side of creating new chromestatus.com entries.  Developers DO check chromestatus to see "what's been added/removed in this release" and it does seed our other outreach processes (blog post, etc.).

Joel Weinberger

non lue,
23 févr. 2016, 12:11:4623/02/2016
à Rick Byers,Joe Medley,Matt Falkenhagen,Michael Nordman,Philip Jägenstedt,Mike West,Elliott Sprehn,PhistucK,blink-dev,Joshua Bell,Dru Knox,Paul Kinlan
Okay, I'll make a new tracking bug for "Removing Old Powerful Features from Insecure Origins." I'll post back here shortly. 

PhistucK

non lue,
23 févr. 2016, 12:19:0823/02/2016
à Joel Weinberger,Rick Byers,Joe Medley,Matt Falkenhagen,Michael Nordman,Philip Jägenstedt,Mike West,Elliott Sprehn,blink-dev,Joshua Bell,Dru Knox,Paul Kinlan
I think they meant that you create one specifically for AppCache...


PhistucK

Joel Weinberger

non lue,
23 févr. 2016, 12:22:1423/02/2016
à PhistucK,Rick Byers,Joe Medley,Matt Falkenhagen,Michael Nordman,Philip Jägenstedt,Mike West,Elliott Sprehn,blink-dev,Joshua Bell,Dru Knox,Paul Kinlan
Well, I already created one for general removal of powerful features on insecure origins :-) https://www.chromestatus.com/feature/5635223400218624

Happy to create more specific ones if that's what people feel are appropriate, although it seems like if we keep that general one up to date, we should be able to point people there, no?

Rick Byers

non lue,
23 févr. 2016, 12:27:4123/02/2016
à Joel Weinberger,PhistucK,Joe Medley,Matt Falkenhagen,Michael Nordman,Philip Jägenstedt,Mike West,Elliott Sprehn,blink-dev,Joshua Bell,Dru Knox,Paul Kinlan
The problem is that people and our tools/process browse over the list of eg. "what's exactly changed in Chrome 48".  The main UI is sorted by milestone (with links to each milestone) for this reasons.  I don't have a super strong opinion though, but in other instances we've erred on creating really light weight entries for specific changes per-milestone (just make sure there are links to the details with more information).

Rick

Joel Weinberger

non lue,
23 févr. 2016, 12:31:0123/02/2016
à Rick Byers,PhistucK,Joe Medley,Matt Falkenhagen,Michael Nordman,Philip Jägenstedt,Mike West,Elliott Sprehn,blink-dev,Joshua Bell,Dru Knox,Paul Kinlan
Thanks for the clarification. I'll remove that feature and add a specific one for AppCache deprecation and removal.

Joel Weinberger

non lue,
23 févr. 2016, 12:36:2723/02/2016
à Rick Byers,PhistucK,Joe Medley,Matt Falkenhagen,Michael Nordman,Philip Jägenstedt,Mike West,Elliott Sprehn,blink-dev,Joshua Bell,Dru Knox,Paul Kinlan
Here is the AppCache specific status: https://www.chromestatus.com/feature/5714236168732672

Unfortunately, I can't figure out how to delete an old feature, so if anyone can point me in the right direction, I'd love to get rid of the over-general status I created: https://www.chromestatus.com/feature/5635223400218624

Joe Medley

non lue,
23 févr. 2016, 12:36:5123/02/2016