672284 - Consider bypassing safebrowsing checks on Session restore

Status: REOPENED

(Toolkit :: Safe Browsing, defect, P5)

Description

[Gian-Carlo Pascutto [:gcp]]

This is really a question about a potential optimization which we might want to investigate.

Right now, the browser will block all URL loads until the site can be verified against the phishing protection. The current implementation needs to do some small disk probes to check this, which is usually fast. In bug 669410 and related ones, this might be replaced by more compact structures (but which might take longer to construct).

Regardless of the technique being used, blocking the URL loading is potentially a clearly noticeable slowdown. I believe that if there is heavy disk I/O, this could be quite noticeable already with the current implementation.

The point where this is most critical is the browser start-up. I can see 3 situations there:

1) We're being launched with a supplied URL. It might come from anywhere, so we must check.
2) The initial page is about:home or about:blank. This should bypass phishing protection entirely (check that!).
3) The user is restoring a session, potentially with many pages. 
4) The initial page is whatever the user set as his homepage.

The question is if we can *consider* bypassing the URL classifier for case 3 and/or case 4 to get faster startup performance.

Comment 1

[Mehdi Mulani [:mmm] (I don't check this)]

I thought case 2 was covered, as there is some verification of the URL. (We don't check chrome:// URLs, for instance)

As for case 3: If we recheck a page that was previously open, we will only end up with a different answer if the page has been added to the database after it was last visited (assuming the user did not ignore the warning), since we do not update the database before checking pages from the last session.
So bypassing when restoring a session seems reasonable in most cases. We can mitigate the above problem by moving the lookup to the session save and use cached values when restoring a session. This could be pretty bad though, as session saves are somewhat frequent (every 2 minutes at the worst, I believe).

Case 4 is beyond my scope, though it seems like a judgement call based on the likelihood of malware changing a user's homepage should be taken into account. (Technically that could affect case 3 too though :/)

Comment 2

[Dave Camp (:dcamp)]

(In reply to comment #1)
> Case 4 is beyond my scope, though it seems like a judgement call based on
> the likelihood of malware changing a user's homepage should be taken into
> account. (Technically that could affect case 3 too though :/)

Do you mean malware already on the user's machine?  That could also just delete the safebrowsing db or turn off the pref if it wanted to...

Comment 3

[Mehdi Mulani [:mmm] (I don't check this)]

(In reply to comment #2)
> Do you mean malware already on the user's machine?  That could also just
> delete the safebrowsing db or turn off the pref if it wanted to...

Of course, I meant that the current prevalence of malware changing a user's homepage should be taken into account. I don't know why a malicious adversary wouldn't just turn off the pref either though :P

Comment 4

[François Marier [:francois]]

I agree we could skip Safe Browsing checks for Case 3, but I disagree about Case 4.

If you set a site as your homepage, it's still possible that it could be compromised in the future and start to serve malware. For a session restore, the window of opportunity is much smaller so the risk is not as high IMHO.

Comment 5

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.

Comment 6

[Gian-Carlo Pascutto [:gcp]]

Unassigning myself, leaving it to Dimi to see how to proceed.