In Bug 1264562, we isolated the OCSP cache by first-party domain (when "privacy.firstparty.isolate" is true). Tor Browser also sends each HTTP request over a Tor circuit corresponding to the by first-party domain, which means each OCSP request object needs to provide a way to retrieve that first-party domain. I imagine the right way is to create give the OCSP request with a new OriginAttribute containing the correct first-party domain.

Ideally we want to land this bug into Firefox 52, otherwise Tor has to uplift it.

Ethan, you made this a P1, can you please also assign it to someone and [domsecurity-active] in case it's really a P1? Thanks!

(In reply to Christoph Kerschbaumer [:ckerschb (limited availability)] from comment #2) > Ethan, you made this a P1, can you please also assign it to someone and > [domsecurity-active] in case it's really a P1? Thanks! Chris, thanks for the reminder. Jonathan is the best person to take this bug. He's on PTO and will be back in two days. I expect he can work on this bug pretty soon. We'll set it as [domsecurity-active] then.

(In reply to Arthur Edelstein [:arthuredelstein] from comment #0) > In Bug 1264562, we isolated the OCSP cache by first-party domain (when > "privacy.firstparty.isolate" is true). Tor Browser also sends each HTTP > request over a Tor circuit corresponding to the by first-party domain, which > means each OCSP request object needs to provide a way to retrieve that > first-party domain. I imagine the right way is to create give the OCSP > request with a new OriginAttribute containing the correct first-party domain. The loading principal of the OCSP request is system principal. http://searchfox.org/mozilla-central/rev/2142de26c16c05f23e543be4fa1a651c4d29604e/security/manager/ssl/nsNSSCallbacks.cpp#98 I don't think it's a good idea to put origin attributes in the loading principal in this case. Arthur, do you think we can put the first party domain in the triggering principal? Christoph, can we use triggering principal to carry first party information for Tor (only when pref is on)?

(In reply to Jonathan Hao [:jhao] from comment #4) > The loading principal of the OCSP request is system principal. > http://searchfox.org/mozilla-central/rev/ > 2142de26c16c05f23e543be4fa1a651c4d29604e/security/manager/ssl/nsNSSCallbacks. > cpp#98 > > I don't think it's a good idea to put origin attributes in the loading > principal in this case. What do you think would be wrong with this approach? Could we create a new loading principal, derived from the system principal, that contains the origin attributes we need? > Arthur, do you think we can put the first party > domain in the triggering principal? I think for predicability, HTTP requests should offer a consistent way to retrieve the first party domain for every HTTP request. In the tests for isolating content (Bug 1264577) and favicon (Bug 1277803) requests, the method used for retrieving it is > channel.loadInfo.originAttributes.firstPartyDomain I wonder if it is somehow possible to ensure it works the same for OCSP requests?

Hmm, I'll try to do that first.

Comment on attachment 8806648 [details] Bug 1312794 - Annotate OCSP requests by first party domain. (adapted from Tor Browser patch #13670) https://reviewboard.mozilla.org/r/90024/#review89858 This looks good (just a few comments), but it needs some tests. ::: security/manager/ssl/nsNSSCallbacks.cpp:118 (Diff revision 1) > > + if (!mRequestSession->mFirstPartyDomain.IsEmpty()) { > + NeckoOriginAttributes attrs; > + attrs.mFirstPartyDomain = > + NS_ConvertUTF8toUTF16(mRequestSession->mFirstPartyDomain); > + nit: this "blank" line has unnecessary spaces in it ::: security/manager/ssl/nsNSSCallbacks.cpp:119 (Diff revision 1) > + if (!mRequestSession->mFirstPartyDomain.IsEmpty()) { > + NeckoOriginAttributes attrs; > + attrs.mFirstPartyDomain = > + NS_ConvertUTF8toUTF16(mRequestSession->mFirstPartyDomain); > + > + nsCOMPtr<nsILoadInfo> loadInfo = chan->GetLoadInfo(); GetLoadInfo doesn't necessarily seem to be infallible - we should probably null-check loadInfo before using it (unless I'm wrong about the guarantees here). ::: security/manager/ssl/nsNSSCallbacks.cpp:120 (Diff revision 1) > + NeckoOriginAttributes attrs; > + attrs.mFirstPartyDomain = > + NS_ConvertUTF8toUTF16(mRequestSession->mFirstPartyDomain); > + > + nsCOMPtr<nsILoadInfo> loadInfo = chan->GetLoadInfo(); > + loadInfo->SetOriginAttributes(attrs); nit: check for failure here

Comment on attachment 8806648 [details] Bug 1312794 - Annotate OCSP requests by first party domain. (adapted from Tor Browser patch #13670) https://reviewboard.mozilla.org/r/90024/#review91400 Great - just some nits. ::: security/manager/ssl/tests/unit/test_ocsp_caching.js:228 (Diff revision 2) > > // This test makes sure that OCSP cache are isolated by firstPartyDomain. > > + let gObservedCnt = 0; > + let protocolProxyService = Cc["@mozilla.org/network/protocol-proxy-service;1"] > + .getService(Ci.nsIProtocolProxyService); nit: indent this so the first '.' is under the '[' from the previous line ::: security/manager/ssl/tests/unit/test_ocsp_caching.js:236 (Diff revision 2) > + // origin attributes are aFirstPartyDomain. > + function startObservingChannels(aFirstPartyDomain) { > + // We use a dummy proxy filter to catch all channels, even those that do not > + // generate an "http-on-modify-request" notification. > + let proxyFilter = { > + applyFilter : function (aProxyService, aChannel, aProxy) { nit: no space before ':'

Pushed by jhao@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8b988d56154b Annotate OCSP requests by first party domain. (adapted from Tor Browser patch #13670) r=keeler

Thank you, David. And Arthur, if this patch doesn't do what you want, please let me know. Thanks.

Hi Jonathan -- sorry I didn't find the time to review this one. But it does look good to me as well. Thanks so much for writing this patch!