Skip to content

Commit

Permalink
ChromeOSTokenManager: small refactoring
Browse files Browse the repository at this point in the history
Rename tpm_slot_ into system_slot_ because it is only used for
storing the system slot (and the private slot is also a tpm
slot). So the renaming eliminates some confusion.

Delete some crypto:: namepspace specifiers since the code is
already in the "crypto" namespace.

Bug: b:197836899
Test: Compilation
Change-Id: Iaf9cdd1a468d5b99b9b4865eca203b08d743993e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3122328
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Commit-Queue: Michael Ershov <miersh@google.com>
Cr-Commit-Position: refs/heads/main@{#915598}
  • Loading branch information
Michael Ershov authored and Chromium LUCI CQ committed Aug 26, 2021
1 parent 5064427 commit b5c71c7
Showing 1 changed file with 21 additions and 22 deletions.
43 changes: 21 additions & 22 deletions crypto/nss_util_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ class ChromeOSTokenManager {
explicit TPMModuleAndSlot(SECMODModule* init_chaps_module)
: chaps_module(init_chaps_module) {}
SECMODModule* chaps_module;
crypto::ScopedPK11Slot tpm_slot;
ScopedPK11Slot tpm_slot;
};

ScopedPK11Slot OpenPersistentNSSDBForPath(const std::string& db_name,
Expand Down Expand Up @@ -161,9 +161,9 @@ class ChromeOSTokenManager {
}

// If everything is already initialized, then return true.
// Note that only |tpm_slot_| is checked, since |chaps_module_| could be
// nullptr in tests while |tpm_slot_| has been set to the test DB.
if (tpm_slot_) {
// Note that only |system_slot_| is checked, since |chaps_module_| could be
// nullptr in tests while |system_slot_| has been set to the test DB.
if (system_slot_) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(std::move(callback), true));
return;
Expand Down Expand Up @@ -215,25 +215,25 @@ class ChromeOSTokenManager {
<< ", got tpm slot: " << !!tpm_args->tpm_slot;

chaps_module_ = tpm_args->chaps_module;
tpm_slot_ = std::move(tpm_args->tpm_slot);
system_slot_ = std::move(tpm_args->tpm_slot);
if (!chaps_module_ && test_system_slot_) {
// chromeos_unittests try to test the TPM initialization process. If we
// have a test DB open, pretend that it is the TPM slot.
tpm_slot_.reset(PK11_ReferenceSlot(test_system_slot_.get()));
system_slot_.reset(PK11_ReferenceSlot(test_system_slot_.get()));
}
initializing_tpm_token_ = false;

if (tpm_slot_)
if (system_slot_)
RunAndClearTPMReadyCallbackList();

std::move(callback).Run(!!tpm_slot_);
std::move(callback).Run(!!system_slot_);
}

void RunAndClearTPMReadyCallbackList() { tpm_ready_callback_list_.Notify(); }

bool IsTPMTokenReady(base::OnceClosure callback) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
if (tpm_slot_)
if (system_slot_)
return true;

if (!callback.is_null())
Expand All @@ -245,9 +245,8 @@ class ChromeOSTokenManager {
// Note that CK_SLOT_ID is an unsigned long, but cryptohome gives us the slot
// id as an int. This should be safe since this is only used with chaps, which
// we also control.
static crypto::ScopedPK11Slot GetTPMSlotForIdInThreadPool(
SECMODModule* chaps_module,
CK_SLOT_ID slot_id) {
static ScopedPK11Slot GetTPMSlotForIdInThreadPool(SECMODModule* chaps_module,
CK_SLOT_ID slot_id) {
DCHECK(chaps_module);

DVLOG(3) << "Poking chaps module.";
Expand All @@ -258,7 +257,7 @@ class ChromeOSTokenManager {
PK11SlotInfo* slot = SECMOD_LookupSlot(chaps_module->moduleID, slot_id);
if (!slot)
LOG(ERROR) << "TPM slot " << slot_id << " not found.";
return crypto::ScopedPK11Slot(slot);
return ScopedPK11Slot(slot);
}

bool InitializeNSSForChromeOSUser(const std::string& username_hash,
Expand Down Expand Up @@ -400,10 +399,10 @@ class ChromeOSTokenManager {
DCHECK(!slot || !test_system_slot_);
test_system_slot_ = std::move(slot);
if (test_system_slot_) {
tpm_slot_.reset(PK11_ReferenceSlot(test_system_slot_.get()));
system_slot_.reset(PK11_ReferenceSlot(test_system_slot_.get()));
RunAndClearTPMReadyCallbackList();
} else {
tpm_slot_.reset();
system_slot_.reset();
}
}

Expand All @@ -413,9 +412,9 @@ class ChromeOSTokenManager {
// Ensure that a previous value of test_system_slot_ is not overwritten.
// Unsetting, i.e. setting a nullptr, however is allowed.
DCHECK(!slot || !test_system_slot_);
if (tpm_slot_ && tpm_slot_ == test_system_slot_) {
// Unset |tpm_slot_| if it was initialized from |test_system_slot_|.
tpm_slot_.reset();
if (system_slot_ && system_slot_ == test_system_slot_) {
// Unset |system_slot_| if it was initialized from |test_system_slot_|.
system_slot_.reset();
}
test_system_slot_ = std::move(slot);
}
Expand All @@ -432,15 +431,15 @@ class ChromeOSTokenManager {
void GetSystemNSSKeySlotCallback(
base::OnceCallback<void(ScopedPK11Slot)> callback) {
std::move(callback).Run(
ScopedPK11Slot(PK11_ReferenceSlot(tpm_slot_.get())));
ScopedPK11Slot(PK11_ReferenceSlot(system_slot_.get())));
}

ScopedPK11Slot GetSystemNSSKeySlot(
base::OnceCallback<void(ScopedPK11Slot)> callback) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
// TODO(mattm): chromeos::TPMTokenloader always calls
// InitializeTPMTokenAndSystemSlot with slot 0. If the system slot is
// disabled, tpm_slot_ will be the first user's slot instead. Can that be
// disabled, system_slot_ will be the first user's slot instead. Can that be
// detected and return nullptr instead?

base::OnceClosure wrapped_callback;
Expand All @@ -450,7 +449,7 @@ class ChromeOSTokenManager {
base::Unretained(this) /* singleton is leaky */, std::move(callback));
}
if (IsTPMTokenReady(std::move(wrapped_callback)))
return ScopedPK11Slot(PK11_ReferenceSlot(tpm_slot_.get()));
return ScopedPK11Slot(PK11_ReferenceSlot(system_slot_.get()));
return ScopedPK11Slot();
}

Expand All @@ -469,7 +468,7 @@ class ChromeOSTokenManager {
using TPMReadyCallbackList = base::OnceClosureList;
TPMReadyCallbackList tpm_ready_callback_list_;
SECMODModule* chaps_module_ = nullptr;
crypto::ScopedPK11Slot tpm_slot_;
ScopedPK11Slot system_slot_;
std::map<std::string, std::unique_ptr<ChromeOSUserData>> chromeos_user_map_;
ScopedPK11Slot test_system_slot_;
ScopedPK11Slot prepared_test_private_slot_;
Expand Down

0 comments on commit b5c71c7

Please sign in to comment.