diff --git a/netwerk/protocol/http/QuicSocketControl.cpp b/netwerk/protocol/http/QuicSocketControl.cpp index a60cf5693979..a9a142d2b542 100644 --- a/netwerk/protocol/http/QuicSocketControl.cpp +++ b/netwerk/protocol/http/QuicSocketControl.cpp @@ -86,6 +86,7 @@ void QuicSocketControl::HandshakeCompleted() { } void QuicSocketControl::SetNegotiatedNPN(const nsACString& aValue) { + MutexAutoLock lock(mMutex); mNegotiatedNPN = aValue; mNPNCompleted = true; } @@ -96,6 +97,7 @@ void QuicSocketControl::SetInfo(uint16_t aCipherSuite, SSLCipherSuiteInfo cipherInfo; if (SSL_GetCipherSuiteInfo(aCipherSuite, &cipherInfo, sizeof cipherInfo) == SECSuccess) { + MutexAutoLock lock(mMutex); mHaveCipherSuiteAndProtocol = true; mCipherSuite = aCipherSuite; mProtocolVersion = aProtocolVersion & 0xFF; diff --git a/security/manager/ssl/CommonSocketControl.cpp b/security/manager/ssl/CommonSocketControl.cpp index 5f42575da4bb..ed5dd4df2934 100644 --- a/security/manager/ssl/CommonSocketControl.cpp +++ b/security/manager/ssl/CommonSocketControl.cpp @@ -33,6 +33,7 @@ CommonSocketControl::CommonSocketControl(uint32_t aProviderFlags) NS_IMETHODIMP CommonSocketControl::GetNotificationCallbacks( nsIInterfaceRequestor** aCallbacks) { + MutexAutoLock lock(mMutex); *aCallbacks = mCallbacks; NS_IF_ADDREF(*aCallbacks); return NS_OK; @@ -41,6 +42,7 @@ CommonSocketControl::GetNotificationCallbacks( NS_IMETHODIMP CommonSocketControl::SetNotificationCallbacks( nsIInterfaceRequestor* aCallbacks) { + MutexAutoLock lock(mMutex); mCallbacks = aCallbacks; return NS_OK; } @@ -90,8 +92,11 @@ CommonSocketControl::TestJoinConnection(const nsACString& npnProtocol, // Different ports may not be joined together if (port != GetPort()) return NS_OK; - // Make sure NPN has been completed and matches requested npnProtocol - if (!mNPNCompleted || !mNegotiatedNPN.Equals(npnProtocol)) return NS_OK; + { + MutexAutoLock lock(mMutex); + // Make sure NPN has been completed and matches requested npnProtocol + if (!mNPNCompleted || !mNegotiatedNPN.Equals(npnProtocol)) return NS_OK; + } IsAcceptableForHost(hostname, _retval); // sets _retval return NS_OK; @@ -202,7 +207,7 @@ CommonSocketControl::IsAcceptableForHost(const nsACString& hostname, nsresult nsrv = mozilla::psm::PublicKeyPinningService::ChainHasValidPins( derCertSpanList, PromiseFlatCString(hostname).BeginReading(), Now(), - enforceTestMode, GetOriginAttributes(), chainHasValidPins, nullptr); + enforceTestMode, GetOriginAttributes(lock), chainHasValidPins, nullptr); if (NS_FAILED(nsrv)) { return NS_OK; } diff --git a/security/manager/ssl/TransportSecurityInfo.cpp b/security/manager/ssl/TransportSecurityInfo.cpp index 1365284c1f10..defb672a0eb7 100644 --- a/security/manager/ssl/TransportSecurityInfo.cpp +++ b/security/manager/ssl/TransportSecurityInfo.cpp @@ -68,6 +68,8 @@ NS_IMPL_ISUPPORTS(TransportSecurityInfo, nsITransportSecurityInfo, nsIInterfaceRequestor, nsISerializable, nsIClassInfo) void TransportSecurityInfo::SetHostName(const char* host) { + MutexAutoLock lock(mMutex); + mHostName.Assign(host); } @@ -75,6 +77,8 @@ void TransportSecurityInfo::SetPort(int32_t aPort) { mPort = aPort; } void TransportSecurityInfo::SetOriginAttributes( const OriginAttributes& aOriginAttributes) { + MutexAutoLock lock(mMutex); + mOriginAttributes = aOriginAttributes; } @@ -82,8 +86,6 @@ void TransportSecurityInfo::SetOriginAttributes( // case, this returns (by pointer) 0, which is treated as a successful value. NS_IMETHODIMP TransportSecurityInfo::GetErrorCode(int32_t* state) { - MutexAutoLock lock(mMutex); - // We're in an inconsistent state if we think we've been canceled but no error // code was set or we haven't been canceled but an error code was set. MOZ_ASSERT( @@ -103,7 +105,6 @@ void TransportSecurityInfo::SetCanceled(PRErrorCode errorCode) { errorCode = SEC_ERROR_LIBRARY_FAILURE; } - MutexAutoLock lock(mMutex); mErrorCode = errorCode; mCanceled = true; } @@ -122,8 +123,6 @@ void TransportSecurityInfo::SetSecurityState(uint32_t aState) { NS_IMETHODIMP TransportSecurityInfo::GetErrorCodeString(nsAString& aErrorString) { - MutexAutoLock lock(mMutex); - const char* codeName = PR_ErrorToName(mErrorCode); aErrorString.Truncate(); if (codeName) { @@ -139,6 +138,7 @@ TransportSecurityInfo::GetInterface(const nsIID& uuid, void** result) { NS_ERROR("nsNSSSocketInfo::GetInterface called off the main thread"); return NS_ERROR_NOT_SAME_THREAD; } + MutexAutoLock lock(mMutex); nsresult rv; if (!mCallbacks) { @@ -291,7 +291,8 @@ TransportSecurityInfo::Write(nsIObjectOutputStream* aStream) { // This is for backward compatibility to be able to read nsISSLStatus // serialized object. -nsresult TransportSecurityInfo::ReadSSLStatus(nsIObjectInputStream* aStream) { +nsresult TransportSecurityInfo::ReadSSLStatus(nsIObjectInputStream* aStream, + MutexAutoLock& aProofOfLock) { bool nsISSLStatusPresent; nsresult rv = aStream->ReadBoolean(&nsISSLStatusPresent); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); @@ -351,26 +352,26 @@ nsresult TransportSecurityInfo::ReadSSLStatus(nsIObjectInputStream* aStream) { const uint8_t streamFormatVersion = (protocolVersionAndStreamFormatVersion >> 8) & 0xFF; - rv = aStream->ReadBoolean(&mIsDomainMismatch); + rv = ReadBoolAndSetAtomicFieldHelper(aStream, mIsDomainMismatch); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); - rv = aStream->ReadBoolean(&mIsNotValidAtThisTime); + rv = ReadBoolAndSetAtomicFieldHelper(aStream, mIsNotValidAtThisTime); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); - rv = aStream->ReadBoolean(&mIsUntrusted); + rv = ReadBoolAndSetAtomicFieldHelper(aStream, mIsUntrusted); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); - rv = aStream->ReadBoolean(&mIsEV); + rv = ReadBoolAndSetAtomicFieldHelper(aStream, mIsEV); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); - rv = aStream->ReadBoolean(&mHasIsEVStatus); + rv = ReadBoolAndSetAtomicFieldHelper(aStream, mHasIsEVStatus); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); - rv = aStream->ReadBoolean(&mHaveCipherSuiteAndProtocol); + rv = ReadBoolAndSetAtomicFieldHelper(aStream, mHaveCipherSuiteAndProtocol); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); - rv = aStream->ReadBoolean(&mHaveCertErrorBits); + rv = ReadBoolAndSetAtomicFieldHelper(aStream, mHaveCertErrorBits); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); @@ -397,7 +398,7 @@ nsresult TransportSecurityInfo::ReadSSLStatus(nsIObjectInputStream* aStream) { // Added in version 3 (see bug 1406856). if (streamFormatVersion >= 3) { - rv = ReadCertList(aStream, mSucceededCertChain); + rv = ReadCertList(aStream, mSucceededCertChain, aProofOfLock); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); if (NS_FAILED(rv)) { @@ -406,7 +407,7 @@ nsresult TransportSecurityInfo::ReadSSLStatus(nsIObjectInputStream* aStream) { // Read only to consume bytes from the stream. nsTArray> failedCertChain; - rv = ReadCertList(aStream, failedCertChain); + rv = ReadCertList(aStream, failedCertChain, aProofOfLock); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); if (NS_FAILED(rv)) { @@ -419,7 +420,8 @@ nsresult TransportSecurityInfo::ReadSSLStatus(nsIObjectInputStream* aStream) { // This is for backward compatability to be able to read nsIX509CertList // serialized object. nsresult TransportSecurityInfo::ReadCertList( - nsIObjectInputStream* aStream, nsTArray>& aCertList) { + nsIObjectInputStream* aStream, nsTArray>& aCertList, + MutexAutoLock& aProofOfLock) { bool nsIX509CertListPresent; nsresult rv = aStream->ReadBoolean(&nsIX509CertListPresent); @@ -455,12 +457,13 @@ nsresult TransportSecurityInfo::ReadCertList( CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); - return ReadCertificatesFromStream(aStream, certListSize, aCertList); + return ReadCertificatesFromStream(aStream, certListSize, aCertList, + aProofOfLock); } nsresult TransportSecurityInfo::ReadCertificatesFromStream( nsIObjectInputStream* aStream, uint32_t aSize, - nsTArray>& aCertList) { + nsTArray>& aCertList, MutexAutoLock& aProofOfLock) { nsresult rv; for (uint32_t i = 0; i < aSize; ++i) { nsCOMPtr support; @@ -495,7 +498,7 @@ TransportSecurityInfo::Read(nsIObjectInputStream* aStream) { MutexAutoLock lock(mMutex); - rv = aStream->Read32(&mSecurityState); + rv = ReadUint32AndSetAtomicFieldHelper(aStream, mSecurityState); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); if (NS_FAILED(rv)) { return rv; @@ -543,7 +546,7 @@ TransportSecurityInfo::Read(nsIObjectInputStream* aStream) { !serVersion.EqualsASCII("3") && !serVersion.EqualsASCII("4") && !serVersion.EqualsASCII("5") && !serVersion.EqualsASCII("6")) { // nsISSLStatus may be present - rv = ReadSSLStatus(aStream); + rv = ReadSSLStatus(aStream, lock); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); @@ -572,32 +575,32 @@ TransportSecurityInfo::Read(nsIObjectInputStream* aStream) { "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); - rv = aStream->ReadBoolean(&mIsDomainMismatch); + rv = ReadBoolAndSetAtomicFieldHelper(aStream, mIsDomainMismatch); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); - rv = aStream->ReadBoolean(&mIsNotValidAtThisTime); + rv = ReadBoolAndSetAtomicFieldHelper(aStream, mIsNotValidAtThisTime); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); - rv = aStream->ReadBoolean(&mIsUntrusted); + rv = ReadBoolAndSetAtomicFieldHelper(aStream, mIsUntrusted); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); - rv = aStream->ReadBoolean(&mIsEV); + rv = ReadBoolAndSetAtomicFieldHelper(aStream, mIsEV); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); - rv = aStream->ReadBoolean(&mHasIsEVStatus); + rv = ReadBoolAndSetAtomicFieldHelper(aStream, mHasIsEVStatus); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); - rv = aStream->ReadBoolean(&mHaveCipherSuiteAndProtocol); + rv = ReadBoolAndSetAtomicFieldHelper(aStream, mHaveCipherSuiteAndProtocol); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); - rv = aStream->ReadBoolean(&mHaveCertErrorBits); + rv = ReadBoolAndSetAtomicFieldHelper(aStream, mHaveCertErrorBits); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); @@ -620,7 +623,7 @@ TransportSecurityInfo::Read(nsIObjectInputStream* aStream) { if (!serVersion.EqualsASCII("3") && !serVersion.EqualsASCII("4") && !serVersion.EqualsASCII("5") && !serVersion.EqualsASCII("6")) { // The old data structure of certList(nsIX509CertList) presents - rv = ReadCertList(aStream, mSucceededCertChain); + rv = ReadCertList(aStream, mSucceededCertChain, lock); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); @@ -631,7 +634,8 @@ TransportSecurityInfo::Read(nsIObjectInputStream* aStream) { "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); - rv = ReadCertificatesFromStream(aStream, certCount, mSucceededCertChain); + rv = ReadCertificatesFromStream(aStream, certCount, mSucceededCertChain, + lock); NS_ENSURE_SUCCESS(rv, rv); } } @@ -639,7 +643,7 @@ TransportSecurityInfo::Read(nsIObjectInputStream* aStream) { if (!serVersion.EqualsASCII("3") && !serVersion.EqualsASCII("4") && !serVersion.EqualsASCII("5") && !serVersion.EqualsASCII("6")) { // The old data structure of certList(nsIX509CertList) presents - rv = ReadCertList(aStream, mFailedCertChain); + rv = ReadCertList(aStream, mFailedCertChain, lock); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); @@ -650,7 +654,7 @@ TransportSecurityInfo::Read(nsIObjectInputStream* aStream) { "Deserialization should not fail"); NS_ENSURE_SUCCESS(rv, rv); - rv = ReadCertificatesFromStream(aStream, certCount, mFailedCertChain); + rv = ReadCertificatesFromStream(aStream, certCount, mFailedCertChain, lock); NS_ENSURE_SUCCESS(rv, rv); } @@ -658,7 +662,7 @@ TransportSecurityInfo::Read(nsIObjectInputStream* aStream) { if (serVersion.EqualsASCII("2") || serVersion.EqualsASCII("3") || serVersion.EqualsASCII("4") || serVersion.EqualsASCII("5") || serVersion.EqualsASCII("6")) { - rv = aStream->ReadBoolean(&mIsDelegatedCredential); + rv = ReadBoolAndSetAtomicFieldHelper(aStream, mIsDelegatedCredential); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); if (NS_FAILED(rv)) { @@ -669,7 +673,7 @@ TransportSecurityInfo::Read(nsIObjectInputStream* aStream) { // mNPNCompleted, mNegotiatedNPN, mResumed added in bug 1584104 if (serVersion.EqualsASCII("4") || serVersion.EqualsASCII("5") || serVersion.EqualsASCII("6")) { - rv = aStream->ReadBoolean(&mNPNCompleted); + rv = ReadBoolAndSetAtomicFieldHelper(aStream, mNPNCompleted); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); if (NS_FAILED(rv)) { @@ -683,7 +687,7 @@ TransportSecurityInfo::Read(nsIObjectInputStream* aStream) { return rv; } - rv = aStream->ReadBoolean(&mResumed); + rv = ReadBoolAndSetAtomicFieldHelper(aStream, mResumed); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); if (NS_FAILED(rv)) { @@ -693,7 +697,8 @@ TransportSecurityInfo::Read(nsIObjectInputStream* aStream) { // mIsBuiltCertChainRootBuiltInRoot added in bug 1485652 if (serVersion.EqualsASCII("5") || serVersion.EqualsASCII("6")) { - rv = aStream->ReadBoolean(&mIsBuiltCertChainRootBuiltInRoot); + rv = ReadBoolAndSetAtomicFieldHelper(aStream, + mIsBuiltCertChainRootBuiltInRoot); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); if (NS_FAILED(rv)) { @@ -703,7 +708,7 @@ TransportSecurityInfo::Read(nsIObjectInputStream* aStream) { // mIsAcceptedEch added in bug 1678079 if (serVersion.EqualsASCII("6")) { - rv = aStream->ReadBoolean(&mIsAcceptedEch); + rv = ReadBoolAndSetAtomicFieldHelper(aStream, mIsAcceptedEch); CHILD_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv), "Deserialization should not fail"); if (NS_FAILED(rv)) { @@ -721,29 +726,29 @@ void TransportSecurityInfo::SerializeToIPC(IPC::Message* aMsg) { int32_t errorCode = static_cast(mErrorCode); - WriteParam(aMsg, mSecurityState); + WriteParam(aMsg, static_cast(mSecurityState)); WriteParam(aMsg, errorCode); WriteParam(aMsg, mServerCert); WriteParam(aMsg, mCipherSuite); WriteParam(aMsg, mProtocolVersion); - WriteParam(aMsg, mIsDomainMismatch); - WriteParam(aMsg, mIsNotValidAtThisTime); - WriteParam(aMsg, mIsUntrusted); - WriteParam(aMsg, mIsEV); - WriteParam(aMsg, mHasIsEVStatus); - WriteParam(aMsg, mHaveCipherSuiteAndProtocol); - WriteParam(aMsg, mHaveCertErrorBits); + WriteParam(aMsg, static_cast(mIsDomainMismatch)); + WriteParam(aMsg, static_cast(mIsNotValidAtThisTime)); + WriteParam(aMsg, static_cast(mIsUntrusted)); + WriteParam(aMsg, static_cast(mIsEV)); + WriteParam(aMsg, static_cast(mHasIsEVStatus)); + WriteParam(aMsg, static_cast(mHaveCipherSuiteAndProtocol)); + WriteParam(aMsg, static_cast(mHaveCertErrorBits)); WriteParam(aMsg, mCertificateTransparencyStatus); WriteParam(aMsg, mKeaGroup); WriteParam(aMsg, mSignatureSchemeName); WriteParam(aMsg, mSucceededCertChain); WriteParam(aMsg, mFailedCertChain); - WriteParam(aMsg, mIsDelegatedCredential); - WriteParam(aMsg, mNPNCompleted); + WriteParam(aMsg, static_cast(mIsDelegatedCredential)); + WriteParam(aMsg, static_cast(mNPNCompleted)); WriteParam(aMsg, mNegotiatedNPN); - WriteParam(aMsg, mResumed); - WriteParam(aMsg, mIsBuiltCertChainRootBuiltInRoot); - WriteParam(aMsg, mIsAcceptedEch); + WriteParam(aMsg, static_cast(mResumed)); + WriteParam(aMsg, static_cast(mIsBuiltCertChainRootBuiltInRoot)); + WriteParam(aMsg, static_cast(mIsAcceptedEch)); } bool TransportSecurityInfo::DeserializeFromIPC(const IPC::Message* aMsg, @@ -752,29 +757,29 @@ bool TransportSecurityInfo::DeserializeFromIPC(const IPC::Message* aMsg, int32_t errorCode = 0; - if (!ReadParam(aMsg, aIter, &mSecurityState) || + if (!ReadParamAtomicHelper(aMsg, aIter, mSecurityState) || !ReadParam(aMsg, aIter, &errorCode) || !ReadParam(aMsg, aIter, &mServerCert) || !ReadParam(aMsg, aIter, &mCipherSuite) || !ReadParam(aMsg, aIter, &mProtocolVersion) || - !ReadParam(aMsg, aIter, &mIsDomainMismatch) || - !ReadParam(aMsg, aIter, &mIsNotValidAtThisTime) || - !ReadParam(aMsg, aIter, &mIsUntrusted) || - !ReadParam(aMsg, aIter, &mIsEV) || - !ReadParam(aMsg, aIter, &mHasIsEVStatus) || - !ReadParam(aMsg, aIter, &mHaveCipherSuiteAndProtocol) || - !ReadParam(aMsg, aIter, &mHaveCertErrorBits) || + !ReadParamAtomicHelper(aMsg, aIter, mIsDomainMismatch) || + !ReadParamAtomicHelper(aMsg, aIter, mIsNotValidAtThisTime) || + !ReadParamAtomicHelper(aMsg, aIter, mIsUntrusted) || + !ReadParamAtomicHelper(aMsg, aIter, mIsEV) || + !ReadParamAtomicHelper(aMsg, aIter, mHasIsEVStatus) || + !ReadParamAtomicHelper(aMsg, aIter, mHaveCipherSuiteAndProtocol) || + !ReadParamAtomicHelper(aMsg, aIter, mHaveCertErrorBits) || !ReadParam(aMsg, aIter, &mCertificateTransparencyStatus) || !ReadParam(aMsg, aIter, &mKeaGroup) || !ReadParam(aMsg, aIter, &mSignatureSchemeName) || !ReadParam(aMsg, aIter, &mSucceededCertChain) || !ReadParam(aMsg, aIter, &mFailedCertChain) || - !ReadParam(aMsg, aIter, &mIsDelegatedCredential) || - !ReadParam(aMsg, aIter, &mNPNCompleted) || + !ReadParamAtomicHelper(aMsg, aIter, mIsDelegatedCredential) || + !ReadParamAtomicHelper(aMsg, aIter, mNPNCompleted) || !ReadParam(aMsg, aIter, &mNegotiatedNPN) || - !ReadParam(aMsg, aIter, &mResumed) || - !ReadParam(aMsg, aIter, &mIsBuiltCertChainRootBuiltInRoot) || - !ReadParam(aMsg, aIter, &mIsAcceptedEch)) { + !ReadParamAtomicHelper(aMsg, aIter, mResumed) || + !ReadParamAtomicHelper(aMsg, aIter, mIsBuiltCertChainRootBuiltInRoot) || + !ReadParamAtomicHelper(aMsg, aIter, mIsAcceptedEch)) { return false; } @@ -917,8 +922,6 @@ void RememberCertErrorsTable::LookupCertErrorBits( void TransportSecurityInfo::SetStatusErrorBits(nsNSSCertificate* cert, uint32_t collected_errors) { - MutexAutoLock lock(mMutex); - SetServerCert(cert, EVStatus::NotEV); mHaveCertErrorBits = true; @@ -933,6 +936,7 @@ NS_IMETHODIMP TransportSecurityInfo::GetFailedCertChain( nsTArray>& aFailedCertChain) { MOZ_ASSERT(aFailedCertChain.IsEmpty()); + MutexAutoLock lock(mMutex); if (!aFailedCertChain.IsEmpty()) { return NS_ERROR_INVALID_ARG; } @@ -957,11 +961,14 @@ static nsresult CreateCertChain(nsTArray>& aOutput, nsresult TransportSecurityInfo::SetFailedCertChain( nsTArray>&& aCertList) { + MutexAutoLock lock(mMutex); + return CreateCertChain(mFailedCertChain, std::move(aCertList)); } NS_IMETHODIMP TransportSecurityInfo::GetServerCert(nsIX509Cert** aServerCert) { NS_ENSURE_ARG_POINTER(aServerCert); + MutexAutoLock lock(mMutex); nsCOMPtr cert = mServerCert; cert.forget(aServerCert); @@ -971,6 +978,7 @@ NS_IMETHODIMP TransportSecurityInfo::GetServerCert(nsIX509Cert** aServerCert) { void TransportSecurityInfo::SetServerCert(nsNSSCertificate* aServerCert, EVStatus aEVStatus) { MOZ_ASSERT(aServerCert); + MutexAutoLock lock(mMutex); mServerCert = aServerCert; mIsEV = (aEVStatus == EVStatus::EV); @@ -1010,6 +1018,8 @@ NS_IMETHODIMP TransportSecurityInfo::GetIsBuiltCertChainRootBuiltInRoot( NS_IMETHODIMP TransportSecurityInfo::GetCipherName(nsACString& aCipherName) { + MutexAutoLock lock(mMutex); + if (!mHaveCipherSuiteAndProtocol) { return NS_ERROR_NOT_AVAILABLE; } @@ -1027,6 +1037,7 @@ TransportSecurityInfo::GetCipherName(nsACString& aCipherName) { NS_IMETHODIMP TransportSecurityInfo::GetKeyLength(uint32_t* aKeyLength) { NS_ENSURE_ARG_POINTER(aKeyLength); + MutexAutoLock lock(mMutex); if (!mHaveCipherSuiteAndProtocol) { return NS_ERROR_NOT_AVAILABLE; } @@ -1044,6 +1055,7 @@ TransportSecurityInfo::GetKeyLength(uint32_t* aKeyLength) { NS_IMETHODIMP TransportSecurityInfo::GetSecretKeyLength(uint32_t* aSecretKeyLength) { NS_ENSURE_ARG_POINTER(aSecretKeyLength); + MutexAutoLock lock(mMutex); if (!mHaveCipherSuiteAndProtocol) { return NS_ERROR_NOT_AVAILABLE; } @@ -1060,6 +1072,8 @@ TransportSecurityInfo::GetSecretKeyLength(uint32_t* aSecretKeyLength) { NS_IMETHODIMP TransportSecurityInfo::GetKeaGroupName(nsACString& aKeaGroup) { + MutexAutoLock lock(mMutex); + if (!mHaveCipherSuiteAndProtocol) { return NS_ERROR_NOT_AVAILABLE; } @@ -1070,6 +1084,8 @@ TransportSecurityInfo::GetKeaGroupName(nsACString& aKeaGroup) { NS_IMETHODIMP TransportSecurityInfo::GetSignatureSchemeName(nsACString& aSignatureScheme) { + MutexAutoLock lock(mMutex); + if (!mHaveCipherSuiteAndProtocol) { return NS_ERROR_NOT_AVAILABLE; } @@ -1080,6 +1096,8 @@ TransportSecurityInfo::GetSignatureSchemeName(nsACString& aSignatureScheme) { NS_IMETHODIMP TransportSecurityInfo::GetProtocolVersion(uint16_t* aProtocolVersion) { + MutexAutoLock lock(mMutex); + NS_ENSURE_ARG_POINTER(aProtocolVersion); if (!mHaveCipherSuiteAndProtocol) { return NS_ERROR_NOT_AVAILABLE; @@ -1093,6 +1111,7 @@ NS_IMETHODIMP TransportSecurityInfo::GetCertificateTransparencyStatus( uint16_t* aCertificateTransparencyStatus) { NS_ENSURE_ARG_POINTER(aCertificateTransparencyStatus); + MutexAutoLock lock(mMutex); *aCertificateTransparencyStatus = mCertificateTransparencyStatus; return NS_OK; @@ -1142,7 +1161,6 @@ nsTArray> TransportSecurityInfo::CreateCertBytesArray( NS_IMETHODIMP TransportSecurityInfo::GetIsDomainMismatch(bool* aIsDomainMismatch) { NS_ENSURE_ARG_POINTER(aIsDomainMismatch); - *aIsDomainMismatch = mHaveCertErrorBits && mIsDomainMismatch; return NS_OK; } @@ -1150,7 +1168,6 @@ TransportSecurityInfo::GetIsDomainMismatch(bool* aIsDomainMismatch) { NS_IMETHODIMP TransportSecurityInfo::GetIsNotValidAtThisTime(bool* aIsNotValidAtThisTime) { NS_ENSURE_ARG_POINTER(aIsNotValidAtThisTime); - *aIsNotValidAtThisTime = mHaveCertErrorBits && mIsNotValidAtThisTime; return NS_OK; } @@ -1158,7 +1175,6 @@ TransportSecurityInfo::GetIsNotValidAtThisTime(bool* aIsNotValidAtThisTime) { NS_IMETHODIMP TransportSecurityInfo::GetIsUntrusted(bool* aIsUntrusted) { NS_ENSURE_ARG_POINTER(aIsUntrusted); - *aIsUntrusted = mHaveCertErrorBits && mIsUntrusted; return NS_OK; } @@ -1167,7 +1183,6 @@ NS_IMETHODIMP TransportSecurityInfo::GetIsExtendedValidation(bool* aIsEV) { NS_ENSURE_ARG_POINTER(aIsEV); *aIsEV = false; - // Never allow bad certs for EV, regardless of overrides. if (mHaveCertErrorBits) { return NS_OK; @@ -1203,6 +1218,8 @@ TransportSecurityInfo::GetIsDelegatedCredential(bool* aIsDelegCred) { NS_IMETHODIMP TransportSecurityInfo::GetNegotiatedNPN(nsACString& aNegotiatedNPN) { + MutexAutoLock lock(mMutex); + if (!mNPNCompleted) { return NS_ERROR_NOT_CONNECTED; } @@ -1217,10 +1234,7 @@ TransportSecurityInfo::GetResumed(bool* aResumed) { return NS_OK; } -void TransportSecurityInfo::SetResumed(bool aResumed) { - MutexAutoLock lock(mMutex); - mResumed = aResumed; -} +void TransportSecurityInfo::SetResumed(bool aResumed) { mResumed = aResumed; } } // namespace psm } // namespace mozilla diff --git a/security/manager/ssl/TransportSecurityInfo.h b/security/manager/ssl/TransportSecurityInfo.h index d832e886c496..3680956a034f 100644 --- a/security/manager/ssl/TransportSecurityInfo.h +++ b/security/manager/ssl/TransportSecurityInfo.h @@ -18,6 +18,7 @@ #include "mozpkix/pkixtypes.h" #include "nsDataHashtable.h" #include "nsIClassInfo.h" +#include "nsIObjectInputStream.h" #include "nsIInterfaceRequestor.h" #include "nsITransportSecurityInfo.h" #include "nsNSSCertificate.h" @@ -56,7 +57,10 @@ class TransportSecurityInfo : public nsITransportSecurityInfo, return result; } - const nsACString& GetHostName() const { return mHostName; } + const nsACString& GetHostName() const { + MutexAutoLock lock(mMutex); + return mHostName; + } void SetHostName(const char* host); @@ -64,6 +68,10 @@ class TransportSecurityInfo : public nsITransportSecurityInfo, void SetPort(int32_t aPort); const OriginAttributes& GetOriginAttributes() const { + MutexAutoLock lock(mMutex); + return mOriginAttributes; + } + const OriginAttributes& GetOriginAttributes(MutexAutoLock& aProofOfLock) const { return mOriginAttributes; } void SetOriginAttributes(const OriginAttributes& aOriginAttributes); @@ -79,7 +87,10 @@ class TransportSecurityInfo : public nsITransportSecurityInfo, nsresult SetSucceededCertChain(nsTArray>&& certList); - bool HasServerCert() { return mServerCert != nullptr; } + bool HasServerCert() { + MutexAutoLock lock(mMutex); + return mServerCert != nullptr; + } static uint16_t ConvertCertificateTransparencyInfoToStatus( const mozilla::psm::CertificateTransparencyInfo& info); @@ -92,6 +103,7 @@ class TransportSecurityInfo : public nsITransportSecurityInfo, void SetCertificateTransparencyStatus( uint16_t aCertificateTransparencyStatus) { + MutexAutoLock lock(mMutex); mCertificateTransparencyStatus = aCertificateTransparencyStatus; } @@ -103,19 +115,19 @@ class TransportSecurityInfo : public nsITransportSecurityInfo, nsCString mKeaGroup; nsCString mSignatureSchemeName; - bool mIsAcceptedEch; - bool mIsDelegatedCredential; - bool mIsDomainMismatch; - bool mIsNotValidAtThisTime; - bool mIsUntrusted; - bool mIsEV; + Atomic mIsAcceptedEch; + Atomic mIsDelegatedCredential; + Atomic mIsDomainMismatch; + Atomic mIsNotValidAtThisTime; + Atomic mIsUntrusted; + Atomic mIsEV; - bool mHasIsEVStatus; - bool mHaveCipherSuiteAndProtocol; + Atomic mHasIsEVStatus; + Atomic mHaveCipherSuiteAndProtocol; /* mHaveCertErrrorBits is relied on to determine whether or not a SPDY connection is eligible for joining in nsNSSSocketInfo::JoinConnection() */ - bool mHaveCertErrorBits; + Atomic mHaveCertErrorBits; private: // True if SetCanceled has been called (or if this was deserialized with a @@ -128,17 +140,51 @@ class TransportSecurityInfo : public nsITransportSecurityInfo, nsCOMPtr mCallbacks; nsTArray> mSucceededCertChain; - bool mNPNCompleted; + Atomic mNPNCompleted; nsCString mNegotiatedNPN; - bool mResumed; - bool mIsBuiltCertChainRootBuiltInRoot; + Atomic mResumed; + Atomic mIsBuiltCertChainRootBuiltInRoot; private: - uint32_t mSecurityState; + static nsresult ReadBoolAndSetAtomicFieldHelper(nsIObjectInputStream* stream, + Atomic& atomic) { + bool tmpBool; + nsresult rv = stream->ReadBoolean(&tmpBool); + if (NS_FAILED(rv)) { + return rv; + } + atomic = tmpBool; + return rv; + } - PRErrorCode mErrorCode; + static nsresult ReadUint32AndSetAtomicFieldHelper( + nsIObjectInputStream* stream, Atomic& atomic) { + uint32_t tmpInt; + nsresult rv = stream->Read32(&tmpInt); + if (NS_FAILED(rv)) { + return rv; + } + atomic = tmpInt; + return rv; + } - int32_t mPort; + template + static bool ReadParamAtomicHelper(const IPC::Message* aMsg, + PickleIterator* aIter, Atomic

& atomic) { + P tmpStore; + bool result = ReadParam(aMsg, aIter, &tmpStore); + if (result == false) { + return result; + } + atomic = tmpStore; + return result; + } + + Atomic mSecurityState; + + Atomic mErrorCode; + + Atomic mPort; nsCString mHostName; OriginAttributes mOriginAttributes; @@ -147,15 +193,18 @@ class TransportSecurityInfo : public nsITransportSecurityInfo, /* Peer cert chain for failed connections (for error reporting) */ nsTArray> mFailedCertChain; - nsresult ReadSSLStatus(nsIObjectInputStream* aStream); + nsresult ReadSSLStatus(nsIObjectInputStream* aStream, + MutexAutoLock& aProofOfLock); // This function is used to read the binary that are serialized // by using nsIX509CertList nsresult ReadCertList(nsIObjectInputStream* aStream, - nsTArray>& aCertList); + nsTArray>& aCertList, + MutexAutoLock& aProofOfLock); nsresult ReadCertificatesFromStream(nsIObjectInputStream* aStream, uint32_t aSize, - nsTArray>& aCertList); + nsTArray>& aCertList, + MutexAutoLock& aProofOfLock); }; class RememberCertErrorsTable { diff --git a/security/manager/ssl/nsNSSIOLayer.cpp b/security/manager/ssl/nsNSSIOLayer.cpp index 6e31d1fb36ed..b95506eabcdb 100644 --- a/security/manager/ssl/nsNSSIOLayer.cpp +++ b/security/manager/ssl/nsNSSIOLayer.cpp @@ -191,6 +191,7 @@ nsNSSSocketInfo::SetClientCert(nsIX509Cert* aClientCert) { } void nsNSSSocketInfo::NoteTimeUntilReady() { + MutexAutoLock lock(mMutex); if (mNotedTimeUntilReady) return; mNotedTimeUntilReady = true; @@ -216,7 +217,7 @@ void nsNSSSocketInfo::SetHandshakeCompleted() { : mFalseStartCallbackCalled ? ChoseNotToFalseStart : NotAllowedToFalseStart; - + MutexAutoLock lock(mMutex); // This will include TCP and proxy tunnel wait time Telemetry::AccumulateTimeDelta( Telemetry::SSL_TIME_UNTIL_HANDSHAKE_FINISHED_KEYED_BY_KA, mKeaGroup, @@ -252,6 +253,7 @@ void nsNSSSocketInfo::SetHandshakeCompleted() { } void nsNSSSocketInfo::SetNegotiatedNPN(const char* value, uint32_t length) { + MutexAutoLock lock(mMutex); if (!value) { mNegotiatedNPN.Truncate(); } else {