[tbb-commits] [tor-browser] 08/08: fixup! Bug 23247: Communicating security expectations for .onion
gitolite role
git at cupani.torproject.org
Tue Aug 23 15:45:19 UTC 2022
This is an automated email from the git hooks/post-receive script.
richard pushed a commit to branch tor-browser-91.13.0esr-11.5-1
in repository tor-browser.
commit a4c279e9f8e5e1ce82c32d4973622b1911f83a2a
Author: Dan Ballard <dan at mindstab.net>
AuthorDate: Tue Aug 23 15:36:44 2022 +0000
fixup! Bug 23247: Communicating security expectations for .onion
Bug 41075: The Tor Browser is showing caution sign but your document said it won't
---
browser/base/content/browser-siteIdentity.js | 6 ++++--
security/certverifier/CertVerifier.cpp | 22 ++++++++++++++++++----
security/manager/ssl/SSLServerCertVerification.cpp | 15 +++++++++++++--
security/manager/ssl/nsNSSIOLayer.cpp | 13 ++++++++++---
security/nss/lib/mozpkix/include/pkix/Result.h | 2 ++
security/nss/lib/mozpkix/include/pkix/pkixnss.h | 1 +
6 files changed, 48 insertions(+), 11 deletions(-)
diff --git a/browser/base/content/browser-siteIdentity.js b/browser/base/content/browser-siteIdentity.js
index b7d59db3dd34d..e45b65ddac158 100644
--- a/browser/base/content/browser-siteIdentity.js
+++ b/browser/base/content/browser-siteIdentity.js
@@ -767,8 +767,10 @@ var gIdentityHandler = {
issuerCert = this._secInfo.succeededCertChain[
this._secInfo.succeededCertChain.length - 1
];
-
- return !issuerCert.isBuiltInRoot;
+ if (issuerCert) {
+ return !issuerCert.isBuiltInRoot;
+ }
+ return false;
},
/**
diff --git a/security/certverifier/CertVerifier.cpp b/security/certverifier/CertVerifier.cpp
index c427539bd67ea..e513eddb31e0c 100644
--- a/security/certverifier/CertVerifier.cpp
+++ b/security/certverifier/CertVerifier.cpp
@@ -918,6 +918,8 @@ Result CertVerifier::VerifySSLServerCert(
return Result::ERROR_BAD_CERT_DOMAIN;
}
+ bool errOnionWithSelfSignedCert = false;
+
// CreateCertErrorRunnable assumes that CheckCertHostname is only called
// if VerifyCert succeeded.
Result rv =
@@ -931,9 +933,16 @@ Result CertVerifier::VerifySSLServerCert(
CertIsSelfSigned(peerCert, pinarg)) {
// In this case we didn't find any issuer for the certificate and the
// certificate is self-signed.
- return Result::ERROR_SELF_SIGNED_CERT;
+ if (StringEndsWith(hostname, ".onion"_ns)) {
+ // Self signed cert over onion is deemed secure, the hidden service provides authentication.
+ // We defer returning this error and keep processing to determine if there are other legitimate
+ // certificate errors (such as expired, wrong domain) that we would like to surface to the user
+ errOnionWithSelfSignedCert = true;
+ } else {
+ return Result::ERROR_SELF_SIGNED_CERT;
+ }
}
- if (rv == Result::ERROR_UNKNOWN_ISSUER) {
+ if (rv == Result::ERROR_UNKNOWN_ISSUER && !errOnionWithSelfSignedCert) {
// In this case we didn't get any valid path for the cert. Let's see if
// the issuer is the same as the issuer for our canary probe. If yes, this
// connection is connecting via a misconfigured proxy.
@@ -951,7 +960,9 @@ Result CertVerifier::VerifySSLServerCert(
return Result::ERROR_MITM_DETECTED;
}
}
- return rv;
+ if (!errOnionWithSelfSignedCert) {
+ return rv;
+ }
}
if (dcInfo) {
@@ -995,7 +1006,7 @@ Result CertVerifier::VerifySSLServerCert(
}
bool isBuiltInRoot;
rv = IsCertChainRootBuiltInRoot(builtChain, isBuiltInRoot);
- if (rv != Success) {
+ if (rv != Success && !errOnionWithSelfSignedCert) {
return rv;
}
@@ -1016,6 +1027,9 @@ Result CertVerifier::VerifySSLServerCert(
return rv;
}
+ if (errOnionWithSelfSignedCert) {
+ return Result::ERROR_ONION_WITH_SELF_SIGNED_CERT;
+ }
return Success;
}
diff --git a/security/manager/ssl/SSLServerCertVerification.cpp b/security/manager/ssl/SSLServerCertVerification.cpp
index 0a84aecc6c724..a0c14be276dd6 100644
--- a/security/manager/ssl/SSLServerCertVerification.cpp
+++ b/security/manager/ssl/SSLServerCertVerification.cpp
@@ -299,6 +299,7 @@ SECStatus DetermineCertOverrideErrors(const UniqueCERTCertificate& cert,
case mozilla::pkix::MOZILLA_PKIX_ERROR_MITM_DETECTED:
case mozilla::pkix::MOZILLA_PKIX_ERROR_NOT_YET_VALID_ISSUER_CERTIFICATE:
case mozilla::pkix::MOZILLA_PKIX_ERROR_SELF_SIGNED_CERT:
+ case mozilla::pkix::MOZILLA_PKIX_ERROR_ONION_WITH_SELF_SIGNED_CERT:
case mozilla::pkix::MOZILLA_PKIX_ERROR_V1_CERT_USED_AS_CA: {
collectedErrors = nsICertOverrideService::ERROR_UNTRUSTED;
errorCodeTrust = defaultErrorCodeToReport;
@@ -984,6 +985,17 @@ PRErrorCode AuthCertificateParseResults(
gPIPNSSLog, LogLevel::Debug,
("[0x%" PRIx64 "] Certificate error was not overridden\n", aPtrForLog));
+ // If Onion with self signed cert we want to prioritize any other error
+ if (errorCodeTrust == MOZILLA_PKIX_ERROR_ONION_WITH_SELF_SIGNED_CERT) {
+ if (errorCodeMismatch) {
+ return errorCodeMismatch;
+ } else if (errorCodeTime) {
+ return errorCodeTime;
+ } else {
+ return MOZILLA_PKIX_ERROR_ONION_WITH_SELF_SIGNED_CERT;
+ }
+ }
+
// pick the error code to report by priority
return errorCodeTrust ? errorCodeTrust
: errorCodeMismatch ? errorCodeMismatch
@@ -1389,8 +1401,7 @@ SSLServerCertVerificationResult::Run() {
std::move(mPeerCertChain),
mCertificateTransparencyStatus, mEVStatus,
mSucceeded, mIsBuiltCertChainRootBuiltInRoot);
-
- if (!mSucceeded && mCollectedErrors != 0) {
+ if (!mSucceeded && mCollectedErrors != 0 && mFinalError != MOZILLA_PKIX_ERROR_ONION_WITH_SELF_SIGNED_CERT) {
mInfoObject->SetStatusErrorBits(mCert, mCollectedErrors);
}
mInfoObject->SetCertVerificationResult(mFinalError);
diff --git a/security/manager/ssl/nsNSSIOLayer.cpp b/security/manager/ssl/nsNSSIOLayer.cpp
index 21687447072d4..10d74b9eb3eb4 100644
--- a/security/manager/ssl/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/nsNSSIOLayer.cpp
@@ -411,7 +411,11 @@ void nsNSSSocketInfo::SetCertVerificationResult(PRErrorCode errorCode) {
"Invalid state transition to cert_verification_finished");
if (mFd) {
- SECStatus rv = SSL_AuthCertificateComplete(mFd, errorCode);
+ PRErrorCode passCode = errorCode;
+ if (errorCode == MOZILLA_PKIX_ERROR_ONION_WITH_SELF_SIGNED_CERT) {
+ passCode = 0;
+ }
+ SECStatus rv = SSL_AuthCertificateComplete(mFd, passCode);
// Only replace errorCode if there was originally no error
if (rv != SECSuccess && errorCode == 0) {
errorCode = PR_GetError();
@@ -422,12 +426,15 @@ void nsNSSSocketInfo::SetCertVerificationResult(PRErrorCode errorCode) {
}
}
- if (errorCode) {
+ if (errorCode &&
+ errorCode != MOZILLA_PKIX_ERROR_ONION_WITH_SELF_SIGNED_CERT) {
mFailedVerification = true;
SetCanceled(errorCode);
}
- if (mPlaintextBytesRead && !errorCode) {
+ if (mPlaintextBytesRead &&
+ (!errorCode ||
+ errorCode == MOZILLA_PKIX_ERROR_ONION_WITH_SELF_SIGNED_CERT)) {
Telemetry::Accumulate(Telemetry::SSL_BYTES_BEFORE_CERT_CALLBACK,
AssertedCast<uint32_t>(mPlaintextBytesRead));
}
diff --git a/security/nss/lib/mozpkix/include/pkix/Result.h b/security/nss/lib/mozpkix/include/pkix/Result.h
index 29461dc1a510b..b2ad3a383ceb3 100644
--- a/security/nss/lib/mozpkix/include/pkix/Result.h
+++ b/security/nss/lib/mozpkix/include/pkix/Result.h
@@ -188,6 +188,8 @@ static const unsigned int FATAL_ERROR_FLAG = 0x800;
SEC_ERROR_LIBRARY_FAILURE) \
MOZILLA_PKIX_MAP(FATAL_ERROR_NO_MEMORY, FATAL_ERROR_FLAG | 4, \
SEC_ERROR_NO_MEMORY) \
+ MOZILLA_PKIX_MAP(ERROR_ONION_WITH_SELF_SIGNED_CERT, 155, \
+ MOZILLA_PKIX_ERROR_ONION_WITH_SELF_SIGNED_CERT) \
/* nothing here */
enum class Result {
diff --git a/security/nss/lib/mozpkix/include/pkix/pkixnss.h b/security/nss/lib/mozpkix/include/pkix/pkixnss.h
index b181ca541e01c..16513a5dfb0b1 100644
--- a/security/nss/lib/mozpkix/include/pkix/pkixnss.h
+++ b/security/nss/lib/mozpkix/include/pkix/pkixnss.h
@@ -88,6 +88,7 @@ enum ErrorCode {
MOZILLA_PKIX_ERROR_ADDITIONAL_POLICY_CONSTRAINT_FAILED = ERROR_BASE + 13,
MOZILLA_PKIX_ERROR_SELF_SIGNED_CERT = ERROR_BASE + 14,
MOZILLA_PKIX_ERROR_MITM_DETECTED = ERROR_BASE + 15,
+ MOZILLA_PKIX_ERROR_ONION_WITH_SELF_SIGNED_CERT = ERROR_BASE + 100,
END_OF_LIST
};
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the tbb-commits
mailing list