Closed Bug 1549405 Opened 7 months ago Closed 6 months ago

Investigate if TRR::SendHTTPRequest requires LOAD_BYPASS_URL_ClASSIFIER flag

Categories

(Toolkit :: Safe Browsing, task, P2)

task

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: dimi, Assigned: dimi)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Check if TRR::SendHTTPRequest[1] requires LOAD_BYPASS_URL_ClASSIFIER

[1] https://searchfox.org/mozilla-central/rev/b2015fdd464f598d645342614593d4ebda922d95/netwerk/dns/TRR.cpp#164

Hi Valentin,
This is the same question as Bug 1547704 about whether we should include LOAD_BYPASS_URL_ClASSIFIER flag as an extra safeguard.
Thank you for your help!

Flags: needinfo?(valentin.gosu)

The channel is only sent to a preset URL, but sometimes it may use GET parameters with base64 encoded values. Could those get incorrectly classified? If yes, then we should add the flag.
It's sent with the system principal, so it probably shouldn't be a problem, right?

Flags: needinfo?(valentin.gosu)

(In reply to Valentin Gosu [:valentin] from comment #2)

The channel is only sent to a preset URL, but sometimes it may use GET parameters with base64 encoded values. Could those get incorrectly classified? If yes, then we should add the flag.

No

It's sent with the system principal, so it probably shouldn't be a problem, right?

Yes, we don't classify channels send via system principal, so in general, TRR request will not be classified

The reason we are still talking about whether we should add LOAD_BYPASS_URL_ClASSIFIER flag to this channel is because
we want to identify channels that if it is blocked, it has a serious consequence.

For example, if somehow, there is a bug, the channel is not sent with system principal, and at the same time, safebrowsing database is polluted with an entry "mozilla.cloudflare-dns.com", then the channel will be blocked(I know it sounds the chance is pretty low :) )

But in case this happened to firefox update channel, we won't be able to recover from that because the channel will always be blocked. But for channels with the load flag, we won't use the system principal, top-level document...etc criteria to determine if it should be classified, hence reduce the possibility that misclassified it because of a bug.

So the question is more like what happens if TRR::SendHTTPRequest? How serious it could be?
Also, if there is any network channel you think of that it has serious consequence if get blocked, please let me know :)
Thank you!

Flags: needinfo?(valentin.gosu)

Right, so if this channel gets blocked there's a chance DNS resolution would fail. We still normally fall back to regular DNS, except for people that set network.trr.mode=3 (strict-mode); in that case DNS resolution would completely fail.

So that is a strong yes for adding the flag. TRR is a critical path for the browser once enabled.

Flags: needinfo?(valentin.gosu)

(In reply to Valentin Gosu [:valentin] from comment #4)

Right, so if this channel gets blocked there's a chance DNS resolution would fail. We still normally fall back to regular DNS, except for people that set network.trr.mode=3 (strict-mode); in that case DNS resolution would completely fail.

So that is a strong yes for adding the flag. TRR is a critical path for the browser once enabled.

Great, thanks for the information!

Assignee: nobody → dlee
Status: NEW → ASSIGNED
Priority: -- → P2

We should add LOAD_BYPASS_URL_ClASSIFIER to TRR request because if
"network.trr.mode" is set to 3, blocked TRR request means DNS resolution
fail, we don't fall back to regular DNS in this scenario.

Pushed by dlee@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/28912333e8b8
Use LOAD_BYPASS_URL_ClASSIFIER flag for TRR request. r=valentin
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.