Closed Bug 1935088 Opened 2 months ago Closed 1 month ago

Cross-origin cookies are not sent from iframe for fetch keepalive

Categories

(Core :: DOM: Networking, defect, P1)

defect

Tracking

()

RESOLVED FIXED
136 Branch
Tracking Status
firefox136 --- fixed

People

(Reporter: smayya, Assigned: smayya)

References

(Blocks 1 open bug)

Details

(Whiteboard: [necko-triaged][necko-priority-queue])

Attachments

(1 file)

As mentioned in this comment, we need to check why cookies are not sent with fetch keepalive requests.

Assignee: nobody → smayya
Severity: -- → S3
Priority: -- → P1
Severity: S3 → S2
Whiteboard: [necko-triaged][necko-priority-new]

the scenario where we see this (sorry, haven't had time to make a isolated repro):

iframe navigated to http://different.domain/entry
/entry responds with Set-Cookie: SESSIONID=;HttpOnly;Secure;SameSite=None and Location: /entry?cc=1
/entry?cc=1 sees the cookie so does Set-Cookie: SESSIONID=realid;allthesameattributes and Location: /page.html
/page.html immediately after load does fetch("/service", {keepalive:true})
on fetch error /page.html reloads (via redirect loop to / and back to /page) and tries again

on the first request i usually see /service being called without cookies at all. on the following requests i see it called with "SESSIONID=" cookie. My colleague says they see the cookie with out value already on the very first service request.

(In reply to Karlis from comment #1)

the scenario where we see this (sorry, haven't had time to make a isolated repro):

iframe navigated to http://different.domain/entry

sorry for misleading comment - we do use https://, not plain http.

Karlis, Thanks for reporting this.
We will try to reproduce this internally.
Meanwhile, it would be great if you could share http logs from about:logging for fetch request with keepalive (failure) and without keepalive (success).
Kindly log into a FILE and with the following logging configuration
timestamp,sync,nsHttp:5,cache2:5,nsSocketTransport:5,nsHostResolver:5,cookies:5

Flags: needinfo?(knagis)

Made a reproduction: https://github.com/Knagis/ffkeepalive

in FF133 you can see that server doesn't receive cookie header in keepalive request but does in normal request. In chrome both requests get the cookie.

Flags: needinfo?(knagis)

(In reply to Karlis from comment #4)

Made a reproduction: https://github.com/Knagis/ffkeepalive

in FF133 you can see that server doesn't receive cookie header in keepalive request but does in normal request. In chrome both requests get the cookie.

Thanks Karlis for the steps. However, i could see that the cookies are NOT sent for both the requests in my setup. This is true for safari as well. However, for chrome I could see that we send the cookie for both the request.
Once I disable Enhanced Tracking Protection for firefox, I could see that cookies ARE sent for both keepalive and normal requests.
I believe we have a bug in Firefox due to keepalive. However, I need more support with regard to issue reproduction. It would be helpful to share your firefox configuration to help reproduce this issue.
Could you please share the Important Modified Preferences output from the about:support page.

It would be great if you could share the http logs. Kindly use log into a file with timestamp,sync,nsHttp:5,cache2:5,cookies:5 setttings.
If you are hesitant to share this publicly, please consider sending the logs to necko@mozilla.com.
If the logs contain sensitive information, feel free to remove that data before sharing.

Thanks

Flags: needinfo?(knagis)

Important Modified Preferences (removed the hex encoded values):

Name 	Value accessibility.typeaheadfind.flashBar	0
browser.contentblocking.category	standard
browser.search.region	LV
browser.sessionstore.upgradeBackup.latestBuildID	20241121140525
browser.startup.homepage_override.buildID	20241121140525
browser.startup.homepage_override.mstone	133.0
browser.toolbars.bookmarks.visibility	always
browser.urlbar.placeholderName	Google
browser.urlbar.placeholderName.private	Google
browser.urlbar.quicksuggest.migrationVersion	2
browser.urlbar.quicksuggest.scenario	history
browser.urlbar.tipShownCount.searchTip_onboard	4
doh-rollout.doneFirstRun	true
doh-rollout.home-region	LV
extensions.formautofill.creditCards.reauth.optout	...
extensions.lastAppVersion	133.0
idle.lastDailyNotification	1733338418
media.gmp-gmpopenh264.abi	x86_64-msvc-x64
media.gmp-gmpopenh264.hashValue	...
media.gmp-gmpopenh264.lastDownload	1733131384
media.gmp-gmpopenh264.lastInstallStart	1733131384
media.gmp-gmpopenh264.lastUpdate	1733131384
media.gmp-gmpopenh264.version	2.3.2
media.gmp-manager.buildID	20241121140525
media.gmp-manager.lastCheck	1733332851
media.gmp-manager.lastEmptyCheck	1733332851
media.gmp-widevinecdm.abi	x86_64-msvc-x64
media.gmp-widevinecdm.hashValue	...
media.gmp-widevinecdm.lastDownload	1733131386
media.gmp-widevinecdm.lastInstallStart	1733131384
media.gmp-widevinecdm.lastUpdate	1733131386
media.gmp-widevinecdm.version	4.10.2830.0
media.gmp.storage.version.observed	1
media.hardware-video-decoding.failed	false
places.database.lastMaintenance	1733132634
privacy.bounceTrackingProtection.hasMigratedUserActivationData	true
privacy.purge_trackers.date_in_cookie_database	0
privacy.purge_trackers.last_purge	1733338418296
privacy.sanitize.clearOnShutdown.hasMigratedToNewPrefs2	true
privacy.sanitize.pending	[{"id":"newtab-container","itemsToClear":[],"options":{}}]
security.webauthn.show_ms_settings_link	true
services.sync.declinedEngines	
signon.management.page.os-auth.optout	...
signon.suggestImportCount	1
storage.vacuum.last.content-prefs.sqlite	1733338418
storage.vacuum.last.index	1
storage.vacuum.last.places.sqlite	1733132634
ui.osk.debug.keyboardDisplayReason	IKPOS: Touch screen not found.

sent the logs (from the reproduction page) to the mentioned email address.

Flags: needinfo?(knagis)

Karlis, now I am able to reproduce this finally by toggling ETP.

Disable Enhanced Tracking Protection and refresh the page - Cookies are sent for both the requests
Enable Enchanced Tracking Proection and refresh the page - Cookie is only sent for Keepalive requests and NOT for normal requests.
Please note in my case cookie is sent only for keepalive.

The logs you shared didnt not have cookie logs. I think this is because of a typo from my side. Please use cookie preset for logging (this should result in the following modules timestamp,sync,nsHttp:5,cache2:5,cookie:5 (previously it was cookies:5 which was wrong). Sincere apologies! Kindly clear all the cache and cookie data before reproducing the issue.

Just to ensure we dont have two different issue could you please confirm if you observe any difference when toggling the enhanced tracking protection.

I believe for cookies storage in certain cases we differentiate between keepalive and non-keepalive requests. We will investigate further in this direction.

Flags: needinfo?(knagis)

Sent new logs to the email address.

As for Enhanced Tracking Protection - i see the issue with standard config. If i enable strict config, cookies are skipped for both requests as expected. If i select custom config and unselect cookies, then cookies are sent for both requests.

This problem was initially reported by multiple customers, it was reproduced by our customer support and QAs. The fix we rolled out was to not send keepalive flag on Firefox. We are rather confident this is not a browser configuration issue and happens on default configuration for most users.

Flags: needinfo?(knagis)

colleague just reproduced the issue on the reproduction page by using completely fresh install of Firefox 133.0 on MacOS 14.5 - in default config keepalive request doesn't include the cookie

We have blocked third-party cookie access by default in the Nightly channel. You can try to change it via the pref network.cookie.cookieBehavior.optInPartitioning and test if this allows the cookies to be sent again.

Tim, I can confirm that this works for me by changing network.cookie.cookieBehavior.optInPartitioning to false.

Flags: needinfo?(tihuang)

Karlis, coul you please confirm if the pref flip works for you?
visit about:config and change network.cookie.cookieBehavior.optInPartitioning to false

Flags: needinfo?(knagis)
Severity: S2 → S3

should be noted that it is important to restart browser after changing the optInPartitioning setting, without restarting the behavior does change but in rather random manner.

with optInPartitioning=true the cookies are completely blocked in the reproduction page so the issue isn't visible. Are you saying that this issue will not be fixed because in the next release partitioning will become the default?

Flags: needinfo?(knagis)

sorry to disappoint, but i added Partitioned attribute to the cookie in the reproduction page (update pushed to github) and the issue persists even with optInPartitioning=true - cookie is sent for normal request but not for keepalive. So the severity probably shouldn't be lowered.

Thanks for quickly checking.
(In reply to Karlis from comment #15)
I think you have network.cookie.cookieBehavior.optInPartitioning disabled by default in your firefox and still see the issue of cookie sent in only one of the fetch requests.

sorry to disappoint, but i added Partitioned attribute to the cookie in the reproduction page (update pushed to github) and the issue persists even with optInPartitioning=true - cookie is sent for normal request but not for keepalive. So the severity probably shouldn't be lowered.

Severity is lowered because we have a workaround for this.

with optInPartitioning=true the cookies are completely blocked in the reproduction page so the issue isn't visible. Are you saying that this issue will not be fixed because in the next release partitioning will become the default?
No we will be working on fixing this.
The issue from our initial observation suggests we are differentiating cookie storage for keepalive and non-keepalive requests. This is the bug here.
We will discuss this internally on further identifying the root-cause.

Tim, looking at the logs looks like we are not able to fetch cookies for keepalive requests.
I will give some background of normal fetch request and keepalive requests.
fetch requests use HttpChannelChild for creation of channels whereas fetch keepalive uses Pfetch for creating the channel.
For PFetch the channel is only created in the parent process unlike HttpChannelChild.
Hence, there could be a possibility that we are missing some settings for keepalive, resulting in different mapping rules compared to normal fetch request.
Can you point me to the code / list attributes that help map a request to its cookie storage.

Severity is lowered because we have a workaround for this.

What is the workaround? If it is setting network.cookie.cookieBehavior.optInPartitioning=true then that does not work. It works only if the cookies are not partitioned - because it is effectively disabling the cookies. However, it does not work when the cookie is created with Partitioned attribute - you can see this in the last version in https://github.com/Knagis/ffkeepalive

Hence, there could be a possibility that we are missing some settings for keepalive, resulting in different mapping rules compared to normal fetch request.

i did notice that when you do Ctrl+Shift+R, normal request is sent with "cache-control": "no-cache" while the keepalive request includes "if-none-match", the same as it does on soft Ctrl+R reload.

I am able to reproduce with a fresh install of v133, ETP standard. Let's go through this case by case because this can get confusing. Note that when changing non/partitioned cookie state to clear the browser's cookie. Also, refreshing hard or soft should not alter the results at all.

  1. optInPartitioning off, CHIPS off, non-partitioned cookie:
  • fetch keep alive no cookie
  • fetch normal has cookie (not consistent, maybe bug)
  1. optInPartitioning off, CHIPS on, non-partitioned cookie:
  • fetch keep alive has cookie (WEIRD, but ok?)
  • fetch normal has cookie
  1. optInPartitioning off, CHIPS off, partitioned cookie:
  • fetch keep alive no cookie
  • fetch normal has cookie (again, not consistent)
  1. optInPartitioning off, CHIPS on, partitioned cookie:
  • fetch keep alive has cookie
  • fetch normal has cookie

Ok, lets try with optInPartitioning ON now. Note that by changing the state via about:config the ETP settings change to custom. Let's ignore for now. Remember to restart the browser.

  1. optInPartitioning ON, CHIPS off, non-partitioned cookie:
  • fetch keep alive no cookie (blocks as expected)
  • fetch normal no cookie (at least we are consistent)
  1. optInPartitioning ON, CHIPS on, non-partitioned cookie:
  • fetch keep alive no cookie (no effect, as expected)
  • fetch normal no cookie
  1. optInPartitioning ON, CHIPS off, partitioned cookie:
  • fetch keep alive no cookie
  • fetch normal has cookie (I would have expected a block here)
  1. optInPartitioning ON, CHIPS on, partitioned cookie:
  • fetch keep alive has cookie (CHIPS works as expected)
  • fetch normal has cookie (as expected)

Ok. So the workaround appears to be to set network.cookie.CHIPS.enabled=true, which will allow cookies with Partitioned attribute through, despite any value of optInPartitioning. Weirdly CHIPS will also help an Unpartitioned cookie if OptInPartitioning is disabled, but maybe this is expected (?). But if optInPartitioning is enabled then an Unpartitioned cookie has no hope of getting through.

It's not totally clear to me what the behaviour should be when OptInPartitioning is disabled, maybe it depends on what ETP is doing, but in all cases I would expect normal fetches to do the same thing as keep-alive fetches, though I am not fetch expert.

When OptInPartitioning is enabled I would have expected fetch normal blocking in the optInPartitioning ON, CHIPS off, partitioned cookie scenario.

I suspect we are dealing with an issue with how/when fetch accesses cookies as well as one in the cookie service.

According to the comment 18, I believe the fetch keep alive(PFetch) doesn't use the same partitionKey as the normal fetch requests(HttpChannel). We can verify this by checking the cookieJarSettings' partitionKey for the channels. You should be able to access it via the loadInfo.

My speculation is that we don't properly config the channel for PFetch case. The channel needs to call AntiTrackingUtils::UpdateAntiTrackingInfoForChannel() in the parent process to populate the data we need for third-party checks.

Flags: needinfo?(tihuang)

(In reply to Tim Huang[:timhuang] from comment #19)

According to the comment 18, I believe the fetch keep alive(PFetch) doesn't use the same partitionKey as the normal fetch requests(HttpChannel). We can verify this by checking the cookieJarSettings' partitionKey for the channels. You should be able to access it via the loadInfo.

My speculation is that we don't properly config the channel for PFetch case. The channel needs to call AntiTrackingUtils::UpdateAntiTrackingInfoForChannel() in the parent process to populate the data we need for third-party checks.

Thank you Tim. I have verified that we have same partitioned keys for both, however we have different return values for IsThirdPartyChannel.
For keepalive, we are getting this value as not a thirdparty channel. Whereas for normal fetch requests, we are getting this as third-party channel. This could lead to different storage policy and hence the difference. I think we have some missing settings in the loadinfo for keepalive as mIsThirdPartyContext is false.

Whiteboard: [necko-triaged][necko-priority-new] → [necko-triaged][necko-priority-queue]
Attachment #9442815 - Attachment description: WIP: Bug 1935088 - set thirdparty contexts for fetch keepalive requests → Bug 1935088 - set thirdparty contexts for fetch keepalive requests. r=edenchuang,timhuang
Pushed by smayya@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5efbaa9e5c01 set thirdparty contexts for fetch keepalive requests. r=timhuang
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 135 Branch
Flags: needinfo?(smayya)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: 135 Branch → ---
Pushed by smayya@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/82616a0973fa set thirdparty contexts for fetch keepalive requests. r=timhuang
Flags: needinfo?(smayya)
Status: REOPENED → RESOLVED
Closed: 2 months ago1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 136 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: