Closed Bug 1419589 Opened 5 years ago Closed 5 years ago

Ensure all storage is removed when resetDefault() is called within the ContextualIdentity service

Categories

(Firefox :: Security, enhancement, P1)

Product:
Firefox
Component:
Security
Version:
52 Branch
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 59
Tracking Flags:
Tracking Status
firefox59 --- fixed

People

(Reporter: tanvi, Assigned: baku)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

context3.patch
6.20 KB, patch
jkt
: review+
Details | Diff | Splinter Review 
To avoid privacy issues with regressions like bug 1347515, we should remove all site storage (cookies, localstorage, serviceworkers, etc), along with icons/colors/names when resetDefault() is called:

https://searchfox.org/mozilla-central/rev/797c93d81fe446f78babf20894f0729f15f71ee6/toolkit/components/contextualidentity/ContextualIdentityService.jsm#152
See Also: → 1419591
Attached patch context3.patch — — Splinter Review 

        Attachment #8930829 -
        Flags: review?(jkt)

    
    

    
  
Comment on attachment 8930829 [details] [diff] [review]
context3.patch

This looks fine however the code is blocked on Bug 1419591 landing first right?

        Attachment #8930829 -
        Flags: review?(jkt) → review+

    
    

    
  
Assuming we move minUserContextId to 1 we should also remove that from the function signature.

    
    

    
  
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/213e5dc04b90
ContextualIdentityService should reset all the container data when resetting the identities, r=jkt

    
    

    
  
Backed out for failing Chrome tests on Linux Debug toolkit/content/tests/chrome/test_bug437844.xul

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/e8813c46b4f3446389c4a333fa1697aa56f0c23d

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=213e5dc04b9090f3a5fb9fc74c7d4f36781e44e8

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=148337838&repo=mozilla-inbound&lineNumber=24398

INFO - GECKO(4887) | ++DOMWINDOW == 73 (0xd71b0000) [pid = 4887] [serial = 104] [outer = 0xcde53940]
24398
[task 2017-11-29T01:30:02.580Z] 01:30:02     INFO - TEST-UNEXPECTED-FAIL | toolkit/content/tests/chrome/test_bug437844.xul | assertion count 37 is more than expected 17 to 36 assertions
24399
[task 2017-11-29T01:30:02.582Z] 01:30:02     INFO - TEST-START | toolkit/content/tests/chrome/test_bug451540.xul
Flags: needinfo?(amarchesini)

    
    

    
  
This test seems completely unrelated are we sure this was the cause of it?
Flags: needinfo?(ncsoregi)

    
    

    
  
The c3 failures were quite frequent, and there was a push to try with the changes from here and Bug 1419591 backed out https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae6b050df7dcf29da2f00465387a9998c3bccf94

It seems that the tests from the above push passed, so it pointed to this push https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=213e5dc04b9090f3a5fb9fc74c7d4f36781e44e8 as being the culprit for the failures. 

If needed, I will look again into it.
Flags: needinfo?(ncsoregi)

    
  
Flags: needinfo?(amarchesini)

    
    

    
  
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1eafaf6a7642
ContextualIdentityService should reset all the container data when resetting the identities, r=jkt

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/1eafaf6a7642
Status: NEW → RESOLVED
Closed: 5 years ago

          status-firefox59:
          --- → fixed
Resolution: --- → FIXED

          You need to log in
          before you can comment on or make changes to this bug.