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