Closed Bug 1119778 Opened 5 years ago Closed 3 years ago

Forget about this site does not clear HSTS setting

Categories

(Firefox :: Bookmarks & History, defect)

34 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 51
Tracking Status
firefox47 --- wontfix
firefox48 --- wontfix
firefox49 --- fixed
firefox50 --- fixed
firefox51 --- verified

People

(Reporter: in-bugzilla, Assigned: keeler)

References

Details

(Keywords: privacy, regression, sec-want, Whiteboard: [adv-main49-])

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:34.0) Gecko/20100101 Firefox/34.0
Build ID: 20141201171754

Steps to reproduce:

- Close all tabs.
- Choose "Forget about this site" in history
- Restart Firefox (may be optional)


Actual results:

- The site still reports wrong certificate (HSTS error) or still can't be accessed using HTTP
- grep -r "site.tld" in profile directory finds SiteSecurityServiceState.txt


Expected results:

- The site can be accessed using HTTP only
- grep -r "site.tld" does not find anything
Component: Untriaged → Bookmarks & History
OS: Linux → All
Hardware: x86_64 → All
see also Bug 1092243 and Bug 902884
Pretty sure this used to work (see bug 1089473 comment 1) but then HSTS settings were moved out of the permission manager into their own settings file. It's not longer a happy accident we need to code this intentionally.
Status: UNCONFIRMED → NEW
Ever confirmed: true
See Also: → 1089473
Duplicate of this bug: 1232586
Should add "regression" keyword and blocking for bug 775370.
Blocks: 775370
Keywords: regression
Just ran into this issue, which is really, *really* frustrating.
Please resolve this issue.
This is indeed pretty annoying, could you folks take a look?
Flags: needinfo?(past)
Judging by the dependency that regressed this I think this is in platform's turf?
Flags: needinfo?(past) → needinfo?(dkeeler)
Assignee: nobody → dkeeler
Flags: needinfo?(dkeeler)
Attachment #8775644 - Flags: review?(MattN+bmo) → review+
Comment on attachment 8775644 [details]
bug 1119778 - make "Forget About This Site" clear HSTS and HPKP info

https://reviewboard.mozilla.org/r/67792/#review64844

r=me with the caveat that subdomains really should be cleared too.

::: toolkit/forgetaboutsite/ForgetAboutSite.jsm:221
(Diff revision 1)
> +      let sss = Cc["@mozilla.org/ssservice;1"].
> +                getService(Ci.nsISiteSecurityService);
> +      sss.removeState(Ci.nsISiteSecurityService.HEADER_HSTS, httpsURI, 0);
> +      sss.removeState(Ci.nsISiteSecurityService.HEADER_HPKP, httpsURI, 0);

This is better than nothing but isn't this supposed to also clear entries for subdomains too? See how other consumers use `hasRootDomain` for this.

I'm fine with doing this in a follow-up if you want. If you do that you can add the tests as TODO in advance.

::: toolkit/forgetaboutsite/ForgetAboutSite.jsm:225
(Diff revision 1)
> +    } catch (e) {
> +      dump("something went wrong while clearing HSTS/HPKP: " + e + "\n");
> +    }

Please use Cu.reportError instead. If you don't mind, could you fix the push code above to do so too. dump() is off by default in builds. reportError also gives the source line IIRC.
Comment on attachment 8775644 [details]
bug 1119778 - make "Forget About This Site" clear HSTS and HPKP info

https://reviewboard.mozilla.org/r/67792/#review64976

I'm OK with this (with MattN's comments addressed).
Attachment #8775644 - Flags: review?(mgoodwin) → review+
Comment on attachment 8775644 [details]
bug 1119778 - make "Forget About This Site" clear HSTS and HPKP info

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67792/diff/1-2/
Thanks for the reviews. I filed bug 1290529 to do the follow-up work to clear subdomains (since it requires some platform support to implement - see bug 1115712).
Comment on attachment 8775644 [details]
bug 1119778 - make "Forget About This Site" clear HSTS and HPKP info

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67792/diff/2-3/
Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c80456e5e3dd
make "Forget About This Site" clear HSTS and HPKP info r=MattN,mgoodwin
backed out for failures in own test like https://treeherder.mozilla.org/logviewer.html#?job_id=1182327&repo=autoland
Flags: needinfo?(dkeeler)
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a4d524f40881
Backed out changeset c80456e5e3dd for causing s4 failures in test_forget_about_site_security_headers.js
Comment on attachment 8775644 [details]
bug 1119778 - make "Forget About This Site" clear HSTS and HPKP info

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67792/diff/3-4/
Apparently the other forget about this site tests don't work on Android/B2G either ( https://dxr.mozilla.org/mozilla-central/rev/465d150bc8be5bbf9f02a8607d4552b6a5e1697c/toolkit/forgetaboutsite/test/unit/xpcshell.ini#4 ), so I just marked this as skip on those platforms.
Flags: needinfo?(dkeeler)
Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e8b7e5dc3004
make "Forget About This Site" clear HSTS and HPKP info r=MattN,mgoodwin
https://hg.mozilla.org/mozilla-central/rev/e8b7e5dc3004
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
I have reproduced this bug with Nightly 37.0a1 (2015-01-09) on Elementary OS 64bit. 

This bug's fix is now verified on Latest Nightly 51.0a1.

Build ID 	20160806030806
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:51.0) Gecko/20100101 Firefox/51.0

[bugday-20160803]
Hi :keeler,
Since this bug is a regression and also affects 49/50, are you also considering to uplift this patch to 49/50?
Flags: needinfo?(dkeeler)
Comment on attachment 8775644 [details]
bug 1119778 - make "Forget About This Site" clear HSTS and HPKP info

Approval Request Comment
[Feature/regressing bug #]: bug 775370
[User impact if declined]: "Forget About This Site" won't clear HSTS/HPKP state, which is the easiest way for a user to reset these headers if a site accidentally DOSes themselves by setting HSTS when they don't fully support https or by setting a bad key pin.
[Describe test coverage new/current, TreeHerder]: has a test
[Risks and why]: low - has a test, the new code is wrapped in a try/catch block, so it's hard to see how this could break things
[String/UUID change made/needed]: none
Flags: needinfo?(dkeeler)
Attachment #8775644 - Flags: approval-mozilla-beta?
Attachment #8775644 - Flags: approval-mozilla-aurora?
Status: RESOLVED → VERIFIED
Comment on attachment 8775644 [details]
bug 1119778 - make "Forget About This Site" clear HSTS and HPKP info

Fix was verified on Nightly, Aurora50+, Beta49+
Attachment #8775644 - Flags: approval-mozilla-beta?
Attachment #8775644 - Flags: approval-mozilla-beta+
Attachment #8775644 - Flags: approval-mozilla-aurora?
Attachment #8775644 - Flags: approval-mozilla-aurora+
Whiteboard: [adv-main49-]
see Bug 1334048
You need to log in before you can comment on or make changes to this bug.