From 9aae55b3fb50895f03bbcacf99a6a44299c11f58 Mon Sep 17 00:00:00 2001 From: Dana Keeler Date: Fri, 13 Mar 2020 16:28:46 +0000 Subject: [PATCH] bug 1622016 - fix up some declarations used in bug 1512471 that break when build chunking changes r=kjacobs Bug 1512471 added VerifySSLServerCertParent.cpp, which uses SSLServerCertVerificationJob::Dispatch, which isn't exposed in a header. It works in unified builds where the chunking happens to put that file with SSLServerCertVerification.cpp, but when that changes the build breaks. Similarly, VerifySSLServerCertChild.cpp uses gPIPNSSLog without declaring it. Differential Revision: https://phabricator.services.mozilla.com/D66618 --HG-- extra : moz-landing-system : lando --- .../manager/ssl/SSLServerCertVerification.cpp | 167 +++++------------- .../manager/ssl/SSLServerCertVerification.h | 74 +++++++- .../manager/ssl/VerifySSLServerCertChild.cpp | 2 + .../manager/ssl/VerifySSLServerCertParent.cpp | 2 + 4 files changed, 123 insertions(+), 122 deletions(-) diff --git a/security/manager/ssl/SSLServerCertVerification.cpp b/security/manager/ssl/SSLServerCertVerification.cpp index d2014f0b1b6f..0c2d97c8f845 100644 --- a/security/manager/ssl/SSLServerCertVerification.cpp +++ b/security/manager/ssl/SSLServerCertVerification.cpp @@ -452,77 +452,6 @@ static nsresult OverrideAllowedForHost( return NS_OK; } -class SSLServerCertVerificationJob : public Runnable { - public: - // Must be called only on the socket transport thread - static SECStatus Dispatch(uint64_t addrForLogging, void* aPinArg, - const UniqueCERTCertificate& serverCert, - nsTArray>&& peerCertChain, - const nsACString& aHostName, int32_t aPort, - const OriginAttributes& aOriginAttributes, - Maybe>& stapledOCSPResponse, - Maybe>& sctsFromTLSExtension, - Maybe& dcInfo, - uint32_t providerFlags, Time time, PRTime prtime, - uint32_t certVerifierFlags, - BaseSSLServerCertVerificationResult* aResultTask); - - private: - NS_DECL_NSIRUNNABLE - - // Must be called only on the socket transport thread - SSLServerCertVerificationJob( - uint64_t addrForLogging, void* aPinArg, const UniqueCERTCertificate& cert, - nsTArray>&& peerCertChain, const nsACString& aHostName, - int32_t aPort, const OriginAttributes& aOriginAttributes, - Maybe>& stapledOCSPResponse, - Maybe>& sctsFromTLSExtension, - Maybe& dcInfo, uint32_t providerFlags, Time time, - PRTime prtime, uint32_t certVerifierFlags, - BaseSSLServerCertVerificationResult* aResultTask); - uint64_t mAddrForLogging; - void* mPinArg; - const UniqueCERTCertificate mCert; - nsTArray> mPeerCertChain; - nsCString mHostName; - int32_t mPort; - OriginAttributes mOriginAttributes; - const uint32_t mProviderFlags; - const uint32_t mCertVerifierFlags; - const Time mTime; - const PRTime mPRTime; - Maybe> mStapledOCSPResponse; - Maybe> mSCTsFromTLSExtension; - Maybe mDCInfo; - RefPtr mResultTask; -}; - -SSLServerCertVerificationJob::SSLServerCertVerificationJob( - uint64_t addrForLogging, void* aPinArg, const UniqueCERTCertificate& cert, - nsTArray>&& peerCertChain, const nsACString& aHostName, - int32_t aPort, const OriginAttributes& aOriginAttributes, - Maybe>& stapledOCSPResponse, - Maybe>& sctsFromTLSExtension, - Maybe& dcInfo, uint32_t providerFlags, Time time, - PRTime prtime, uint32_t certVerifierFlags, - BaseSSLServerCertVerificationResult* aResultTask) - : Runnable("psm::SSLServerCertVerificationJob"), - mAddrForLogging(addrForLogging), - mPinArg(aPinArg), - mCert(CERT_DupCertificate(cert.get())), - mPeerCertChain(std::move(peerCertChain)), - mHostName(aHostName), - mPort(aPort), - mOriginAttributes(aOriginAttributes), - mProviderFlags(providerFlags), - mCertVerifierFlags(certVerifierFlags), - mTime(time), - mPRTime(prtime), - mStapledOCSPResponse(std::move(stapledOCSPResponse)), - mSCTsFromTLSExtension(std::move(sctsFromTLSExtension)), - mDCInfo(std::move(dcInfo)), - mResultTask(aResultTask) {} - // This function assumes that we will only use the SPDY connection coalescing // feature on connections where we have negotiated SPDY using NPN. If we ever // talk SPDY without having negotiated it with SPDY, this code will give wrong @@ -1185,52 +1114,6 @@ Result AuthCertificate( return rv; } -/*static*/ -SECStatus SSLServerCertVerificationJob::Dispatch( - uint64_t addrForLogging, void* aPinArg, - const UniqueCERTCertificate& serverCert, - nsTArray>&& peerCertChain, const nsACString& aHostName, - int32_t aPort, const OriginAttributes& aOriginAttributes, - Maybe>& stapledOCSPResponse, - Maybe>& sctsFromTLSExtension, - Maybe& dcInfo, uint32_t providerFlags, Time time, - PRTime prtime, uint32_t certVerifierFlags, - BaseSSLServerCertVerificationResult* aResultTask) { - // Runs on the socket transport thread - if (!aResultTask || !serverCert) { - NS_ERROR("Invalid parameters for SSL server cert validation"); - PR_SetError(PR_INVALID_ARGUMENT_ERROR, 0); - return SECFailure; - } - - if (!gCertVerificationThreadPool) { - PR_SetError(PR_INVALID_STATE_ERROR, 0); - return SECFailure; - } - - RefPtr job(new SSLServerCertVerificationJob( - addrForLogging, aPinArg, serverCert, std::move(peerCertChain), aHostName, - aPort, aOriginAttributes, stapledOCSPResponse, sctsFromTLSExtension, - dcInfo, providerFlags, time, prtime, certVerifierFlags, aResultTask)); - - nsresult nrv = gCertVerificationThreadPool->Dispatch(job, NS_DISPATCH_NORMAL); - if (NS_FAILED(nrv)) { - // We can't call SetCertVerificationResult here to change - // mCertVerificationState because SetCertVerificationResult will call - // libssl functions that acquire SSL locks that are already being held at - // this point. However, we can set an error with PR_SetError and return - // SECFailure, and the correct thing will happen (the error will be - // propagated and this connection will be terminated). - PRErrorCode error = nrv == NS_ERROR_OUT_OF_MEMORY ? PR_OUT_OF_MEMORY_ERROR - : PR_INVALID_STATE_ERROR; - PR_SetError(error, 0); - return SECFailure; - } - - PR_SetError(PR_WOULD_BLOCK_ERROR, 0); - return SECWouldBlock; -} - PRErrorCode AuthCertificateParseResults( uint64_t aPtrForLog, const nsACString& aHostName, int32_t aPort, const OriginAttributes& aOriginAttributes, @@ -1346,6 +1229,54 @@ PRErrorCode AuthCertificateParseResults( : errorCodeTime ? errorCodeTime : aDefaultErrorCodeToReport; } +} // unnamed namespace + +/*static*/ +SECStatus SSLServerCertVerificationJob::Dispatch( + uint64_t addrForLogging, void* aPinArg, + const UniqueCERTCertificate& serverCert, + nsTArray>&& peerCertChain, const nsACString& aHostName, + int32_t aPort, const OriginAttributes& aOriginAttributes, + Maybe>& stapledOCSPResponse, + Maybe>& sctsFromTLSExtension, + Maybe& dcInfo, uint32_t providerFlags, Time time, + PRTime prtime, uint32_t certVerifierFlags, + BaseSSLServerCertVerificationResult* aResultTask) { + // Runs on the socket transport thread + if (!aResultTask || !serverCert) { + NS_ERROR("Invalid parameters for SSL server cert validation"); + PR_SetError(PR_INVALID_ARGUMENT_ERROR, 0); + return SECFailure; + } + + if (!gCertVerificationThreadPool) { + PR_SetError(PR_INVALID_STATE_ERROR, 0); + return SECFailure; + } + + RefPtr job(new SSLServerCertVerificationJob( + addrForLogging, aPinArg, serverCert, std::move(peerCertChain), aHostName, + aPort, aOriginAttributes, stapledOCSPResponse, sctsFromTLSExtension, + dcInfo, providerFlags, time, prtime, certVerifierFlags, aResultTask)); + + nsresult nrv = gCertVerificationThreadPool->Dispatch(job, NS_DISPATCH_NORMAL); + if (NS_FAILED(nrv)) { + // We can't call SetCertVerificationResult here to change + // mCertVerificationState because SetCertVerificationResult will call + // libssl functions that acquire SSL locks that are already being held at + // this point. However, we can set an error with PR_SetError and return + // SECFailure, and the correct thing will happen (the error will be + // propagated and this connection will be terminated). + PRErrorCode error = nrv == NS_ERROR_OUT_OF_MEMORY ? PR_OUT_OF_MEMORY_ERROR + : PR_INVALID_STATE_ERROR; + PR_SetError(error, 0); + return SECFailure; + } + + PR_SetError(PR_WOULD_BLOCK_ERROR, 0); + return SECWouldBlock; +} + NS_IMETHODIMP SSLServerCertVerificationJob::Run() { // Runs on a cert verification thread and only on parent process. @@ -1409,8 +1340,6 @@ SSLServerCertVerificationJob::Run() { return NS_OK; } -} // unnamed namespace - // Takes information needed for cert verification, does some consistency // checks and calls SSLServerCertVerificationJob::Dispatch. SECStatus AuthCertificateHookInternal( diff --git a/security/manager/ssl/SSLServerCertVerification.h b/security/manager/ssl/SSLServerCertVerification.h index f2fda7b3a61a..4e5e1beddf6b 100644 --- a/security/manager/ssl/SSLServerCertVerification.h +++ b/security/manager/ssl/SSLServerCertVerification.h @@ -6,17 +6,21 @@ #ifndef _SSLSERVERCERTVERIFICATION_H #define _SSLSERVERCERTVERIFICATION_H +#include "CertVerifier.h" +#include "ScopedNSSTypes.h" #include "mozilla/Maybe.h" +#include "mozpkix/pkix.h" #include "nsTArray.h" #include "nsThreadUtils.h" -#include "ScopedNSSTypes.h" +#include "prerror.h" +#include "prio.h" #include "seccomon.h" #include "secoidt.h" -#include "prio.h" -#include "prerror.h" class nsNSSCertificate; +using namespace mozilla::pkix; + namespace mozilla { namespace psm { @@ -84,6 +88,70 @@ class SSLServerCertVerificationResult final uint32_t mCollectedErrors; }; +class SSLServerCertVerificationJob : public Runnable { + public: + // Must be called only on the socket transport thread + static SECStatus Dispatch(uint64_t addrForLogging, void* aPinArg, + const UniqueCERTCertificate& serverCert, + nsTArray>&& peerCertChain, + const nsACString& aHostName, int32_t aPort, + const OriginAttributes& aOriginAttributes, + Maybe>& stapledOCSPResponse, + Maybe>& sctsFromTLSExtension, + Maybe& dcInfo, + uint32_t providerFlags, Time time, PRTime prtime, + uint32_t certVerifierFlags, + BaseSSLServerCertVerificationResult* aResultTask); + + private: + NS_DECL_NSIRUNNABLE + + // Must be called only on the socket transport thread + SSLServerCertVerificationJob(uint64_t addrForLogging, void* aPinArg, + const UniqueCERTCertificate& cert, + nsTArray>&& peerCertChain, + const nsACString& aHostName, int32_t aPort, + const OriginAttributes& aOriginAttributes, + Maybe>& stapledOCSPResponse, + Maybe>& sctsFromTLSExtension, + Maybe& dcInfo, + uint32_t providerFlags, Time time, PRTime prtime, + uint32_t certVerifierFlags, + BaseSSLServerCertVerificationResult* aResultTask) + : Runnable("psm::SSLServerCertVerificationJob"), + mAddrForLogging(addrForLogging), + mPinArg(aPinArg), + mCert(CERT_DupCertificate(cert.get())), + mPeerCertChain(std::move(peerCertChain)), + mHostName(aHostName), + mPort(aPort), + mOriginAttributes(aOriginAttributes), + mProviderFlags(providerFlags), + mCertVerifierFlags(certVerifierFlags), + mTime(time), + mPRTime(prtime), + mStapledOCSPResponse(std::move(stapledOCSPResponse)), + mSCTsFromTLSExtension(std::move(sctsFromTLSExtension)), + mDCInfo(std::move(dcInfo)), + mResultTask(aResultTask) {} + + uint64_t mAddrForLogging; + void* mPinArg; + const UniqueCERTCertificate mCert; + nsTArray> mPeerCertChain; + nsCString mHostName; + int32_t mPort; + OriginAttributes mOriginAttributes; + const uint32_t mProviderFlags; + const uint32_t mCertVerifierFlags; + const Time mTime; + const PRTime mPRTime; + Maybe> mStapledOCSPResponse; + Maybe> mSCTsFromTLSExtension; + Maybe mDCInfo; + RefPtr mResultTask; +}; + } // namespace psm } // namespace mozilla diff --git a/security/manager/ssl/VerifySSLServerCertChild.cpp b/security/manager/ssl/VerifySSLServerCertChild.cpp index c3b570fed16f..420817698e0f 100644 --- a/security/manager/ssl/VerifySSLServerCertChild.cpp +++ b/security/manager/ssl/VerifySSLServerCertChild.cpp @@ -13,6 +13,8 @@ #include "nsNSSIOLayer.h" #include "nsSerializationHelper.h" +extern mozilla::LazyLogModule gPIPNSSLog; + namespace mozilla { namespace psm { diff --git a/security/manager/ssl/VerifySSLServerCertParent.cpp b/security/manager/ssl/VerifySSLServerCertParent.cpp index 47addf813b07..88da0b376733 100644 --- a/security/manager/ssl/VerifySSLServerCertParent.cpp +++ b/security/manager/ssl/VerifySSLServerCertParent.cpp @@ -21,6 +21,8 @@ extern mozilla::LazyLogModule gPIPNSSLog; using mozilla::ipc::AssertIsOnBackgroundThread; using mozilla::ipc::IsOnBackgroundThread; +using namespace mozilla::pkix; + namespace mozilla { namespace psm {