Same-site lax cookies not sent by fetch event handler after page reload

• Component: Service Workers
• Status: RESOLVED FIXED

[Steffen Weber]

Requests sent by Safari lack SameSite=Lax cookies when a service worker with a fetch event handler is involved.

How to reproduce:

1. Fetch a SameSite=Lax demo cookie by visiting https://www.computerbase.de/api/samesite-demo-cookie (*)
2. Search Google for "computerbase"
3. Open the "Network" tab of the developer tools
4. Click on the search result that leads to our homepage (may not the first one depending on your location)
5. In the "Network" tab, find the "document" request and have a look at the cookies sent by Safari. For now, the "samesite-demo-cookie" is there.
6. Now reload our homepage. (Maybe this causes our Service Worker to fully initialize / activate?)
7. Repeat steps 2-4. Now the cookie doesn't get sent anymore.

(*) I've created this script because other cookies set by ComputerBase don't use SameSite=Lax in Safari anymore because of this bug.

The fetch event handler of our service worker doesn't handle all requests, for some requests it just returns "null" to let the browser handle the request (as if the service worker didn't exist). For example, the fetch event handler of our service worker returns "null" for all forum URLs (all URLs starting with "https://www.computerbase.de/forum/"). It turns out that forum URLs have no problem with same-site cookies: When I click on the search results entry for our forum (should be displayed when searching for "computerbase", if not then search for "computerbase forum") then the same-site demo cookie is always there as expected. If I then go back to the SERP and click on our homepage link again, the cookie is missing. Go back to the SERP and click on our forum link again, the cookie is there again. And so on...

So we have the following: 1) Same-site cookie is sent to our homepage on first try, bot not anymore after one or two reloads (probably after the service-worker has been initialized and our fetch event handler is used). 2) Same-site cookie is always sent to our forum pages who are ignored by the fetch event handler.

I think the combination of these two observations indicates that there is an issue with same-site cookies for requests sent by the fetch event handler.

Split from Bug #219650 as suggested by John Wilander.

[John Wilander]

Thanks, Steffen! Cc’ing Youenn who knows most about Fetch and ServiceWorkers. Youenn, I can assist on the SameSite cookie part.

[youenn fablet]

Hi Steffen, did you notice the lack of cookies from service worker by using web inspector or by noticing issues with the document being received or through server logs?
If it is web inspector only, I am wondering whether we do not have an issue retrieving  request/response cookie information when requests/responses go through the service worker.

[youenn fablet]

Sadly, debugging service worker in STP seems broken, I do not see service workers be listed in the debug menu even though they are serving content.

[youenn fablet]

Per Arne, I know there was a regression that we fixed some time ago where sandbox hardening broke service worker debugging. Maybe the issue came back.
Bisecting might be feasible here.

[Steffen Weber]

Initially, I only looked at the Safari Developer Tools to check the sent cookies but I've now verified that they are indeed not sent (by temporarily modifying our homepage to dump all cookies for requests originating at a specific BrowserStack IP address).

Btw, I'm able to reproduce the issue by simply repeating step 6: The same-site cookie is gone after the second reload. Step 7 is not necessary.

[Radar WebKit Bug Importer]

<rdar://problem/78878853>

[youenn fablet]

Created attachment 430733 [details]
Patch

[youenn fablet]

Steffen, I wrote the following test that is using same site cookie and everything seems to work as expected from this test.
I used 'secure; HttpOnly; SameSite=Lax' as seems to be the case in your website.

Let me know if you can spot a difference with your setup.

As can be seen, the cookies are not provided in web inspector when doing a fetch through service worker. This should probably be fixed in a different bug.

[youenn fablet]

Comment on attachment 430733 [details]
Patch

Might be nice to have a test landed so adding r?
Steffen, please let me know if there is any difference with your setup.

[Steffen Weber]

I'm on vacation. I'll have another look at the end of June. Sorry!

[Steffen Weber]

I'm not familiar with Web Platform Tests but does the test actually simulate the process of navigating away from the site and back to the site via an external link? This is the crucial step that triggers the issue.

It definitely isn't just a cosmetic Developer Tools issue because the problem was reported to us by actual Safari users (who were logged-out after clicking on Google result links).

[youenn fablet]

*** Bug 232440 has been marked as a duplicate of this bug. ***

Hey Youenn, thanks for linking to this issue. Is there any chances this gets fixed? I was concerced about the progress on my ticket: https://bugs.webkit.org/show_bug.cgi?id=232440 but seeing this one is open since May, I am getting a bit anxious.

This is a huge issue on effected sites as I outlined in the ticket above. Happy to help if you need any more details

[youenn fablet]

Created attachment 446168 [details]
Patch

[youenn fablet]

(In reply to erik.witt from comment #13)
> Hey Youenn, thanks for linking to this issue. Is there any chances this gets
> fixed? I was concerced about the progress on my ticket:
> https://bugs.webkit.org/show_bug.cgi?id=232440 but seeing this one is open
> since May, I am getting a bit anxious.
> 
> This is a huge issue on effected sites as I outlined in the ticket above.
> Happy to help if you need any more details

I think I have a fix. You repro steps helped a lot.

[youenn fablet]

Created attachment 446330 [details]
Patch

[youenn fablet]

Created attachment 446332 [details]
Patch

[Chris Dumez]

Comment on attachment 446332 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=446332&action=review

> Source/WebCore/loader/FrameLoader.h:320
> +    enum class IsServiceWorkerNavigationLoad { No, Yes };

: bool

[youenn fablet]

Created attachment 446364 [details]
Patch for landing

[EWS]

Committed r286656 (244969@main): <https://commits.webkit.org/244969@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 446364 [details].

[Brent Fulgham]

This change should be present in STP 139, iOS 15.4 Beta, and macOS 12.3 Beta.