Intent to Temporarily Remove: sendBeacon() with a Blob whose type is not CORS-safelisted

[Takeshi Yoshino]

Primary eng (and PM) emails

tyos...@chromium.org

Summary

Temporarily make Navigator.sendBeacon() throw when called with a Blob whose type is not any of the CORS-safelisted values for the Content-Type request header as the data argument.

Motivation

I got this removal plan reviewed by API owners in advance to this mail as it's a bit irregular. I'm now sending a formal I-to-D as suggested there.

Navigator.sendBeacon() (in the Beacon API spec) implementation in Chrome has a security issue when called with a Blob whose type is not any of the non-CORS-safelisted for the Content-Type request header.

We've been trying to fix the issue earlier, however, due to the amount of refactoring required, we've been failing to do so. Besides that, we want to make our beacon-ing infrastructure clean so that it'll be ready for sendBeacon V2 that would be realized in the form of the keepalive attribute of the Fetch API.

Because of the reasons, I'd like to temporarily remove it now by prioritizing the security problem first, and then get the underlying architecture problem fixed in non-ad-hoc way before re-enabling the feature again.

See the following for detailed background.

[Standard resource loading code path in Blink]

In Blink, blink::ResourceFetcher (or blink::DocumentThreadableLoader) must be used for resource loading (including XHR, Fetch API, etc.) so that the fetch algorithm is performed as specified in the Fetch Standard for various reason including security perspectives.

blink::ResourceFetcher uses content::WebURLLoader to talk to net/ to perform lower level HTTP networking.

[sendBeacon() is using non-standard code path for resource loading]

sendBeacon() is, however, using this content::WebURLLoader directly and fulfilling the requirements in the fetch algorithm in an ad-hoc way (invoking the sub-procedures in blink::ResourceFetcher in PingLoader).

Because of this, it doesn’t have any code path to run the CORS preflight steps which is implemented in blink::DocumentThreadableLoader for the standard path.

[sendBeacon() needs CORS preflight]

CORS preflight is a request with its method set to OPTIONS sent in advance to the actual request that has non-CORS-safelisted methods/headers to check that the peer server understands CORS, and therefore the client can safely issue the requests.

Unlike XHR and Fetch API, sendBeacon() uses only the POST method and doesn’t accept any custom header. But when it's called with a payload in the form of a Blob, the type of the Blob is used for generating the Content-Type request header. It can be not a CORS-safelisted value for the Content-Type request header. Such a request must be sent after CORS preflight steps are performed.

[Renderer may be gone when to issue the actual request]

sendBeacon() must continue fetching even after the origin page is gone (e.g. navigating, closing the tab). But the CORS preflight logic lives in the renderer process. So, even if we simply enable the CORS preflight logic for sendBeacon(), it doesn’t work correctly since the renderer may be already killed when the CORS preflight completes.

Possible non-ad-hoc solutions for this are discussed in yhirano@’s Fetch API keepalive design doc. But it would take some time to finish any of the solutions.

Interoperability and Compatibility Risk

This removal does cause interoperability issues, but we'll remove it by prioritizing fixing the security issue sooner.

We've been warning developers for 8 months (since this CL which is included from M54) by making sendBeacon throw on Dev releases. The description of the exception thrown include the address of the crbug entry. For now, we haven't received any push back from developers on the crbug.

(The CL has also already put this feature under Finch control)

Alternative implementation suggestion for web developers

Embed the Content-Type information into the payload by using some encoding.

Usage information from UseCounter

Current usage of sendBeacon() with non-CORS-safelisted Content-Type is 0.08% (recent peak).

OWP launch tracking bug

http://crbug.com/720283

Entry on the feature dashboard

https://www.chromestatus.com/feature/5654267444592640

[PhistucK]

On Wed, May 10, 2017 at 1:26 PM, 'Takeshi Yoshino' via blink-network-dev <blink-ne...@chromium.org> wrote:

We've been warning developers for 8 months (since this CL which is included from M54) by making sendBeacon throw on Dev releases

​Is there a deprecation warning on non-Dev releases?​

☆PhistucK

[Mike West]

Non-OWNERS' LGTM.

The user-facing impact of restricting Beacon in the way discussed here should be negligable, even given it's 0.08% usage, as Beacon is a fire-and-forget mechanism that a developer would be hard-pressed to surface to users (as there's no success/failure notification). Some analytics packages might be affected in the short term. IMO, it's better to cause that (mostly invisible) breakage than to bypass CORS for these kinds of requests.

-mike

[Takeshi Yoshino]

On Wed, May 10, 2017 at 8:26 PM, PhistucK <phis...@gmail.com> wrote:

On Wed, May 10, 2017 at 1:26 PM, 'Takeshi Yoshino' via blink-network-dev <blink-network-dev@chromium.org> wrote:

We've been warning developers for 8 months (since this CL which is included from M54) by making sendBeacon throw on Dev releases

​Is there a deprecation warning on non-Dev releases?​

No. I should have had, right.

 

[Joe Medley]

Are you hoping this will go out in Chrome 60?

Please make sure I'm kept up to date so I can make sure developers know about this.

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

If an API's not documented it doesn't exist.

--
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+unsubscribe@chromium.org.

[Takeshi Yoshino]

Oh, sorry for not CC-ing you though you told me so on the other thread.

Yes. I'm going to make this happen for M60.

[Jochen Eisinger]

lgtm1

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

[Simon Pieters]

On Wed, 10 May 2017 12:26:31 +0200, 'Takeshi Yoshino' via blink-dev
<blin...@chromium.org> wrote:

> *Primary eng (and PM) emails*
>
> tyos...@chromium.org
>
> *Summary*

>
> Temporarily make Navigator.sendBeacon() throw when called with a Blob
> whose
> type is not any of the CORS-safelisted values for the Content-Type
> request
> header as the data argument.
>

> *Motivation*

>
> I got this removal plan reviewed by API owners in advance to this mail as
> it's a bit irregular. I'm now sending a formal I-to-D as suggested there.
>
> Navigator.sendBeacon() (in the Beacon API spec)

> <https://w3c.github.io/beacon/> implementation in Chrome has a security
> issue <https://bugs.chromium.org/p/chromium/issues/detail?id=490015> when

> called with a Blob whose type is not any of the non-CORS-safelisted for
> the
> Content-Type request header.
>
> We've been trying to fix the issue earlier, however, due to the amount of
> refactoring required, we've been failing to do so. Besides that, we want
> to
> make our beacon-ing infrastructure clean so that it'll be ready for
> sendBeacon V2 that would be realized in the form of the keepalive
> attribute

> of the Fetch API <https://fetch.spec.whatwg.org/#dom-request-keepalive>.

>
> Because of the reasons, I'd like to temporarily remove it now by
> prioritizing the security problem first, and then get the underlying
> architecture problem fixed in non-ad-hoc way before re-enabling the
> feature
> again.
>
> See the following for detailed background.
>
> [Standard resource loading code path in Blink]
>
> In Blink, blink::ResourceFetcher (or blink::DocumentThreadableLoader)
> must
> be used for resource loading (including XHR, Fetch API, etc.) so that the

> fetch algorithm <https://fetch.spec.whatwg.org/#concept-fetch> is

> performed
> as specified in the Fetch Standard for various reason including security
> perspectives.
>
> blink::ResourceFetcher uses content::WebURLLoader to talk to net/ to
> perform lower level HTTP networking.
>
> [sendBeacon() is using non-standard code path for resource loading]
>
> sendBeacon() is, however, using this content::WebURLLoader directly and
> fulfilling the requirements in the fetch algorithm in an ad-hoc way
> (invoking the sub-procedures in blink::ResourceFetcher in PingLoader).
>
> Because of this, it doesn’t have any code path to run the CORS preflight

> <https://fetch.spec.whatwg.org/#cors-preflight-request> steps which is

> implemented in blink::DocumentThreadableLoader for the standard path.
>
> [sendBeacon() needs CORS preflight]
>
> CORS preflight is a request with its method set to OPTIONS sent in
> advance
> to the actual request that has non-CORS-safelisted

> <https://fetch.spec.whatwg.org/#cors-safelisted-request-header> methods
> <https://fetch.spec.whatwg.org/#cors-safelisted-method>/headers
> <https://fetch.spec.whatwg.org/#cors-safelisted-request-header> to check

> that the peer server understands CORS, and therefore the client can
> safely
> issue the requests.
>
> Unlike XHR and Fetch API, sendBeacon() uses only the POST method and
> doesn’t accept any custom header. But when it's called with a payload in
> the form of a Blob, the type of the Blob is used for generating the
> Content-Type request header. It can be not a CORS-safelisted value for
> the
> Content-Type request header. Such a request must be sent after CORS
> preflight steps are performed.
>
> [Renderer may be gone when to issue the actual request]
>
> sendBeacon() must continue fetching even after the origin page is gone
> (e.g. navigating, closing the tab). But the CORS preflight logic lives in
> the renderer process. So, even if we simply enable the CORS preflight
> logic
> for sendBeacon(), it doesn’t work correctly since the renderer may be
> already killed when the CORS preflight completes.
>
> Possible non-ad-hoc solutions for this are discussed in yhirano@’s Fetch
> API keepalive design doc

> <https://docs.google.com/document/d/1iHJtFa3jOo5n9QXHb6Ok5nK8kavXSk2DrLoubPWi9ys/edit>.

> But it would take some time to finish any of the solutions.
>
> Interoperability and Compatibility Risk
>
> This removal does cause interoperability issues, but we'll remove it by
> prioritizing fixing the security issue sooner.
>
> We've been warning developers for 8 months (since this CL

> <https://codereview.chromium.org/2184973002> which is included from M54)

> by
> making sendBeacon throw on Dev releases. The description of the exception
> thrown include the address of the crbug entry. For now, we haven't
> received
> any push back from developers on the crbug.
>
> (The CL has also already put this feature under Finch control)
>
> Alternative implementation suggestion for web developers
>
> Embed the Content-Type information into the payload by using some
> encoding.

Could you expand on this, maybe give a concrete example, so that it is
more straight-forward for web developers to fix?

Non-owner LGTM.

> Usage information from UseCounter
>
> Current usage of sendBeacon() with non-CORS-safelisted Content-Type is
> 0.08%

> <https://www.chromestatus.com/metrics/feature/timeline/popularity/1485>
> (recent
> peak).
>
> *OWP launch tracking bug*
>
> http://crbug.com/720283
>
> *Entry on the feature dashboard*
>
> https://www.chromestatus.com/feature/5654267444592640
>


--
Simon Pieters
Opera Software

[Chris Harrelson]

Hi,

I'm not sure I understand why the compatibility risk is low enough for this intent. As stated, it's at 0.08%, and it is a "fire and forget" mechansim,

but the server logging of sites that use beacons would still break.

Chris

--
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+unsubscribe@chromium.org.

[Mike West]

On Fri, May 12, 2017 at 1:01 AM, Chris Harrelson <chri...@chromium.org> wrote:

Hi,

I'm not sure I understand why the compatibility risk is low enough for this intent. As stated, it's at 0.08%, and it is a "fire and forget" mechansim,

but the server logging of sites that use beacons would still break.

That's correct. I'd suggest that we can live with a higher bar because the breakage will not be user-visible, and because the status quo is a security issue, as detailed in the associated bug. We've lived with the bug for a while on the assumption that it would be fixed; since that fix doesn't seem forthcoming in the near future, I think we need to bite the bullet and remove the feature until we can ship it safely.

I'd also suggest that our current behavior is putting pressure on other browsers to _remove_ their security protections in order to become compatible with our behavior. "But it works in Chrome!" is, for better or worse, a powerful argument. I think it's our responsibility to remove this pressure.

This change will impact developers, but I believe that that's a good reason to increase pressure on ourselves to invest in a real fix by requiring preflights. My understanding is that that's tyoshino@ et al's next step.

-mike

[Jochen Eisinger]

I agree that breaking this functionality would improve both security and increase predictability as we'd align with other browsers.

[Takeshi Yoshino]

One very simple way is to have the Content-Type header value at the head of the body,

const body = new Blob([originalBody.type, "\r\n", originalBody], {type: "text/plain"});

navigator.sendBeacon(url, body);

and at the server side let the application layer process it.

This workaround needs to move Content-Type processing up to the application layer and therefore it shouldn't be feasible for everyone.

 

[Chris Harrelson]

LGTM2. I think the security and predictability improvements, and that it wouldn't directly break user experience, are enough to justify this change.

Sounds like Firefox already has the behavior described in this Intent? Could you clarify?

--

[TAMURA, Kent]

LGTM3.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAOMQ%2Bw_TXzbpO_3g%3DwPH78hp9gC3UGFEALHJrV2UM3Djt5WO1w%40mail.gmail.com.

--

TAMURA Kent
Software Engineer, Google

[Mike West]

On Mon, May 15, 2017 at 8:55 PM, Chris Harrelson <chri...@chromium.org> wrote:

LGTM2. I think the security and predictability improvements, and that it wouldn't directly break user experience, are enough to justify this change.

Sounds like Firefox already has the behavior described in this Intent? Could you clarify?

Firefox sends CORS preflights for Beacon requests with a non-simple `Content-Type`. The proposal here is for Blink to fail these requests until such time as we can send preflights as well.

-mike

[Anne van Kesteren]

On Tue, May 16, 2017 at 3:53 PM, Mike West <mk...@chromium.org> wrote:
> Firefox sends CORS preflights for Beacon requests with a non-simple
> `Content-Type`. The proposal here is for Blink to fail these requests until
> such time as we can send preflights as well.

To be pedantic, Firefox preflights for Blob objects. The plan is to
make that a Content-Type-based check in
https://bugzilla.mozilla.org/show_bug.cgi?id=1289387 once Chromium has
shipped some improvements, such as the one being discussed here.


--
https://annevankesteren.nl/

[PhistucK]

Now that I think about it, throwing (as opposed to a silent error with a console warning/error but still not throwing) might be inappropriate here. It can break the execution of the script which may, for example, write to localStorage or do anything else following the sendBeacon call (it does not even have to happen at unload/beforeunload).

☆PhistucK

--
You received this message because you are subscribed to the Google Groups "blink-network-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-network-dev+unsubscribe@chromium.org.
To post to this group, send email to blink-ne...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-network-dev/CADnb78h0w%2BEKqGnCph6Mer%3DGAFK4Ld%3D7ck6mfNs9tG0-m49yNw%40mail.gmail.com.

[Takeshi Yoshino]

I understand silently disabling might be fine and better for some users, but I don't think we can say that's true for most.

I'd like to make it louder by throwing than just logging for higher probability that developers notice the change and take some action based on their priority.

[benjami...@glassechidna.com.au]

I've been asked to cross-post (or perhaps more appropriately post) here regarding my feedback given at https://bugs.chromium.org/p/chromium/issues/detail?id=490015

It's obviously too late now, however I'll provide this feedback in that it my be valuable in the future.

I generally understand the motivation behind prominent errors, in this case a thrown exception, as an attempt to better get the attention of developers. However, if an existing documented error/failure reporting mechanism exists I'd prefer that it were used.

In this particular case sendBeacon() was not documented as throwing exceptions, however it is clearly documented that sendBeacon() may return false if the Blob was able to be queued for later submission. Given that was documented behaviour, I was handling that case appropriately and falling back to a (much less desirable) synchronous XHR. However, the exception stopped my client's website dead in its tracks, meaning certain analytics (in particular dwell time) simply weren't being reported. We were yet to notice (and may not have for some time) a chance in analytics, I was just fortunate enough to stumble across the thrown exception as I had my dev tools open whilst closing the page whilst developing completely unrelated functionality (I don't normally leave dev tools open when closing a page, it was purely coincidental).

The other bit of feedback is that half-complete solutions aren't particularly nice in general. I do realise that security fixes are typically urgent in nature and can warrant a somewhere more heavy-handed approach - I'm unsure if that was truly necessary this time around. I don't know if this was being exploited in practice (honestly without further guidance I'm unsure how this could possibly be exploited on its lonesome), but ideally a complete fix could have been shipped rather than a partial fix making its way into what is now a stable published build (Chrome 59).

For developers these sorts of changes can be a tad frustrating when they're unexpected, more-so when we are consuming the API in accordance with the spec - in my case I'm just trying to POST an octet stream to a CORS compatible end-point.

Anyway, I do respect that end-user security absolutely should come before developer convenience, it's just that these aren't necessarily mutually exclusively if handled with caution.

 

On Monday, May 22, 2017 at 3:33:44 PM UTC+10, Takeshi Yoshino wrote:

I understand silently disabling might be fine and better for some users, but I don't think we can say that's true for most.

I'd like to make it louder by throwing than just logging for higher probability that developers notice the change and take some action based on their priority.

On Thu, May 18, 2017 at 3:25 PM, PhistucK <phis...@gmail.com> wrote:

Now that I think about it, throwing (as opposed to a silent error with a console warning/error but still not throwing) might be inappropriate here. It can break the execution of the script which may, for example, write to localStorage or do anything else following the sendBeacon call (it does not even have to happen at unload/beforeunload).

☆PhistucK

On Tue, May 16, 2017 at 5:03 PM, Anne van Kesteren <ann...@annevk.nl> wrote:

On Tue, May 16, 2017 at 3:53 PM, Mike West <mk...@chromium.org> wrote:
> Firefox sends CORS preflights for Beacon requests with a non-simple
> `Content-Type`. The proposal here is for Blink to fail these requests until
> such time as we can send preflights as well.

To be pedantic, Firefox preflights for Blob objects. The plan is to
make that a Content-Type-based check in
https://bugzilla.mozilla.org/show_bug.cgi?id=1289387 once Chromium has
shipped some improvements, such as the one being discussed here.


--
https://annevankesteren.nl/

--
You received this message because you are subscribed to the Google Groups "blink-network-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to blink-network-...@chromium.org.

To post to this group, send email to blink-ne...@chromium.org.

[Rick Byers]

Thank you so much for sharing your experience Benjamin, and I'm sorry for the trouble this caused you!

I agree that it probably would have been better to return false than throw in this case (although it's hard to be sure since we don't know how many sites would have been surprised/harmed by a silent failure if they weren't checking the return value).  The fact that console warnings don't do as much to raise developer awareness as throwing an exception (which, for example, can generate telemetry via an onerror handler) is a bug that we (Paul cc'd) are working to fix ASAP.

I've updated our web compat principles document to include this case as an example to consider in the future. We're constantly working to strike the best possible tradeoff between web compat and other concerns like increasing security and can only do that effectively by learning from the cases that could have gone better.  Thank you!

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/034b36ec-b8ca-4f33-a0e9-24d2248a4723%40chromium.org.

[adrian....@gmail.com]

I'm on Chrome 61.0.3163.91. The fix breaks same-origin requests using a Blob with a custom "type". For example, we use:

var serializedData = JSON.stringify(data);
var rawData = new Blob([serializedData], {
  type: 'application/json'
});
// This is a same-origin request!
navigator.sendBeacon('/track', rawData);

---

This code is now throwing the exception:

"Failed to execute 'sendBeacon' on 'Navigator': sendBeacon() with a Blob whose type is not any of the CORS-safelisted values for the Content-Type request header is disabled temporarily. See  http://crbug.com/490015  for details."

In Firefox 55.0.3, the request is sent correctly (and of course without an exception). There is *no* preflight CORS request sent since it's not cross-origin. However, if we change to a different origin (e.g. "127.0.0.1" instead of "localhost"), the preflight request is sent as expected.

This new behavior in Chrome is undesirable because it assumes that every request is CORS. As a workaround, we catch exceptions caused by "sendBeacon" and retry with "application/x-www-form-urlencoded" (e.g. using "jQuery.param()" instead of "JSON.stringify()"). If there was no exception (like other people are saying), our fallback to fetch/AJAX would have worked.

[PhistucK]

It was shipped in Chrome 59 (so three stable versions have had it already), despite this known issue, so I would not expect that to change (instead, I hope the feature will simply be implemented as expected, across origins and over the same origin).

☆PhistucK

[Yoav Weiss]

I would say this mostly counts as a bug fix, so don't think a separate intent is strictly required. Maybe just a PSA, to make sure folks are aware of it?

On Tue, Jan 14, 2020 at 6:50 AM <yhi...@chromium.org> wrote:

Reviving this old thread.

As Out-of-Renderer CORS (OOR-CORS) is rolling out, I think we are ready to undo this temporarily remove on M81. Do I need a separate intent thread?

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

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-network-dev/072aa916-9b7d-4070-af2e-68fa3c6855ef%40chromium.org.

[yhi...@chromium.org]

Reviving this old thread.

As Out-of-Renderer CORS (OOR-CORS) is rolling out, I think we are ready to undo this temporarily remove on M81. Do I need a separate intent thread?

On Wednesday, May 10, 2017 at 7:26:59 PM UTC+9, Takeshi Yoshino wrote: