corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1945985 - rsclientcerts: only search for new objects when the platform is actually looking for client auth certs r=jschanck

Browse Source 
Previously, rsclientcerts would have to heuristically determine when the
platform was searching for client auth certificates in order to avoid
re-querying the underlying certificate and key sources every time any part of
NSS looked for PKCS#11 objects. This heuristic was imprecise and constrained
the rest of the implementation that handles client auth certificates.

Now that osclientcerts and ipcclientcerts (the consumers of rsclientcerts) are
part of libxul, they can directly check if gecko is searching for client auth
certificates and update their list of known certificates and keys accordingly.

In theory, multiple simultaneous connections could cause multiple updates of
the known list of certificates and keys, so to prevent unnecessary costly work,
rsclientcerts will still only refresh its list of objects every 2 seconds (down
from 3 seconds previously).

Differential Revision: https://phabricator.services.mozilla.com/D237108
This commit is contained in:
Dana Keeler
2025-02-18 19:26:59 +00:00
parent 70e23c057f
commit b1e99b2e7e
6 changed files with 65 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
security/manager/ssl/NSSSocketControl.cpp
View File

@@ -14,6 +14,7 @@
#include "mozilla/dom/Promise.h"
#include "mozilla/glean/SecurityManagerSslMetrics.h"
#include "nsNSSCallbacks.h"
#include "nsNSSComponent.h"
#include "nsProxyRelease.h"
using namespace mozilla;
@@ -460,6 +461,9 @@ void NSSSocketControl::ClientAuthCertificateSelected(
const_cast<uint8_t*>(certBytes.Elements()),
static_cast<unsigned int>(certBytes.Length()),
};
// Ensure that osclientcerts (or ipcclientcerts, in the socket process) will
// populate its list of certificates and keys.
AutoSearchingForClientAuthCertificates _;
UniqueCERTCertificate cert(CERT_NewTempCertificate(
CERT_GetDefaultCertDB(), &certItem, nullptr, false, true));
UniqueSECKEYPrivateKey key;

15
security/manager/ssl/TLSClientAuthCertSelection.cpp
View File

@@ -771,16 +771,6 @@ SECStatus SSLGetClientAuthDataHook(void* arg, PRFileDesc* socket,
nsTArray<nsTArray<uint8_t>> caNames(CollectCANames(caNamesDecoded));
// Currently, the IPC client certs module only refreshes its view of
// available certificates and keys if the platform issues a search for all
// certificates or keys. In the socket process, such a search may not have
// happened, so this ensures it has.
// Additionally, instantiating certificates in NSS is not thread-safe and has
// performance implications, so search for them here (on the socket thread)
// when not in the socket process.
UniqueCERTCertList potentialClientCertificates(
FindClientCertificatesWithPrivateKeys());
RefPtr<ClientAuthCertificateSelected> continuation(
new ClientAuthCertificateSelected(info));
// If this is the socket process, dispatch an IPC call to select a client
@@ -866,6 +856,11 @@ SECStatus SSLGetClientAuthDataHook(void* arg, PRFileDesc* socket,
return SECWouldBlock;
}
// Instantiating certificates in NSS is not thread-safe and has performance
// implications, so search for them here (on the socket thread).
UniqueCERTCertList potentialClientCertificates(
FindClientCertificatesWithPrivateKeys());
nsTArray<nsTArray<nsTArray<uint8_t>>> potentialClientCertificateChains;
FilterPotentialClientCertificatesByCANames(potentialClientCertificates,
caNames, enterpriseCertificates,

2
security/manager/ssl/nsNSSCertificateDB.cpp
View File

@@ -1185,6 +1185,8 @@ NS_IMETHODIMP nsNSSCertificateDB::AsPKCS7Blob(
NS_IMETHODIMP
nsNSSCertificateDB::GetCerts(nsTArray<RefPtr<nsIX509Cert>>& _retval) {
AutoSearchingForClientAuthCertificates _;
nsresult rv = BlockUntilLoadableCertsLoaded();
if (NS_FAILED(rv)) {
return rv;

22
security/manager/ssl/nsNSSComponent.cpp
View File

@@ -4,6 +4,8 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include <atomic>
#include "nsNSSComponent.h"
#include "BinaryPath.h"
@@ -1931,6 +1933,25 @@ nsNSSComponent::AsyncClearSSLExternalAndInternalSessionCache(
return NS_OK;
}
std::atomic<bool> sSearchingForClientAuthCertificates{false};
extern "C" {
bool IsGeckoSearchingForClientAuthCertificates() {
return sSearchingForClientAuthCertificates;
}
}
AutoSearchingForClientAuthCertificates::
AutoSearchingForClientAuthCertificates() {
sSearchingForClientAuthCertificates = true;
}
AutoSearchingForClientAuthCertificates::
~AutoSearchingForClientAuthCertificates() {
sSearchingForClientAuthCertificates = false;
}
namespace mozilla {
namespace psm {
@@ -1978,6 +1999,7 @@ static inline void CopyCertificatesTo(UniqueCERTCertList& from,
UniqueCERTCertList FindClientCertificatesWithPrivateKeys() {
MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
("FindClientCertificatesWithPrivateKeys"));
AutoSearchingForClientAuthCertificates _;
(void)BlockUntilLoadableCertsLoaded();
(void)CheckForSmartCardChanges();

20
security/manager/ssl/nsNSSComponent.h
View File

@@ -51,6 +51,26 @@ bool HandleTLSPrefChange(const nsCString& aPref);
void SetValidationOptionsCommon();
void PrepareForShutdownInSocketProcess();
// RAII helper class to indicate that gecko is searching for client auth
// certificates. Will automatically stop indicating that a search is happening
// when it goes out of scope.
// osclientcerts (or ipcclientcerts, in the socket process) will call
// IsGeckoSearchingForClientAuthCertificates() to determine if gecko is
// searching for client auth certificates. If so, the module knows to refresh
// its list of certificates and keys (which can be costly).
// In theory, two separate threads could both create a
// AutoSearchingForClientAuthCertificates at overlapping times. If one goes out
// of scope sooner than the other, IsGeckoSearchingForClientAuthCertificates()
// could potentially incorrectly return false for the slower thread. However,
// as long as the faster thread has ensured that osclientcerts/ipcclientcerts
// has updated its list of known certificates, a second search would be
// redundant anyway, so it doesn't matter.
class AutoSearchingForClientAuthCertificates {
public:
AutoSearchingForClientAuthCertificates();
~AutoSearchingForClientAuthCertificates();
};
// Implementation of the PSM component interface.
class nsNSSComponent final : public nsINSSComponent, public nsIObserver {
public:

53
security/manager/ssl/rsclientcerts/src/manager.rs
View File

@@ -12,7 +12,10 @@ use std::time::{Duration, Instant};
use crate::error::{Error, ErrorType};
use crate::error_here;
use crate::util::*;
extern "C" {
fn IsGeckoSearchingForClientAuthCertificates() -> bool;
}
/// Helper enum to differentiate between sessions on the modern slot and sessions on the legacy
/// slot. The former is for EC keys and RSA keys that can be used with RSA-PSS whereas the latter is
@@ -321,37 +324,6 @@ impl ManagerProxy {
}
}
// Determines if the attributes of a given search correspond to NSS looking for all certificates or
// private keys. Returns true if so, and false otherwise.
// These searches are of the form:
// { { type: CKA_TOKEN, value: [1] },
// { type: CKA_CLASS, value: [CKO_CERTIFICATE or CKO_PRIVATE_KEY, as serialized bytes] } }
// (although not necessarily in that order - see nssToken_TraverseCertificates and
// nssToken_FindPrivateKeys)
fn search_is_for_all_certificates_or_keys(
attrs: &[(CK_ATTRIBUTE_TYPE, Vec<u8>)],
) -> Result<bool, Error> {
if attrs.len() != 2 {
return Ok(false);
}
let token_bytes = vec![1_u8];
let mut found_token = false;
let cko_certificate_bytes = serialize_uint(CKO_CERTIFICATE)?;
let cko_private_key_bytes = serialize_uint(CKO_PRIVATE_KEY)?;
let mut found_certificate_or_private_key = false;
for (attr_type, attr_value) in attrs.iter() {
if attr_type == &CKA_TOKEN && attr_value == &token_bytes {
found_token = true;
}
if attr_type == &CKA_CLASS
&& (attr_value == &cko_certificate_bytes || attr_value == &cko_private_key_bytes)
{
found_certificate_or_private_key = true;
}
}
Ok(found_token && found_certificate_or_private_key)
}
const SUPPORTED_ATTRIBUTES: &[CK_ATTRIBUTE_TYPE] = &[
CKA_CLASS,
CKA_TOKEN,
@@ -439,7 +411,7 @@ pub struct Manager<B: ClientCertsBackend> {
/// The next object handle to hand out.
next_handle: CK_OBJECT_HANDLE,
/// The last time the implementation looked for new objects in the backend.
/// The implementation does this search no more than once every 3 seconds.
/// The implementation does this search no more than once every 2 seconds.
last_scan_time: Option<Instant>,
backend: B,
}
@@ -460,14 +432,14 @@ impl<B: ClientCertsBackend> Manager<B> {
}
}
/// When a new search session is opened (provided at least 3 seconds have elapsed since the
/// When a new search session is opened (provided at least 2 seconds have elapsed since the
/// last session was opened), this searches for certificates and keys to expose. We
/// de-duplicate previously-found certificates and keys by keeping track of their IDs.
fn maybe_find_new_objects(&mut self) -> Result<(), Error> {
let now = Instant::now();
match self.last_scan_time {
Some(last_scan_time) => {
if now.duration_since(last_scan_time) < Duration::new(3, 0) {
if now.duration_since(last_scan_time) < Duration::new(2, 0) {
return Ok(());
}
}
@@ -552,12 +524,11 @@ impl<B: ClientCertsBackend> Manager<B> {
return Ok(());
}
}
// When NSS wants to find all certificates or all private keys, it will perform a search
// with a particular set of attributes. This implementation uses these searches as an
// indication for the backend to re-scan for new objects from tokens that may have been
// inserted or certificates that may have been imported into the OS. Since these searches
// are relatively rare, this minimizes the impact of doing these re-scans.
if search_is_for_all_certificates_or_keys(&attrs)? {
// Only search for new objects when gecko has indicated that it is looking for client
// authentication certificates (or all certificates).
// Since these searches are relatively rare, this minimizes the impact of doing these
// re-scans.
if unsafe { IsGeckoSearchingForClientAuthCertificates() } {
self.maybe_find_new_objects()?;
}
let mut handles = Vec::new();