568502 - Security UI binding for multiprocess Fennec

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect)

Description

[Honza Bambas (:mayhemer)]

After mozilla-central gets merged with the electrolysis tree, security UI of Fennec will stop working.

After discussion with Benjamin Smedberg we need to change browser.xml here: http://hg.mozilla.org/mozilla-central/annotate/d4e5f5f19777/toolkit/content/widgets/browser.xml#l538

New class in the e10s tree TabParent (IFrameEmbedding) will implement nsISecureBrowserUI after bug 536301 lands.  browser.xml shall then bind to that implementation.

Comment 1

[Honza Bambas (:mayhemer)]

Sorry, wrong source reference, the correct is:

http://hg.mozilla.org/mobile-browser/annotate/4cfb4164ba20/chrome/content/browser.js#l3198

and as I can see, if browser.webProgress is TabParent, then this bug seems not to be valid as Fennec is not using nsISecureBrowserUI at all.

Comment 2

[Benjamin Smedberg]

Sure it is: http://mxr.mozilla.org/mobile-browser/source/chrome/content/browser.js#2174

Comment 3

[Honza Bambas (:mayhemer)]

Ah, I was searching for the interface.  I'm not much familiar with binding and xul, so where is the securityUI property instantiated?  By the code from mozilla-central's browser.xml?

Comment 4

[Benjamin Smedberg]

Yes. browser.xml is what implements the <xul:browser> element. After you fix this bug I think Firefox should "just work" (or at least the basic larry stuff: the more complex certificate viewer UI may not).

Comment 5

[Honza Bambas (:mayhemer)]

Thanks for clearing it for me.  

I think it's backwards: after I fix the Firefox bug (browser.xul code) this one should 'just work', mobile browser depends on it.

The cert detail UI is probably another bug, not needed for Fennec (?).

Comment 6

[Benjamin Smedberg]

The Firefox/Fennec bugs are probably dups. And yes, the cert detail UI is a different bug: I don't know whether Fennec has any UI like that or not.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Fennec had originally planned on moving the security data collection into the
content process. If the securityUI will be accessible in chrome, then we might
save ourselves some work.

No cert UI for Fennec

Comment 8

[Benjamin Smedberg]

Well, you'll need at least some cooperation with chrome since only chrome will have EV data.

Comment 9

[Honza Bambas (:mayhemer)]

Security data collection is made on the content process as by of means of checking web progress notifications and security information of each top level request and all their sub-requests (as is being made in the single process model).

The security info is being passed between parent and child processes because it is determined on the parent process but needs to be handled on the content process.

Security UI (nsISecureBrowserUI interface) will be accessible on the chrome process.  It will be implemented by the TabParent class.

Comment 10

[Honza Bambas (:mayhemer)]

And EV data is processed on the chrome as well (in TabParent).

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

Sounds like we will not need to change much with the current mobile code. Everything should just work, once bug 536301 lands.

If we need to do some small cleanup, it won't be a problem.

Comment 12

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

We should remove all the security UI processing from the content process, and instead have the content process notify the chrome process when something happens that should modify the security indicators. Then we can avoid serializing nsNSSSocketInfo and nsSSLStatus for IPC between the processes. These structures can be pretty large (several KB) because each one embeds a SSL certificate in its serialization. (And, nsNSSSocketInfo embeds an nSSSLStatus, so we're sending *two* serialized certs across processes.)

Also, because none of these classes (nsNSSCertificate, nsSSLStatus, nsNSSSocketInfo) serialize the EV status of the cert, we end up doing certificate validation multiple times, and sometimes on the main thread. This causes the main thread to block on network IO (OCSP responses). We can change the serialization/deserialization logic to store/restore the EV status, but this gets tricky because the HTTP disk cache also uses the same serialization for these structures; changing the serialization would thus cause compatibility problems. (OTOH, we probably need to serialize/deserialize the EV status anyway, to properly handle WYCIWYG, especially for session restore.)

Once we avoid sending nsNSSSocketInfo/nsSSLStatus across processes, we can remove the serialization code from those two classes. (For the cache, we should use specialized serialization logic instead of the current and inefficient generic serialization of these structures.)

Comment 13

[BMO Automation]

Closing all opened bug in a graveyard component