net/cert: Deprecate use of base::ProtectedMemory

base::ProtectedMemory is being deprecated because it's not widely used
enough to make a security impact and justify its maintenance burden.
Replace use of base::ProtectedMemory with raw function pointers and add
an attribute to disable CFI-icall checking.

Bug: 1018834
Change-Id: Ia79aa823ab42e89b7103b8b89e65cda0c24dbb3b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1885010
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710618}

net/cert/cert_verify_proc_nss.cc

@@ -15,10 +15,9 @@
 #include <string>
 #include <vector>
 
+#include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/macros.h"
-#include "base/memory/protected_memory.h"
-#include "base/memory/protected_memory_cfi.h"
 #include "base/stl_util.h"
 #include "build/build_config.h"
 #include "crypto/nss_util.h"
@@ -49,22 +48,6 @@ using CacheOCSPResponseFunction = SECStatus (*)(CERTCertDBHandle* handle,
                                                 const SECItem* encodedResponse,
                                                 void* pwArg);
 
-static PROTECTED_MEMORY_SECTION base::ProtectedMemory<CacheOCSPResponseFunction>
-    g_cache_ocsp_response;
-
-// The function pointer for CERT_CacheOCSPResponseFromSideChannel is saved to
-// read-only memory after being dynamically resolved as a security mitigation to
-// prevent the pointer from being tampered with. See crbug.com/771365 for
-// details.
-const base::ProtectedMemory<CacheOCSPResponseFunction>&
-ResolveCacheOCSPResponse() {
-  static base::ProtectedMemory<CacheOCSPResponseFunction>::Initializer init(
-      &g_cache_ocsp_response,
-      reinterpret_cast<CacheOCSPResponseFunction>(
-          dlsym(RTLD_DEFAULT, "CERT_CacheOCSPResponseFromSideChannel")));
-  return g_cache_ocsp_response;
-}
-
 typedef std::unique_ptr<
     CERTCertificatePolicies,
     crypto::NSSDestroyer<CERTCertificatePolicies,
@@ -826,6 +809,7 @@ bool CertVerifyProcNSS::SupportsAdditionalTrustAnchors() const {
   return true;
 }
 
+NO_SANITIZE("cfi-icall")
 int CertVerifyProcNSS::VerifyInternalImpl(
     X509Certificate* cert,
     const std::string& hostname,
@@ -851,7 +835,10 @@ int CertVerifyProcNSS::VerifyInternalImpl(
   }
   CERTCertificate* cert_handle = input_chain[0].get();
 
-  if (!ocsp_response.empty() && *ResolveCacheOCSPResponse() != nullptr) {
+  static CacheOCSPResponseFunction cache_ocsp_response_from_side_channel =
+      reinterpret_cast<CacheOCSPResponseFunction>(
+          dlsym(RTLD_DEFAULT, "CERT_CacheOCSPResponseFromSideChannel"));
+  if (!ocsp_response.empty() && cache_ocsp_response_from_side_channel) {
     // Note: NSS uses a thread-safe global hash table, so this call will
     // affect any concurrent verification operations on |cert| or copies of
     // the same certificate. This is an unavoidable limitation of NSS's OCSP
@@ -860,9 +847,9 @@ int CertVerifyProcNSS::VerifyInternalImpl(
     ocsp_response_item.data = reinterpret_cast<unsigned char*>(
         const_cast<char*>(ocsp_response.data()));
     ocsp_response_item.len = ocsp_response.size();
-    UnsanitizedCfiCall(ResolveCacheOCSPResponse())(
-        CERT_GetDefaultCertDB(), cert_handle, PR_Now(), &ocsp_response_item,
-        nullptr);
+    cache_ocsp_response_from_side_channel(CERT_GetDefaultCertDB(), cert_handle,
+                                          PR_Now(), &ocsp_response_item,
+                                          nullptr);
   }
 
   // Setup a callback to call into CheckChainRevocationWithCRLSet with the

net/cert/known_roots_nss.cc

@@ -11,8 +11,7 @@
 
 #include <memory>
 
-#include "base/memory/protected_memory.h"
-#include "base/memory/protected_memory_cfi.h"
+#include "base/compiler_specific.h"
 #include "crypto/nss_util_internal.h"
 #include "net/base/hash_value.h"
 #include "net/cert/x509_util_nss.h"
@@ -30,31 +29,20 @@ using PK11HasAttributeSetFunction = CK_BBOOL (*)(PK11SlotInfo* slot,
                                                  CK_OBJECT_HANDLE id,
                                                  CK_ATTRIBUTE_TYPE type,
                                                  PRBool haslock);
-static PROTECTED_MEMORY_SECTION
-    base::ProtectedMemory<PK11HasAttributeSetFunction>
-        g_pk11_has_attribute_set;
-
-// The function pointer for PK11_HasAttributeSet is saved to read-only memory
-// after being dynamically resolved as a security mitigation to prevent the
-// pointer from being tampered with. See https://crbug.com/771365 for details.
-const base::ProtectedMemory<PK11HasAttributeSetFunction>&
-ResolvePK11HasAttributeSet() {
-  static base::ProtectedMemory<PK11HasAttributeSetFunction>::Initializer init(
-      &g_pk11_has_attribute_set,
-      reinterpret_cast<PK11HasAttributeSetFunction>(
-          dlsym(RTLD_DEFAULT, "PK11_HasAttributeSet")));
-  return g_pk11_has_attribute_set;
-}
 
 }  // namespace
 
 // IsKnownRoot returns true if the given certificate is one that we believe
 // is a standard (as opposed to user-installed) root.
+NO_SANITIZE("cfi-icall")
 bool IsKnownRoot(CERTCertificate* root) {
   if (!root || !root->slot)
     return false;
 
-  if (*ResolvePK11HasAttributeSet() != nullptr) {
+  static PK11HasAttributeSetFunction pk11_has_attribute_set =
+      reinterpret_cast<PK11HasAttributeSetFunction>(
+          dlsym(RTLD_DEFAULT, "PK11_HasAttributeSet"));
+  if (pk11_has_attribute_set) {
     // Historically, the set of root certs was determined based on whether or
     // not it was part of nssckbi.[so,dll], the read-only PKCS#11 module that
     // exported the certs with trust settings. However, some distributions,
@@ -76,9 +64,9 @@ bool IsKnownRoot(CERTCertificate* root) {
         if (PK11_IsPresent(slot) && PK11_HasRootCerts(slot)) {
           CK_OBJECT_HANDLE handle = PK11_FindCertInSlot(slot, root, nullptr);
           if (handle != CK_INVALID_HANDLE &&
-              UnsanitizedCfiCall(ResolvePK11HasAttributeSet())(
-                  root->slot, handle, CKA_NSS_MOZILLA_CA_POLICY, PR_FALSE) ==
-                  CK_TRUE) {
+              pk11_has_attribute_set(root->slot, handle,
+                                     CKA_NSS_MOZILLA_CA_POLICY,
+                                     PR_FALSE) == CK_TRUE) {
             return true;
           }
         }