[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #14059 [Tor Browser]: Revision of existing double key cookie logic to meet requirements
#14059: Revision of existing double key cookie logic to meet requirements
-----------------------------+----------------------------
Reporter: michael | Owner: michael
Type: defect | Status: needs_revision
Priority: normal | Milestone:
Component: Tor Browser | Version:
Resolution: | Keywords:
Actual Points: | Parent ID: #3246
Points: |
-----------------------------+----------------------------
Comment (by michael):
Replying to [comment:18 gk]:
> 1) Please document why you use one time
`mThirdPartyUtil->GetFirstPartyURIFromChannel` and the other time
`mThirdPartyUtil->GetFirstPartyIsolationURI` and what that implies.
>
GetFirstPartyIsolationURI is a special case of GetFirstPartyURIFromChannel
which operates on the condition of party isolation (see requirement above
'Conditional operation') while the latter (GetFirstPartyURIFromChannel)
passes the first party URI unconditionally.
Since we unconditionally store both keys when writing but make decisions
(send cookie or not?) on the condition of privacy.thirdparty.isolate and
the private browsing channel attribute, we use GetFirstPartyURIFromChannel
when writing (SetCookieString) and GetFirstPartyIsolationURI when reading
(GetCookieStringCommon.)
To clarify this difference I wrote two new comment lines in the source
code near `nsCookieService::GetCookieStringCommon`. I think that's what
you meant by 'document.'
[[br]]
> 2) You can't reuse `requireHostMatch` in `SetCookieStringInternal` as
this would mean that the URL bar domain could influence unrelated cookies
checks which it must not do.
>
Good catch, I'm still considering a solution.
[[br]]
> 3)
> {{{
> // origin matches matches
> }}}
>
Thanks, I fixed that.
[[br]]
> 4) There are several places where you just use `baseDomain` in
nsCookie::Create() which is especially consifusing in `GetCookieFromRow()`
as the first comment is talks about to skip reading the baseDomain what we
do that nevertheless. Could you add a comment on this baseDomain usage
please?
>
There are two places that I've named the '''baseDomain''' variable near a
call to `nsCookie::Create`, namely patch line '''310''' and line '''344'''
but I haven't touched the persistent cookie code including
`GetCookieFromRow()`. Should there be a comment near both lines '''310'''
and '''344''' (in the patch msvb14058-283f7c6.patch) resembling
'unconditionally write the second (baseDomain) key as well as the first
(host) key when storing cookies.' Is that what you mean?
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/14059#comment:20>
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