820466 - ###!!! ASSERTION: nsSSLStatus has null mServerCert or was called in the content process

Status: RESOLVED DUPLICATE of bug 764496

(Core :: Security: PSM, defect)

Description

[Honza Bambas (:mayhemer)]

Debug desktop build of Fennec.  Entered https://addons.mozilla.org/en-US/firefox/search/?q=telemetry to the address bar.

Child process asserts (http://hg.mozilla.org/mozilla-central/annotate/3905010a2346/security/manager/ssl/src/nsIdentityChecking.cpp#l1172):

 	xul.dll!NS_DebugBreak_P(unsigned int aSeverity=1, const char * aStr=0x56312e20, const char * aExpr=0x56312e18, const char * aFile=0x56312dc8, int aLine=1173)  Line 415 + 0xc bytes	C++
>	xul.dll!nsSSLStatus::GetIsExtendedValidation(bool * aIsEV=0x0048bf8e)  Line 1173 + 0x1b bytes	C++
 	xul.dll!nsSecureBrowserUIImpl::EvaluateAndUpdateSecurityState(nsIRequest * aRequest=0x02c67be4, nsISupports * info=0x044d3220, bool withNewLocation=true)  Line 496 + 0x1d bytes	C++
 	xul.dll!nsSecureBrowserUIImpl::OnLocationChange(nsIWebProgress * aWebProgress=0x04337d84, nsIRequest * aRequest=0x02c67be4, nsIURI * aLocation=0x042d9e10, unsigned int aFlags=0)  Line 1475 + 0x1a bytes	C++
 	xul.dll!nsDocLoader::FireOnLocationChange(nsIWebProgress * aWebProgress=0x04337d84, nsIRequest * aRequest=0x02c67be4, nsIURI * aUri=0x042d9e10, unsigned int aFlags=0)  Line 1346	C++
 	xul.dll!nsDocShell::CreateContentViewer(const char * aContentType=0x04425fe8, nsIRequest * request=0x02c67be4, nsIStreamListener * * aContentHandler=0x04400ac0)  Line 7749	C++
 	xul.dll!nsDSURIContentListener::DoContent(const char * aContentType=0x04425fe8, bool aIsContentPreferred=false, nsIRequest * request=0x02c67be4, nsIStreamListener * * aContentHandler=0x04400ac0, bool * aAbortProcess=0x0048c203)  Line 122 + 0x20 bytes	C++
 	xul.dll!nsDocumentOpenInfo::TryContentListener(nsIURIContentListener * aListener=0x04323280, nsIChannel * aChannel=0x02c67be4)  Line 659 + 0x42 bytes	C++
 	xul.dll!nsDocumentOpenInfo::DispatchContent(nsIRequest * request=0x02c67be4, nsISupports * aCtxt=0x00000000)  Line 360 + 0x35 bytes	C++
 	xul.dll!nsDocumentOpenInfo::OnStartRequest(nsIRequest * request=0x02c67be4, nsISupports * aCtxt=0x00000000)  Line 252 + 0x10 bytes	C++
 	xul.dll!mozilla::net::HttpChannelChild::OnStartRequest(const nsHttpResponseHead & responseHead={...}, const bool & useResponseHead=true, const nsHttpHeaderArray & requestHeaders={...}, const bool & isFromCache=false, const bool & cacheEntryAvailable=true, const unsigned int & cacheExpirationTime=0, const nsCString & cachedCharset={...}, const nsCString & securityInfoSerialization={...}, const PRNetAddr & selfAddr={...}, const PRNetAddr & peerAddr={...})  Line 278 + 0x47 bytes	C++
 	xul.dll!mozilla::net::HttpChannelChild::RecvOnStartRequest(const nsHttpResponseHead & responseHead={...}, const bool & useResponseHead=true, const nsHttpHeaderArray & requestHeaders={...}, const bool & isFromCache=false, const bool & cacheEntryAvailable=true, const unsigned int & cacheExpirationTime=0, const nsCString & cachedCharset={...}, const nsCString & securityInfoSerialization={...}, const PRNetAddr & selfAddr={...}, const PRNetAddr & peerAddr={...})  Line 236	C++
 	xul.dll!mozilla::net::PHttpChannelChild::OnMessageReceived(const IPC::Message & __msg={...})  Line 543 + 0x59 bytes	C++
 	xul.dll!mozilla::dom::PContentChild::OnMessageReceived(const IPC::Message & __msg={...})  Line 2078 + 0x17 bytes	C++
 	xul.dll!mozilla::ipc::AsyncChannel::OnDispatchMessage(const IPC::Message & msg={...})  Line 473 + 0x1e bytes	C++
 	xul.dll!mozilla::ipc::RPCChannel::OnMaybeDequeueOne()  Line 404	C++
 	xul.dll!DispatchToMethod<mozilla::ipc::RPCChannel,bool (__thiscall mozilla::ipc::RPCChannel::*)(void)>(mozilla::ipc::RPCChannel * obj=0x00661110, bool (void)* method=0x52ca8f8a, const Tuple0 & arg={...})  Line 384	C++
 	xul.dll!RunnableMethod<mozilla::ipc::RPCChannel,bool (__thiscall mozilla::ipc::RPCChannel::*)(void),Tuple0>::Run()  Line 307 + 0x1e bytes	C++
 	xul.dll!mozilla::ipc::RPCChannel::RefCountedTask::Run()  Line 425 + 0x1a bytes	C++
 	xul.dll!mozilla::ipc::RPCChannel::DequeueTask::Run()  Line 448 + 0x19 bytes	C++
 	xul.dll!MessageLoop::RunTask(Task * task=0x00659408)  Line 334	C++
 	xul.dll!MessageLoop::DeferOrRunPendingTask(const MessageLoop::PendingTask & pending_task={...})  Line 344	C++
 	xul.dll!MessageLoop::DoWork()  Line 441 + 0xc bytes	C++
 	xul.dll!mozilla::ipc::DoWorkRunnable::Run()  Line 43	C++
 	xul.dll!nsThread::ProcessNextEvent(bool mayWait=true, bool * result=0x0048d287)  Line 627 + 0x19 bytes	C++
 	xul.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x0066ec00, bool mayWait=true)  Line 221 + 0x17 bytes	C++
 	xul.dll!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate * aDelegate=0x0048f3b0)  Line 117 + 0xe bytes	C++
 	xul.dll!mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate * aDelegate=0x0048f3b0)  Line 232	C++
 	xul.dll!MessageLoop::RunInternal()  Line 216	C++
 	xul.dll!MessageLoop::RunHandler()  Line 209	C++
 	xul.dll!MessageLoop::Run()  Line 183	C++
 	xul.dll!nsBaseAppShell::Run()  Line 165	C++
 	xul.dll!nsAppShell::Run()  Line 232 + 0x9 bytes	C++
 	xul.dll!XRE_RunAppShell()  Line 646 + 0x19 bytes	C++
 	xul.dll!mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate * aDelegate=0x0048f3b0)  Line 198 + 0x5 bytes	C++
 	xul.dll!MessageLoop::RunInternal()  Line 216	C++
 	xul.dll!MessageLoop::RunHandler()  Line 209	C++
 	xul.dll!MessageLoop::Run()  Line 183	C++
 	xul.dll!XRE_InitChildProcess(int aArgc=2, char * * aArgv=0x00657e40, GeckoProcessType aProcess=GeckoProcessType_Content)  Line 489	C++
 	plugin-container.exe!NS_internal_main(int argc=4, char * * argv=0x00657e40)  Line 48 + 0x12 bytes	C++
 	plugin-container.exe!wmain(int argc=5, wchar_t * * argv=0x01fcc370)  Line 105 + 0xd bytes	C++

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Also hit during mochitests; see <https://tbpl.mozilla.org/php/getParsedLog.php?id=21024459&tree=Firefox&full=1> for example.

Comment 2

[Mark Hammond [:markh] [:mhammond]]

This is also hit by the background thumbnail service when it attempts to thumbnail a page which wants to use a certificate.  Note that bug 902439 is arranging to use LOAD_ANONYMOUS on those requests so the certificate prompt doesn't show up, but this assertion happens both with and without the patch there.  The assertion is being hit in the child process.

This code was added in bug 686248, so blocking that bug and adding Brian to the CC list.

Comment 3

[Chris Peterson [:cpeterson]]

Brian: is this assertion reporting a real bug?

I hit this assertion failure all the time (e.g. google.com) with e10s enabled. It's pretty annoying. If this is a website problem, this assertion should probably be changed to just log a warning.

Comment 4

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

(In reply to Chris Peterson (:cpeterson) from comment #3)
> Brian: is this assertion reporting a real bug?
> 
> I hit this assertion failure all the time (e.g. google.com) with e10s
> enabled. It's pretty annoying. If this is a website problem, this assertion
> should probably be changed to just log a warning.

This is a real bug. This is a significant architectural issue with how the security indicators are calculated in e10s. In particular, the determination of the security status of a page is being made in the untrustworthy content process, instead of in the trusted parent process.

I believe I explained this to quite a few MoCo employees in the past, so I think it shouldn't be too hard to find somebody within Mozilla who understands the issue.

Comment 6

[Jason Duell]

Honza, do you have any idea what needs to be done here?  It sounds like there's some check that we need to do on the parent instead of the child (ideally we could return it as part of the channel response, i.e. in OnStart or OnStop, to save an IPDL round trip)

Comment 7

[Honza Bambas (:mayhemer)]

As Brian mentioned, this is one of the results of the 10s architecture.  Docshells, load groups and sec UI are all running only on the content process.  Parent is actually dumb and only makes the "real" network request w/o any knowledge which one is which and how those are grouped/related.  At least to my current knowledge..

The "TODO" here is a bit larger project.

Adding Tanvi, she may have some ideas what way to go here.

Comment 8

[Tanvi Vyas[:tanvi]]

(In reply to Honza Bambas (:mayhemer) from comment #7)
> As Brian mentioned, this is one of the results of the 10s architecture. 
> Docshells, load groups and sec UI are all running only on the content
> process.  Parent is actually dumb and only makes the "real" network request
> w/o any knowledge which one is which and how those are grouped/related.  At
> least to my current knowledge..
> 
> The "TODO" here is a bit larger project.
> 
> Adding Tanvi, she may have some ideas what way to go here.

I am having similar problems with Mixed Content Blocker and Monica is also having issues with Tracking Protection.  The information I need to make security decisions is available via nodes, docshells, securityUI, etc which we are not able to send from child to parent in e10s.  So either we have to send some data across (ex: flags or state that we retrieve in the child and then pass to the parent) or we have to run the code in the child.  From what I've heard, if your code is not making network requests, it is okay to run in the child.  Sounds like Brian has concerns about security code running in the child, regardless of whether it is making network requests.

Comment 10

[Brad Lassey [:blassey] (use needinfo?)]

Tanvi, can you shed some light here? Is this something we should care about for rolling out e10s?

Comment 11

[Tanvi Vyas[:tanvi]]

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #10)
> Tanvi, can you shed some light here? Is this something we should care about
> for rolling out e10s?

It looks like the offending code was removed from the codebase:
http://hg.mozilla.org/mozilla-central/annotate/3905010a2346/security/manager/ssl/src/nsIdentityChecking.cpp#l1172
http://mxr.mozilla.org/mozilla-central/source/security/manager/ssl/nsSSLStatus.cpp#117

So we can probably close this bug.  Moving the needinfo to keeler to confirm.

Comment 12

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

This was fixed in bug 764496, looks like.