From 86e8ca7e0b8d07d9472e6ad6700bc3e63b50d336 Mon Sep 17 00:00:00 2001 From: David Keeler Date: Fri, 27 Feb 2015 11:33:36 -0800 Subject: [PATCH] bug 1137538 - remove nsIIdentityInfo and nsNSSSocketInfo::GetPreviousCert r=mayhemer --- security/manager/ssl/public/moz.build | 1 - .../manager/ssl/public/nsIIdentityInfo.idl | 17 ---- .../ssl/src/SSLServerCertVerification.cpp | 2 +- .../manager/ssl/src/TransportSecurityInfo.cpp | 7 +- .../manager/ssl/src/TransportSecurityInfo.h | 2 +- security/manager/ssl/src/nsNSSCallbacks.cpp | 36 ++------ security/manager/ssl/src/nsNSSCertificate.cpp | 3 +- security/manager/ssl/src/nsNSSCertificate.h | 4 +- security/manager/ssl/src/nsNSSIOLayer.cpp | 83 ------------------- security/manager/ssl/src/nsNSSIOLayer.h | 2 - security/manager/ssl/src/nsSSLStatus.cpp | 11 ++- security/manager/ssl/src/nsSSLStatus.h | 3 +- .../manager/ssl/tests/unit/test_ev_certs.js | 4 - 13 files changed, 23 insertions(+), 152 deletions(-) delete mode 100644 security/manager/ssl/public/nsIIdentityInfo.idl diff --git a/security/manager/ssl/public/moz.build b/security/manager/ssl/public/moz.build index 6307482c56a..e2c6f3e07e9 100644 --- a/security/manager/ssl/public/moz.build +++ b/security/manager/ssl/public/moz.build @@ -17,7 +17,6 @@ XPIDL_SOURCES += [ 'nsIDataSignatureVerifier.idl', 'nsIDOMCryptoDialogs.idl', 'nsIGenKeypairInfoDlg.idl', - 'nsIIdentityInfo.idl', 'nsIKeygenThread.idl', 'nsIKeyModule.idl', 'nsINSSCertCache.idl', diff --git a/security/manager/ssl/public/nsIIdentityInfo.idl b/security/manager/ssl/public/nsIIdentityInfo.idl deleted file mode 100644 index eebcddb2024..00000000000 --- a/security/manager/ssl/public/nsIIdentityInfo.idl +++ /dev/null @@ -1,17 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#include "nsISupports.idl" - -[scriptable, uuid(d842dcec-b032-443e-ab53-54aeb7b569f3)] -interface nsIIdentityInfo : nsISupports -{ - /** - * A "true" value means: - * The object that implements this interface uses a certificate that - * was successfully verified as an Extended Validation (EV) cert. - * The test is bound to SSL Server Cert Usage. - */ - readonly attribute boolean isExtendedValidation; -}; diff --git a/security/manager/ssl/src/SSLServerCertVerification.cpp b/security/manager/ssl/src/SSLServerCertVerification.cpp index 4f554105705..c074da8ee4e 100644 --- a/security/manager/ssl/src/SSLServerCertVerification.cpp +++ b/security/manager/ssl/src/SSLServerCertVerification.cpp @@ -655,7 +655,7 @@ CreateCertErrorRunnable(CertVerifier& certVerifier, return nullptr; } - infoObject->SetStatusErrorBits(*nssCert, collected_errors); + infoObject->SetStatusErrorBits(nssCert, collected_errors); return new CertErrorRunnable(fdForLogging, static_cast(nssCert.get()), diff --git a/security/manager/ssl/src/TransportSecurityInfo.cpp b/security/manager/ssl/src/TransportSecurityInfo.cpp index 2ce7e103848..e533cb1517d 100644 --- a/security/manager/ssl/src/TransportSecurityInfo.cpp +++ b/security/manager/ssl/src/TransportSecurityInfo.cpp @@ -1088,15 +1088,16 @@ RememberCertErrorsTable::LookupCertErrorBits(TransportSecurityInfo* infoObject, } void -TransportSecurityInfo::SetStatusErrorBits(nsIX509Cert & cert, +TransportSecurityInfo::SetStatusErrorBits(nsNSSCertificate* cert, uint32_t collected_errors) { MutexAutoLock lock(mMutex); - if (!mSSLStatus) + if (!mSSLStatus) { mSSLStatus = new nsSSLStatus(); + } - mSSLStatus->SetServerCert(&cert, nsNSSCertificate::ev_status_invalid); + mSSLStatus->SetServerCert(cert, nsNSSCertificate::ev_status_invalid); mSSLStatus->mHaveCertErrorBits = true; mSSLStatus->mIsDomainMismatch = diff --git a/security/manager/ssl/src/TransportSecurityInfo.h b/security/manager/ssl/src/TransportSecurityInfo.h index 92267cbca07..c5e6241f677 100644 --- a/security/manager/ssl/src/TransportSecurityInfo.h +++ b/security/manager/ssl/src/TransportSecurityInfo.h @@ -74,7 +74,7 @@ public: /* Set SSL Status values */ nsresult SetSSLStatus(nsSSLStatus *aSSLStatus); nsSSLStatus* SSLStatus() { return mSSLStatus; } - void SetStatusErrorBits(nsIX509Cert & cert, uint32_t collected_errors); + void SetStatusErrorBits(nsNSSCertificate* cert, uint32_t collected_errors); nsresult SetFailedCertChain(ScopedCERTCertList& certList); diff --git a/security/manager/ssl/src/nsNSSCallbacks.cpp b/security/manager/ssl/src/nsNSSCallbacks.cpp index bc9edb88e98..88f0e134934 100644 --- a/security/manager/ssl/src/nsNSSCallbacks.cpp +++ b/security/manager/ssl/src/nsNSSCallbacks.cpp @@ -1269,8 +1269,6 @@ void HandshakeCallback(PRFileDesc* fd, void* client_data) { nsContentUtils::LogSimpleConsoleError(msg, "SSL"); } - ScopedCERTCertificate serverCert(SSL_PeerCertificate(fd)); - /* Set the SSL Status information */ RefPtr status(infoObject->SSLStatus()); if (!status) { @@ -1281,33 +1279,15 @@ void HandshakeCallback(PRFileDesc* fd, void* client_data) { RememberCertErrorsTable::GetInstance().LookupCertErrorBits(infoObject, status); - RefPtr nssc(nsNSSCertificate::Create(serverCert.get())); - nsCOMPtr prevcert; - infoObject->GetPreviousCert(getter_AddRefs(prevcert)); - - bool equals_previous = false; - if (prevcert && nssc) { - nsresult rv = nssc->Equals(prevcert, &equals_previous); - if (NS_FAILED(rv)) { - equals_previous = false; - } - } - - if (equals_previous) { + if (status->HasServerCert()) { PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, - ("HandshakeCallback using PREV cert %p\n", prevcert.get())); - status->SetServerCert(prevcert, nsNSSCertificate::ev_status_unknown); - } - else { - if (status->HasServerCert()) { - PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, - ("HandshakeCallback KEEPING existing cert\n")); - } - else { - PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, - ("HandshakeCallback using NEW cert %p\n", nssc.get())); - status->SetServerCert(nssc, nsNSSCertificate::ev_status_unknown); - } + ("HandshakeCallback KEEPING existing cert\n")); + } else { + ScopedCERTCertificate serverCert(SSL_PeerCertificate(fd)); + RefPtr nssc(nsNSSCertificate::Create(serverCert.get())); + PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, + ("HandshakeCallback using NEW cert %p\n", nssc.get())); + status->SetServerCert(nssc, nsNSSCertificate::ev_status_unknown); } infoObject->NoteTimeUntilReady(); diff --git a/security/manager/ssl/src/nsNSSCertificate.cpp b/security/manager/ssl/src/nsNSSCertificate.cpp index 9f0c5b63f86..6d1b85d5f9a 100644 --- a/security/manager/ssl/src/nsNSSCertificate.cpp +++ b/security/manager/ssl/src/nsNSSCertificate.cpp @@ -66,7 +66,6 @@ extern PRLogModuleInfo* gPIPNSSLog; NS_IMPL_ISUPPORTS(nsNSSCertificate, nsIX509Cert, - nsIIdentityInfo, nsISerializable, nsIClassInfo) @@ -1440,7 +1439,7 @@ nsNSSCertificate::getValidEVOidTag(SECOidTag& resultOidTag, bool& validEV) #endif // MOZ_NO_EV_CERTS -NS_IMETHODIMP +nsresult nsNSSCertificate::GetIsExtendedValidation(bool* aIsEV) { #ifdef MOZ_NO_EV_CERTS diff --git a/security/manager/ssl/src/nsNSSCertificate.h b/security/manager/ssl/src/nsNSSCertificate.h index 16ece1339a5..3f71fc9ab35 100644 --- a/security/manager/ssl/src/nsNSSCertificate.h +++ b/security/manager/ssl/src/nsNSSCertificate.h @@ -10,7 +10,6 @@ #include "nsIX509CertDB.h" #include "nsIX509CertList.h" #include "nsIASN1Object.h" -#include "nsIIdentityInfo.h" #include "nsCOMPtr.h" #include "nsNSSShutDown.h" #include "nsISimpleEnumerator.h" @@ -26,7 +25,6 @@ class nsINSSComponent; class nsIASN1Sequence; class nsNSSCertificate MOZ_FINAL : public nsIX509Cert, - public nsIIdentityInfo, public nsISerializable, public nsIClassInfo, public nsNSSShutDownObject @@ -34,7 +32,6 @@ class nsNSSCertificate MOZ_FINAL : public nsIX509Cert, public: NS_DECL_THREADSAFE_ISUPPORTS NS_DECL_NSIX509CERT - NS_DECL_NSIIDENTITYINFO NS_DECL_NSISERIALIZABLE NS_DECL_NSICLASSINFO @@ -48,6 +45,7 @@ public: static nsNSSCertificate* Create(CERTCertificate*cert = nullptr, SECOidTag* evOidPolicy = nullptr); static nsNSSCertificate* ConstructFromDER(char* certDER, int derLen); + nsresult GetIsExtendedValidation(bool* aIsEV); enum EVStatus { ev_status_invalid = 0, diff --git a/security/manager/ssl/src/nsNSSIOLayer.cpp b/security/manager/ssl/src/nsNSSIOLayer.cpp index 25c51badf22..aa08bfff0dc 100644 --- a/security/manager/ssl/src/nsNSSIOLayer.cpp +++ b/security/manager/ssl/src/nsNSSIOLayer.cpp @@ -27,13 +27,6 @@ #include "SSLServerCertVerification.h" #include "nsNSSCertHelper.h" -#ifndef MOZ_NO_EV_CERTS -#include "nsIDocShell.h" -#include "nsIDocShellTreeItem.h" -#include "nsISecureBrowserUI.h" -#include "nsIInterfaceRequestorUtils.h" -#endif - #include "nsCharSeparatedTokenizer.h" #include "nsIConsoleService.h" #include "PSMRunnable.h" @@ -274,39 +267,6 @@ nsNSSSocketInfo::SetNotificationCallbacks(nsIInterfaceRequestor* aCallbacks) return NS_OK; } -#ifndef MOZ_NO_EV_CERTS -static void -getSecureBrowserUI(nsIInterfaceRequestor* callbacks, - nsISecureBrowserUI** result) -{ - NS_ASSERTION(result, "result parameter to getSecureBrowserUI is null"); - *result = nullptr; - - NS_ASSERTION(NS_IsMainThread(), - "getSecureBrowserUI called off the main thread"); - - if (!callbacks) - return; - - nsCOMPtr secureUI = do_GetInterface(callbacks); - if (secureUI) { - secureUI.forget(result); - return; - } - - nsCOMPtr item = do_GetInterface(callbacks); - if (item) { - nsCOMPtr rootItem; - (void) item->GetSameTypeRootTreeItem(getter_AddRefs(rootItem)); - - nsCOMPtr docShell = do_QueryInterface(rootItem); - if (docShell) { - (void) docShell->GetSecurityUI(result); - } - } -} -#endif - void nsNSSSocketInfo::NoteTimeUntilReady() { @@ -580,49 +540,6 @@ nsNSSSocketInfo::SetFileDescPtr(PRFileDesc* aFilePtr) return NS_OK; } -#ifndef MOZ_NO_EV_CERTS -class PreviousCertRunnable : public SyncRunnableBase -{ -public: - explicit PreviousCertRunnable(nsIInterfaceRequestor* callbacks) - : mCallbacks(callbacks) - { - } - - virtual void RunOnTargetThread() - { - nsCOMPtr secureUI; - getSecureBrowserUI(mCallbacks, getter_AddRefs(secureUI)); - nsCOMPtr statusProvider = do_QueryInterface(secureUI); - if (statusProvider) { - nsCOMPtr status; - (void) statusProvider->GetSSLStatus(getter_AddRefs(status)); - if (status) { - (void) status->GetServerCert(getter_AddRefs(mPreviousCert)); - } - } - } - - nsCOMPtr mPreviousCert; // out -private: - nsCOMPtr mCallbacks; // in -}; -#endif - -void -nsNSSSocketInfo::GetPreviousCert(nsIX509Cert** _result) -{ - NS_ASSERTION(_result, "_result parameter to GetPreviousCert is null"); - *_result = nullptr; - -#ifndef MOZ_NO_EV_CERTS - RefPtr runnable(new PreviousCertRunnable(mCallbacks)); - DebugOnly rv = runnable->DispatchToMainThreadAndWait(); - NS_ASSERTION(NS_SUCCEEDED(rv), "runnable->DispatchToMainThreadAndWait() failed"); - runnable->mPreviousCert.forget(_result); -#endif -} - void nsNSSSocketInfo::SetCertVerificationWaiting() { diff --git a/security/manager/ssl/src/nsNSSIOLayer.h b/security/manager/ssl/src/nsNSSIOLayer.h index 0c45d6ad277..0b8a791878e 100644 --- a/security/manager/ssl/src/nsNSSIOLayer.h +++ b/security/manager/ssl/src/nsNSSIOLayer.h @@ -44,8 +44,6 @@ public: bool IsHandshakePending() const { return mHandshakePending; } void SetHandshakeNotPending() { mHandshakePending = false; } - void GetPreviousCert(nsIX509Cert** _result); - void SetTLSVersionRange(SSLVersionRange range) { mTLSVersionRange = range; } SSLVersionRange GetTLSVersionRange() const { return mTLSVersionRange; }; diff --git a/security/manager/ssl/src/nsSSLStatus.cpp b/security/manager/ssl/src/nsSSLStatus.cpp index 40b8305f734..09b7ab5fb7a 100644 --- a/security/manager/ssl/src/nsSSLStatus.cpp +++ b/security/manager/ssl/src/nsSSLStatus.cpp @@ -7,7 +7,6 @@ #include "nsSSLStatus.h" #include "plstr.h" #include "nsIClassInfoImpl.h" -#include "nsIIdentityInfo.h" #include "nsIProgrammingLanguage.h" #include "nsIObjectOutputStream.h" #include "nsIObjectInputStream.h" @@ -288,7 +287,8 @@ nsSSLStatus::~nsSSLStatus() } void -nsSSLStatus::SetServerCert(nsIX509Cert* aServerCert, nsNSSCertificate::EVStatus aEVStatus) +nsSSLStatus::SetServerCert(nsNSSCertificate* aServerCert, + nsNSSCertificate::EVStatus aEVStatus) { mServerCert = aServerCert; @@ -299,10 +299,9 @@ nsSSLStatus::SetServerCert(nsIX509Cert* aServerCert, nsNSSCertificate::EVStatus } #ifndef MOZ_NO_EV_CERTS - nsCOMPtr idinfo = do_QueryInterface(mServerCert); - if (idinfo) { - nsresult rv = idinfo->GetIsExtendedValidation(&mIsEV); - if (NS_WARN_IF(NS_FAILED(rv))) { + if (aServerCert) { + nsresult rv = aServerCert->GetIsExtendedValidation(&mIsEV); + if (NS_FAILED(rv)) { return; } mHasIsEVStatus = true; diff --git a/security/manager/ssl/src/nsSSLStatus.h b/security/manager/ssl/src/nsSSLStatus.h index 29fdd723115..dec1a623409 100644 --- a/security/manager/ssl/src/nsSSLStatus.h +++ b/security/manager/ssl/src/nsSSLStatus.h @@ -30,7 +30,8 @@ public: nsSSLStatus(); - void SetServerCert(nsIX509Cert* aServerCert, nsNSSCertificate::EVStatus aEVStatus); + void SetServerCert(nsNSSCertificate* aServerCert, + nsNSSCertificate::EVStatus aEVStatus); bool HasServerCert() { return mServerCert != nullptr; diff --git a/security/manager/ssl/tests/unit/test_ev_certs.js b/security/manager/ssl/tests/unit/test_ev_certs.js index 794a58af858..3652651ea6d 100644 --- a/security/manager/ssl/tests/unit/test_ev_certs.js +++ b/security/manager/ssl/tests/unit/test_ev_certs.js @@ -255,9 +255,5 @@ function check_no_ocsp_requests(cert_name, expected_error) { // Since we're not doing OCSP requests, no certificate will be EV. do_check_eq(hasEVPolicy.value, false); do_check_eq(expected_error, error); - // Also check that isExtendedValidation doesn't cause OCSP requests. - let identityInfo = cert.QueryInterface(Ci.nsIIdentityInfo); - do_check_eq(identityInfo.isExtendedValidation, false); ocspResponder.stop(run_next_test); } -