From 3b89434010281a7a6e701e11c5f083b3482838d7 Mon Sep 17 00:00:00 2001 From: "kaie@kuix.de" Date: Fri, 11 Apr 2008 21:47:22 -0700 Subject: [PATCH] Single patch for 3 overlapping fixes. Bug 420187, hang in nsNSSHttpRequestSession::internal_send_receive_attempt r=rrelyea Bug 383369, fixing a regression from 335801 r=rrelyea Bug 358438, fix proposed by and portions contributed by Honza Bambas r=honzab, r=rrelyea approval1.9 for combined patch = beltzner --- .../boot/src/nsSecureBrowserUIImpl.cpp | 197 +++++++++++++----- .../manager/boot/src/nsSecureBrowserUIImpl.h | 13 +- security/manager/ssl/public/Makefile.in | 1 + .../public/nsIAssociatedContentSecurity.idl | 56 +++++ security/manager/ssl/src/nsNSSCallbacks.cpp | 88 +++++--- security/manager/ssl/src/nsNSSCallbacks.h | 8 + security/manager/ssl/src/nsNSSComponent.cpp | 13 ++ security/manager/ssl/src/nsNSSIOLayer.cpp | 132 ++++++++++-- security/manager/ssl/src/nsNSSIOLayer.h | 14 +- security/manager/ssl/src/nsSSLThread.cpp | 22 +- security/manager/ssl/src/nsSSLThread.h | 4 +- 11 files changed, 422 insertions(+), 126 deletions(-) create mode 100644 security/manager/ssl/public/nsIAssociatedContentSecurity.idl diff --git a/security/manager/boot/src/nsSecureBrowserUIImpl.cpp b/security/manager/boot/src/nsSecureBrowserUIImpl.cpp index 5ff20d6f7183..221c2e4a82ec 100644 --- a/security/manager/boot/src/nsSecureBrowserUIImpl.cpp +++ b/security/manager/boot/src/nsSecureBrowserUIImpl.cpp @@ -166,6 +166,10 @@ nsSecureBrowserUIImpl::nsSecureBrowserUIImpl() mNewToplevelIsEV = PR_FALSE; mNewToplevelSecurityStateKnown = PR_TRUE; ResetStateTracking(); + mSubRequestsHighSecurity = 0; + mSubRequestsLowSecurity = 0; + mSubRequestsBrokenSecurity = 0; + mSubRequestsNoSecurity = 0; #if defined(PR_LOGGING) if (!gSecureDocLog) @@ -359,31 +363,28 @@ static nsresult IsChildOfDomWindow(nsIDOMWindow *parent, nsIDOMWindow *child, return NS_OK; } -static PRUint32 GetSecurityStateFromChannel(nsIChannel* aChannel) +static PRUint32 GetSecurityStateFromSecurityInfo(nsISupports *info) { nsresult res; PRUint32 securityState; - // qi for the psm information about this channel load. - nsCOMPtr info; - aChannel->GetSecurityInfo(getter_AddRefs(info)); nsCOMPtr psmInfo(do_QueryInterface(info)); if (!psmInfo) { - PR_LOG(gSecureDocLog, PR_LOG_DEBUG, ("SecureUI: GetSecurityState:%p - no nsITransportSecurityInfo for %p\n", - aChannel, (nsISupports *)info)); + PR_LOG(gSecureDocLog, PR_LOG_DEBUG, ("SecureUI: GetSecurityState: - no nsITransportSecurityInfo for %p\n", + (nsISupports *)info)); return nsIWebProgressListener::STATE_IS_INSECURE; } - PR_LOG(gSecureDocLog, PR_LOG_DEBUG, ("SecureUI: GetSecurityState:%p - info is %p\n", aChannel, + PR_LOG(gSecureDocLog, PR_LOG_DEBUG, ("SecureUI: GetSecurityState: - info is %p\n", (nsISupports *)info)); res = psmInfo->GetSecurityState(&securityState); if (NS_FAILED(res)) { - PR_LOG(gSecureDocLog, PR_LOG_DEBUG, ("SecureUI: GetSecurityState:%p - GetSecurityState failed: %d\n", - aChannel, res)); + PR_LOG(gSecureDocLog, PR_LOG_DEBUG, ("SecureUI: GetSecurityState: - GetSecurityState failed: %d\n", + res)); securityState = nsIWebProgressListener::STATE_IS_BROKEN; } - PR_LOG(gSecureDocLog, PR_LOG_DEBUG, ("SecureUI: GetSecurityState:%p - Returning %d\n", aChannel, + PR_LOG(gSecureDocLog, PR_LOG_DEBUG, ("SecureUI: GetSecurityState: - Returning %d\n", securityState)); return securityState; } @@ -471,10 +472,6 @@ void nsSecureBrowserUIImpl::ResetStateTracking() mInfoTooltip.Truncate(); mDocumentRequestsInProgress = 0; - mSubRequestsHighSecurity = 0; - mSubRequestsLowSecurity = 0; - mSubRequestsBrokenSecurity = 0; - mSubRequestsNoSecurity = 0; if (mTransferringRequests.ops) { PL_DHashTableFinish(&mTransferringRequests); mTransferringRequests.ops = nsnull; @@ -484,10 +481,8 @@ void nsSecureBrowserUIImpl::ResetStateTracking() } nsresult -nsSecureBrowserUIImpl::EvaluateAndUpdateSecurityState(nsIRequest *aRequest) +nsSecureBrowserUIImpl::EvaluateAndUpdateSecurityState(nsIRequest* aRequest, nsISupports *info) { - nsCOMPtr channel(do_QueryInterface(aRequest)); - /* I explicitly ignore the camelCase variable naming style here, I want to make it clear these are temp variables that relate to the member variables with the same suffix.*/ @@ -501,16 +496,12 @@ nsSecureBrowserUIImpl::EvaluateAndUpdateSecurityState(nsIRequest *aRequest) PRBool updateTooltip = PR_FALSE; nsXPIDLString temp_InfoTooltip; - if (channel) { - temp_NewToplevelSecurityState = GetSecurityStateFromChannel(channel); + temp_NewToplevelSecurityState = GetSecurityStateFromSecurityInfo(info); PR_LOG(gSecureDocLog, PR_LOG_DEBUG, ("SecureUI:%p: OnStateChange: remember mNewToplevelSecurityState => %x\n", this, temp_NewToplevelSecurityState)); - // Get SSL Status information if possible - nsCOMPtr info; - channel->GetSecurityInfo(getter_AddRefs(info)); nsCOMPtr sp = do_QueryInterface(info); if (sp) { // Ignore result @@ -533,7 +524,6 @@ nsSecureBrowserUIImpl::EvaluateAndUpdateSecurityState(nsIRequest *aRequest) } } } - } // assume temp_NewToplevelSecurityState was set in this scope! // see code that is directly above @@ -549,22 +539,21 @@ nsSecureBrowserUIImpl::EvaluateAndUpdateSecurityState(nsIRequest *aRequest) if (updateTooltip) { mInfoTooltip = temp_InfoTooltip; } + PR_LOG(gSecureDocLog, PR_LOG_DEBUG, + ("SecureUI:%p: remember securityInfo %p\n", this, + info)); + mCurrentToplevelSecurityInfo = info; } return UpdateSecurityState(aRequest); } void -nsSecureBrowserUIImpl::UpdateSubrequestMembers(nsIRequest *aRequest) +nsSecureBrowserUIImpl::UpdateSubrequestMembers(nsISupports *securityInfo) { // For wyciwyg channels in subdocuments we only update our // subrequest state members. - PRUint32 reqState = nsIWebProgressListener::STATE_IS_INSECURE; - nsCOMPtr channel(do_QueryInterface(aRequest)); - - if (channel) { - reqState = GetSecurityStateFromChannel(channel); - } + PRUint32 reqState = GetSecurityStateFromSecurityInfo(securityInfo); // the code above this line should run without a lock nsAutoMonitor lock(mMonitor); @@ -749,10 +738,13 @@ nsSecureBrowserUIImpl::OnStateChange(nsIWebProgress* aWebProgress, } #endif + nsCOMPtr securityInfo; nsCOMPtr channel(do_QueryInterface(aRequest)); if (channel) { + channel->GetSecurityInfo(getter_AddRefs(securityInfo)); + nsCOMPtr uri; channel->GetURI(getter_AddRefs(uri)); if (uri) @@ -914,6 +906,11 @@ nsSecureBrowserUIImpl::OnStateChange(nsIWebProgress* aWebProgress, f -= nsIWebProgressListener::STATE_SECURE_LOW; info.Append("SECURE_LOW "); } + if (f & nsIWebProgressListener::STATE_RESTORING) + { + f -= nsIWebProgressListener::STATE_RESTORING; + info.Append("STATE_RESTORING "); + } if (f > 0) { @@ -930,7 +927,7 @@ nsSecureBrowserUIImpl::OnStateChange(nsIWebProgress* aWebProgress, { PR_LOG(gSecureDocLog, PR_LOG_DEBUG, ("SecureUI:%p: OnStateChange: seeing STOP with security state: %d\n", this, - GetSecurityStateFromChannel(channel) + GetSecurityStateFromSecurityInfo(securityInfo) )); } #endif @@ -964,12 +961,13 @@ nsSecureBrowserUIImpl::OnStateChange(nsIWebProgress* aWebProgress, } } + PRBool allowSecurityStateChange = PR_TRUE; if (loadFlags & nsIChannel::LOAD_RETARGETED_DOCUMENT_URI) { // The original consumer (this) is no longer the target of the load. // Ignore any events with this flag, do not allow them to update // our secure UI state. - return NS_OK; + allowSecurityStateChange = PR_FALSE; } if (aProgressStateFlags & STATE_START @@ -980,27 +978,99 @@ nsSecureBrowserUIImpl::OnStateChange(nsIWebProgress* aWebProgress, && loadFlags & nsIChannel::LOAD_DOCUMENT_URI) { - nsAutoMonitor lock(mMonitor); - if (!mDocumentRequestsInProgress) + PRBool inProgress; + + PRInt32 saveSubHigh; + PRInt32 saveSubLow; + PRInt32 saveSubBroken; + PRInt32 saveSubNo; + nsCOMPtr prevContentSecurity; + + PRInt32 newSubHigh = 0; + PRInt32 newSubLow = 0; + PRInt32 newSubBroken = 0; + PRInt32 newSubNo = 0; + + { + nsAutoMonitor lock(mMonitor); + inProgress = (mDocumentRequestsInProgress!=0); + + if (allowSecurityStateChange && !inProgress) + { + saveSubHigh = mSubRequestsHighSecurity; + saveSubLow = mSubRequestsLowSecurity; + saveSubBroken = mSubRequestsBrokenSecurity; + saveSubNo = mSubRequestsNoSecurity; + prevContentSecurity = do_QueryInterface(mCurrentToplevelSecurityInfo); + } + } + + if (allowSecurityStateChange && !inProgress) { PR_LOG(gSecureDocLog, PR_LOG_DEBUG, ("SecureUI:%p: OnStateChange: start for toplevel document\n", this )); - ResetStateTracking(); - mNewToplevelSecurityStateKnown = PR_FALSE; + if (prevContentSecurity) + { + PR_LOG(gSecureDocLog, PR_LOG_DEBUG, + ("SecureUI:%p: OnStateChange: start, saving current sub state\n", this + )); + + // before resetting our state, let's save information about + // sub element loads, so we can restore it later + prevContentSecurity->SetCountSubRequestsHighSecurity(saveSubHigh); + prevContentSecurity->SetCountSubRequestsLowSecurity(saveSubLow); + prevContentSecurity->SetCountSubRequestsBrokenSecurity(saveSubBroken); + prevContentSecurity->SetCountSubRequestsNoSecurity(saveSubNo); + } + + if (securityInfo && + (aProgressStateFlags & nsIWebProgressListener::STATE_RESTORING) != 0) + { + // When restoring from bfcache, we will not get events for the + // page's sub elements, so let's load the state of sub elements + // from the cache. + + nsCOMPtr + newContentSecurity(do_QueryInterface(securityInfo)); + + if (newContentSecurity) + { + PR_LOG(gSecureDocLog, PR_LOG_DEBUG, + ("SecureUI:%p: OnStateChange: start, loading old sub state\n", this + )); + + newContentSecurity->GetCountSubRequestsHighSecurity(&newSubHigh); + newContentSecurity->GetCountSubRequestsLowSecurity(&newSubLow); + newContentSecurity->GetCountSubRequestsBrokenSecurity(&newSubBroken); + newContentSecurity->GetCountSubRequestsNoSecurity(&newSubNo); + } + } } - // By using a counter, this code also works when the toplevel - // document get's redirected, but the STOP request for the - // previous toplevel document has not yet have been received. + { + nsAutoMonitor lock(mMonitor); - PR_LOG(gSecureDocLog, PR_LOG_DEBUG, - ("SecureUI:%p: OnStateChange: ++mDocumentRequestsInProgress\n", this - )); + if (allowSecurityStateChange && !inProgress) + { + ResetStateTracking(); + mSubRequestsHighSecurity = newSubHigh; + mSubRequestsLowSecurity = newSubLow; + mSubRequestsBrokenSecurity = newSubBroken; + mSubRequestsNoSecurity = newSubNo; + mNewToplevelSecurityStateKnown = PR_FALSE; + } + + // By using a counter, this code also works when the toplevel + // document get's redirected, but the STOP request for the + // previous toplevel document has not yet have been received. + PR_LOG(gSecureDocLog, PR_LOG_DEBUG, + ("SecureUI:%p: OnStateChange: ++mDocumentRequestsInProgress\n", this + )); + ++mDocumentRequestsInProgress; + } - ++mDocumentRequestsInProgress; - return NS_OK; } @@ -1018,7 +1088,10 @@ nsSecureBrowserUIImpl::OnStateChange(nsIWebProgress* aWebProgress, { nsAutoMonitor lock(mMonitor); temp_DocumentRequestsInProgress = mDocumentRequestsInProgress; - temp_ToplevelEventSink = mToplevelEventSink; + if (allowSecurityStateChange) + { + temp_ToplevelEventSink = mToplevelEventSink; + } } if (temp_DocumentRequestsInProgress <= 0) @@ -1034,20 +1107,26 @@ nsSecureBrowserUIImpl::OnStateChange(nsIWebProgress* aWebProgress, if (!temp_ToplevelEventSink && channel) { - ObtainEventSink(channel, temp_ToplevelEventSink); + if (allowSecurityStateChange) + { + ObtainEventSink(channel, temp_ToplevelEventSink); + } } { nsAutoMonitor lock(mMonitor); - mToplevelEventSink = temp_ToplevelEventSink; + if (allowSecurityStateChange) + { + mToplevelEventSink = temp_ToplevelEventSink; + } --mDocumentRequestsInProgress; } - if (requestHasTransferedData) { + if (allowSecurityStateChange && requestHasTransferedData) { // Data has been transferred for the single toplevel // request. Evaluate the security state. - return EvaluateAndUpdateSecurityState(aRequest); + return EvaluateAndUpdateSecurityState(aRequest, securityInfo); } return NS_OK; @@ -1064,9 +1143,9 @@ nsSecureBrowserUIImpl::OnStateChange(nsIWebProgress* aWebProgress, // We only care for the security state of sub requests which have actually transfered data. - if (requestHasTransferedData) + if (allowSecurityStateChange && requestHasTransferedData) { - UpdateSubrequestMembers(aRequest); + UpdateSubrequestMembers(securityInfo); // Care for the following scenario: // A new top level document load might have already started, @@ -1251,8 +1330,8 @@ void nsSecureBrowserUIImpl::UpdateMyFlags(PRBool &showWarning, lockIconState &wa mNotifiedToplevelIsEV = mNewToplevelIsEV; } -nsresult nsSecureBrowserUIImpl::TellTheWorld(PRBool &showWarning, - lockIconState &warnSecurityState, +nsresult nsSecureBrowserUIImpl::TellTheWorld(PRBool showWarning, + lockIconState warnSecurityState, nsIRequest* aRequest) { nsCOMPtr temp_ToplevelEventSink; @@ -1322,6 +1401,9 @@ nsSecureBrowserUIImpl::OnLocationChange(nsIWebProgress* aWebProgress, NS_ASSERTION(mOnStateLocationChangeReentranceDetection == 1, "unexpected parallel nsIWebProgress OnStateChange and/or OnLocationChange notification"); + PR_LOG(gSecureDocLog, PR_LOG_DEBUG, + ("SecureUI:%p: OnLocationChange\n", this)); + PRBool updateIsViewSource = PR_FALSE; PRBool temp_IsViewSource = PR_FALSE; nsCOMPtr window; @@ -1368,13 +1450,18 @@ nsSecureBrowserUIImpl::OnLocationChange(nsIWebProgress* aWebProgress, nsCOMPtr windowForProgress; aWebProgress->GetDOMWindow(getter_AddRefs(windowForProgress)); + nsCOMPtr securityInfo; + nsCOMPtr channel(do_QueryInterface(aRequest)); + if (channel) + channel->GetSecurityInfo(getter_AddRefs(securityInfo)); + if (windowForProgress.get() == window.get()) { // For toplevel channels, update the security state right away. - return EvaluateAndUpdateSecurityState(aRequest); + return EvaluateAndUpdateSecurityState(aRequest, securityInfo); } // For channels in subdocuments we only update our subrequest state members. - UpdateSubrequestMembers(aRequest); + UpdateSubrequestMembers(securityInfo); // Care for the following scenario: diff --git a/security/manager/boot/src/nsSecureBrowserUIImpl.h b/security/manager/boot/src/nsSecureBrowserUIImpl.h index 7e7baf510b25..d3c4bb420780 100644 --- a/security/manager/boot/src/nsSecureBrowserUIImpl.h +++ b/security/manager/boot/src/nsSecureBrowserUIImpl.h @@ -58,6 +58,7 @@ #include "nsISecurityEventSink.h" #include "nsWeakReference.h" #include "nsISSLStatusProvider.h" +#include "nsIAssociatedContentSecurity.h" #include "pldhash.h" #include "prmon.h" @@ -120,7 +121,6 @@ protected: nsXPIDLString mInfoTooltip; PRInt32 mDocumentRequestsInProgress; - PRInt32 mSubRequestsInProgress; PRInt32 mSubRequestsHighSecurity; PRInt32 mSubRequestsLowSecurity; PRInt32 mSubRequestsBrokenSecurity; @@ -129,17 +129,18 @@ protected: static nsresult MapInternalToExternalState(PRUint32* aState, lockIconState lock, PRBool ev); nsresult UpdateSecurityState(nsIRequest* aRequest); void UpdateMyFlags(PRBool &showWarning, lockIconState &warnSecurityState); - nsresult TellTheWorld(PRBool &showWarning, - lockIconState &warnSecurityState, + nsresult TellTheWorld(PRBool showWarning, + lockIconState warnSecurityState, nsIRequest* aRequest); - nsresult EvaluateAndUpdateSecurityState(nsIRequest *aRequest); - void UpdateSubrequestMembers(nsIRequest *aRequest); + nsresult EvaluateAndUpdateSecurityState(nsIRequest* aRequest, nsISupports *info); + void UpdateSubrequestMembers(nsISupports *securityInfo); void ObtainEventSink(nsIChannel *channel, nsCOMPtr &sink); - + nsCOMPtr mSSLStatus; + nsCOMPtr mCurrentToplevelSecurityInfo; void GetBundleString(const PRUnichar* name, nsAString &outString); diff --git a/security/manager/ssl/public/Makefile.in b/security/manager/ssl/public/Makefile.in index b3b4795c3253..b65e357fd69b 100644 --- a/security/manager/ssl/public/Makefile.in +++ b/security/manager/ssl/public/Makefile.in @@ -62,6 +62,7 @@ XPIDLSRCS = \ nsIBadCertListener2.idl \ nsISSLErrorListener.idl \ nsIIdentityInfo.idl \ + nsIAssociatedContentSecurity.idl \ nsICertOverrideService.idl \ nsIRecentBadCertsService.idl \ nsIFormSigningDialog.idl \ diff --git a/security/manager/ssl/public/nsIAssociatedContentSecurity.idl b/security/manager/ssl/public/nsIAssociatedContentSecurity.idl new file mode 100644 index 000000000000..4d15018e1a8c --- /dev/null +++ b/security/manager/ssl/public/nsIAssociatedContentSecurity.idl @@ -0,0 +1,56 @@ +/* ***** BEGIN LICENSE BLOCK ***** + * Version: MPL 1.1/GPL 2.0/LGPL 2.1 + * + * The contents of this file are subject to the Mozilla Public License Version + * 1.1 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * http://www.mozilla.org/MPL/ + * + * Software distributed under the License is distributed on an "AS IS" basis, + * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License + * for the specific language governing rights and limitations under the + * License. + * + * The Original Code is mozilla.org code. + * + * The Initial Developer of the Original Code is Google Inc. + * Portions created by the Initial Developer are Copyright (C) 2006 + * the Initial Developer. All Rights Reserved. + * + * Contributor(s): + * Honza Bambas + * + * Alternatively, the contents of this file may be used under the terms of + * either the GNU General Public License Version 2 or later (the "GPL"), or + * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), + * in which case the provisions of the GPL or the LGPL are applicable instead + * of those above. If you wish to allow use of your version of this file only + * under the terms of either the GPL or the LGPL, and not to allow others to + * use your version of this file under the terms of the MPL, indicate your + * decision by deleting the provisions above and replace them with the notice + * and other provisions required by the GPL or the LGPL. If you do not delete + * the provisions above, a recipient may use your version of this file under + * the terms of any one of the MPL, the GPL or the LGPL. + * + * ***** END LICENSE BLOCK ***** */ + +/** + * This interface is used to cache associated (sub) content security + * state. That is determined asynchronously based on callback notification + * while loading the content and its sub content particles. + * + * Some optimizations like bfcaching removes these callback notifications + * and therefor the subcontent state could not be determined. In such + * a case it is loaded from this object stored in nsIChannel.securityInfo. + */ + +#include "nsISupports.idl" + +[scriptable, uuid(8DB92DDE-799F-4d33-80F7-459CAC800DC9)] +interface nsIAssociatedContentSecurity : nsISupports +{ + attribute long countSubRequestsHighSecurity; + attribute long countSubRequestsLowSecurity; + attribute long countSubRequestsBrokenSecurity; + attribute long countSubRequestsNoSecurity; +}; diff --git a/security/manager/ssl/src/nsNSSCallbacks.cpp b/security/manager/ssl/src/nsNSSCallbacks.cpp index f96a8e0947ce..0e4616e66483 100644 --- a/security/manager/ssl/src/nsNSSCallbacks.cpp +++ b/security/manager/ssl/src/nsNSSCallbacks.cpp @@ -80,12 +80,13 @@ NSSCleanupAutoPtrClass(CERTCertificate, CERT_DestroyCertificate) extern PRLogModuleInfo* gPIPNSSLog; #endif -struct nsHTTPDownloadEvent : nsRunnable { +class nsHTTPDownloadEvent : public nsRunnable { +public: nsHTTPDownloadEvent(); ~nsHTTPDownloadEvent(); NS_IMETHOD Run(); - + nsNSSHttpRequestSession *mRequestSession; // no ownership nsCOMPtr mListener; @@ -120,9 +121,8 @@ nsHTTPDownloadEvent::Run() // Create a loadgroup for this new channel. This way if the channel // is redirected, we'll have a way to cancel the resulting channel. - nsCOMPtr loadGroup = - do_CreateInstance(NS_LOADGROUP_CONTRACTID); - chan->SetLoadGroup(loadGroup); + nsCOMPtr lg = do_CreateInstance(NS_LOADGROUP_CONTRACTID); + chan->SetLoadGroup(lg); if (mRequestSession->mHasPostData) { @@ -148,11 +148,13 @@ nsHTTPDownloadEvent::Run() rv = hchan->SetRequestMethod(mRequestSession->mRequestMethod); NS_ENSURE_SUCCESS(rv, rv); - nsSSLThread::rememberPendingHTTPRequest(loadGroup); - mResponsibleForDoneSignal = PR_FALSE; mListener->mResponsibleForDoneSignal = PR_TRUE; + mListener->mLoadGroup = lg.get(); + NS_ADDREF(mListener->mLoadGroup); + mListener->mLoadGroupOwnerThread = PR_GetCurrentThread(); + rv = NS_NewStreamLoader(getter_AddRefs(mListener->mLoader), mListener); @@ -162,16 +164,21 @@ nsHTTPDownloadEvent::Run() if (NS_FAILED(rv)) { mListener->mResponsibleForDoneSignal = PR_FALSE; mResponsibleForDoneSignal = PR_TRUE; - - nsSSLThread::rememberPendingHTTPRequest(nsnull); + + NS_RELEASE(mListener->mLoadGroup); + mListener->mLoadGroup = nsnull; + mListener->mLoadGroupOwnerThread = nsnull; } return NS_OK; } struct nsCancelHTTPDownloadEvent : nsRunnable { + nsCOMPtr mListener; + NS_IMETHOD Run() { - nsSSLThread::cancelPendingHTTPRequest(); + mListener->FreeLoadGroup(PR_TRUE); + mListener = nsnull; return NS_OK; } }; @@ -367,7 +374,6 @@ nsNSSHttpRequestSession::internal_send_receive_attempt(PRBool &retryable_error, } PRBool request_canceled = PR_FALSE; - PRBool aborted_wait = PR_FALSE; { nsAutoLock locker(waitLock); @@ -410,30 +416,26 @@ nsNSSHttpRequestSession::internal_send_receive_attempt(PRBool &retryable_error, if (!request_canceled) { - if ((PRIntervalTime)(PR_IntervalNow() - start_time) > mTimeoutInterval) + PRBool wantExit = nsSSLThread::exitRequested(); + PRBool timeout = + (PRIntervalTime)(PR_IntervalNow() - start_time) > mTimeoutInterval; + + if (wantExit || timeout) { request_canceled = PR_TRUE; - // but we'll to continue to wait for waitFlag - - nsCOMPtr cancelevent = new nsCancelHTTPDownloadEvent; + + nsRefPtr cancelevent = new nsCancelHTTPDownloadEvent; + cancelevent->mListener = mListener; rv = NS_DispatchToMainThread(cancelevent); - if (NS_FAILED(rv)) - { + if (NS_FAILED(rv)) { NS_WARNING("cannot post cancel event"); - aborted_wait = PR_TRUE; - break; } + break; } } } } - if (aborted_wait) - { - // we couldn't cancel it, let's no longer reference it - nsSSLThread::rememberPendingHTTPRequest(nsnull); - } - if (request_canceled) return SECFailure; @@ -535,7 +537,9 @@ nsHTTPListener::nsHTTPListener() mLock(nsnull), mCondition(nsnull), mWaitFlag(PR_TRUE), - mResponsibleForDoneSignal(PR_FALSE) + mResponsibleForDoneSignal(PR_FALSE), + mLoadGroup(nsnull), + mLoadGroupOwnerThread(nsnull) { } @@ -575,6 +579,34 @@ nsHTTPListener::~nsHTTPListener() NS_IMPL_THREADSAFE_ISUPPORTS1(nsHTTPListener, nsIStreamLoaderObserver) +void +nsHTTPListener::FreeLoadGroup(PRBool aCancelLoad) +{ + nsILoadGroup *lg = nsnull; + + if (mLock) { + nsAutoLock locker(mLock); + + if (mLoadGroup) { + if (mLoadGroupOwnerThread != PR_GetCurrentThread()) { + NS_ASSERTION(PR_FALSE, + "attempt to access nsHTTPDownloadEvent::mLoadGroup on multiple threads, leaking it!"); + } + else { + lg = mLoadGroup; + mLoadGroup = nsnull; + } + } + } + + if (lg) { + if (aCancelLoad) { + lg->Cancel(NS_ERROR_ABORT); + } + NS_RELEASE(lg); + } +} + NS_IMETHODIMP nsHTTPListener::OnStreamComplete(nsIStreamLoader* aLoader, nsISupports* aContext, @@ -584,6 +616,8 @@ nsHTTPListener::OnStreamComplete(nsIStreamLoader* aLoader, { mResultCode = aStatus; + FreeLoadGroup(PR_FALSE); + nsCOMPtr req; nsCOMPtr hchan; @@ -628,8 +662,6 @@ nsHTTPListener::OnStreamComplete(nsIStreamLoader* aLoader, void nsHTTPListener::send_done_signal() { - nsSSLThread::rememberPendingHTTPRequest(nsnull); - mResponsibleForDoneSignal = PR_FALSE; { diff --git a/security/manager/ssl/src/nsNSSCallbacks.h b/security/manager/ssl/src/nsNSSCallbacks.h index 68d5f358a0f6..abf314df902e 100644 --- a/security/manager/ssl/src/nsNSSCallbacks.h +++ b/security/manager/ssl/src/nsNSSCallbacks.h @@ -86,6 +86,14 @@ public: PRBool mResponsibleForDoneSignal; void send_done_signal(); + + // no nsCOMPtr. When I use it, I get assertions about + // loadgroup not being thread safe. + // So, let's use a raw pointer and ensure we only create and destroy + // it on the network thread ourselves. + nsILoadGroup *mLoadGroup; + PRThread *mLoadGroupOwnerThread; + void FreeLoadGroup(PRBool aCancelLoad); }; class nsNSSHttpServerSession diff --git a/security/manager/ssl/src/nsNSSComponent.cpp b/security/manager/ssl/src/nsNSSComponent.cpp index fe08e35b1a5a..45adcb91b2a1 100644 --- a/security/manager/ssl/src/nsNSSComponent.cpp +++ b/security/manager/ssl/src/nsNSSComponent.cpp @@ -1722,6 +1722,19 @@ nsNSSComponent::Init() return rv; } + // Access our string bundles now, this prevents assertions from I/O + // - nsStandardURL not thread-safe + // - wrong thread: 'NS_IsMainThread()' in nsIOService.cpp + // when loading error strings on the SSL threads. + { + NS_NAMED_LITERAL_STRING(dummy_name, "dummy"); + nsXPIDLString result; + mPIPNSSBundle->GetStringFromName(dummy_name.get(), + getter_Copies(result)); + mNSSErrorsBundle->GetStringFromName(dummy_name.get(), + getter_Copies(result)); + } + if (!mPrefBranch) { mPrefBranch = do_GetService(NS_PREFSERVICE_CONTRACTID); NS_ASSERTION(mPrefBranch, "Unable to get pref service"); diff --git a/security/manager/ssl/src/nsNSSIOLayer.cpp b/security/manager/ssl/src/nsNSSIOLayer.cpp index 74467ec914e8..f581deb10bc1 100644 --- a/security/manager/ssl/src/nsNSSIOLayer.cpp +++ b/security/manager/ssl/src/nsNSSIOLayer.cpp @@ -204,6 +204,11 @@ nsNSSSocketInfo::nsNSSSocketInfo() : mFd(nsnull), mBlockingState(blocking_state_unknown), mSecurityState(nsIWebProgressListener::STATE_IS_INSECURE), + mSubRequestsHighSecurity(0), + mSubRequestsLowSecurity(0), + mSubRequestsBrokenSecurity(0), + mSubRequestsNoSecurity(0), + mDocShellDependentStuffKnown(PR_FALSE), mExternalErrorReporting(PR_FALSE), mForSTARTTLS(PR_FALSE), mHandshakePending(PR_TRUE), @@ -232,12 +237,13 @@ void nsNSSSocketInfo::virtualDestroyNSSReference() { } -NS_IMPL_THREADSAFE_ISUPPORTS7(nsNSSSocketInfo, +NS_IMPL_THREADSAFE_ISUPPORTS8(nsNSSSocketInfo, nsITransportSecurityInfo, nsISSLSocketControl, nsIInterfaceRequestor, nsISSLStatusProvider, nsIIdentityInfo, + nsIAssociatedContentSecurity, nsISerializable, nsIClassInfo) @@ -319,15 +325,30 @@ nsNSSSocketInfo::SetNotificationCallbacks(nsIInterfaceRequestor* aCallbacks) return NS_OK; } + mCallbacks = aCallbacks; + mDocShellDependentStuffKnown = PR_FALSE; + + return NS_OK; +} + +nsresult +nsNSSSocketInfo::EnsureDocShellDependentStuffKnown() +{ + if (mDocShellDependentStuffKnown) + return NS_OK; + + if (!mCallbacks || nsSSLThread::exitRequested()) + return NS_ERROR_FAILURE; + + mDocShellDependentStuffKnown = PR_TRUE; + nsCOMPtr proxiedCallbacks; NS_GetProxyForObject(NS_PROXY_TO_MAIN_THREAD, NS_GET_IID(nsIInterfaceRequestor), - static_cast(aCallbacks), + static_cast(mCallbacks), NS_PROXY_SYNC, getter_AddRefs(proxiedCallbacks)); - mCallbacks = proxiedCallbacks; - // Are we running within a context that wants external SSL error reporting? // We'll look at the presence of a security UI object inside docshell. // If the docshell wants the lock icon, you'll get the ssl error pages, too. @@ -339,7 +360,7 @@ nsNSSSocketInfo::SetNotificationCallbacks(nsIInterfaceRequestor* aCallbacks) nsCOMPtr docshell; - nsCOMPtr item(do_GetInterface(mCallbacks)); + nsCOMPtr item(do_GetInterface(proxiedCallbacks)); if (item) { nsCOMPtr proxiedItem; @@ -395,6 +416,8 @@ nsNSSSocketInfo::SetNotificationCallbacks(nsIInterfaceRequestor* aCallbacks) nsresult nsNSSSocketInfo::GetExternalErrorReporting(PRBool* state) { + nsresult rv = EnsureDocShellDependentStuffKnown(); + NS_ENSURE_SUCCESS(rv, rv); *state = mExternalErrorReporting; return NS_OK; } @@ -420,6 +443,54 @@ nsNSSSocketInfo::SetSecurityState(PRUint32 aState) return NS_OK; } +/* attribute unsigned long countSubRequestsHighSecurity; */ +NS_IMETHODIMP nsNSSSocketInfo::GetCountSubRequestsHighSecurity(PRInt32 *aSubRequestsHighSecurity) +{ + *aSubRequestsHighSecurity = mSubRequestsHighSecurity; + return NS_OK; +} +NS_IMETHODIMP nsNSSSocketInfo::SetCountSubRequestsHighSecurity(PRInt32 aSubRequestsHighSecurity) +{ + mSubRequestsHighSecurity = aSubRequestsHighSecurity; + return NS_ERROR_NOT_IMPLEMENTED; +} + +/* attribute unsigned long countSubRequestsLowSecurity; */ +NS_IMETHODIMP nsNSSSocketInfo::GetCountSubRequestsLowSecurity(PRInt32 *aSubRequestsLowSecurity) +{ + *aSubRequestsLowSecurity = mSubRequestsLowSecurity; + return NS_OK; +} +NS_IMETHODIMP nsNSSSocketInfo::SetCountSubRequestsLowSecurity(PRInt32 aSubRequestsLowSecurity) +{ + mSubRequestsLowSecurity = aSubRequestsLowSecurity; + return NS_OK; +} + +/* attribute unsigned long countSubRequestsBrokenSecurity; */ +NS_IMETHODIMP nsNSSSocketInfo::GetCountSubRequestsBrokenSecurity(PRInt32 *aSubRequestsBrokenSecurity) +{ + *aSubRequestsBrokenSecurity = mSubRequestsBrokenSecurity; + return NS_OK; +} +NS_IMETHODIMP nsNSSSocketInfo::SetCountSubRequestsBrokenSecurity(PRInt32 aSubRequestsBrokenSecurity) +{ + mSubRequestsBrokenSecurity = aSubRequestsBrokenSecurity; + return NS_OK; +} + +/* attribute unsigned long countSubRequestsNoSecurity; */ +NS_IMETHODIMP nsNSSSocketInfo::GetCountSubRequestsNoSecurity(PRInt32 *aSubRequestsNoSecurity) +{ + *aSubRequestsNoSecurity = mSubRequestsNoSecurity; + return NS_OK; +} +NS_IMETHODIMP nsNSSSocketInfo::SetCountSubRequestsNoSecurity(PRInt32 aSubRequestsNoSecurity) +{ + mSubRequestsNoSecurity = aSubRequestsNoSecurity; + return NS_OK; +} + NS_IMETHODIMP nsNSSSocketInfo::GetShortSecurityDescription(PRUnichar** aText) { if (mShortDesc.IsEmpty()) @@ -465,10 +536,17 @@ NS_IMETHODIMP nsNSSSocketInfo::GetInterface(const nsIID & uuid, void * *result) rv = ir->GetInterface(uuid, result); } else { - // Proxy of the channel callbacks should probably go here, rather - // than in the password callback code + if (nsSSLThread::exitRequested()) + return NS_ERROR_FAILURE; - rv = mCallbacks->GetInterface(uuid, result); + nsCOMPtr proxiedCallbacks; + NS_GetProxyForObject(NS_PROXY_TO_MAIN_THREAD, + NS_GET_IID(nsIInterfaceRequestor), + mCallbacks, + NS_PROXY_SYNC, + getter_AddRefs(proxiedCallbacks)); + + rv = proxiedCallbacks->GetInterface(uuid, result); } return rv; } @@ -619,6 +697,8 @@ nsresult nsNSSSocketInfo::SetFileDescPtr(PRFileDesc* aFilePtr) nsresult nsNSSSocketInfo::GetPreviousCert(nsIX509Cert** _result) { NS_ENSURE_ARG_POINTER(_result); + nsresult rv = EnsureDocShellDependentStuffKnown(); + NS_ENSURE_SUCCESS(rv, rv); *_result = mPreviousCert; NS_IF_ADDREF(*_result); @@ -1142,6 +1222,9 @@ displayAlert(nsAFlatString &formattedString, nsNSSSocketInfo *infoObject) // The interface requestor object may not be safe, so proxy the call to get // the nsIPrompt. + if (nsSSLThread::exitRequested()) + return NS_ERROR_FAILURE; + nsCOMPtr proxiedCallbacks; NS_GetProxyForObject(NS_PROXY_TO_MAIN_THREAD, NS_GET_IID(nsIInterfaceRequestor), @@ -1176,6 +1259,10 @@ nsHandleSSLError(nsNSSSocketInfo *socketInfo, PRInt32 err) return NS_OK; } + if (nsSSLThread::exitRequested()) { + return NS_ERROR_FAILURE; + } + nsresult rv; NS_DEFINE_CID(nssComponentCID, NS_NSSCOMPONENT_CID); nsCOMPtr nssComponent(do_GetService(nssComponentCID, &rv)); @@ -1190,9 +1277,16 @@ nsHandleSSLError(nsNSSSocketInfo *socketInfo, PRInt32 err) socketInfo->GetPort(&port); // Try to get a nsISSLErrorListener implementation from the socket consumer. - nsCOMPtr callbacks; - socketInfo->GetNotificationCallbacks(getter_AddRefs(callbacks)); - if (callbacks) { + nsCOMPtr cb; + socketInfo->GetNotificationCallbacks(getter_AddRefs(cb)); + if (cb) { + nsCOMPtr callbacks; + NS_GetProxyForObject(NS_PROXY_TO_MAIN_THREAD, + NS_GET_IID(nsIInterfaceRequestor), + cb, + NS_PROXY_SYNC, + getter_AddRefs(callbacks)); + nsCOMPtr sel = do_GetInterface(callbacks); if (sel) { nsISSLErrorListener *proxy_sel = nsnull; @@ -2746,6 +2840,9 @@ nsNSSBadCertHandler(void *arg, PRFileDesc *sslSocket) if (!infoObject) return SECFailure; + if (nsSSLThread::exitRequested()) + return cancel_and_failure(infoObject); + CERTCertificate *peerCert = nsnull; CERTCertificateCleaner peerCertCleaner(peerCert); peerCert = SSL_PeerCertificate(sslSocket); @@ -2915,9 +3012,16 @@ nsNSSBadCertHandler(void *arg, PRFileDesc *sslSocket) nsresult rv; // Try to get a nsIBadCertListener2 implementation from the socket consumer. - nsCOMPtr callbacks; - infoObject->GetNotificationCallbacks(getter_AddRefs(callbacks)); - if (callbacks) { + nsCOMPtr cb; + infoObject->GetNotificationCallbacks(getter_AddRefs(cb)); + if (cb) { + nsCOMPtr callbacks; + NS_GetProxyForObject(NS_PROXY_TO_MAIN_THREAD, + NS_GET_IID(nsIInterfaceRequestor), + cb, + NS_PROXY_SYNC, + getter_AddRefs(callbacks)); + nsCOMPtr bcl = do_GetInterface(callbacks); if (bcl) { nsIBadCertListener2 *proxy_bcl = nsnull; diff --git a/security/manager/ssl/src/nsNSSIOLayer.h b/security/manager/ssl/src/nsNSSIOLayer.h index 54ea1be056d9..b530d98f9f89 100644 --- a/security/manager/ssl/src/nsNSSIOLayer.h +++ b/security/manager/ssl/src/nsNSSIOLayer.h @@ -52,6 +52,7 @@ #include "nsSSLStatus.h" #include "nsISSLStatusProvider.h" #include "nsIIdentityInfo.h" +#include "nsIAssociatedContentSecurity.h" #include "nsXPIDLString.h" #include "nsNSSShutDown.h" #include "nsAutoPtr.h" @@ -128,6 +129,7 @@ class nsNSSSocketInfo : public nsITransportSecurityInfo, public nsIInterfaceRequestor, public nsISSLStatusProvider, public nsIIdentityInfo, + public nsIAssociatedContentSecurity, public nsISerializable, public nsIClassInfo, public nsNSSShutDownObject, @@ -143,6 +145,7 @@ public: NS_DECL_NSIINTERFACEREQUESTOR NS_DECL_NSISSLSTATUSPROVIDER NS_DECL_NSIIDENTITYINFO + NS_DECL_NSIASSOCIATEDCONTENTSECURITY NS_DECL_NSISERIALIZABLE NS_DECL_NSICLASSINFO @@ -198,14 +201,19 @@ protected: nsCOMPtr mCallbacks; PRFileDesc* mFd; nsCOMPtr mCert; - nsCOMPtr mPreviousCert; + nsCOMPtr mPreviousCert; // DocShellDependent enum { blocking_state_unknown, is_nonblocking_socket, is_blocking_socket } mBlockingState; PRUint32 mSecurityState; + PRInt32 mSubRequestsHighSecurity; + PRInt32 mSubRequestsLowSecurity; + PRInt32 mSubRequestsBrokenSecurity; + PRInt32 mSubRequestsNoSecurity; nsString mShortDesc; nsString mErrorMessage; - PRPackedBool mExternalErrorReporting; + PRPackedBool mDocShellDependentStuffKnown; + PRPackedBool mExternalErrorReporting; // DocShellDependent PRPackedBool mForSTARTTLS; PRPackedBool mHandshakePending; PRPackedBool mCanceled; @@ -223,6 +231,8 @@ protected: nsSSLSocketThreadData *mThreadData; + nsresult EnsureDocShellDependentStuffKnown(); + private: virtual void virtualDestroyNSSReference(); void destructorSafeDestroyNSSReference(); diff --git a/security/manager/ssl/src/nsSSLThread.cpp b/security/manager/ssl/src/nsSSLThread.cpp index 1304f695f664..8c5ccc3aa890 100644 --- a/security/manager/ssl/src/nsSSLThread.cpp +++ b/security/manager/ssl/src/nsSSLThread.cpp @@ -1120,28 +1120,14 @@ void nsSSLThread::Run(void) } } -void nsSSLThread::rememberPendingHTTPRequest(nsIRequest *aRequest) +PRBool nsSSLThread::exitRequested() { if (!ssl_thread_singleton) - return; + return PR_FALSE; - nsAutoLock threadLock(ssl_thread_singleton->mMutex); + // no lock - ssl_thread_singleton->mPendingHTTPRequest = aRequest; -} - -void nsSSLThread::cancelPendingHTTPRequest() -{ - if (!ssl_thread_singleton) - return; - - nsAutoLock threadLock(ssl_thread_singleton->mMutex); - - if (ssl_thread_singleton->mPendingHTTPRequest) - { - ssl_thread_singleton->mPendingHTTPRequest->Cancel(NS_ERROR_ABORT); - ssl_thread_singleton->mPendingHTTPRequest = nsnull; - } + return ssl_thread_singleton->mExitRequested; } nsSSLThread *nsSSLThread::ssl_thread_singleton = nsnull; diff --git a/security/manager/ssl/src/nsSSLThread.h b/security/manager/ssl/src/nsSSLThread.h index 5e60f11a52a3..d91b2ee77278 100644 --- a/security/manager/ssl/src/nsSSLThread.h +++ b/security/manager/ssl/src/nsSSLThread.h @@ -152,9 +152,7 @@ public: static nsresult requestActivateSSL(nsNSSSocketInfo *si); - // Called from either Necko or SSL thread. - static void rememberPendingHTTPRequest(nsIRequest *aRequest); - static void cancelPendingHTTPRequest(); + static PRBool exitRequested(); }; #endif //_NSSSLTHREAD_H_