diff --git a/security/certverifier/CertVerifier.h b/security/certverifier/CertVerifier.h index 93ab1b5231e..0816990dc15 100644 --- a/security/certverifier/CertVerifier.h +++ b/security/certverifier/CertVerifier.h @@ -79,8 +79,6 @@ private: }; void InitCertVerifierLog(); -SECStatus IsCertBuiltInRoot(CERTCertificate* cert, bool& result); - } } // namespace mozilla::psm #endif // mozilla_psm__CertVerifier_h diff --git a/security/manager/ssl/src/SSLServerCertVerification.cpp b/security/manager/ssl/src/SSLServerCertVerification.cpp index 316e8cb44dd..8c9837b37cd 100644 --- a/security/manager/ssl/src/SSLServerCertVerification.cpp +++ b/security/manager/ssl/src/SSLServerCertVerification.cpp @@ -98,7 +98,6 @@ #include "pkix/pkixtypes.h" #include "pkix/pkixnss.h" -#include "pkix/ScopedPtr.h" #include "CertVerifier.h" #include "CryptoTask.h" #include "ExtendedValidation.h" @@ -122,7 +121,6 @@ #include "PSMRunnable.h" #include "SharedSSLState.h" #include "nsContentUtils.h" -#include "nsURLHelper.h" #include "ssl.h" #include "secerr.h" @@ -741,215 +739,6 @@ BlockServerCertChangeForSpdy(nsNSSSocketInfo* infoObject, return SECFailure; } -void -AccumulateSubjectCommonNameTelemetry(const char* commonName, - bool commonNameInSubjectAltNames) -{ - if (!commonName) { - // 1 means no common name present - Telemetry::Accumulate(Telemetry::BR_9_2_2_SUBJECT_COMMON_NAME, 1); - } else if (!commonNameInSubjectAltNames) { - PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, - ("BR telemetry: common name '%s' not in subject alt. names " - "(or the subject alt. names extension is not present)\n", - commonName)); - // 2 means the common name is not present in subject alt names - Telemetry::Accumulate(Telemetry::BR_9_2_2_SUBJECT_COMMON_NAME, 2); - } else { - // 0 means the common name is present in subject alt names - Telemetry::Accumulate(Telemetry::BR_9_2_2_SUBJECT_COMMON_NAME, 0); - } -} - -// Returns true if and only if commonName ends with altName (minus its leading -// "*"). altName has already been checked to be of the form "*.". -// commonName may be NULL. -static bool -TryMatchingWildcardSubjectAltName(const char* commonName, - nsDependentCString altName) -{ - if (!commonName) { - return false; - } - // altNameSubstr is now "." - nsDependentCString altNameSubstr(altName.get() + 1, altName.Length() - 1); - nsDependentCString commonNameStr(commonName, strlen(commonName)); - int32_t altNameIndex = commonNameStr.Find(altNameSubstr); - // This only matches if the end of commonNameStr is the altName without - // the '*'. - // Consider this.example.com and *.example.com: - // "this.example.com".Find(".example.com") is 4 - // 4 + ".example.com".Length() == 4 + 12 == 16 == "this.example.com".Length() - // Now this.example.com and *.example: - // "this.example.com".Find(".example") is 4 - // 4 + ".example".Length() == 4 + 8 == 12 != "this.example.com".Length() - return altNameIndex >= 0 && - altNameIndex + altNameSubstr.Length() == commonNameStr.Length(); -} - -// Gathers telemetry on Baseline Requirements 9.2.1 (Subject Alternative -// Names Extension) and 9.2.2 (Subject Common Name Field). -// Specifically: -// - whether or not the subject common name field is present -// - whether or not the subject alternative names extension is present -// - if there is a malformed entry in the subject alt. names extension -// - if there is an entry in the subject alt. names extension corresponding -// to the subject common name -// Telemetry is only gathered for certificates that chain to a trusted root -// in Mozilla's Root CA program. -// certList consists of a validated certificate chain. The end-entity -// certificate is first and the root (trust anchor) is last. -void -GatherBaselineRequirementsTelemetry(const ScopedCERTCertList& certList) -{ - CERTCertListNode* endEntityNode = CERT_LIST_HEAD(certList); - CERTCertListNode* rootNode = CERT_LIST_TAIL(certList); - PR_ASSERT(endEntityNode && rootNode); - if (!endEntityNode || !rootNode) { - return; - } - CERTCertificate* cert = endEntityNode->cert; - mozilla::pkix::ScopedPtr commonName( - CERT_GetCommonName(&cert->subject)); - // This only applies to certificates issued by authorities in our root - // program. - bool isBuiltIn = false; - SECStatus rv = IsCertBuiltInRoot(rootNode->cert, isBuiltIn); - if (rv != SECSuccess || !isBuiltIn) { - PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, - ("BR telemetry: '%s' not a built-in root (or IsCertBuiltInRoot " - "failed)\n", commonName.get())); - return; - } - SECItem altNameExtension; - rv = CERT_FindCertExtension(cert, SEC_OID_X509_SUBJECT_ALT_NAME, - &altNameExtension); - if (rv != SECSuccess) { - PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, - ("BR telemetry: no subject alt names extension for '%s'\n", - commonName.get())); - // 1 means there is no subject alt names extension - Telemetry::Accumulate(Telemetry::BR_9_2_1_SUBJECT_ALT_NAMES, 1); - AccumulateSubjectCommonNameTelemetry(commonName.get(), false); - return; - } - - ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE)); - CERTGeneralName* subjectAltNames = - CERT_DecodeAltNameExtension(arena, &altNameExtension); - // CERT_FindCertExtension takes a pointer to a SECItem and allocates memory - // in its data field. This is a bad way to do this because we can't use a - // ScopedSECItem and neither is that memory tracked by an arena. We have to - // manually reach in and free the memory. - PORT_Free(altNameExtension.data); - if (!subjectAltNames) { - PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, - ("BR telemetry: could not decode subject alt names for '%s'\n", - commonName.get())); - // 2 means the subject alt names extension could not be decoded - Telemetry::Accumulate(Telemetry::BR_9_2_1_SUBJECT_ALT_NAMES, 2); - AccumulateSubjectCommonNameTelemetry(commonName.get(), false); - return; - } - - CERTGeneralName* currentName = subjectAltNames; - bool commonNameInSubjectAltNames = false; - bool nonDNSNameOrIPAddressPresent = false; - bool malformedDNSNameOrIPAddressPresent = false; - bool nonFQDNPresent = false; - do { - nsDependentCString altName; - if (currentName->type == certDNSName) { - altName.Assign(reinterpret_cast(currentName->name.other.data), - currentName->name.other.len); - nsDependentCString altNameWithoutWildcard(altName); - if (altNameWithoutWildcard.Find("*.") == 0) { - altNameWithoutWildcard.Assign(altName.get() + 2, altName.Length() - 2); - commonNameInSubjectAltNames |= - TryMatchingWildcardSubjectAltName(commonName.get(), altName); - } - // net_IsValidHostName appears to return true for valid IP addresses, - // which would be invalid for a DNS name. - // Note that the net_IsValidHostName check will catch things like - // "a.*.example.com". - if (!net_IsValidHostName(altNameWithoutWildcard) || - net_IsValidIPv4Addr(altName.get(), altName.Length()) || - net_IsValidIPv6Addr(altName.get(), altName.Length())) { - PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, - ("BR telemetry: DNSName '%s' not valid (for '%s')\n", - altName.get(), commonName.get())); - malformedDNSNameOrIPAddressPresent = true; - } - if (altName.FindChar('.') == kNotFound) { - nonFQDNPresent = true; - } - } else if (currentName->type == certIPAddress) { - char buf[INET6_ADDRSTRLEN + 1] = { 0 }; - PRNetAddr addr; - if (currentName->name.other.len == 4) { - addr.inet.family = PR_AF_INET; - memcpy(&addr.inet.ip, currentName->name.other.data, - currentName->name.other.len); - if (PR_NetAddrToString(&addr, buf, sizeof(buf) - 1) != PR_SUCCESS) { - PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, - ("BR telemetry: IPAddress (v4) not valid (for '%s')\n", - commonName.get())); - malformedDNSNameOrIPAddressPresent = true; - } else { - altName.Assign(buf, strlen(buf)); - } - } else if (currentName->name.other.len == 16) { - addr.inet.family = PR_AF_INET6; - memcpy(&addr.ipv6.ip, currentName->name.other.data, - currentName->name.other.len); - if (PR_NetAddrToString(&addr, buf, sizeof(buf) - 1) != PR_SUCCESS) { - PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, - ("BR telemetry: IPAddress (v6) not valid (for '%s')\n", - commonName.get())); - malformedDNSNameOrIPAddressPresent = true; - } else { - altName.Assign(buf, strlen(buf)); - } - } else { - PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, - ("BR telemetry: IPAddress not valid (for '%s')\n", - commonName.get())); - malformedDNSNameOrIPAddressPresent = true; - } - } else { - PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, - ("BR telemetry: non-DNSName, non-IPAddress present for '%s'\n", - commonName.get())); - nonDNSNameOrIPAddressPresent = true; - } - if (commonName && altName.Equals(commonName.get())) { - commonNameInSubjectAltNames = true; - } - currentName = CERT_GetNextGeneralName(currentName); - } while (currentName && currentName != subjectAltNames); - - if (nonDNSNameOrIPAddressPresent) { - // 3 means there's an entry that isn't an ip address or dns name - Telemetry::Accumulate(Telemetry::BR_9_2_1_SUBJECT_ALT_NAMES, 3); - } - if (malformedDNSNameOrIPAddressPresent) { - // 4 means there's a malformed ip address or dns name entry - Telemetry::Accumulate(Telemetry::BR_9_2_1_SUBJECT_ALT_NAMES, 4); - } - if (nonFQDNPresent) { - // 5 means there's a DNS name entry with a non-fully-qualified domain name - Telemetry::Accumulate(Telemetry::BR_9_2_1_SUBJECT_ALT_NAMES, 5); - } - if (!nonDNSNameOrIPAddressPresent && !malformedDNSNameOrIPAddressPresent && - !nonFQDNPresent) { - // 0 means the extension is acceptable - Telemetry::Accumulate(Telemetry::BR_9_2_1_SUBJECT_ALT_NAMES, 0); - } - - AccumulateSubjectCommonNameTelemetry(commonName.get(), - commonNameInSubjectAltNames); -} - SECStatus AuthCertificate(CertVerifier& certVerifier, TransportSecurityInfo* infoObject, @@ -970,11 +759,10 @@ AuthCertificate(CertVerifier& certVerifier, !(providerFlags & nsISocketProvider::NO_PERMANENT_STORAGE); SECOidTag evOidPolicy; - ScopedCERTCertList certList; rv = certVerifier.VerifySSLServerCert(cert, stapledOCSPResponse, time, infoObject, infoObject->GetHostNameRaw(), - saveIntermediates, 0, &certList, + saveIntermediates, 0, nullptr, &evOidPolicy); // We want to remember the CA certs in the temp db, so that the application can find the @@ -994,7 +782,6 @@ AuthCertificate(CertVerifier& certVerifier, } if (rv == SECSuccess) { - GatherBaselineRequirementsTelemetry(certList); // The connection may get terminated, for example, if the server requires // a client cert. Let's provide a minimal SSLStatus // to the caller that contains at least the cert and its status. diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index 708dd3711d6..b1abd651407 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -6507,18 +6507,6 @@ "n_buckets": 10, "description": "Sidebar showing: seconds that the sidebar has been opened" }, - "BR_9_2_1_SUBJECT_ALT_NAMES": { - "expires_in_version": "never", - "kind": "enumerated", - "n_values": 8, - "description": "Baseline Requirements section 9.2.1: subject alternative names extension (0: ok, 1 or more: error)" - }, - "BR_9_2_2_SUBJECT_COMMON_NAME": { - "expires_in_version": "never", - "kind": "enumerated", - "n_values": 8, - "description": "Baseline Requirements section 9.2.2: subject common name field (0: present, in subject alt. names; 1: not present; 2: not present in subject alt. names)" - }, "TRACKING_PROTECTION_ENABLED": { "expires_in_version": "never", "kind": "boolean",