Closed Bug 1571234 Opened 4 months ago Closed 3 months ago

Forget About This Site does not delete cookies by base domain

Categories

(Toolkit :: Data Sanitization, defect, P1, major)

68 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla71
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 - verified
firefox68 --- wontfix
firefox69 + wontfix
firefox70 blocking verified
firefox71 + verified

People

(Reporter: gpimple, Assigned: johannh)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

Open History, search for a website, right-click on it, select Forget About This Site.

Actual results:

Pages from this site are removed from history, cookies stay in place.

Expected results:

As https://support.mozilla.org/en-US/kb/delete-browsing-search-download-history-firefox#w_remove-a-single-website-from-your-history states, "All history items (browsing and download history, cookies, cache, active logins, passwords, saved form data, exceptions for cookies, images, pop-ups) for that site will be removed."

Hi Gpimple,

Thanks for taking the time of opening this bug and being very clear about the steps. However, this issue has already been pointed out on bug 1546295. Please do take a look at it. They are on their way to solving this issue.

Best regards, Flor.

Status: UNCONFIRMED → RESOLVED
Closed: 4 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1546295

That doesn't feel like a dupe of bug 1546295 to me.

Can you reproduce this on a clean profile?

Blocks: 1550317
Status: RESOLVED → REOPENED
Component: Untriaged → Data Sanitization
Ever confirmed: true
Flags: needinfo?(gpimple)
Priority: -- → P3
Product: Firefox → Toolkit
Resolution: DUPLICATE → ---

(In reply to Johann Hofmann [:johannh] from comment #2)

That doesn't feel like a dupe of bug 1546295 to me.

Can you reproduce this on a clean profile?

Yes, it does happen on a clean profile too.
I created a new profile, navigated to cnn.com, closed the tab. Chose "Forget About This Site" in History, then opened Preferencies > Privacy: cookies for cnn.com are still present.

Flags: needinfo?(gpimple)

[Tracking Requested - why for this release]:
Data Sanitization bug

So, for me, Forget About Site does delete some cookies, but apparently only the edition.cnn.com-specific subdomain cookies, none of the cookies for the cnn.com domain, which is not how it should be.

This seems to have regressed with 1515913 by using removeCookiesFromRootDomain without ensuring that we actually got passed a root domain in ClearDataService.

We should probably track this for 70 to get it fixed quickly. Not sure about 69.

Status: REOPENED → NEW
Priority: P3 → P1
Summary: "Forget This Site" option in History's contextual menus doesn't delete cookies anymore → Forget About This Site does not delete cookies by base domain
Keywords: regression
Regressed by: 1515913

Too late for the 69.0 release, but I'll keep this on the radar for a dot release ride-along if a low-risk patch is available.

Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Attachment #9089422 - Attachment is obsolete: true

This will change RemoveCookiesFromRootDomain to not remove cookies from all sub-domains but instead
only remove the single host that is passed. This is necessary to support removing all sub-domain
cookie data in ForgetAboutSite without compromising on the cookie permissions that are supported by
Sanitizer.jsm

As a side effect, this also fixes the issues described in https://bugzilla.mozilla.org/show_bug.cgi?id=1515913#c24
(which were caused by deleting sub-domains without checking their permission).

Sanitizer.jsm should retain its functionality even with this change, because it already collects
all the principals we have to delete individually and tries to delete them (so it would consider
mozilla.org and support.mozilla.org separately).

This is made necessary because ClearDataService will not delete sub-domains anymore. It
didn't correctly delete base domains before either, so we just switch to collecting the
correct hosts entirely outside of ClearDataService.

Depends on D45012

Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3a3d06a4087d
Change RemoveCookiesFromRootDomain to RemoveCookiesFromExactHost. r=baku
https://hg.mozilla.org/integration/autoland/rev/4a9668e42ac0
Manually collect hosts to delete cookies from in ForgetAboutSite. r=baku
Status: ASSIGNED → RESOLVED
Closed: 4 months ago3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71

Is this something you wanted to nominate for Beta approval (and maybe more)?

Flags: needinfo?(jhofmann)

I think Beta is a good idea (definitely not more). Let's give this a tiny bit more bake time (and it would be great to get QA on Nightly) before uplift.

Comment on attachment 9091037 [details]
Bug 1571234 - Change RemoveCookiesFromRootDomain to RemoveCookiesFromExactHost. r=baku

Beta/Release Uplift Approval Request

  • User impact if declined: "Forget About This Site" will not delete enough cookies, resulting in a hypothetical breaking of privacy guarantees
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: - Go to https://edition.cnn.com/
  • Close the tab
  • Open History
  • Right click on the history entry for edition.cnn.com and choose "Forget About this Site"
  • Go to about:preferences -> Privacy -> Cookies and Site Data -> Manage Data
  • Search for "cnn.com"
  • Confirm that there are no entries
  • List of other uplifts needed: None
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): This is touching data sanitization, which is a bit tricky to get right with many rules and exceptions and unfortunately not as many tests as we should have.
  • String changes made/needed: None
Flags: needinfo?(jhofmann)
Attachment #9091037 - Flags: approval-mozilla-beta?
Attachment #9091038 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

I have managed to reproduce this issue using Firefox 70.0a1 (BuildId:20190803221448) on Windows 10 64bit.

This issue is verified fixed using Firefox 71.0a1 (BuildId:20190916155843) on Windows 10 64bit, macOS 10.13.6 and Ubuntu 18.04 64bit.

Leaving a ni on myself as a reminder to verify this once it lands in beta.

Status: RESOLVED → VERIFIED
Flags: needinfo?(emil.ghitta)
Flags: in-testsuite+

Comment on attachment 9091037 [details]
Bug 1571234 - Change RemoveCookiesFromRootDomain to RemoveCookiesFromExactHost. r=baku

Fixes a privacy issue which can cause some cookies to not be deleted when they're supposed to be. Thanks for adding a new test. Approved for 70.0b8.

Attachment #9091037 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9091038 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

This issue is verified fixed using Firefox 70.0b8 (provided in comment 16) on Windows 10 64bit, macOS 10.13.6 and Ubuntu 18.04.

Pending a possible esr uplift.

Ciprian, I think that this is a good candidate for our "Anti-Tracking" test suite.

Flags: in-qa-testsuite?(ciprian.georgiu)

(In reply to Emil Ghitta, QA [:emilghitta] from comment #17)

Ciprian, I think that this is a good candidate for our "Anti-Tracking" test suite.

Although this seems to be covered by automated tests, I also added a manual regression test case here, in order to cover this scenario.

Flags: needinfo?(emil.ghitta)
Flags: in-qa-testsuite?(ciprian.georgiu)
Flags: in-qa-testsuite+

Sounds like this is a wontfix for ESR68 based on comment 12. Feel free to nominate for uplift if that's incorrect.

Hmm. This seems pretty unproblematic so far, I was mostly hesitant to go to release quickly but we could consider ESR. Adding NI to consider uplifting.

Flags: needinfo?(jhofmann)

Comment on attachment 9091037 [details]
Bug 1571234 - Change RemoveCookiesFromRootDomain to RemoveCookiesFromExactHost. r=baku

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Local privacy issues for some users with advanced settings
  • User impact if declined: "Forget About This Site" will not delete enough cookies, resulting in a hypothetical breaking of privacy guarantees. This also resolves bug 1515913.
  • Fix Landed on Version: 70
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is touching data sanitization, which is a bit tricky to get right and fixing one thing is prone to break something else.

However, by now we have a lot of tests for this behavior (we've been laboring at it for a while) and we added even more tests in bug 1515913 that confirm this is working as expected.

  • String or UUID changes made by this patch: None
Flags: needinfo?(jhofmann)
Attachment #9091037 - Flags: approval-mozilla-esr68?
Attachment #9091038 - Flags: approval-mozilla-esr68?

Comment on attachment 9091037 [details]
Bug 1571234 - Change RemoveCookiesFromRootDomain to RemoveCookiesFromExactHost. r=baku

Fixes a privacy regression. Approved for 68.2esr.

Attachment #9091037 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+
Attachment #9091038 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+

This issue is verified fixed using Firefox 68.2.0esr (provided in comment 23) on Windows 10 64bit, macOS 10.13.6 and Ubuntu 18.04 64bit.

Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.