Closed Bug 1452568 Opened 2 years ago Closed 2 years ago

DNS-Over-HTTPS calls in PBM dumped in cache2 folder

Categories

(Core :: Networking: DNS, defect, P2)

61 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: modi.konark, Assigned: bagder)

Details

(Whiteboard: [necko-triaged][trr])

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:59.0) Gecko/20100101 Firefox/59.0
Build ID: 20180329143049

Steps to reproduce:

Firefox Nightly Version: 61.0a1 build ID: 20180408220126
Flip prefs to use TRR:
1. network.trr.bootstrapAddress : 1.1.1.1
2. network.trr.mode: 3
3. network.trr.uri: https://mozilla.cloudflare-dns.com/dns-query
4. network.trr.useGET: true
5. Monitor Cache2 folder

Location can be found:
1. about:cache?storage=disk&context=
2. Storage disk location.




Actual results:

Cache2 folder contains files which look like dumps from DOH calls. Even calls done in Private mode.
(File attached for websites visited only in private mode.)
A. 
   1. Cliqz.com : https://0bin.net/paste/YO0sF30E7DYmxrnr#0IiW92hLmzOFFCU672PQhuLYUfORj3trIDUYgMgTYXp
   2. static.cliqz.com: https://0bin.net/paste/41qu6fhFthQ5uLFK#K7JAU34GhFRM7K0devP0G1+a6ac4EH4I4mZ5dDjQv2f
   etc etc.

B. xvideos.com: https://0bin.net/paste/sctBdrXABDQVxGM7#hJUVixQCkHKm+qom99Gs451UYrlGY3NBGsfqz4UV0Kf
    static-hw.xvideos.com: https://0bin.net/paste/QQKdNiVPXxzIEe0D#3PLk4PLUGKinYnNHA9i1KqzIVDgZtM+kYRZXKcZxD2I



Expected results:

Domains visited in PBM should not be saved on Disk, it seems like when the TRR mode is configured with GET the leaks seems to happen. 

I could not reproduce the same with network.trr.useGET: false.

Not marking it secure as it cannot be exploited remotely and is only an issue in Nightly.
Assignee: nobody → daniel
Component: Untriaged → Networking: DNS
Priority: -- → P2
Product: Firefox → Core
Whiteboard: [necko-triaged][trr]
Comment on attachment 8966144 [details]
bug 1452568 - inhibit cache use when doing TRR in PB

https://reviewboard.mozilla.org/r/234876/#review240542

::: netwerk/dns/TRR.cpp:191
(Diff revision 1)
>                       nsIContentPolicy::TYPE_OTHER,
>                       nullptr,   // PerformanceStorage
>                       nullptr, // aLoadGroup
>                       this,
> -                     nsIRequest::LOAD_ANONYMOUS, ios);
> +                     nsIRequest::LOAD_ANONYMOUS |
> +                     (mPB ? nsIRequest::INHIBIT_CACHING: 0), ios);

I think the better way would have been to pass a loadInfo to the channel that has originAttributes.mPrivateBrowsingId > 0

If we can't do that, this is OK too.
Attachment #8966144 - Flags: review?(valentin.gosu) → review+
Comment on attachment 8966144 [details]
bug 1452568 - inhibit cache use when doing TRR in PB

https://reviewboard.mozilla.org/r/234876/#review240542

> I think the better way would have been to pass a loadInfo to the channel that has originAttributes.mPrivateBrowsingId > 0
> 
> If we can't do that, this is OK too.

I suppose everything is possible since this is just code, but that seems to require more code and fiddling so my question would be what benefits we would get from doing it with a loadInfo instead of just setting this bit?
The main(/only?) benefit would be that the responses would still be cached in memory, with a private browsing flag.
Given that we have a separate cache for DNS entries, probably not that relevant.
> Given that we have a separate cache for DNS entries, probably not that relevant

Agreed. For in-memory, the existing DNS cache will be used anyway.
Pushed by daniel@haxx.se:
https://hg.mozilla.org/integration/autoland/rev/7bff640ac3d4
inhibit cache use when doing TRR in PB r=valentin
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
https://hg.mozilla.org/mozilla-central/rev/7bff640ac3d4
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.