1763073 - Telemetry for SameSite cookies with non-samesite redirect chain

Status: RESOLVED FIXED

(Core :: Networking: Cookies, task, P2)

Description

[Tom Schuster (MoCo)]

We already correctly consider the redirect chain for SameSite cookies when determining if a cookie is same-site foreign: https://searchfox.org/mozilla-central/rev/fbb1e8462ad82b0e76b5c13dd0d6280cfb69e68d/netwerk/cookie/CookieCommons.cpp#588
Chrome however doesn't do this yet. Thus if we ship Firefox with this restriction in place we might run into numerous web compatibility issues. Chrome's platform stats shows that roughly 1% of page loads use SameSite cookies with a bad redirect chain. The purposes of this bug is to add similar telemetry to Firefox.

Comment 1

[Tom Schuster (MoCo)]

Attachment: Bug 1763073 - Add telemetry (and messaging) for SameSite cookies blocked due to redirects. r?freddyb

Comment 2

[Tom Schuster (MoCo)]

Attachment: Request for data collection review form

Thanks to everyone who helped me complete this form.

Comment 3

[Tom Schuster (MoCo)]

Attachment: Request for data collection review form

Thanks again for everyone who helped. Re-submitted this because copy-pasting from Google Docs caused issues. We have some open question about the categorization.

Comment 4

[Chris H-C :chutten]

Comment on attachment 9276669 [details]
Request for data collection review form

DATA COLLECTION REVIEW RESPONSE:

Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes.

Is there a control mechanism that allows the user to turn the data collection on and off?

Yes. This collection is Telemetry so can be controlled through Firefox's Preferences.

If the request is for permanent data collection, is there someone who will monitor the data over time?

No. This collection will expire in Firefox 106.

Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 3, Stored Content and Communications.

Is the data collection request for default-on or default-off?

Default on for all channels.

Does the instrumentation include the addition of any new identifiers?

No.

Is the data collection covered by the existing Firefox privacy notice?

Almost certainly yes. To Legal to confirm.

Does the data collection use a third-party collection tool?

No.

---

Result: datareview-, pending Legal review for default-on Cat3 data collection.

(( The next step is that Legal will review this data collection to see if it's aligned with the Privacy Notice and enumerate the product risk and kick off any necessary conversations. For this case I expect this to be resolved quickly without any of that and with just a note here on the bug (given the narrow scope of the Cat3 part of the collection). When that happens, I'll come back 'round and flip this over to datareview+))

Comment 5

[AL]

Tom or Chris: can we briefly meet so I can better understand the issue?

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:tschuster, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.

Comment 7

[Tom Schuster (MoCo)]

This will land when the data-review is given.

Comment 8

[AL]

After meeting the team and assessing the data collected, we're good to move forward. No issue or concern from the Privacy/Legal side.

Comment 9

[Chris H-C :chutten]

Comment on attachment 9276669 [details]
Request for data collection review form

Flipping to data-review+ on Legal review. Good to go!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2d02cef08622
Add telemetry (and messaging) for SameSite cookies blocked due to redirects. r=freddyb,dveditz

Comment 11

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/2d02cef08622

Comment 12

[Tom Schuster (MoCo)]

Comment on attachment 9271060 [details]
Bug 1763073 - Add telemetry (and messaging) for SameSite cookies blocked due to redirects. r?freddyb

Beta/Release Uplift Approval Request

• User impact if declined: We need this telemetry to get the required information for landing samesite=lax by default, which we had to delay shipping multiple times.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): This patch "only" adds console logging and telemetry and shouldn't change any normal behavior.
• String changes made/needed: New string added
• Is Android affected?: Yes

Comment 13

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9271060 [details]
Bug 1763073 - Add telemetry (and messaging) for SameSite cookies blocked due to redirects. r?freddyb

We are out of the beta cycle and releng is currently merging mozilla-beta to mozilla-release before we build our RC. Morphing this into a release build uplift request.

Comment 14

[Pascal Chevrel:pascalc (PTO until April 26)]

Flod, this uplift request adds a string, are we OK uplifting this late l10n change?

Comment 15

[Pascal Chevrel:pascalc (PTO until April 26)]

Tom, you indicated that the risk is medium, what kind of regression could it cause?

Have you verified that the added telemetry works as expected on Nightly?

Thanks

Comment 16

[Tom Schuster (MoCo)]

I can also provide a patch without the string changes and logging, we care more about the telemetry.

Tom, you indicated that the risk is medium, what kind of regression could it cause?

I don't have any specific risk in mind, but the patch isn't exactly trivial. I don't expect any real issues after 5 days of Nightly.

Have you verified that the added telemetry works as expected on Nightly?

Yes it's working and already accessible on telemetry.mozilla.org.

Comment 17

[Francesco Lodolo [:flod]]

(In reply to Pascal Chevrel:pascalc from comment #14)

Flod, this uplift request adds a string, are we OK uplifting this late l10n change?

I'd really prefer not to have an extra string added this late in the schedule.

Comment 18

[Pascal Chevrel:pascalc (PTO until April 26)]

Let's target a dot release for this uplift, the risk/benefit ratio does not seem favorable to include it directly in our release build.

Comment 19

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9271060 [details]
Bug 1763073 - Add telemetry (and messaging) for SameSite cookies blocked due to redirects. r?freddyb

After talking with dveditz, he thinks the patch is low risk and there is a strong need for their team to have telemetry on release as soon as possible, approved for RC on 102 and 102esr

Comment 20

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-release/rev/f1604ed27bc0

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Looks like this didn't get uplifted to ESR102 in time for the 102.0esr RC build. Seems like something we can land for the 102.1esr release next cycle, however.

Comment 22

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-esr102/rev/4180ec744bb7

Uplifted with RC2 and ESR respins