chaps: Remove the unnecessary locks
We don't need these locks after we cleanup the token init & unload
thread model.
BUG=b:205087097
TEST=tast run $DUT hwsec.Chaps*
Change-Id: Ic2acb5e401ca46598d993b685eec5580806b8948
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/3261881
Reviewed-by: John L Chen <zuan@chromium.org>
Tested-by: Yi Chou <yich@google.com>
Commit-Queue: Yi Chou <yich@google.com>
diff --git a/chaps/chaps_adaptor.cc b/chaps/chaps_adaptor.cc
index 10eae94..16c9422 100644
--- a/chaps/chaps_adaptor.cc
+++ b/chaps/chaps_adaptor.cc
@@ -8,7 +8,6 @@
#include <base/files/file_util.h>
#include <base/logging.h>
#include <base/strings/string_number_conversions.h>
-#include <base/synchronization/lock.h>
#include <dbus/object_path.h>
#include "chaps/chaps.h"
@@ -17,9 +16,7 @@
#include "chaps/dbus_bindings/constants.h"
#include "chaps/token_manager_interface.h"
-using base::AutoLock;
using base::FilePath;
-using base::Lock;
using brillo::SecureBlob;
using std::string;
using std::vector;
diff --git a/chaps/chaps_adaptor.h b/chaps/chaps_adaptor.h
index 793e337..183abac 100644
--- a/chaps/chaps_adaptor.h
+++ b/chaps/chaps_adaptor.h
@@ -15,10 +15,6 @@
#include <dbus/bus.h>
#include <chaps/proto_bindings/ck_structs.pb.h>
-namespace base {
-class Lock;
-} // namespace base
-
namespace chaps {
const char kPersistentLogLevelPath[] = "/var/lib/chaps/.loglevel";
diff --git a/chaps/chaps_proxy.cc b/chaps/chaps_proxy.cc
index c8e224c..b2d7f85 100644
--- a/chaps/chaps_proxy.cc
+++ b/chaps/chaps_proxy.cc
@@ -19,7 +19,6 @@
#include "chaps/isolate.h"
#include "pkcs11/cryptoki.h"
-using base::AutoLock;
using brillo::Blob;
using brillo::SecureBlob;
using brillo::dbus_utils::ExtractMethodCallResults;
diff --git a/chaps/chaps_proxy.h b/chaps/chaps_proxy.h
index 2512713..34f3283 100644
--- a/chaps/chaps_proxy.h
+++ b/chaps/chaps_proxy.h
@@ -11,7 +11,6 @@
#include <base/at_exit.h>
#include <base/memory/ref_counted.h>
-#include <base/synchronization/lock.h>
#include <base/threading/thread.h>
#include <brillo/secure_blob.h>
#include <chaps/proto_bindings/ck_structs.pb.h>
diff --git a/chaps/slot_manager_impl.cc b/chaps/slot_manager_impl.cc
index 26757e1..fc5c363 100644
--- a/chaps/slot_manager_impl.cc
+++ b/chaps/slot_manager_impl.cc
@@ -35,7 +35,6 @@
#include "chaps/slot_policy_shared_slot.h"
#include "pkcs11/cryptoki.h"
-using base::AutoLock;
using base::FilePath;
using brillo::SecureBlob;
using std::map;
@@ -935,7 +934,6 @@
}
int SlotManagerImpl::CreateHandle() {
- AutoLock lock(handle_generator_lock_);
// If we use this many handles, we have a problem.
CHECK(last_handle_ < std::numeric_limits<int>::max());
return ++last_handle_;
diff --git a/chaps/slot_manager_impl.h b/chaps/slot_manager_impl.h
index 6970922..970fc35 100644
--- a/chaps/slot_manager_impl.h
+++ b/chaps/slot_manager_impl.h
@@ -18,7 +18,6 @@
#include <vector>
#include <base/macros.h>
-#include <base/synchronization/lock.h>
#include "chaps/chaps_factory.h"
#include "chaps/object_pool.h"
@@ -237,7 +236,6 @@
std::map<int, int> session_slot_map_;
std::map<brillo::SecureBlob, Isolate> isolate_map_;
AsyncTPMUtility* tpm_utility_;
- base::Lock handle_generator_lock_;
bool auto_load_system_token_;
bool is_initialized_;
SystemShutdownBlocker* system_shutdown_blocker_;
diff --git a/chaps/tpm2_utility_impl.cc b/chaps/tpm2_utility_impl.cc
index 7050ea2..2ec6e51 100644
--- a/chaps/tpm2_utility_impl.cc
+++ b/chaps/tpm2_utility_impl.cc
@@ -31,7 +31,6 @@
#include <trunks/trunks_dbus_proxy.h>
#include <trunks/trunks_factory_impl.h>
-using base::AutoLock;
using brillo::SecureBlob;
using std::map;
using std::set;
@@ -332,7 +331,7 @@
bool is_owned = false;
bool is_owner_password_present = false;
bool has_reset_lock_permissions = false;
- if (!tpm_manager_utility_.load()->GetTpmNonsensitiveStatus(
+ if (!tpm_manager_utility_->GetTpmNonsensitiveStatus(
&is_enabled, &is_owned, &is_owner_password_present,
&has_reset_lock_permissions)) {
LOG(ERROR) << ": failed to get TPM status from tpm_manager.";
@@ -344,7 +343,6 @@
return false;
}
#ifndef CHAPS_TPM2_USE_PER_OP_SESSIONS
- AutoLock lock(lock_);
result = session_->StartUnboundSession(false /* salted */,
false /* enable_encryption */);
if (result != TPM_RC_SUCCESS) {
@@ -376,7 +374,7 @@
bool is_owned = false;
bool is_owner_password_present = false;
bool has_reset_lock_permissions = false;
- if (!tpm_manager_utility_.load()->GetTpmNonsensitiveStatus(
+ if (!tpm_manager_utility_->GetTpmNonsensitiveStatus(
&is_enabled, &is_owned, &is_owner_password_present,
&has_reset_lock_permissions)) {
LOG(ERROR) << ": failed to get TPM status from tpm_manager.";
@@ -394,7 +392,6 @@
const std::string& encrypted_root_key,
SecureBlob* root_key) {
CHECK(root_key);
- AutoLock lock(lock_);
int key_handle = 0;
if (!LoadKeyWithParentInternal(std::nullopt, auth_key_blob, auth_data,
kStorageRootKey, &key_handle)) {
@@ -415,7 +412,6 @@
const SecureBlob& new_auth_data,
const std::string& old_auth_key_blob,
std::string* new_auth_key_blob) {
- AutoLock lock(lock_);
int key_handle = 0;
if (new_auth_data.size() > SHA256_DIGEST_SIZE) {
LOG(ERROR) << "Authorization cannot be larger than SHA256 Digest size.";
@@ -446,7 +442,6 @@
}
bool TPM2UtilityImpl::GenerateRandom(int num_bytes, std::string* random_data) {
- AutoLock lock(lock_);
TPM_RC result =
trunks_tpm_utility_->GenerateRandom(num_bytes, nullptr, random_data);
if (result != TPM_RC_SUCCESS) {
@@ -458,7 +453,6 @@
}
bool TPM2UtilityImpl::StirRandom(const std::string& entropy_data) {
- AutoLock lock(lock_);
TPM_RC result = trunks_tpm_utility_->StirRandom(entropy_data, nullptr);
if (result != TPM_RC_SUCCESS) {
LOG(ERROR) << "Error seeding TPM random number generator: "
@@ -474,7 +468,6 @@
const SecureBlob& auth_data,
std::string* key_blob,
int* key_handle) {
- AutoLock lock(lock_);
if (public_exponent.size() > 4) {
LOG(ERROR) << "Incorrectly formatted public_exponent.";
return false;
@@ -514,7 +507,6 @@
bool TPM2UtilityImpl::GetRSAPublicKey(int key_handle,
std::string* public_exponent,
std::string* modulus) {
- AutoLock lock(lock_);
trunks::TPMT_PUBLIC public_data;
TPM_RC result =
trunks_tpm_utility_->GetKeyPublicArea(key_handle, &public_data);
@@ -542,7 +534,6 @@
const SecureBlob& auth_data,
std::string* key_blob,
int* key_handle) {
- AutoLock lock(lock_);
if (!IsECCurveSupported(nid)) {
LOG(ERROR) << "Not supported NID";
return false;
@@ -577,7 +568,6 @@
}
bool TPM2UtilityImpl::GetECCPublicKey(int key_handle, std::string* ec_point) {
- AutoLock lock(lock_);
trunks::TPMT_PUBLIC public_area;
TPM_RC result =
trunks_tpm_utility_->GetKeyPublicArea(key_handle, &public_area);
@@ -609,7 +599,6 @@
const SecureBlob& auth_data,
std::string* key_blob,
int* key_handle) {
- AutoLock lock(lock_);
if (public_exponent.size() > 4) {
LOG(ERROR) << "Incorrectly formatted public_exponent.";
return false;
@@ -651,8 +640,6 @@
const brillo::SecureBlob& auth_data,
std::string* key_blob,
int* key_handle) {
- AutoLock lock(lock_);
-
ScopedSession session_scope(factory_, &session_);
if (!session_) {
return false;
@@ -683,7 +670,6 @@
const std::string& key_blob,
const SecureBlob& auth_data,
int* key_handle) {
- AutoLock lock(lock_);
return LoadKeyWithParentInternal(slot, key_blob, auth_data, kStorageRootKey,
key_handle);
}
@@ -693,13 +679,11 @@
const SecureBlob& auth_data,
int parent_key_handle,
int* key_handle) {
- AutoLock lock(lock_);
return LoadKeyWithParentInternal(slot, key_blob, auth_data, parent_key_handle,
key_handle);
}
void TPM2UtilityImpl::UnloadKeysForSlot(int slot) {
- AutoLock Lock(lock_);
for (const auto& it : slot_handles_[slot]) {
FlushHandle(it);
}
@@ -769,7 +753,6 @@
bool TPM2UtilityImpl::Unbind(int key_handle,
const std::string& input,
std::string* output) {
- AutoLock lock(lock_);
return UnbindInternal(key_handle, input, output);
}
@@ -778,8 +761,6 @@
const std::string& mechanism_parameter,
const std::string& input,
std::string* signature) {
- AutoLock Lock(lock_);
-
// Parse the various parameters for this method.
DigestAlgorithm digest_algorithm = GetDigestAlgorithm(signing_mechanism);
// Parse RSA PSS Parameters if applicable.
@@ -1091,7 +1072,6 @@
const brillo::SecureBlob& auth_value,
std::string* key_blob,
std::string* encrypted_data) {
- AutoLock lock(lock_);
if (unsealed_data.length() > MAX_SYM_DATA) {
LOG(ERROR) << "The data to seal is too large.";
return false;
@@ -1128,7 +1108,6 @@
return true;
}
- AutoLock lock(lock_);
std::string data;
ScopedSession session_scope(factory_, &session_);
if (!session_) {
diff --git a/chaps/tpm2_utility_impl.h b/chaps/tpm2_utility_impl.h
index d7acbb1..f13a16c 100644
--- a/chaps/tpm2_utility_impl.h
+++ b/chaps/tpm2_utility_impl.h
@@ -7,7 +7,6 @@
#include "chaps/tpm_utility.h"
-#include <atomic>
#include <map>
#include <memory>
#include <optional>
@@ -15,8 +14,6 @@
#include <string>
#include <base/macros.h>
-#include <base/single_thread_task_runner.h>
-#include <base/synchronization/lock.h>
#include <gtest/gtest_prod.h>
#include <tpm_manager/client/tpm_manager_utility.h>
#include <trunks/hmac_session.h>
@@ -164,20 +161,15 @@
std::unique_ptr<trunks::TrunksFactoryImpl> default_factory_;
trunks::TrunksFactory* factory_;
bool is_trunks_proxy_initialized_ = false;
- // |Init| had been completely run. This implies TPM is enabled and owned.
- std::atomic<bool> is_initialized_ = false;
+ bool is_initialized_ = false;
// TPM is enabled.
- std::atomic<bool> is_enabled_ = false;
- base::Lock lock_;
+ bool is_enabled_ = false;
std::unique_ptr<trunks::HmacSession> session_;
std::unique_ptr<trunks::TpmUtility> trunks_tpm_utility_;
std::map<int, std::set<int>> slot_handles_;
std::map<int, brillo::SecureBlob> handle_auth_data_;
std::map<int, std::string> handle_name_;
- // The |tpm_manager_utility_| may be used in |Init| & |IsTPMAvailable| in
- // the same time, using std::atomic here could prevent the potential race
- // condition.
- std::atomic<tpm_manager::TpmManagerUtility*> tpm_manager_utility_;
+ tpm_manager::TpmManagerUtility* tpm_manager_utility_;
FRIEND_TEST(TPM2UtilityTest, IsTPMAvailable);
FRIEND_TEST(TPM2UtilityTest, LoadKeySuccess);
diff --git a/chaps/tpm_utility_impl.cc b/chaps/tpm_utility_impl.cc
index 6301dcd..40c8bbc 100644
--- a/chaps/tpm_utility_impl.cc
+++ b/chaps/tpm_utility_impl.cc
@@ -13,7 +13,6 @@
#include <base/files/file_path.h>
#include <base/files/file_util.h>
#include <base/logging.h>
-#include <base/synchronization/lock.h>
#include <brillo/secure_blob.h>
#include <openssl/rand.h>
#include <trousers/scoped_tss_type.h>
@@ -21,7 +20,6 @@
#include "chaps/chaps_utility.h"
-using base::AutoLock;
using brillo::SecureBlob;
using std::hex;
using std::map;
@@ -185,7 +183,6 @@
}
}
- AutoLock lock(lock_);
TSS_RESULT result = TSS_SUCCESS;
result = Tspi_Context_Create(tsp_context_.ptr());
if (result != TSS_SUCCESS) {
@@ -300,7 +297,6 @@
return false;
string root_key_str;
if (!Unbind(key_handle, encrypted_root_key, &root_key_str)) {
- AutoLock lock(lock_);
FlushHandle(key_handle);
return false;
}
@@ -308,7 +304,6 @@
brillo::SecureClearContainer(root_key_str);
VLOG(1) << "TPMUtilityImpl::Authenticate success";
- AutoLock lock(lock_);
FlushHandle(key_handle);
return true;
}
@@ -325,17 +320,14 @@
// Make sure the old auth data is ok.
string encrypted, decrypted;
if (!Bind(key_handle, "testdata", &encrypted)) {
- AutoLock lock(lock_);
FlushHandle(key_handle);
return false;
}
if (!Unbind(key_handle, encrypted, &decrypted)) {
- AutoLock lock(lock_);
FlushHandle(key_handle);
return false;
}
// Change the secret.
- AutoLock lock(lock_);
TSS_RESULT result = TSS_SUCCESS;
ScopedTssPolicy policy(tsp_context_);
result = Tspi_Context_CreateObject(tsp_context_, TSS_OBJECT_TYPE_POLICY,
@@ -370,7 +362,6 @@
bool TPMUtilityImpl::GenerateRandom(int num_bytes, string* random_data) {
VLOG(1) << "TPMUtilityImpl::GenerateRandom enter";
- AutoLock lock(lock_);
if (!InitSRK())
return false;
TSS_RESULT result = TSS_SUCCESS;
@@ -394,7 +385,6 @@
bool TPMUtilityImpl::StirRandom(const string& entropy_data) {
VLOG(1) << "TPMUtilityImpl::StirRandom enter";
- AutoLock lock(lock_);
if (!InitSRK())
return false;
TSS_RESULT result = TSS_SUCCESS;
@@ -432,7 +422,6 @@
string* key_blob,
int* key_handle) {
VLOG(1) << "TPMUtilityImpl::GenerateRSAKey enter";
- AutoLock lock(lock_);
if (!InitSRK())
return false;
TSS_RESULT result = TSS_SUCCESS;
@@ -478,7 +467,6 @@
string* public_exponent,
string* modulus) {
VLOG(1) << "TPMUtilityImpl::GetRSAPublicKey enter";
- AutoLock lock(lock_);
if (!InitSRK())
return false;
if (!GetKeyAttributeData(GetTssHandle(key_handle), TSS_TSPATTRIB_RSAKEY_INFO,
@@ -518,7 +506,6 @@
string* key_blob,
int* key_handle) {
VLOG(1) << "TPMUtilityImpl::WrapRSAKey enter";
- AutoLock lock(lock_);
if (!InitSRK())
return false;
if (!GetSRKPublicKey())
@@ -616,7 +603,6 @@
const SecureBlob& auth_data,
int parent_key_handle,
int* key_handle) {
- AutoLock lock(lock_);
if (!InitSRK())
return false;
if (slot.has_value() && IsAlreadyLoaded(slot.value(), key_blob, key_handle))
@@ -633,7 +619,6 @@
void TPMUtilityImpl::UnloadKeysForSlot(int slot) {
VLOG(1) << "TPMUtilityImpl::UnloadKeysForSlot enter";
- AutoLock lock(lock_);
if (!InitSRK())
return;
set<int>* handles = &slot_handles_[slot].handles_;
@@ -648,7 +633,6 @@
bool TPMUtilityImpl::Bind(int key_handle, const string& input, string* output) {
VLOG(1) << "TPMUtilityImpl::Bind enter";
- AutoLock lock(lock_);
if (!InitSRK())
return false;
TSSEncryptedData encrypted(tsp_context_);
@@ -671,7 +655,6 @@
const string& input,
string* output) {
VLOG(1) << "TPMUtilityImpl::Unbind enter";
- AutoLock lock(lock_);
if (!InitSRK())
return false;
TSSEncryptedData encrypted(tsp_context_);
@@ -709,7 +692,6 @@
const string& input,
string* signature) {
VLOG(1) << "TPMUtilityImpl::Sign enter";
- AutoLock lock(lock_);
DigestAlgorithm digest_algorithm = GetDigestAlgorithm(signing_mechanism);
// Using the TSS_SS_RSASSAPKCS1V15_DER scheme, we need to manually
@@ -754,7 +736,6 @@
bool TPMUtilityImpl::IsSRKReady() {
VLOG(1) << "TPMUtilityImpl::IsSRKReady";
- AutoLock lock(lock_);
if (!tpm_manager_utility_) {
LOG(ERROR) << "Accessing invalid tpm_manager utility.";
return false;
@@ -1027,11 +1008,9 @@
}
if (!Bind(auth_key_handle, unsealed_data, encrypted_data)) {
LOG(ERROR) << "Failed to bind encryption key for sealing data.";
- AutoLock lock(lock_);
FlushHandle(auth_key_handle);
return false;
}
- AutoLock lock(lock_);
FlushHandle(auth_key_handle);
return true;
}
diff --git a/chaps/tpm_utility_impl.h b/chaps/tpm_utility_impl.h
index 157434a..d1dd4ca 100644
--- a/chaps/tpm_utility_impl.h
+++ b/chaps/tpm_utility_impl.h
@@ -13,7 +13,6 @@
#include <string>
#include <base/macros.h>
-#include <base/synchronization/lock.h>
#include <trousers/scoped_tss_type.h>
#include <trousers/tss.h>
#include <tpm_manager/client/tpm_manager_utility.h>
@@ -181,7 +180,6 @@
const std::string default_exponent_;
std::map<int, HandleInfo> slot_handles_;
std::map<int, KeyInfo> handle_info_;
- base::Lock lock_;
int last_handle_;
tpm_manager::TpmManagerUtility* tpm_manager_utility_;
};
