Forget about this site does not clear HSTS setting

VERIFIED FIXED in Firefox 49

Status

()

VERIFIED FIXED
4 years ago
2 years ago

People

(Reporter: in-bugzilla, Assigned: keeler)

Tracking

({privacy, regression, sec-want})

34 Branch
Firefox 51
privacy, regression, sec-want
Points:
---

Firefox Tracking Flags

(firefox47 wontfix, firefox48 wontfix, firefox49 fixed, firefox50 fixed, firefox51 verified)

Details

(Whiteboard: [adv-main49-])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
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

Comment 1

4 years ago
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
Keywords: privacy, sec-want
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1232586

Comment 4

3 years ago
Should add "regression" keyword and blocking for bug 775370.
Blocks: 775370
Keywords: regression

Comment 5

2 years ago
Just ran into this issue, which is really, *really* frustrating.
Please resolve this issue.
status-firefox47: --- → wontfix
status-firefox48: --- → affected
status-firefox49: --- → affected
status-firefox50: --- → affected
This is indeed pretty annoying, could you folks take a look?
status-firefox48: affected → wontfix
Flags: needinfo?(past)
Judging by the dependency that regressed this I think this is in platform's turf?
Flags: needinfo?(past) → needinfo?(dkeeler)
(Assignee)

Updated

2 years ago
Assignee: nobody → dkeeler
Flags: needinfo?(dkeeler)
(Assignee)

Comment 8

2 years ago
Created attachment 8775644 [details]
bug 1119778 - make "Forget About This Site" clear HSTS and HPKP info

Review commit: https://reviewboard.mozilla.org/r/67792/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67792/
Attachment #8775644 - Flags: review?(mgoodwin)
Attachment #8775644 - Flags: review?(MattN+bmo)
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+
(Assignee)

Updated

2 years ago
See Also: → bug 1290529
(Assignee)

Comment 11

2 years ago
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/
(Assignee)

Comment 12

2 years ago
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).
(Assignee)

Comment 13

2 years ago
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/
(Assignee)

Comment 14

2 years ago
(Had to fix an ES lint issue, but otherwise this looked good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d1e4acc6d19c )

Comment 15

2 years ago
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)

Comment 17

2 years ago
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
(Assignee)

Comment 18

2 years ago
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/
(Assignee)

Comment 19

2 years ago
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)

Comment 20

2 years ago
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

Comment 21

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e8b7e5dc3004
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51

Comment 22

2 years ago
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)
(Assignee)

Comment 24

2 years ago
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?

Updated

2 years ago
Status: RESOLVED → VERIFIED
status-firefox51: fixed → 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+

Comment 26

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/c8fddbc2ee1d
status-firefox50: affected → fixed

Comment 27

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/0339b0c18498
status-firefox49: affected → fixed
Whiteboard: [adv-main49-]

Comment 28

2 years ago
see Bug 1334048
You need to log in before you can comment on or make changes to this bug.