[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #17965 [Tor Browser]: Isolate HPKP pinning to url bar domain
#17965: Isolate HPKP pinning to url bar domain
-------------------------------------------------+-------------------------
Reporter: mikeperry | Owner: tbb-
Type: defect | team
Priority: High | Status:
Component: Tor Browser | needs_revision
Severity: Normal | Milestone:
Keywords: tbb-linkability, | Version:
TorBrowserTeam201601R | Resolution:
Parent ID: | Actual Points:
Sponsor: | Points:
-------------------------------------------------+-------------------------
Changes (by mcs):
* status: needs_review => needs_revision
Comment:
Replying to [comment:5 arthuredelstein]:
> Here is a branch that isolates both HSTS and HPKP.
>
> https://github.com/arthuredelstein/tor-browser/commits/17965+1
>
> The same mechanism is used to store both HSTS and HPKP state, so I
isolate both HSTS and HPKP in the first patch. Note that I left out
isolation for SpeculativeConnect for now, because we have it disabled, and
otherwise the patch would be substantially larger and more complicated.
Kathy and I reviewed the patch. It is unfortunate that so many places need
to be touched in order to pass the isolationKey through, but we do not
have a better idea. We have a few minor comments:
1. Please change the uuid in netwerk/base/nsISiteSecurityService.idl.
2. Add @param aIsolationKey comments where appropriate in that same file.
3. Rename isolationKey to aIsolationKey in the declaration for
getKeyPinsForHostname() to be consistent with most parameter declarations
inside that same idl.
4. It looks like there is an unnecessary change (line break added) inside
nsSiteSecurityService::ProcessPKPHeader().
5. In the commit message, mention that this fix depends on the fix for
#13670.
> The second patch in this branch provides a regression test for HSTS
isolation. I still need to write a regression test for HPKP isolation.
This looks OK. Please remove this line from bug17965_tab.html:
console.log("hi");
Please add a comment to this line in test_tor_bug17965.html that mentions
that it is needed due to #18087:
yield setPref("security.nocertdb", false);
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/17965#comment:6>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs