Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-24 21:31:04 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1672120 - Access of TransportSecurityInfo fields should hold mutex r=keeler,necko-reviewers,valentin

Browse Source 
Differential Revision: https://phabricator.services.mozilla.com/D97632
This commit is contained in:
Moritz Birghan 2020-12-08 15:22:08 +00:00
parent 4539b31797
commit 9c3afcc70e
5 changed files with 97 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
netwerk/protocol/http/QuicSocketControl.cpp
View File

@ -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;

22
security/manager/ssl/CommonSocketControl.cpp
View File

@ -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;
@ -117,11 +122,14 @@ CommonSocketControl::IsAcceptableForHost(const nsACString& hostname,
return NS_OK;
}
// If the cert has error bits (e.g. it is untrusted) then do not join.
// The value of mHaveCertErrorBits is only reliable because we know that
// the handshake completed.
if (mHaveCertErrorBits) {
return NS_OK;
{
MutexAutoLock lock(mMutex);
// If the cert has error bits (e.g. it is untrusted) then do not join.
// The value of mHaveCertErrorBits is only reliable because we know that
// the handshake completed.
if (mHaveCertErrorBits) {
return NS_OK;
}
}
// If the connection is using client certificates then do not join

71
security/manager/ssl/TransportSecurityInfo.cpp
View File

@ -68,13 +68,21 @@ NS_IMPL_ISUPPORTS(TransportSecurityInfo, nsITransportSecurityInfo,
nsIInterfaceRequestor, nsISerializable, nsIClassInfo)
void TransportSecurityInfo::SetHostName(const char* host) {
MutexAutoLock lock(mMutex);
mHostName.Assign(host);
}
void TransportSecurityInfo::SetPort(int32_t aPort) { mPort = aPort; }
void TransportSecurityInfo::SetPort(int32_t aPort) {
MutexAutoLock lock(mMutex);
mPort = aPort;
}
void TransportSecurityInfo::SetOriginAttributes(
const OriginAttributes& aOriginAttributes) {
MutexAutoLock lock(mMutex);
mOriginAttributes = aOriginAttributes;
}
@ -112,11 +120,15 @@ bool TransportSecurityInfo::IsCanceled() { return mCanceled; }
NS_IMETHODIMP
TransportSecurityInfo::GetSecurityState(uint32_t* state) {
MutexAutoLock lock(mMutex);
*state = mSecurityState;
return NS_OK;
}
void TransportSecurityInfo::SetSecurityState(uint32_t aState) {
MutexAutoLock lock(mMutex);
mSecurityState = aState;
}
@ -139,6 +151,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 +304,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");
@ -397,7 +411,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 +420,7 @@ nsresult TransportSecurityInfo::ReadSSLStatus(nsIObjectInputStream* aStream) {
// Read only to consume bytes from the stream.
nsTArray<RefPtr<nsIX509Cert>> 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 +433,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<RefPtr<nsIX509Cert>>& aCertList) {
nsIObjectInputStream* aStream, nsTArray<RefPtr<nsIX509Cert>>& aCertList,
MutexAutoLock& aProofOfLock) {
bool nsIX509CertListPresent;
nsresult rv = aStream->ReadBoolean(&nsIX509CertListPresent);
@ -455,12 +470,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<RefPtr<nsIX509Cert>>& aCertList) {
nsTArray<RefPtr<nsIX509Cert>>& aCertList, MutexAutoLock& aProofOfLock) {
nsresult rv;
for (uint32_t i = 0; i < aSize; ++i) {
nsCOMPtr<nsISupports> support;
@ -543,7 +559,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);
@ -620,7 +636,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 +647,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 +656,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 +667,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);
}
@ -917,9 +934,8 @@ void RememberCertErrorsTable::LookupCertErrorBits(
void TransportSecurityInfo::SetStatusErrorBits(nsNSSCertificate* cert,
uint32_t collected_errors) {
MutexAutoLock lock(mMutex);
SetServerCert(cert, EVStatus::NotEV);
MutexAutoLock lock(mMutex);
mHaveCertErrorBits = true;
mIsDomainMismatch = collected_errors & nsICertOverrideService::ERROR_MISMATCH;
@ -933,6 +949,7 @@ NS_IMETHODIMP
TransportSecurityInfo::GetFailedCertChain(
nsTArray<RefPtr<nsIX509Cert>>& aFailedCertChain) {
MOZ_ASSERT(aFailedCertChain.IsEmpty());
MutexAutoLock lock(mMutex);
if (!aFailedCertChain.IsEmpty()) {
return NS_ERROR_INVALID_ARG;
}
@ -957,11 +974,14 @@ static nsresult CreateCertChain(nsTArray<RefPtr<nsIX509Cert>>& aOutput,
nsresult TransportSecurityInfo::SetFailedCertChain(
nsTArray<nsTArray<uint8_t>>&& 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<nsIX509Cert> cert = mServerCert;
cert.forget(aServerCert);
@ -971,6 +991,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);
@ -997,6 +1018,8 @@ nsresult TransportSecurityInfo::SetSucceededCertChain(
NS_IMETHODIMP TransportSecurityInfo::SetIsBuiltCertChainRootBuiltInRoot(
bool aIsBuiltInRoot) {
MutexAutoLock lock(mMutex);
mIsBuiltCertChainRootBuiltInRoot = aIsBuiltInRoot;
return NS_OK;
}
@ -1004,12 +1027,15 @@ NS_IMETHODIMP TransportSecurityInfo::SetIsBuiltCertChainRootBuiltInRoot(
NS_IMETHODIMP TransportSecurityInfo::GetIsBuiltCertChainRootBuiltInRoot(
bool* aIsBuiltInRoot) {
NS_ENSURE_ARG_POINTER(aIsBuiltInRoot);
MutexAutoLock lock(mMutex);
*aIsBuiltInRoot = mIsBuiltCertChainRootBuiltInRoot;
return NS_OK;
}
NS_IMETHODIMP
TransportSecurityInfo::GetCipherName(nsACString& aCipherName) {
MutexAutoLock lock(mMutex);
if (!mHaveCipherSuiteAndProtocol) {
return NS_ERROR_NOT_AVAILABLE;
}
@ -1027,6 +1053,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 +1071,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 +1088,8 @@ TransportSecurityInfo::GetSecretKeyLength(uint32_t* aSecretKeyLength) {
NS_IMETHODIMP
TransportSecurityInfo::GetKeaGroupName(nsACString& aKeaGroup) {
MutexAutoLock lock(mMutex);
if (!mHaveCipherSuiteAndProtocol) {
return NS_ERROR_NOT_AVAILABLE;
}
@ -1070,6 +1100,8 @@ TransportSecurityInfo::GetKeaGroupName(nsACString& aKeaGroup) {
NS_IMETHODIMP
TransportSecurityInfo::GetSignatureSchemeName(nsACString& aSignatureScheme) {
MutexAutoLock lock(mMutex);
if (!mHaveCipherSuiteAndProtocol) {
return NS_ERROR_NOT_AVAILABLE;
}
@ -1081,6 +1113,7 @@ TransportSecurityInfo::GetSignatureSchemeName(nsACString& aSignatureScheme) {
NS_IMETHODIMP
TransportSecurityInfo::GetProtocolVersion(uint16_t* aProtocolVersion) {
NS_ENSURE_ARG_POINTER(aProtocolVersion);
MutexAutoLock lock(mMutex);
if (!mHaveCipherSuiteAndProtocol) {
return NS_ERROR_NOT_AVAILABLE;
}
@ -1093,6 +1126,7 @@ NS_IMETHODIMP
TransportSecurityInfo::GetCertificateTransparencyStatus(
uint16_t* aCertificateTransparencyStatus) {
NS_ENSURE_ARG_POINTER(aCertificateTransparencyStatus);
MutexAutoLock lock(mMutex);
*aCertificateTransparencyStatus = mCertificateTransparencyStatus;
return NS_OK;
@ -1142,6 +1176,7 @@ nsTArray<nsTArray<uint8_t>> TransportSecurityInfo::CreateCertBytesArray(
NS_IMETHODIMP
TransportSecurityInfo::GetIsDomainMismatch(bool* aIsDomainMismatch) {
NS_ENSURE_ARG_POINTER(aIsDomainMismatch);
MutexAutoLock lock(mMutex);
*aIsDomainMismatch = mHaveCertErrorBits && mIsDomainMismatch;
return NS_OK;
@ -1150,6 +1185,7 @@ TransportSecurityInfo::GetIsDomainMismatch(bool* aIsDomainMismatch) {
NS_IMETHODIMP
TransportSecurityInfo::GetIsNotValidAtThisTime(bool* aIsNotValidAtThisTime) {
NS_ENSURE_ARG_POINTER(aIsNotValidAtThisTime);
MutexAutoLock lock(mMutex);
*aIsNotValidAtThisTime = mHaveCertErrorBits && mIsNotValidAtThisTime;
return NS_OK;
@ -1158,6 +1194,7 @@ TransportSecurityInfo::GetIsNotValidAtThisTime(bool* aIsNotValidAtThisTime) {
NS_IMETHODIMP
TransportSecurityInfo::GetIsUntrusted(bool* aIsUntrusted) {
NS_ENSURE_ARG_POINTER(aIsUntrusted);
MutexAutoLock lock(mMutex);
*aIsUntrusted = mHaveCertErrorBits && mIsUntrusted;
return NS_OK;
@ -1167,6 +1204,7 @@ NS_IMETHODIMP
TransportSecurityInfo::GetIsExtendedValidation(bool* aIsEV) {
NS_ENSURE_ARG_POINTER(aIsEV);
*aIsEV = false;
MutexAutoLock lock(mMutex);
// Never allow bad certs for EV, regardless of overrides.
if (mHaveCertErrorBits) {
@ -1194,6 +1232,7 @@ TransportSecurityInfo::GetIsAcceptedEch(bool* aIsAcceptedEch) {
NS_IMETHODIMP
TransportSecurityInfo::GetIsDelegatedCredential(bool* aIsDelegCred) {
NS_ENSURE_ARG_POINTER(aIsDelegCred);
MutexAutoLock lock(mMutex);
if (!mHaveCipherSuiteAndProtocol) {
return NS_ERROR_NOT_AVAILABLE;
}
@ -1203,6 +1242,8 @@ TransportSecurityInfo::GetIsDelegatedCredential(bool* aIsDelegCred) {
NS_IMETHODIMP
TransportSecurityInfo::GetNegotiatedNPN(nsACString& aNegotiatedNPN) {
MutexAutoLock lock(mMutex);
if (!mNPNCompleted) {
return NS_ERROR_NOT_CONNECTED;
}
@ -1213,6 +1254,8 @@ TransportSecurityInfo::GetNegotiatedNPN(nsACString& aNegotiatedNPN) {
NS_IMETHODIMP
TransportSecurityInfo::GetResumed(bool* aResumed) {
MutexAutoLock lock(mMutex);
*aResumed = mResumed;
return NS_OK;
}

26
security/manager/ssl/TransportSecurityInfo.h
View File

@ -56,14 +56,21 @@ 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);
int32_t GetPort() const { return mPort; }
int32_t GetPort() const {
MutexAutoLock lock(mMutex);
return mPort;
}
void SetPort(int32_t aPort);
const OriginAttributes& GetOriginAttributes() const {
MutexAutoLock lock(mMutex);
return mOriginAttributes;
}
void SetOriginAttributes(const OriginAttributes& aOriginAttributes);
@ -79,7 +86,10 @@ class TransportSecurityInfo : public nsITransportSecurityInfo,
nsresult SetSucceededCertChain(nsTArray<nsTArray<uint8_t>>&& 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 +102,7 @@ class TransportSecurityInfo : public nsITransportSecurityInfo,
void SetCertificateTransparencyStatus(
uint16_t aCertificateTransparencyStatus) {
MutexAutoLock lock(mMutex);
mCertificateTransparencyStatus = aCertificateTransparencyStatus;
}
@ -147,15 +158,18 @@ class TransportSecurityInfo : public nsITransportSecurityInfo,
/* Peer cert chain for failed connections (for error reporting) */
nsTArray<RefPtr<nsIX509Cert>> 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<RefPtr<nsIX509Cert>>& aCertList);
nsTArray<RefPtr<nsIX509Cert>>& aCertList,
MutexAutoLock& aProofOfLock);
nsresult ReadCertificatesFromStream(nsIObjectInputStream* aStream,
uint32_t aSize,
nsTArray<RefPtr<nsIX509Cert>>& aCertList);
nsTArray<RefPtr<nsIX509Cert>>& aCertList,
MutexAutoLock& aProofOfLock);
};
class RememberCertErrorsTable {

4
security/manager/ssl/nsNSSIOLayer.cpp
View File

@ -193,6 +193,7 @@ nsNSSSocketInfo::SetClientCert(nsIX509Cert* aClientCert) {
}
void nsNSSSocketInfo::NoteTimeUntilReady() {
MutexAutoLock lock(mMutex);
if (mNotedTimeUntilReady) return;
mNotedTimeUntilReady = true;
@ -223,7 +224,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,
@ -259,6 +260,7 @@ void nsNSSSocketInfo::SetHandshakeCompleted() {
}
void nsNSSSocketInfo::SetNegotiatedNPN(const char* value, uint32_t length) {
MutexAutoLock lock(mMutex);
if (!value) {
mNegotiatedNPN.Truncate();
} else {