Bug 210739 – [SOUP] Disable HSTS for requests when cookies will be blocked by ITP

• Component: WebKit2
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

"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."

[Carlos Garcia Campos]

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?

[Michael Catanzaro]

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.

[Carlos Garcia Campos]

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.

[Claudio Saavedra]

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.

[Michael Catanzaro]

(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.
"""

[Michael Catanzaro]

(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.")

[John Wilander]

(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.

[John Wilander]

(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.

[Carlos Garcia Campos]

(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

[Claudio Saavedra]

The Upgrade-Insecure-Request header, I suppose.

[John Wilander]

(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.

[Michael Catanzaro]

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.

[Carlos Garcia Campos]

Created attachment 401716 [details]
Patch

I think this is enough in the case of soup.

[Carlos Garcia Campos]

Created attachment 401717 [details]
Patch

[Carlos Garcia Campos]

Created attachment 401718 [details]
Patch

[Claudio Saavedra]

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.

[Michael Catanzaro]

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.

[Carlos Garcia Campos]

Committed r263010: <https://trac.webkit.org/changeset/263010>

[Radar WebKit Bug Importer]

<rdar://problem/64338649>