Find callsites that should use LOAD_BYPASS_URL_CLASSIFIER

ASSIGNED
Assigned to

Status

()

enhancement
P2
normal
ASSIGNED
a year ago
17 days ago

People

(Reporter: francois, Assigned: dimi)

Tracking

(Blocks 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

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.
Reporter

Updated

6 months ago
Blocks: 1207775
See Also: 1207775
Assignee

Updated

6 months ago
Assignee: nobody → dlee
Status: NEW → ASSIGNED
Priority: P3 → P2
Assignee

Comment 1

3 months ago

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
Assignee

Updated

2 months ago
Summary: Find callsites that should NOT use LOAD_CLASSSIFY_URI → Find callsites that should use LOAD_BYPASS_URL_CLASSIFIER
Assignee

Updated

2 months ago
Depends on: 1522412
Assignee

Comment 2

2 months ago

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!

Flags: needinfo?(ehsan)

Comment 3

2 months ago

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...

Flags: needinfo?(ehsan)
Assignee

Comment 4

2 months ago

(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.

Assignee

Comment 5

2 months ago

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

Assignee

Updated

2 months ago
Type: defect → enhancement
Assignee

Updated

24 days ago
Depends on: 1547701
Assignee

Updated

24 days ago
Depends on: 1547704
Assignee

Updated

24 days ago
Depends on: 1547732
Assignee

Updated

17 days ago
Depends on: 1549405
You need to log in before you can comment on or make changes to this bug.