Closed Bug 875867 Opened 7 years ago Closed 5 years ago

Support SafeBrowsing in e10s

Categories

(Toolkit :: Safe Browsing, defect, P2)

defect

Tracking

()

RESOLVED FIXED
Tracking Status
e10s m3+ ---

People

(Reporter: briansmith, Assigned: mrbkap)

References

Details

(Whiteboard: [e10s-m3])

+++ This bug was initially created as a clone of Bug #839120 +++

(In reply to Brian Smith (:bsmith) from bug #839120 comment #14)
> SafeBrowsing is a security check, and network security checks should be done
> by the parent process anyway, not by the child process. So, there should be
> no SafeBrowsing code running in the child process in the first place. IIRC,
> the SafeBrowsing service suspects the channel while the check is done, and
> then resumes it when the check is complete. That suspend/resume should
> happen in the parent process. So, there shouldn't be any need for
> complicated proxying code. Instead, I think the difficulty is in migrating
> the code that connects the safe browsing from the place it is currently done
> (which would run in the child processes) to a place that runs in the parent
> processes. If things are done this way, then a compromised child process
> will not be able to influence the SafeBrowsing decision.
> 
> Also note that, as of now, NSS and most of PSM are unavailable in child
> processes, and URL classifier does use NSS/PSM for its cryptography. And,
> certificate validation and other network security checks are done the same
> way.
GCP, how bad do you think this bug will be? (i.e. give me a time estimate)
Blocks: old-e10s-m2
Flags: needinfo?(gpascutto)
The problem is that although I know how SafeBrowsing updates work, they likely aren't the real problem. i.e. the remote lookups currently happen as async calls on the main thread from the nsUrlClassifierDBService. The database update IO is already done on a worker thread. All of those things likely have to remain in the main process (also because they do crypto). The problem is the interaction with the content processes.

SafeBrowsing is linked with the rest of the browser via these places:
http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpChannel.cpp#4644
http://mxr.mozilla.org/mozilla-central/source/netwerk/base/src/nsChannelClassifier.cpp
http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#4582

Maybe someone who knows nsHttpChannel can give a more informed idea about this? SafeBrowsing works by suspending the Channel until it gets a decision, then setting an error if the link is considered dangerous. Then nsDocShell picks up that error and redirects to about:blocked with suitable parameters (malware/phishing).

Basically I think you need someone who understands nsHttpChannel more than you need someone who understands SafeBrowsing.
Flags: needinfo?(gpascutto)
The nsHttpChannel code runs in the main process, and I would expect the NS_ERROR_PHISHING_URI error to propagate just fine to the child channel and therefore to the docshell code in the child. What exactly are you worried about in this scenario?
I'm not speccifically worried about anything. I'm pointing out how it works. I have no idea how the above relates to e10s, or specifically, what work there is to be done if everything will work "just fine".

If nsHttpChannel runs in the main process, including the nsChannelClassifier part, I'm not sure if there is anything that need to be done?
Assignee: nobody → mrbkap
Priority: -- → P2
Hi all,

I try to run SafeBrowsing feature on Firefox OS, and coded some modifications to Gecko.
https://github.com/KDDI-tech/gecko/tree/safebrowsing

You can see the warning pages which are displayed by malware or attack web pages.
Although, actually, this is WIP code,  there are some issues.

1. can’t update cache DB on safebrowsing feature.
  modified clientID “B2G"  to “Firefox”. in SafeBrowsing.jsm L:131, clientID have a value “B2G”, the safebrowsing feature can’t update cache DB, because google’s safe browsing server reject the connection.

2. jslib doesn’t work correctory 
 in my code, the object of jslib has “FakeBackStagePath”, so we can’t access some modules[1] that over jslib.
 [1] : the modules under “ B2G/gecko/toolkit/components/url-classifier/content “, e.g. jslib.RequestBackoff, jslib.Function.prototype.inherits

We believe that this code help you to implement SafeBrowsing feature.

One last thing, actuary I don't know why SafeBrowsing feature doesn't work on Firefox OS.
Are there any known issues?

Best regards.
Flags: needinfo?(mrbkap)
Flags: needinfo?(gpascutto)
I think you want bug 839120. For this bug I think there's an outstanding issue with the block pages not working correctly. I have no idea what issues there are on Firefox OS but it looks you found some.
Flags: needinfo?(gpascutto)
Hi gcp,
Thanks comments. got it.
Although, I guess I'm having a quite bit confusion that what kind of situation about SafeBrowsing.

Is there the META bug about SafeBrowsing feature, and is there any person who implement about SafeBrowsing feature?
Flags: needinfo?(gpascutto)
This is the meta bug for making SafeBrowsing work with e10s and I linked you the meta bug on making it work on Firefox OS.

All the info we know is in those bugs. If you read them, you'll see we guess that SafeBrowsing *probably* mostly works, but that some small bugs *probably* remain and will have to be fixed. *We don't know for sure what they are until someone carefully tests the feature.* All the info we have is in the bugs.

I tested this a bit on desktop and concluded that the buttons on the about:blocked page don't work on e10s mode. There may be more bugs, that's just the first one I ran into. 

I haven't tested Firefox OS. Nobody, besides you, is currently actively looking into it, which means you probably know more than us about what is needed to make it work. If you find bugs and are able to fix them, we'd *very* much appreciate patches: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch

I hope this answer your question, because I'm not quite sure what you're actually asking.
Flags: needinfo?(gpascutto)
Flags: needinfo?(mrbkap)
No longer blocks: old-e10s-m2
Whiteboard: [e10s-m3]
(In reply to Chris Peterson (:cpeterson) from comment #9)
> Safe Browsing test sites:
> 
> http://www.itisatrap.org/firefox/its-a-trap.html
> http://www.itisatrap.org/firefox/its-an-attack.html

It's true those are test sites, but they trigger a very different code path than non-test sites. I recommended that you try to use sample urls from phishtank.com (make sure that they are also caught in Chrome) as much as possible to avoid these differences.
Moving to M3 milestone. These bugs affect e10s dogfooding but Brad and Gavin did not think they were M2 blockers.
Depends on: 1079242
Depends on: 1079245
Aside from the two bugs I filed here, I'm not finding a lot of issues with pb mode in e10s. Also, I didn't find a lot of disable tests related to pb mode. If anyone knows of something that's broken, please post a description here.
Private browsing has nothing to do with SafeBrowsing.
No longer depends on: 1079242, 1079245
(In reply to Gian-Carlo Pascutto [:gcp] from comment #13)
> Private browsing has nothing to do with SafeBrowsing.

Oh, thought they were basically the same. 

I tested blocked page test pages as well, didn't have any problems. about:blocked comes up and the chrome in the page works.

I'm tempted to resolve this bug wfm. I'm really not finding a lot of issues here. gcp, any areas that are broken that you know of in e10s mode?
Needinfoing myself to take a look and see if everything works.
Flags: needinfo?(gpascutto)
Resolving this for now since its basically a tracker that never accumulated any bugs. If we find something broken with safe browsing and e10s lets file and triage those issues in individual bugs.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Depends on: 1094170
Flags: needinfo?(gpascutto)
Depends on: 1094172
You need to log in before you can comment on or make changes to this bug.