1480780 - Use network.cookie.cookieBehavior prefs for the new cookie restriction project

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[Andrea Marchesini [:baku]]

This bug is about:

1. introducing nsICookieService::BEHAVIOR_REJECT_TRACKER cookie policy.
2. supporting the overwrite of that new cookie behavior via cookie permissions.

Comment 1

[Andrea Marchesini [:baku]]

Attachment: mergePref.patch

Comment 2

[Andrea Marchesini [:baku]]

Comment on attachment 8997428 [details] [diff] [review]
mergePref.patch

It breaks a couple of ServiceWorker tests, but I haven't investigated why yet. I suspect is an easy fix...

Comment 3

[Andrea Marchesini [:baku]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=11b64fdf28e9e8f2ea3409d0481dd5072bef0846 here the last try push.

Comment 4

[(no longer active)]

Comment on attachment 8997428 [details] [diff] [review]
mergePref.patch

Review of attachment 8997428 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/cookie/nsCookieService.cpp
@@ +2410,5 @@
>  nsCookieService::PrefChanged(nsIPrefBranch *aPrefBranch)
>  {
>    int32_t val;
>    if (NS_SUCCEEDED(aPrefBranch->GetIntPref(kPrefCookieBehavior, &val)))
> +    mCookieBehavior = (uint8_t) LIMIT(val, 0, nsICookieService::BEHAVIOR_REJECT_TRACKER, 0);

Instead of this, please define a new constant, BEHAVIOR_LAST and use it here.

::: toolkit/components/antitracking/AntiTrackingCommon.cpp
@@ +89,5 @@
> +
> +  nsCOMPtr<nsICookiePermission> cps =
> +    do_GetService(NS_COOKIEPERMISSION_CONTRACTID);
> +  if (NS_WARN_IF(!cps)) {
> +    return 1;

I think in error situations, it is better to return unknown...

@@ +379,5 @@
> +  }
> +
> +  if (behavior == nsICookieService::BEHAVIOR_REJECT_FOREIGN ||
> +      behavior == nsICookieService::BEHAVIOR_LIMIT_FOREIGN) {
> +    return false;

Nit: please add a TODO entry here for doing something more sophisticated for BEHAVIOR_LIMIT_FOREIGN...

Comment 5

[(no longer active)]

I addressed the review comments myself, and fixed the test failures on try.  I'm now going to land this patch...

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e70b1d7ba1eb
Merge the privacy.3rdpartystorage.enabled pref with the network.cookie.cookieBehavior pref; r=ehsan

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5d982e6568b
follow-up: Fix the build bustage on a CLOSED TREE

Comment 8

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/e70b1d7ba1eb
https://hg.mozilla.org/mozilla-central/rev/e5d982e6568b