Bug 210739 - [SOUP] Disable HSTS for requests when cookies will be blocked by ITP
Summary: [SOUP] Disable HSTS for requests when cookies will be blocked by ITP
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk, InRadar
Depends on:
Blocks:
 
Reported: 2020-04-20 03:10 PDT by Carlos Garcia Campos
Modified: 2020-06-14 02:08 PDT (History)
6 users (show)

See Also:


Attachments
Patch (1.39 KB, patch)
2020-06-12 02:41 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Patch (1.35 KB, patch)
2020-06-12 02:57 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Patch (1.31 KB, patch)
2020-06-12 03:00 PDT, Carlos Garcia Campos
mcatanzaro: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2020-04-20 03:10:00 PDT
"When the original request is HTTP, the request will have its cookies blocked, and has been upgraded by the HSTS mechanism, downgrade back to HTTP, apply all other rules in WebKit that might again upgrade it such as Upgrade Insecure Requests or potential/future auto-upgrade of mixed content, and send out."
Comment 1 Carlos Garcia Campos 2020-04-20 03:12:59 PDT
In the case of soup, I think a request is upgraded from http to https only by HSTS. Why do we wait for HSTS to upgrade the request to downgrade it again? Why not just ignore/disable HSTS when the request is created?
Comment 2 Michael Catanzaro 2020-04-22 08:25:19 PDT
That's fine. It doesn't make any difference, since that's just an internal implementation detail.

The point is simply that prevalent domains should not have HSTS.
Comment 3 Carlos Garcia Campos 2020-04-22 08:37:50 PDT
Then I'll just disable HSTS for requests when cookies are going to be blocked, since that's a lot easier than upgrade -> downgrade -> request again with HSTS ignored. I wonder why cocoa does it this way, though.
Comment 4 Claudio Saavedra 2020-04-22 08:39:54 PDT
I think there was something like that in one of the other ports' implementations of the upgrade mechanism, so we might want to improve that.
Comment 5 Michael Catanzaro 2020-04-22 08:54:25 PDT
(In reply to Carlos Garcia Campos from comment #3)
> Then I'll just disable HSTS for requests when cookies are going to be
> blocked, since that's a lot easier than upgrade -> downgrade -> request
> again with HSTS ignored. I wonder why cocoa does it this way, though.

Yes, that sounds correct. What you're trying to do is implement Mitigation 2 from this blog post: https://webkit.org/blog/8146/protecting-against-hsts-abuse/

"""
We modified WebKit so that when an insecure third-party subresource load from a domain for which we block cookies (such as an invisible tracking pixel) had been upgraded to an authenticated connection because of dynamic HSTS, we ignore the HSTS upgrade request and just use the original URL. This causes HSTS super cookies to become a bit string consisting only of zeroes.
"""
Comment 6 Michael Catanzaro 2020-04-22 08:59:05 PDT
(In reply to Michael Catanzaro from comment #2)
> The point is simply that prevalent domains should not have HSTS.

Well one thing has changed: nowadays ITP blocks *all* third-party cookies. I guess this means HSTS should be disabled for all third-party resources?

Or does Safari still allow HSTS upgrades on non-prevalent third-party domains? We might investigate what Safari does. But it probably shouldn't, because that would be subject to the same issues discussed in https://webkit.org/blog/9661/preventing-tracking-prevention-tracking/. (Note that ITP has since tightened "all third-party cookies blocked on websites without prior user interaction" to "all third-party cookies blocked without storage access API request.")
Comment 7 John Wilander 2020-04-22 18:52:28 PDT
(In reply to Carlos Garcia Campos from comment #1)
> In the case of soup, I think a request is upgraded from http to https only
> by HSTS. Why do we wait for HSTS to upgrade the request to downgrade it
> again? Why not just ignore/disable HSTS when the request is created?

Are you saying UIR support is Cocoa-specific? That would surprise me.
Comment 8 John Wilander 2020-04-22 19:00:58 PDT
(In reply to Michael Catanzaro from comment #6)
> (In reply to Michael Catanzaro from comment #2)
> > The point is simply that prevalent domains should not have HSTS.
> 
> Well one thing has changed: nowadays ITP blocks *all* third-party cookies. I
> guess this means HSTS should be disabled for all third-party resources?
> 
> Or does Safari still allow HSTS upgrades on non-prevalent third-party
> domains? We might investigate what Safari does. But it probably shouldn't,
> because that would be subject to the same issues discussed in
> https://webkit.org/blog/9661/preventing-tracking-prevention-tracking/. (Note
> that ITP has since tightened "all third-party cookies blocked on websites
> without prior user interaction" to "all third-party cookies blocked without
> storage access API request.")

Since all third-party cookies are blocked by default and not based on classification, HSTS is also blocked across the board.

Two things to keep in mind here:

1. Entries on the HSTS preload list should not be downgraded since they are not stateful and thus cannot be used for cross-site tracking purposes. Only downgrade dynamic HSTS.

2. Make sure you also cover HSTS on redirects. This turned out to be the trickiest part for us because it required a new callback from the HTTP layer saying “This redirect was actually to HTTP but I already upgraded it to HTTPS. Do you want to change that?” This is different from the initial request which WebKit sees *before* it goes to the HTTP layer where an upgrade may happen. I.e. you can be proactive on the initial request but need to be reactive on redirects.
Comment 9 Carlos Garcia Campos 2020-04-22 22:33:53 PDT
(In reply to John Wilander from comment #7)
> (In reply to Carlos Garcia Campos from comment #1)
> > In the case of soup, I think a request is upgraded from http to https only
> > by HSTS. Why do we wait for HSTS to upgrade the request to downgrade it
> > again? Why not just ignore/disable HSTS when the request is created?
> 
> Are you saying UIR support is Cocoa-specific? That would surprise me.

I don't even know what UIR is :-P
Comment 10 Claudio Saavedra 2020-04-23 01:06:19 PDT
The Upgrade-Insecure-Request header, I suppose.
Comment 11 John Wilander 2020-04-23 07:22:07 PDT
(In reply to Claudio Saavedra from comment #10)
> The Upgrade-Insecure-Request header, I suppose.

Yes, Upgrade Insecure Requests, as mentioned in the quote in the bug description.
Comment 12 Michael Catanzaro 2020-04-23 07:35:21 PDT
That is: the UIR header always wins, since, if used, it eliminates the potential for HSTS abuse. We don't want to wind up downgrading those requests to HTTP even if they would otherwise be downgraded by HSTS Mitigation 2.
Comment 13 Carlos Garcia Campos 2020-06-12 02:41:46 PDT
Created attachment 401716 [details]
Patch

I think this is enough in the case of soup.
Comment 14 Carlos Garcia Campos 2020-06-12 02:57:41 PDT
Created attachment 401717 [details]
Patch
Comment 15 Carlos Garcia Campos 2020-06-12 03:00:20 PDT
Created attachment 401718 [details]
Patch
Comment 16 Claudio Saavedra 2020-06-12 05:11:47 PDT
Comment on attachment 401718 [details]
Patch

Informal review here. We were using .allowCookies() because we didn't have ITP back at the time. It looks to me like this was the right thing to do from the beginning, so the patch looks godo to me.
Comment 17 Michael Catanzaro 2020-06-12 06:50:35 PDT
Comment on attachment 401718 [details]
Patch

I think this is correct.

The only trick here is that we want Upgrade Insecure Requests to take precedence, so the request should still be upgraded to HTTPS if that's used even if cookies are blocked. But that should be handled... somewhere else... so it should be fine.
Comment 18 Carlos Garcia Campos 2020-06-14 02:07:23 PDT
Committed r263010: <https://trac.webkit.org/changeset/263010>
Comment 19 Radar WebKit Bug Importer 2020-06-14 02:08:15 PDT
<rdar://problem/64338649>