From 5291778fb1e46902e32dc68c58a7044f928605a9 Mon Sep 17 00:00:00 2001 From: Dana Keeler Date: Wed, 3 Jul 2024 23:18:29 +0000 Subject: [PATCH] Bug 1905453 - collect telemetry on the prevalence of third-party pkcs#11 modules r=jschanck Differential Revision: https://phabricator.services.mozilla.com/D215563 --- .../certverifier/NSSCertDBTrustDomain.cpp | 18 +++++----- security/certverifier/NSSCertDBTrustDomain.h | 4 --- security/manager/ssl/EnterpriseRoots.cpp | 5 +-- security/manager/ssl/PKCS11ModuleDB.cpp | 35 ++++++++++++++++++- security/manager/ssl/PKCS11ModuleDB.h | 8 +++++ .../ssl/RootCertificateTelemetryUtils.cpp | 4 +-- security/manager/ssl/metrics.yaml | 16 +++++++++ security/manager/ssl/nsNSSCertHelper.cpp | 6 ---- security/manager/ssl/nsNSSCertHelper.h | 10 ------ security/manager/ssl/nsNSSComponent.cpp | 3 +- security/manager/ssl/nsNSSIOLayer.cpp | 1 - security/manager/ssl/nsPKCS11Slot.cpp | 8 +++-- .../ssl/tests/unit/test_pkcs11_module.js | 22 ++++++++++-- 13 files changed, 99 insertions(+), 41 deletions(-) diff --git a/security/certverifier/NSSCertDBTrustDomain.cpp b/security/certverifier/NSSCertDBTrustDomain.cpp index 9ceb4aa0d61f..32a89caba933 100644 --- a/security/certverifier/NSSCertDBTrustDomain.cpp +++ b/security/certverifier/NSSCertDBTrustDomain.cpp @@ -13,6 +13,7 @@ #include "ExtendedValidation.h" #include "MultiLogCTVerifier.h" #include "NSSErrorsService.h" +#include "PKCS11ModuleDB.h" #include "PublicKeyPinningService.h" #include "cert.h" #include "cert_storage/src/cert_storage.h" @@ -37,7 +38,6 @@ #include "nsCRTGlue.h" #include "nsIObserverService.h" #include "nsNSSCallbacks.h" -#include "nsNSSCertHelper.h" #include "nsNSSCertificate.h" #include "nsNSSCertificateDB.h" #include "nsNSSIOLayer.h" @@ -102,7 +102,7 @@ NSSCertDBTrustDomain::NSSCertDBTrustDomain( mHostname(hostname), mCertStorage(do_GetService(NS_CERT_STORAGE_CID)), mOCSPStaplingStatus(CertVerifier::OCSP_STAPLING_NEVER_CHECKED), - mBuiltInRootsModule(SECMOD_FindModule(kRootModuleName)), + mBuiltInRootsModule(SECMOD_FindModule(kRootModuleName.get())), mOCSPFetchStatus(OCSPFetchStatus::NotFetched) {} static void FindRootsWithSubject(UniqueSECMODModule& rootsModule, @@ -1686,6 +1686,8 @@ SECStatus InitializeNSS(const nsACString& dir, NSSDBConfig nssDbConfig, } } + CollectThirdPartyPKCS11ModuleTelemetry(); + return SECSuccess; } @@ -1750,8 +1752,6 @@ bool LoadUserModuleAt(const char* moduleName, const char* libraryName, return true; } -const char* kIPCClientCertsModuleName = "IPC Client Cert Module"; - bool LoadIPCClientCertsModule(const nsCString& dir) { // The IPC client certs module needs to be able to call back into gecko to be // able to communicate with the parent process over IPC. This is achieved by @@ -1759,14 +1759,14 @@ bool LoadIPCClientCertsModule(const nsCString& dir) { // extra string parameter that will be available when C_Initialize is called // on IPC client certs. nsPrintfCString addrs("%p,%p", DoFindObjects, DoSign); - if (!LoadUserModuleAt(kIPCClientCertsModuleName, "ipcclientcerts", dir, + if (!LoadUserModuleAt(kIPCClientCertsModuleName.get(), "ipcclientcerts", dir, addrs.get())) { return false; } RunOnShutdown( []() { UniqueSECMODModule ipcClientCertsModule( - SECMOD_FindModule(kIPCClientCertsModuleName)); + SECMOD_FindModule(kIPCClientCertsModuleName.get())); if (ipcClientCertsModule) { SECMOD_UnloadUserModule(ipcClientCertsModule.get()); } @@ -1775,14 +1775,12 @@ bool LoadIPCClientCertsModule(const nsCString& dir) { return true; } -const char* kOSClientCertsModuleName = "OS Client Cert Module"; - bool LoadOSClientCertsModule(const nsCString& dir) { nsLiteralCString params = StaticPrefs::security_osclientcerts_assume_rsa_pss_support() ? "RSA-PSS"_ns : ""_ns; - return LoadUserModuleAt(kOSClientCertsModuleName, "osclientcerts", dir, + return LoadUserModuleAt(kOSClientCertsModuleName.get(), "osclientcerts", dir, params.get()); } @@ -1794,7 +1792,7 @@ bool LoadLoadableRoots(const nsCString& dir) { // "Root Certs" module allows us to load the correct one. See bug 1406396. int unusedModType; Unused << SECMOD_DeleteModule("Root Certs", &unusedModType); - return LoadUserModuleAt(kRootModuleName, "nssckbi", dir, nullptr); + return LoadUserModuleAt(kRootModuleName.get(), "nssckbi", dir, nullptr); } nsresult DefaultServerNicknameForCert(const CERTCertificate* cert, diff --git a/security/certverifier/NSSCertDBTrustDomain.h b/security/certverifier/NSSCertDBTrustDomain.h index a21908233977..294e5dbdc0f6 100644 --- a/security/certverifier/NSSCertDBTrustDomain.h +++ b/security/certverifier/NSSCertDBTrustDomain.h @@ -87,8 +87,6 @@ bool LoadLoadableRoots(const nsCString& dir); */ bool LoadOSClientCertsModule(const nsCString& dir); -extern const char* kOSClientCertsModuleName; - /** * Loads the IPC client certs module. * @@ -99,8 +97,6 @@ extern const char* kOSClientCertsModuleName; */ bool LoadIPCClientCertsModule(const nsCString& dir); -extern const char* kIPCClientCertsModuleName; - /** * Unloads the loadable roots module and os client certs module, if loaded. */ diff --git a/security/manager/ssl/EnterpriseRoots.cpp b/security/manager/ssl/EnterpriseRoots.cpp index 940037cfc898..49544300eaa8 100644 --- a/security/manager/ssl/EnterpriseRoots.cpp +++ b/security/manager/ssl/EnterpriseRoots.cpp @@ -6,6 +6,7 @@ #include "EnterpriseRoots.h" +#include "PKCS11ModuleDB.h" #include "mozilla/ArrayUtils.h" #include "mozilla/IntegerPrintfMacros.h" #include "mozilla/Casting.h" @@ -13,7 +14,6 @@ #include "mozilla/Unused.h" #include "mozpkix/Result.h" #include "nsCRT.h" -#include "nsNSSCertHelper.h" #include "nsThreadUtils.h" #ifdef MOZ_WIDGET_ANDROID @@ -33,6 +33,7 @@ extern mozilla::LazyLogModule gPIPNSSLog; using namespace mozilla; +using namespace psm; void EnterpriseCert::CopyBytes(nsTArray& dest) const { dest.Assign(mDER); @@ -498,7 +499,7 @@ nsresult GatherEnterpriseCerts(nsTArray& certs) { } certs.Clear(); - UniqueSECMODModule rootsModule(SECMOD_FindModule(kRootModuleName)); + UniqueSECMODModule rootsModule(SECMOD_FindModule(kRootModuleName.get())); #ifdef XP_WIN GatherEnterpriseCertsWindows(certs, rootsModule); #endif // XP_WIN diff --git a/security/manager/ssl/PKCS11ModuleDB.cpp b/security/manager/ssl/PKCS11ModuleDB.cpp index 3825abb8b2c5..f47fdb2b56dd 100644 --- a/security/manager/ssl/PKCS11ModuleDB.cpp +++ b/security/manager/ssl/PKCS11ModuleDB.cpp @@ -7,6 +7,7 @@ #include "PKCS11ModuleDB.h" #include "ScopedNSSTypes.h" +#include "mozilla/glean/GleanMetrics.h" #include "nsComponentManagerUtils.h" #include "nsIMutableArray.h" #include "nsNSSCertHelper.h" @@ -33,7 +34,7 @@ static nsresult NormalizeModuleNameIn(const nsAString& moduleNameIn, return rv; } if (moduleNameIn.Equals(localizedRootModuleName)) { - moduleNameOut.Assign(kRootModuleName); + moduleNameOut.Assign(kRootModuleName.get()); return NS_OK; } moduleNameOut.Assign(NS_ConvertUTF16toUTF8(moduleNameIn)); @@ -59,6 +60,8 @@ PKCS11ModuleDB::DeleteModule(const nsAString& aModuleName) { return NS_ERROR_FAILURE; } + CollectThirdPartyPKCS11ModuleTelemetry(); + return NS_OK; } @@ -103,6 +106,9 @@ PKCS11ModuleDB::AddModule(const nsAString& aModuleName, if (srv != SECSuccess) { return NS_ERROR_FAILURE; } + + CollectThirdPartyPKCS11ModuleTelemetry(); + return NS_OK; } @@ -180,5 +186,32 @@ PKCS11ModuleDB::GetIsFIPSEnabled(bool* aIsFIPSEnabled) { return NS_OK; } +const nsLiteralCString kBuiltInModuleNames[] = { + kNSSInternalModuleName, + kRootModuleName, + kOSClientCertsModuleName, + kIPCClientCertsModuleName, +}; + +void CollectThirdPartyPKCS11ModuleTelemetry() { + size_t thirdPartyModulesLoaded = 0; + AutoSECMODListReadLock lock; + for (SECMODModuleList* list = SECMOD_GetDefaultModuleList(); list; + list = list->next) { + bool isThirdParty = true; + for (const auto& builtInModuleName : kBuiltInModuleNames) { + if (builtInModuleName.Equals(list->module->commonName)) { + isThirdParty = false; + break; + } + } + if (isThirdParty) { + thirdPartyModulesLoaded++; + } + } + mozilla::glean::pkcs11::third_party_modules_loaded.Set( + thirdPartyModulesLoaded); +} + } // namespace psm } // namespace mozilla diff --git a/security/manager/ssl/PKCS11ModuleDB.h b/security/manager/ssl/PKCS11ModuleDB.h index c167afe41078..2fc0a557140b 100644 --- a/security/manager/ssl/PKCS11ModuleDB.h +++ b/security/manager/ssl/PKCS11ModuleDB.h @@ -7,6 +7,7 @@ #define PKCS11ModuleDB_h #include "nsIPKCS11ModuleDB.h" +#include "nsTLiteralString.h" namespace mozilla { namespace psm { @@ -29,6 +30,13 @@ class PKCS11ModuleDB : public nsIPKCS11ModuleDB { virtual ~PKCS11ModuleDB() = default; }; +const nsLiteralCString kIPCClientCertsModuleName("IPC Client Cert Module"); +const nsLiteralCString kNSSInternalModuleName("NSS Internal PKCS #11 Module"); +const nsLiteralCString kOSClientCertsModuleName("OS Client Cert Module"); +const nsLiteralCString kRootModuleName("Builtin Roots Module"); + +void CollectThirdPartyPKCS11ModuleTelemetry(); + } // namespace psm } // namespace mozilla diff --git a/security/manager/ssl/RootCertificateTelemetryUtils.cpp b/security/manager/ssl/RootCertificateTelemetryUtils.cpp index d6bd54d741c6..0b4c9968f947 100644 --- a/security/manager/ssl/RootCertificateTelemetryUtils.cpp +++ b/security/manager/ssl/RootCertificateTelemetryUtils.cpp @@ -6,12 +6,12 @@ #include "RootCertificateTelemetryUtils.h" +#include "PKCS11ModuleDB.h" #include "RootHashes.inc" // Note: Generated by genRootCAHashes.js #include "ScopedNSSTypes.h" #include "mozilla/ArrayUtils.h" #include "mozilla/Logging.h" #include "nsINSSComponent.h" -#include "nsNSSCertHelper.h" #include "nsServiceManagerUtils.h" #include "pk11pub.h" @@ -109,7 +109,7 @@ int32_t RootCABinNumber(Span cert) { // because then it should have already matched our built-in list. This is // here as a backstop to catch situations where a built-in root was added but // the built-in telemetry information was not updated. - UniqueSECMODModule rootsModule(SECMOD_FindModule(kRootModuleName)); + UniqueSECMODModule rootsModule(SECMOD_FindModule(kRootModuleName.get())); AutoSECMODListReadLock secmodLock; if (!rootsModule || rootsModule->slotCount != 1) { return ROOT_CERTIFICATE_UNKNOWN; diff --git a/security/manager/ssl/metrics.yaml b/security/manager/ssl/metrics.yaml index 3278d867fd81..f2a30decb167 100644 --- a/security/manager/ssl/metrics.yaml +++ b/security/manager/ssl/metrics.yaml @@ -246,3 +246,19 @@ verification_used_cert_from: - dkeeler@mozilla.com expires: never denominator_metric: tls.certificate_verifications + +pkcs11: + third_party_modules_loaded: + type: quantity + description: + The number of third-party PKCS#11 modules loaded. + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1905453 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1905453 + data_sensitivity: + - interaction + notification_emails: + - dkeeler@mozilla.com + expires: never + unit: modules diff --git a/security/manager/ssl/nsNSSCertHelper.cpp b/security/manager/ssl/nsNSSCertHelper.cpp index 2a21400cb678..c0f0cb591008 100644 --- a/security/manager/ssl/nsNSSCertHelper.cpp +++ b/security/manager/ssl/nsNSSCertHelper.cpp @@ -26,12 +26,6 @@ using namespace mozilla; -// To avoid relying on localized strings in PSM, we hard-code the root module -// name internally. When we display it to the user in the list of modules in the -// front-end, we look up the localized value and display that instead of this. -const char* kRootModuleName = "Builtin Roots Module"; -const size_t kRootModuleNameLen = strlen(kRootModuleName); - static nsresult GetPIPNSSBundle(nsIStringBundle** pipnssBundle) { nsCOMPtr bundleService( do_GetService(NS_STRINGBUNDLE_CONTRACTID)); diff --git a/security/manager/ssl/nsNSSCertHelper.h b/security/manager/ssl/nsNSSCertHelper.h index 453e2e21eb3a..eb7aa4050bed 100644 --- a/security/manager/ssl/nsNSSCertHelper.h +++ b/security/manager/ssl/nsNSSCertHelper.h @@ -5,18 +5,8 @@ #ifndef nsNSSCertHelper_h #define nsNSSCertHelper_h -#ifndef INET6_ADDRSTRLEN -# define INET6_ADDRSTRLEN 46 -#endif - -#include "certt.h" #include "nsString.h" -extern const char* kRootModuleName; -extern const size_t kRootModuleNameLen; - -class nsIX509Cert; - // If input is valid UTF-8, converts from UTF-8 to UTF-16. Otherwise, // converts from Latin1 to UTF-16. void LossyUTF8ToUTF16(const char* str, uint32_t len, /*out*/ nsAString& result); diff --git a/security/manager/ssl/nsNSSComponent.cpp b/security/manager/ssl/nsNSSComponent.cpp index 478901c38036..5891ae628f81 100644 --- a/security/manager/ssl/nsNSSComponent.cpp +++ b/security/manager/ssl/nsNSSComponent.cpp @@ -11,6 +11,7 @@ #include "EnterpriseRoots.h" #include "ExtendedValidation.h" #include "NSSCertDBTrustDomain.h" +#include "PKCS11ModuleDB.h" #include "SSLTokensCache.h" #include "ScopedNSSTypes.h" #include "SharedSSLState.h" @@ -610,7 +611,7 @@ void AsyncLoadOrUnloadOSClientCertsModule(bool load) { Unused << task->Dispatch(); } else { UniqueSECMODModule osClientCertsModule( - SECMOD_FindModule(kOSClientCertsModuleName)); + SECMOD_FindModule(kOSClientCertsModuleName.get())); if (osClientCertsModule) { SECMOD_UnloadUserModule(osClientCertsModule.get()); } diff --git a/security/manager/ssl/nsNSSIOLayer.cpp b/security/manager/ssl/nsNSSIOLayer.cpp index 57f5d3ae6ec4..001a60b8aa79 100644 --- a/security/manager/ssl/nsNSSIOLayer.cpp +++ b/security/manager/ssl/nsNSSIOLayer.cpp @@ -44,7 +44,6 @@ #include "nsContentUtils.h" #include "nsISocketProvider.h" #include "nsIWebProgressListener.h" -#include "nsNSSCertHelper.h" #include "nsNSSComponent.h" #include "nsNSSHelper.h" #include "nsPrintfCString.h" diff --git a/security/manager/ssl/nsPKCS11Slot.cpp b/security/manager/ssl/nsPKCS11Slot.cpp index 130e79033d88..88ac9bb0d708 100644 --- a/security/manager/ssl/nsPKCS11Slot.cpp +++ b/security/manager/ssl/nsPKCS11Slot.cpp @@ -6,6 +6,7 @@ #include +#include "PKCS11ModuleDB.h" #include "mozilla/Casting.h" #include "mozilla/Logging.h" #include "mozilla/Telemetry.h" @@ -20,6 +21,7 @@ #include "secmod.h" using mozilla::LogLevel; +using namespace mozilla::psm; extern mozilla::LazyLogModule gPIPNSSLog; @@ -213,8 +215,10 @@ nsPKCS11Module::nsPKCS11Module(SECMODModule* module) { static nsresult NormalizeModuleNameOut(const char* moduleNameIn, nsACString& moduleNameOut) { // Easy case: this isn't the builtin roots module. - if (strnlen(moduleNameIn, kRootModuleNameLen + 1) != kRootModuleNameLen || - strncmp(kRootModuleName, moduleNameIn, kRootModuleNameLen) != 0) { + if (strnlen(moduleNameIn, kRootModuleName.Length() + 1) != + kRootModuleName.Length() || + strncmp(kRootModuleName.get(), moduleNameIn, kRootModuleName.Length()) != + 0) { moduleNameOut.Assign(moduleNameIn); return NS_OK; } diff --git a/security/manager/ssl/tests/unit/test_pkcs11_module.js b/security/manager/ssl/tests/unit/test_pkcs11_module.js index abad2dbb5466..654788f937c3 100644 --- a/security/manager/ssl/tests/unit/test_pkcs11_module.js +++ b/security/manager/ssl/tests/unit/test_pkcs11_module.js @@ -13,7 +13,15 @@ const gModuleDB = Cc["@mozilla.org/security/pkcs11moduledb;1"].getService( Ci.nsIPKCS11ModuleDB ); -function run_test() { +add_task(async function test_pkcs11_module() { + Services.fog.initializeFOG(); + + equal( + 0, + await Glean.pkcs11.thirdPartyModulesLoaded.testGetValue(), + "should have no third-party modules to begin with" + ); + // Check that if we have never added the test module, that we don't find it // in the module list. checkPKCS11ModuleNotPresent("PKCS11 Test Module", "pkcs11testmodule"); @@ -23,6 +31,11 @@ function run_test() { libraryFile.append("pkcs11testmodule"); libraryFile.append(ctypes.libraryName("pkcs11testmodule")); loadPKCS11Module(libraryFile, "PKCS11 Test Module", true); + equal( + 1, + await Glean.pkcs11.thirdPartyModulesLoaded.testGetValue(), + "should have one third-party module after loading it" + ); let testModule = checkPKCS11ModuleExists( "PKCS11 Test Module", "pkcs11testmodule" @@ -50,9 +63,14 @@ function run_test() { Ci.nsIPKCS11ModuleDB ); pkcs11ModuleDB.deleteModule("PKCS11 Test Module"); + equal( + 0, + await Glean.pkcs11.thirdPartyModulesLoaded.testGetValue(), + "should have no third-party modules after unloading it" + ); checkPKCS11ModuleNotPresent("PKCS11 Test Module", "pkcs11testmodule"); // Check miscellaneous module DB methods and attributes. ok(!gModuleDB.canToggleFIPS, "It should NOT be possible to toggle FIPS"); ok(!gModuleDB.isFIPSEnabled, "FIPS should not be enabled"); -} +});