Attachment 606511 Details for Bug 851089 – patch v4 (same as attached upstream)

[patch] patch v4 (same as attached upstream)

p4-783974 (text/plain), 20.73 KB, created by Kai Engert (:kaie) (inactive account) on 2012-08-23 10:50:02 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kai Engert (:kaie) (inactive account)

Created: 2012-08-23 10:50:02 UTC

Size: 20.73 KB

patch

obsolete

>diff --git a/security/manager/ssl/src/SSLServerCertVerification.cpp b/security/manager/ssl/src/SSLServerCertVerification.cpp
>--- a/security/manager/ssl/src/SSLServerCertVerification.cpp
>+++ b/security/manager/ssl/src/SSLServerCertVerification.cpp
>@@ -103,16 +103,17 @@
> #include "nsRecentBadCerts.h"
> #include "nsNSSIOLayer.h"
> 
> #include "mozilla/Assertions.h"
> #include "nsIThreadPool.h"
> #include "nsXPCOMCIDInternal.h"
> #include "nsComponentManagerUtils.h"
> #include "nsServiceManagerUtils.h"
>+#include "nsIConsoleService.h"
> #include "PSMRunnable.h"
> 
> #include "ssl.h"
> #include "secerr.h"
> #include "secport.h"
> #include "sslerr.h"
> 
> #ifdef PR_LOGGING
>@@ -176,16 +177,37 @@ void StopSSLServerCertVerificationThread
>   if (gCertVerificationThreadPool) {
>     gCertVerificationThreadPool->Shutdown();
>     NS_RELEASE(gCertVerificationThreadPool);
>   }
> }
> 
> namespace {
> 
>+void
>+LogInvalidCertError(TransportSecurityInfo *socketInfo, 
>+                    const nsACString &host, 
>+                    const nsACString &hostWithPort,
>+                    PRInt32 port,
>+                    PRErrorCode errorCode,
>+                    ::mozilla::psm::SSLErrorMessageType errorMessageType,
>+                    nsIX509Cert* ix509)
>+{
>+  nsString message;
>+  socketInfo->GetErrorLogMessage(errorCode, errorMessageType, false, false, message);
>+  
>+  if (!message.IsEmpty()) {
>+    nsCOMPtr<nsIConsoleService> console;
>+    console = do_GetService(NS_CONSOLESERVICE_CONTRACTID);
>+    if (console) {
>+      console->LogStringMessage(message.get());
>+    }
>+  }
>+}
>+
> // Dispatched to the STS thread to notify the infoObject of the verification
> // result.
> //
> // This will cause the PR_Poll in the STS thread to return, so things work
> // correctly even if the STS thread is blocked polling (only) on the file
> // descriptor that is waiting for this result.
> class SSLServerCertVerificationResult : public nsRunnable
> {
>@@ -350,19 +372,31 @@ CertErrorRunnable::CheckCertOverrides()
>                                       mInfoObject->SSLStatus());
>   }
> 
>   // pick the error code to report by priority
>   PRErrorCode errorCodeToReport = mErrorCodeTrust    ? mErrorCodeTrust
>                                 : mErrorCodeMismatch ? mErrorCodeMismatch
>                                 : mErrorCodeExpired  ? mErrorCodeExpired
>                                 : mDefaultErrorCodeToReport;
>+                                
>+  SSLServerCertVerificationResult *result = 
>+    new SSLServerCertVerificationResult(mInfoObject, 
>+                                        errorCodeToReport,
>+                                        OverridableCertErrorMessage);
> 
>-  return new SSLServerCertVerificationResult(mInfoObject, errorCodeToReport,
>-                                             OverridableCertErrorMessage);
>+  LogInvalidCertError(mInfoObject,
>+                      nsDependentCString(mInfoObject->GetHostName()),
>+                      hostWithPortString,
>+                      port,
>+                      result->mErrorCode,
>+                      result->mErrorMessageType,
>+                      mCert);
>+
>+  return result;
> }
> 
> void 
> CertErrorRunnable::RunOnTargetThread()
> {
>   MOZ_ASSERT(NS_IsMainThread());
> 
>   mResult = CheckCertOverrides();
>diff --git a/security/manager/ssl/src/TransportSecurityInfo.cpp b/security/manager/ssl/src/TransportSecurityInfo.cpp
>--- a/security/manager/ssl/src/TransportSecurityInfo.cpp
>+++ b/security/manager/ssl/src/TransportSecurityInfo.cpp
>@@ -240,62 +240,96 @@ TransportSecurityInfo::GetErrorMessage(P
> 
>   if (!NS_IsMainThread()) {
>     NS_ERROR("nsNSSSocketInfo::GetErrorMessage called off the main thread");
>     return NS_ERROR_NOT_SAME_THREAD;
>   }
> 
>   MutexAutoLock lock(mMutex);
> 
>-  nsresult rv = formatErrorMessage(lock);
>-  NS_ENSURE_SUCCESS(rv, rv);
>+  if (mErrorMessageCached.IsEmpty()) {
>+    nsresult rv = formatErrorMessage(lock, 
>+                                     mErrorCode, mErrorMessageType,
>+                                     true, true, mErrorMessageCached);
>+    NS_ENSURE_SUCCESS(rv, rv);
>+  }
> 
>   *aText = ToNewUnicode(mErrorMessageCached);
>   return *aText != nullptr ? NS_OK : NS_ERROR_OUT_OF_MEMORY;
> }
> 
>+void
>+TransportSecurityInfo::GetErrorLogMessage(PRErrorCode errorCode,
>+                                          SSLErrorMessageType errorMessageType,
>+                                          bool wantsHtml,
>+                                          bool suppressPort443,
>+                                          nsString &result)
>+{
>+  if (!NS_IsMainThread()) {
>+    NS_ERROR("nsNSSSocketInfo::GetErrorLogMessage called off the main thread");
>+    return;
>+  }
>+
>+  MutexAutoLock lock(mMutex);
>+  (void) formatErrorMessage(lock, errorCode, errorMessageType,
>+                            wantsHtml, suppressPort443, result);
>+}
>+
> static nsresult
> formatPlainErrorMessage(nsXPIDLCString const & host, PRInt32 port,
>-                        PRErrorCode err, nsString &returnedMessage);
>+                        PRErrorCode err, 
>+                        bool suppressPort443,
>+                        nsString &returnedMessage);
> 
> static nsresult
> formatOverridableCertErrorMessage(nsISSLStatus & sslStatus,
>                                   PRErrorCode errorCodeToReport, 
>                                   const nsXPIDLCString & host, PRInt32 port,
>+                                  bool suppressPort443,
>+                                  bool wantsHtml,
>                                   nsString & returnedMessage);
> 
> // XXX: uses nsNSSComponent string bundles off the main thread when called by
> //      nsNSSSocketInfo::Write(). When we remove the error message from the
> //      serialization of nsNSSSocketInfo (bug 697781) we can inline
>-//      formatErrorMessage into GetErrorMessage().
>+//      formatErrorMessage into GetErrorLogMessage().
> nsresult
>-TransportSecurityInfo::formatErrorMessage(MutexAutoLock const & proofOfLock)
>+TransportSecurityInfo::formatErrorMessage(MutexAutoLock const & proofOfLock, 
>+                                          PRErrorCode errorCode,
>+                                          SSLErrorMessageType errorMessageType,
>+                                          bool wantsHtml, bool suppressPort443, 
>+                                          nsString &result)
> {
>-  if (mErrorCode == 0 || !mErrorMessageCached.IsEmpty()) {
>+  if (errorCode == 0) {
>+    result.Truncate();
>     return NS_OK;
>   }
> 
>   nsresult rv;
>   NS_ConvertASCIItoUTF16 hostNameU(mHostName);
>-  NS_ASSERTION(mErrorMessageType != OverridableCertErrorMessage || 
>+  NS_ASSERTION(errorMessageType != OverridableCertErrorMessage || 
>                 (mSSLStatus && mSSLStatus->mServerCert &&
>                  mSSLStatus->mHaveCertErrorBits),
>-                "GetErrorMessage called for cert error without cert");
>-  if (mErrorMessageType == OverridableCertErrorMessage && 
>+                "GetErrorLogMessage called for cert error without cert");
>+  if (errorMessageType == OverridableCertErrorMessage && 
>       mSSLStatus && mSSLStatus->mServerCert) {
>-    rv = formatOverridableCertErrorMessage(*mSSLStatus, mErrorCode,
>+    rv = formatOverridableCertErrorMessage(*mSSLStatus, errorCode,
>                                            mHostName, mPort,
>-                                           mErrorMessageCached);
>+                                           suppressPort443,
>+                                           wantsHtml,
>+                                           result);
>   } else {
>-    rv = formatPlainErrorMessage(mHostName, mPort, mErrorCode,
>-                                 mErrorMessageCached);
>+    rv = formatPlainErrorMessage(mHostName, mPort, 
>+                                 errorCode,
>+                                 suppressPort443,
>+                                 result);
>   }
> 
>   if (NS_FAILED(rv)) {
>-    mErrorMessageCached.Truncate();
>+    result.Truncate();
>   }
> 
>   return rv;
> }
> 
> /* void getInterface (in nsIIDRef uuid, [iid_is (uuid), retval] out nsQIResult result); */
> NS_IMETHODIMP
> TransportSecurityInfo::GetInterface(const nsIID & uuid, void * *result)
>@@ -367,17 +401,19 @@ TransportSecurityInfo::Write(nsIObjectOu
>   // This mask value has been chosen as mSecurityState could
>   // never be assigned such value.
>   PRUint32 version = 3;
>   stream->Write32(version | 0xFFFF0000);
>   stream->Write32(mSecurityState);
>   stream->WriteWStringZ(mShortDesc.get());
> 
>   // XXX: uses nsNSSComponent string bundles off the main thread
>-  nsresult rv = formatErrorMessage(lock); 
>+  nsresult rv = formatErrorMessage(lock, 
>+                                   mErrorCode, mErrorMessageType,
>+                                   true, true, mErrorMessageCached);
>   NS_ENSURE_SUCCESS(rv, rv);
>   stream->WriteWStringZ(mErrorMessageCached.get());
> 
>   stream->WriteCompoundObject(NS_ISUPPORTS_CAST(nsISSLStatus*, status),
>                               NS_GET_IID(nsISupports), true);
> 
>   stream->Write32((PRUint32)mSubRequestsHighSecurity);
>   stream->Write32((PRUint32)mSubRequestsLowSecurity);
>@@ -580,17 +616,19 @@ TransportSecurityInfo::SetSSLStatus(nsSS
> 
> /* Formats an error message for non-certificate-related SSL errors
>  * and non-overridable certificate errors (both are of type
>  * PlainErrormMessage). Use formatOverridableCertErrorMessage
>  * for overridable cert errors.
>  */
> static nsresult
> formatPlainErrorMessage(const nsXPIDLCString &host, PRInt32 port,
>-                        PRErrorCode err, nsString &returnedMessage)
>+                        PRErrorCode err, 
>+                        bool suppressPort443,
>+                        nsString &returnedMessage)
> {
>   const PRUnichar *params[1];
>   nsresult rv;
> 
>   nsCOMPtr<nsINSSComponent> component = do_GetService(kNSSComponentCID, &rv);
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   if (host.Length())
>@@ -600,17 +638,17 @@ formatPlainErrorMessage(const nsXPIDLCSt
>     // For now, hide port when it's 443 and we're reporting the error.
>     // In the future a better mechanism should be used
>     // to make a decision about showing the port number, possibly by requiring
>     // the context object to implement a specific interface.
>     // The motivation is that Mozilla browser would like to hide the port number
>     // in error pages in the common case.
> 
>     hostWithPort.AssignASCII(host);
>-    if (port != 443) {
>+    if (!suppressPort443 || port != 443) {
>       hostWithPort.AppendLiteral(":");
>       hostWithPort.AppendInt(port);
>     }
>     params[0] = hostWithPort.get();
> 
>     nsString formattedString;
>     rv = component->PIPBundleFormatStringFromName("SSLConnectionErrorPrefix", 
>                                                   params, 1, 
>@@ -775,16 +813,17 @@ GetSubjectAltNames(CERTCertificate *nssC
>   PORT_FreeArena(san_arena, false);
>   return true;
> }
> 
> static void
> AppendErrorTextMismatch(const nsString &host,
>                         nsIX509Cert* ix509,
>                         nsINSSComponent *component,
>+                        bool wantsHtml,
>                         nsString &returnedMessage)
> {
>   const PRUnichar *params[1];
>   nsresult rv;
> 
>   CERTCertificate *nssCert = NULL;
>   CERTCertificateCleaner nssCertCleaner(nssCert);
> 
>@@ -835,23 +874,30 @@ AppendErrorTextMismatch(const nsString &
>     if (NS_SUCCEEDED(rv)) {
>       returnedMessage.Append(message);
>       returnedMessage.Append(NS_LITERAL_STRING("\n  "));
>       returnedMessage.Append(allNames);
>       returnedMessage.Append(NS_LITERAL_STRING("  \n"));
>     }
>   }
>   else if (nameCount == 1) {
>-    const PRUnichar *params[1];
>-    params[0] = allNames.get();
>-
>     nsString formattedString;
>-    rv = component->PIPBundleFormatStringFromName("certErrorMismatchSingle2", 
>-                                                  params, 1, 
>-                                                  formattedString);
>+    
>+    if (wantsHtml) {
>+      const PRUnichar *params[1];
>+      params[0] = allNames.get();
>+      rv = component->PIPBundleFormatStringFromName("certErrorMismatchSingle2",
>+                                                    params, 1, 
>+                                                    formattedString);
>+    }
>+    else {
>+      formattedString.Append(NS_LITERAL_STRING("The certificate is only valid for "));
>+      formattedString.Append(allNames.get());
>+    }
>+    
>     if (NS_SUCCEEDED(rv)) {
>       returnedMessage.Append(formattedString);
>       returnedMessage.Append(NS_LITERAL_STRING("\n"));
>     }
>   }
>   else { // nameCount == 0
>     nsString message;
>     nsresult rv = component->GetPIPNSSBundleString("certErrorMismatchNoNames",
>@@ -973,32 +1019,34 @@ AppendErrorTextCode(PRErrorCode errorCod
> /* Formats an error message for overridable certificate errors (of type
>  * OverridableCertErrorMessage). Use formatPlainErrorMessage to format
>  * non-overridable cert errors and non-cert-related errors.
>  */
> static nsresult
> formatOverridableCertErrorMessage(nsISSLStatus & sslStatus,
>                                   PRErrorCode errorCodeToReport, 
>                                   const nsXPIDLCString & host, PRInt32 port,
>+                                  bool suppressPort443,
>+                                  bool wantsHtml,
>                                   nsString & returnedMessage)
> {
>   const PRUnichar *params[1];
>   nsresult rv;
>   nsAutoString hostWithPort;
>   nsAutoString hostWithoutPort;
> 
>   // For now, hide port when it's 443 and we're reporting the error.
>   // In the future a better mechanism should be used
>   // to make a decision about showing the port number, possibly by requiring
>   // the context object to implement a specific interface.
>   // The motivation is that Mozilla browser would like to hide the port number
>   // in error pages in the common case.
>   
>   hostWithoutPort.AppendASCII(host);
>-  if (port == 443) {
>+  if (suppressPort443 && port == 443) {
>     params[0] = hostWithoutPort.get();
>   } else {
>     hostWithPort.AppendASCII(host);
>     hostWithPort.Append(':');
>     hostWithPort.AppendInt(port);
>     params[0] = hostWithPort.get();
>   }
> 
>@@ -1023,17 +1071,17 @@ formatOverridableCertErrorMessage(nsISSL
>     AppendErrorTextUntrusted(errorCodeToReport, hostWithoutPort, ix509, 
>                              component, returnedMessage);
>   }
> 
>   bool isDomainMismatch;
>   rv = sslStatus.GetIsDomainMismatch(&isDomainMismatch);
>   NS_ENSURE_SUCCESS(rv, rv);
>   if (isDomainMismatch) {
>-    AppendErrorTextMismatch(hostWithoutPort, ix509, component, returnedMessage);
>+    AppendErrorTextMismatch(hostWithoutPort, ix509, component, wantsHtml, returnedMessage);
>   }
> 
>   bool isNotValidAtThisTime;
>   rv = sslStatus.GetIsNotValidAtThisTime(&isNotValidAtThisTime);
>   NS_ENSURE_SUCCESS(rv, rv);
>   if (isNotValidAtThisTime) {
>     AppendErrorTextTime(ix509, component, returnedMessage);
>   }
>diff --git a/security/manager/ssl/src/TransportSecurityInfo.h b/security/manager/ssl/src/TransportSecurityInfo.h
>--- a/security/manager/ssl/src/TransportSecurityInfo.h
>+++ b/security/manager/ssl/src/TransportSecurityInfo.h
>@@ -54,16 +54,23 @@ public:
>   nsresult GetHostName(char **aHostName);
>   nsresult SetHostName(const char *aHostName);
> 
>   PRInt32 GetPort() const { return mPort; }
>   nsresult GetPort(PRInt32 *aPort);
>   nsresult SetPort(PRInt32 aPort);
> 
>   PRErrorCode GetErrorCode() const;
>+  
>+  void GetErrorLogMessage(PRErrorCode errorCode,
>+                          ::mozilla::psm::SSLErrorMessageType errorMessageType,
>+                          bool wantsHtml,
>+                          bool suppressPort443,
>+                          nsString &result);
>+  
>   void SetCanceled(PRErrorCode errorCode,
>                    ::mozilla::psm::SSLErrorMessageType errorMessageType);
>   
>   /* Set SSL Status values */
>   nsresult SetSSLStatus(nsSSLStatus *aSSLStatus);
>   nsSSLStatus* SSLStatus() { return mSSLStatus; }
>   void SetStatusErrorBits(nsIX509Cert & cert, PRUint32 collected_errors);
> 
>@@ -86,17 +93,21 @@ private:
>   PRInt32 mSubRequestsLowSecurity;
>   PRInt32 mSubRequestsBrokenSecurity;
>   PRInt32 mSubRequestsNoSecurity;
>   nsString mShortDesc;
> 
>   PRErrorCode mErrorCode;
>   ::mozilla::psm::SSLErrorMessageType mErrorMessageType;
>   nsString mErrorMessageCached;
>-  nsresult formatErrorMessage(::mozilla::MutexAutoLock const & proofOfLock);
>+  nsresult formatErrorMessage(::mozilla::MutexAutoLock const & proofOfLock, 
>+                              PRErrorCode errorCode,
>+                              ::mozilla::psm::SSLErrorMessageType errorMessageType,
>+                              bool wantsHtml, bool suppressPort443, 
>+                              nsString &result);
> 
>   PRInt32 mPort;
>   nsXPIDLCString mHostName;
>   PRErrorCode mIsCertIssuerBlacklisted;
> 
>   /* SSL Status */
>   nsRefPtr<nsSSLStatus> mSSLStatus;
> 
>diff --git a/security/manager/ssl/src/nsNSSIOLayer.cpp b/security/manager/ssl/src/nsNSSIOLayer.cpp
>--- a/security/manager/ssl/src/nsNSSIOLayer.cpp
>+++ b/security/manager/ssl/src/nsNSSIOLayer.cpp
>@@ -18,16 +18,17 @@
> #include "SSLServerCertVerification.h"
> #include "nsNSSCertHelper.h"
> #include "nsNSSCleaner.h"
> #include "nsIDocShell.h"
> #include "nsIDocShellTreeItem.h"
> #include "nsISecureBrowserUI.h"
> #include "nsIInterfaceRequestorUtils.h"
> #include "nsCharSeparatedTokenizer.h"
>+#include "nsIConsoleService.h"
> #include "PSMRunnable.h"
> 
> #include "ssl.h"
> #include "secerr.h"
> #include "sslerr.h"
> #include "secder.h"
> #include "keyhi.h"
> 
>@@ -480,17 +481,19 @@ void nsSSLIOLayerHelpers::Cleanup()
> 
>   if (mutex) {
>     delete mutex;
>     mutex = nullptr;
>   }
> }
> 
> static void
>-nsHandleSSLError(nsNSSSocketInfo *socketInfo, PRErrorCode err)
>+nsHandleSSLError(nsNSSSocketInfo *socketInfo, 
>+                 ::mozilla::psm::SSLErrorMessageType errtype, 
>+                 PRErrorCode err)
> {
>   if (!NS_IsMainThread()) {
>     NS_ERROR("nsHandleSSLError called off the main thread");
>     return;
>   }
> 
>   // SetCanceled is only called by the main thread or the socket transport
>   // thread. Whenever this function is called on the main thread, the SSL
>@@ -522,20 +525,34 @@ nsHandleSSLError(nsNSSSocketInfo *socket
>     if (sel) {
>       nsIInterfaceRequestor *csi = static_cast<nsIInterfaceRequestor*>(socketInfo);
>       nsCString hostWithPortString = hostName;
>       hostWithPortString.AppendLiteral(":");
>       hostWithPortString.AppendInt(port);
>     
>       bool suppressMessage = false; // obsolete, ignored
>       rv = sel->NotifySSLError(csi, err, hostWithPortString, &suppressMessage);
>+      if (NS_FAILED(rv))
>+        suppressMessage = false;
>     }
>   }
>-
>+  
>+  // We must cancel first, which sets the error code,
>+  // only afterwards the call to GetErrorMessage will work correctly.
>   socketInfo->SetCanceled(err, PlainErrorMessage);
>+  nsXPIDLString errorString;
>+  socketInfo->GetErrorMessage(getter_Copies(errorString));
>+  
>+  if (!errorString.IsEmpty()) {
>+    nsCOMPtr<nsIConsoleService> console;
>+    console = do_GetService(NS_CONSOLESERVICE_CONTRACTID);
>+    if (console) {
>+      console->LogStringMessage(errorString.get());
>+    }
>+  }
> }
> 
> namespace {
> 
> enum Operation { reading, writing, not_reading_or_writing };
> 
> PRInt32 checkHandshake(PRInt32 bytesTransfered, bool wasReading,
>                        PRFileDesc* ssl_layer_fd,
>@@ -801,27 +818,32 @@ isTLSIntoleranceError(PRInt32 err, bool 
>   }
>   
>   return false;
> }
> 
> class SSLErrorRunnable : public SyncRunnableBase
> {
>  public:
>-  SSLErrorRunnable(nsNSSSocketInfo * infoObject, PRErrorCode errorCode)
>-    : mInfoObject(infoObject), mErrorCode(errorCode)
>+  SSLErrorRunnable(nsNSSSocketInfo * infoObject, 
>+                   ::mozilla::psm::SSLErrorMessageType errtype, 
>+                   PRErrorCode errorCode)
>+    : mInfoObject(infoObject)
>+    , mErrType(errtype)
>+    , mErrorCode(errorCode)
>   {
>   }
> 
>   virtual void RunOnTargetThread()
>   {
>-    nsHandleSSLError(mInfoObject, mErrorCode);
>+    nsHandleSSLError(mInfoObject, mErrType, mErrorCode);
>   }
>   
>   nsRefPtr<nsNSSSocketInfo> mInfoObject;
>+  ::mozilla::psm::SSLErrorMessageType mErrType;
>   const PRErrorCode mErrorCode;
> };
> 
> namespace {
> 
> PRInt32 checkHandshake(PRInt32 bytesTransfered, bool wasReading,
>                        PRFileDesc* ssl_layer_fd,
>                        nsNSSSocketInfo *socketInfo)
>@@ -885,16 +907,17 @@ PRInt32 checkHandshake(PRInt32 bytesTran
>     // do the synchronous dispatch to the main thread unnecessarily after we've
>     // already handled a certificate error. (SSLErrorRunnable calls
>     // nsHandleSSLError, which has logic to avoid replacing the error message,
>     // so without the !socketInfo->GetErrorCode(), it would just be an
>     // expensive no-op.)
>     if (!wantRetry && (IS_SSL_ERROR(err) || IS_SEC_ERROR(err)) &&
>         !socketInfo->GetErrorCode()) {
>       nsRefPtr<SyncRunnableBase> runnable = new SSLErrorRunnable(socketInfo,
>+                                                                 PlainErrorMessage,
>                                                                  err);
>       (void) runnable->DispatchToMainThreadAndWait();
>     }
>   }
>   else if (wasReading && 0 == bytesTransfered) // zero bytes on reading, socket closed
>   {
>     if (handleHandshakeResultNow)
>     {

Actions: View | Diff

Attachments on bug 851089: 606511 | 606512 | 616533