1397544 - Use the IP malware Safe Browsing list

Status: RESOLVED WONTFIX

(Toolkit :: Safe Browsing, enhancement, P3)

Description

[François Marier [:francois]]

Chrome uses a list of IP addresses known to serve malware:

ListIdentifier GetIpMalwareId() {
  return ListIdentifier(GetCurrentPlatformType(), IP_RANGE, MALWARE_THREAT);
}

https://cs.chromium.org/chromium/src/components/safe_browsing_db/v4_protocol_manager_util.cc?l=115&rcl=aa7726c0ac6fd261d1cd9932fb5e4e6e51c43676

It's not restricted to Chrome, it's also available in Chromium:

https://cs.chromium.org/chromium/src/components/safe_browsing_db/v4_local_database_manager.cc?l=62&rcl=aa7726c0ac6fd261d1cd9932fb5e4e6e51c43676

We should determine whether or not we should add this to Firefox.

Comment 1

[François Marier [:francois]]

Google says that we should use this. There is no documentation for the hash format though so we'll have to look at the Chromium source code.

Comment 2

[Thomas Nguyen (:tnguyen)]

We can trace it from https://cs.chromium.org/chromium/src/components/safe_browsing_db/v4_local_database_manager.cc?l=425&rcl=a3c45f3749fe0fab81aea4391518fd2bd9f090dc
I think it's similar to URL type, IP_RANGE type will store prefixes and hash of an IPV6 
Unit test
https://cs.chromium.org/chromium/src/components/safe_browsing_db/v4_local_database_manager_unittest.cc?l=852&rcl=a3c45f3749fe0fab81aea4391518fd2bd9f090dc
Looks like they convert the ip v6 string into hash string and check prefix matches with local database. We can figure it out and check more details when starting this bug.
Is there perf impact if we intend to implement this? Are we intending to check the IP malware matching list at the same place in nsChannelClassifier or earlier?

Comment 3

[François Marier [:francois]]

(In reply to Thomas Nguyen[:tnguyen] ni plz from comment #2)
> Is there perf impact if we intend to implement this? Are we intending to
> check the IP malware matching list at the same place in nsChannelClassifier
> or earlier?

We'd check it at the same time as we check URLs against the malware/phishing/unwanted list so it shouldn't change much from a performance point of view.

Comment 4

[Dimi Lee [:dimi]]

Attachment: WIP patch

First patch (and coding) since June, I need some time to get familiar with safebrowsing...

Comment 5

[Dimi Lee [:dimi]]

Hi Francois,
After checking chromium source code, I think how they are using MalwareIp database isn't the same as
how we plan to do.

If I understand correctly, chromium use malware ip database in client-side malware detection.
After page loads, they check all IP address requested together with the page content[1][2].
Then chromium sends all those bad IPs with other information to the server to determine if this is a malware URL.

Since they didn't use the "malware IP table" directly to decide if a URL load is valid, I wonder if we
should use it that way.

[1] https://cs.chromium.org/chromium/src/chrome/browser/safe_browsing/browser_feature_extractor.h?gsn=ShouldClassifyForMalware&l=66
[2] https://cs.chromium.org/chromium/src/chrome/browser/safe_browsing/browser_feature_extractor.cc?type=cs&q=MatchMalwareIp&l=48
[3] https://cs.chromium.org/chromium/src/components/safe_browsing/proto/csd.proto?type=cs&q=ClientMalwareRequest&l=154

Comment 6

[François Marier [:francois]]

(In reply to Dimi Lee[:dimi][:dlee] from comment #5)
> If I understand correctly, chromium use malware ip database in client-side
> malware detection.
> After page loads, they check all IP address requested together with the page
> content[1][2].
> Then chromium sends all those bad IPs with other information to the server
> to determine if this is a malware URL.

You're right, they're currently only using it in this way.

> Since they didn't use the "malware IP table" directly to decide if a URL
> load is valid, I wonder if we
> should use it that way.

I will try to find out whether it's meant to be used for more than just the client-side detection code you have found. Last time I talked to Google, they implied it was meant to be used like the URL blacklist for malware sites.

Keeping the NEEDINFO open.

Comment 7

[François Marier [:francois]]

The list is currently not yet being populated.

Let's reopen this bug if/when the list becomes available.

Thanks for the patch Dimi and sorry about the confusion.