1119778 - Forget about this site does not clear HSTS setting

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Robert]

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

Comment 1

[Ralf Hauser]

see also Bug 1092243 and Bug 902884

Comment 2

[Daniel Veditz [:dveditz]]

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.

Comment 4

[fireattack [:fireattack]]

Should add "regression" keyword and blocking for bug 775370.

Comment 5

[BenWiederhake.GitHub]

Just ran into this issue, which is really, *really* frustrating.
Please resolve this issue.

Comment 6

[James Willcox (:snorp) ([e-mail]) (he/him)]

This is indeed pretty annoying, could you folks take a look?

Comment 7

[Panos Astithas (he/him) [:past] (please ni?)]

Judging by the dependency that regressed this I think this is in platform's turf?

Comment 8

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Attachment: 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/

Comment 9

[Matthew N. [:MattN]]

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 10

[Mark Goodwin [:mgoodwin]]

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).

Comment 11

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

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/

Comment 12

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

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 13

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

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/

Comment 14

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

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

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c80456e5e3dd
make "Forget About This Site" clear HSTS and HPKP info r=MattN,mgoodwin

Comment 16

[Carsten Book [:Tomcat]]

backed out for failures in own test like https://treeherder.mozilla.org/logviewer.html#?job_id=1182327&repo=autoland

Comment 17

[Pulsebot]

Backout by [e-mail]:
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 18

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

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/

Comment 19

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

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.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e8b7e5dc3004
make "Forget About This Site" clear HSTS and HPKP info r=MattN,mgoodwin

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e8b7e5dc3004

Comment 22

[Sayed [:LiquidO2]]

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]

Comment 23

[Gerry Chang [:gchang]]

Hi :keeler,
Since this bug is a regression and also affects 49/50, are you also considering to uplift this patch to 49/50?

Comment 24

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

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

Comment 25

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8775644 [details]
bug 1119778 - make "Forget About This Site" clear HSTS and HPKP info

Fix was verified on Nightly, Aurora50+, Beta49+

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c8fddbc2ee1d

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0339b0c18498

Comment 28

[Ralf Hauser]

see Bug 1334048