1442496 - Find callsites that should use LOAD_BYPASS_URL_CLASSIFIER

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, enhancement, P3)

Description

[François Marier [:francois]]

I noticed that blocklist requests were being run through the URL classifier:

D/nsChannelClassifier nsChannelClassifier[0x97c46e20]: suspended channel 0x95b9c030
D/UrlClassifierDbService nsUrlClassifierLookupCallback::LookupComplete [0x956500d0] 0 pending completions
D/UrlClassifierDbService nsUrlClassifierLookupCallback::HandleResults [0x956500d0, 0 results]
D/nsChannelClassifier nsChannelClassifier[0x97c460c0]:OnClassifyComplete NS_OK (suspended channel)
D/nsChannelClassifier nsChannelClassifier::MarkEntryClassified[NS_OK] https://blocklists.settings.services.mozilla.com/v1/blocklist/3/%7Baa3c5121-dab2-40e2-81ca-7ea25febc110%7D/60.0a1/Fennec/20180301150822/Android_x86-gcc3/en-US/default/Linux%2017/default/default/1/1/new/
D/nsChannelClassifier nsChannelClassifier[0x97c460c0]: resuming channel 0x95560030 from OnClassifyComplete
D/UrlClassifierDbService nsUrlClassifierLookupCallback::LookupComplete [0x95650550] 0 pending completions
D/UrlClassifierDbService nsUrlClassifierLookupCallback::HandleResults [0x95650550, 0 results]
D/nsChannelClassifier nsChannelClassifier[0x97c46e20]:OnClassifyComplete NS_OK (suspended channel)
D/nsChannelClassifier nsChannelClassifier::MarkEntryClassified[NS_OK] https://firefox.settings.services.mozilla.com/v1/buckets/monitor/collections/changes/records
D/nsChannelClassifier nsChannelClassifier[0x97c46e20]: resuming channel 0x95b9c030 from OnClassifyComplete
D/nsChannelClassifier nsChannelClassifier::~nsChannelClassifier 0x97c46e20
D/nsChannelClassifier nsChannelClassifier::nsChannelClassifier 0x9a115160
D/nsChannelClassifier nsChannelClassifier[0x9a115160]: Classifying principal https://firefox.settings.services.mozilla.com/v1/buckets/blocklists/collections/addons/records?_sort=-last_modified on channel with uri https://firefox.settings.services.mozilla.com/v1/buckets/blocklists/collections/addons/records?_sort=-last_modified

same thing for the geolocation calls:

D/nsChannelClassifier nsChannelClassifier[0x97c46060]: suspended channel 0x9555f030
D/UrlClassifierDbService nsUrlClassifierLookupCallback::LookupComplete [0x955955b0] 0 pending completions
D/UrlClassifierDbService nsUrlClassifierLookupCallback::HandleResults [0x955955b0, 0 results]
D/nsChannelClassifier nsChannelClassifier[0x97c46060]:OnClassifyComplete NS_OK (suspended channel)
D/nsChannelClassifier nsChannelClassifier::MarkEntryClassified[NS_OK] https://location.services.mozilla.com/v1/country?key=fff72d56-b040-4205-9a11-82feda9d83a3
D/nsChannelClassifier nsChannelClassifier[0x97c46060]: resuming channel 0x9555f030 from OnClassifyComplete

and some kind of update:

D/nsChannelClassifier nsChannelClassifier[0x9a115e40]: suspended channel 0x95567830
D/UrlClassifierDbService nsUrlClassifierLookupCallback::LookupComplete [0x95650520] 0 pending completions
D/UrlClassifierDbService nsUrlClassifierLookupCallback::HandleResults [0x95650520, 0 results]
D/nsChannelClassifier nsChannelClassifier[0x9a115e40]:OnClassifyComplete NS_OK (suspended channel)
D/nsChannelClassifier nsChannelClassifier::MarkEntryClassified[NS_OK] https://aus5.mozilla.org/update/3/GMP/60.0a1/20180301150822/Android_x86-gcc3/en-US/default/Linux%2017/default/default/update.xml
D/nsChannelClassifier nsChannelClassifier[0x9a115e40]: resuming channel 0x95567830 from OnClassifyComplete
D/nsChannelClassifier nsChannelClassifier::~nsChannelClassifier 0x9a107580
D/nsChannelClassifier nsChannelClassifier::nsChannelClassifier 0x99bfba40

These internal loads should not be run through the classifier otherwise a bad Safe Browsing list, or a bug in the classifier, could break critical Firefox functionality.

Comment 1

[Dimi Lee [:dimi]]

Here is the list of the call sites we can skip URLClassifier check.
Since we are going to remove the flag LOAD_CLASSSIFY_URI in Bug 1522412, we don't really need to do anything here. However, we might still want to check if some of the call sites here we want to enforce bypass URLClassifier check.

The reason is even those call sites here will be exempted from classification automatically, some of them might be "critical" for Firefox, which means if somehow the "critical" channels are classified and marked as bad, it may cause Firefox fails to update, startup.

• OCSP request
• SafeBrowsing update, lookup
• Download protection lookup
• Telemetry pings
• Add-on checker
• CaptivePortal detect
• Firefox update
• Firefox warning page
• TRR request(trusted recursive resolver)
• SSL certificate exception
• ASAN reporter
• HPKP
• User Agent update
• Wifi geolocation request

Comment 2

[Dimi Lee [:dimi]]

Hi Ehsan,
I am going to needinfo or ping individual component owner to know what channels listed here are "critical channels".
But I assume we need a more clear definition of "critical channel" before checking with them.
I assume if a channel may affect launching firefox, triggering an update or may have serious security issue if it is blocked is "critical" channel.
But take "download protection request" for example, I am not sure if this is critical or not. And of course, we can still add the flag without a problem in this case because I know it won't be a malicious URL.

Do you have something in mind about what is the criteria to add or not to add this bypass flag? thanks!

Comment 3

[(no longer active)]

I think your criteria sounds good (a channel being blocked causing issues with critical Firefox functionality such as triggering an update, causing security issues, or preventing the browser to launch or block the basic functionality.)

BTW note that such channels should also be marked as beConservative as well, so this is an issue that is larger than the scope of this work. I think a dev-platform post may be better than needinfoing a ton of people here...

Comment 4

[Dimi Lee [:dimi]]

(In reply to :Ehsan Akhgari from comment #3)

BTW note that such channels should also be marked as beConservative as well, so this is an issue that is larger than the scope of this work. I think a dev-platform post may be better than needinfoing a ton of people here...

Thanks for the suggestions! I'll include this in the mail to dev-platform, and perhaps still needinfo some people after that to make sure things moving forward.

Comment 5

[Dimi Lee [:dimi]]

I am stuck in the SafeBrowsing performance stuff, will work on this next week.

Comment 6

[Dimi Lee [:dimi]]

close because this work is done.