diff options
author | Utkarsh Sanghi <usanghi@google.com> | 2015-10-28 21:14:49 +0000 |
---|---|---|
committer | Gerrit Code Review <noreply-gerritcodereview@google.com> | 2015-10-28 21:14:49 +0000 |
commit | 203c680b569b4a5f82cdc262c013c16bd601260c (patch) | |
tree | c331e18d87a4dd488ac06d5202a5038381762c49 | |
parent | 2c9a05c401de0b9eb7bc6ecc8304462dc72b5b5f (diff) | |
parent | e9f4df5412da4aa513b8e73ff0a7e2adb0ceedc4 (diff) | |
download | tpm_manager-203c680b569b4a5f82cdc262c013c16bd601260c.tar.gz |
Merge "tpm_manager: Add OwnershipDependencies"
-rw-r--r-- | client/tpm_ownership_dbus_proxy_test.cc | 2 | ||||
-rw-r--r-- | common/local_data.proto | 1 | ||||
-rw-r--r-- | common/print_local_data_proto.cc | 6 | ||||
-rw-r--r-- | common/tpm_manager_constants.h | 14 | ||||
-rw-r--r-- | server/dbus_service_test.cc | 4 | ||||
-rw-r--r-- | server/tpm2_initializer_impl.cc | 19 | ||||
-rw-r--r-- | server/tpm2_initializer_test.cc | 7 | ||||
-rw-r--r-- | server/tpm_initializer_impl.cc | 7 |
8 files changed, 34 insertions, 26 deletions
diff --git a/client/tpm_ownership_dbus_proxy_test.cc b/client/tpm_ownership_dbus_proxy_test.cc index a22afe6..c9ef319 100644 --- a/client/tpm_ownership_dbus_proxy_test.cc +++ b/client/tpm_ownership_dbus_proxy_test.cc @@ -59,7 +59,6 @@ TEST_F(TpmOwnershipDBusProxyTest, GetTpmStatus) { reply.set_status(STATUS_SUCCESS); reply.set_enabled(true); reply.set_owned(true); - reply.mutable_local_data()->set_owned_by_this_install(true); reply.set_dictionary_attack_counter(3); reply.set_dictionary_attack_threshold(4); reply.set_dictionary_attack_lockout_in_effect(true); @@ -77,7 +76,6 @@ TEST_F(TpmOwnershipDBusProxyTest, GetTpmStatus) { EXPECT_EQ(STATUS_SUCCESS, reply.status()); EXPECT_TRUE(reply.enabled()); EXPECT_TRUE(reply.owned()); - EXPECT_TRUE(reply.local_data().owned_by_this_install()); EXPECT_EQ(3, reply.dictionary_attack_counter()); EXPECT_EQ(4, reply.dictionary_attack_threshold()); EXPECT_TRUE(reply.dictionary_attack_lockout_in_effect()); diff --git a/common/local_data.proto b/common/local_data.proto index 8bb714f..a6ba6e1 100644 --- a/common/local_data.proto +++ b/common/local_data.proto @@ -24,7 +24,6 @@ package tpm_manager; // when all the clients have the owner password injected, this protobuf is // cleared of all passwords. message LocalData { - optional bool owned_by_this_install = 1; optional bytes owner_password = 2; repeated string owner_dependency = 3; optional bytes endorsement_password = 4; diff --git a/common/print_local_data_proto.cc b/common/print_local_data_proto.cc index 81c0864..76cb98e 100644 --- a/common/print_local_data_proto.cc +++ b/common/print_local_data_proto.cc @@ -34,12 +34,6 @@ std::string GetProtoDebugStringWithIndent(const LocalData& value, std::string output = base::StringPrintf("[%s] {\n", value.GetTypeName().c_str()); - if (value.has_owned_by_this_install()) { - output += indent + " owned_by_this_install: "; - base::StringAppendF(&output, "%s", - value.owned_by_this_install() ? "true" : "false"); - output += "\n"; - } if (value.has_owner_password()) { output += indent + " owner_password: "; base::StringAppendF(&output, "%s", diff --git a/common/tpm_manager_constants.h b/common/tpm_manager_constants.h index eabf378..8d994cd 100644 --- a/common/tpm_manager_constants.h +++ b/common/tpm_manager_constants.h @@ -19,9 +19,23 @@ namespace tpm_manager { +// These constants are used to set up and access the D-Bus interface for +// TpmManager. constexpr char kTpmManagerServiceName[] = "org.chromium.TpmManager"; constexpr char kTpmManagerServicePath[] = "/org/chromium/TpmManager"; +// These constants define the ownership dependencies. On a chromeos system, +// there are dependencies on Attestation and InstallAttributes, but at the +// moment TpmManager has no clients, so we only add a dependency on Test. +// TODO(usanghi): Figure out a way to handle ownership dependencies +// dynamically (b/25341605). +constexpr const char* kTestDependency = "Test"; + +// Array to easily access the list of ownership dependencies. +// Note: When dependencies are added/removed from the above list, they should +// be modified here as well. +constexpr const char* kInitialTpmOwnerDependencies[] = { kTestDependency }; + } // namespace tpm_manager #endif // TPM_MANAGER_COMMON_TPM_MANAGER_CONSTANTS_H_ diff --git a/server/dbus_service_test.cc b/server/dbus_service_test.cc index 25c7bbe..2696dcc 100644 --- a/server/dbus_service_test.cc +++ b/server/dbus_service_test.cc @@ -67,7 +67,7 @@ class DBusServiceTest : public testing::Test { dbus::MessageWriter writer(call.get()); writer.AppendProtoAsArrayOfBytes(request); auto response = brillo::dbus_utils::testing::CallMethod( - dbus_service_->dbus_object_, method_call); + dbus_service_->dbus_object_, call.get()); dbus::MessageReader reader(response.get()); EXPECT_TRUE(reader.PopArrayOfBytesAsProto(reply)); } @@ -112,7 +112,6 @@ TEST_F(DBusServiceTest, GetTpmStatus) { reply.set_status(STATUS_SUCCESS); reply.set_enabled(true); reply.set_owned(true); - reply.mutable_local_data()->set_owned_by_this_install(true); reply.set_dictionary_attack_counter(3); reply.set_dictionary_attack_threshold(4); reply.set_dictionary_attack_lockout_in_effect(true); @@ -124,7 +123,6 @@ TEST_F(DBusServiceTest, GetTpmStatus) { EXPECT_EQ(STATUS_SUCCESS, reply.status()); EXPECT_TRUE(reply.enabled()); EXPECT_TRUE(reply.owned()); - EXPECT_TRUE(reply.local_data().owned_by_this_install()); EXPECT_EQ(3, reply.dictionary_attack_counter()); EXPECT_EQ(4, reply.dictionary_attack_threshold()); EXPECT_TRUE(reply.dictionary_attack_lockout_in_effect()); diff --git a/server/tpm2_initializer_impl.cc b/server/tpm2_initializer_impl.cc index 1ae9b2b..8d30643 100644 --- a/server/tpm2_initializer_impl.cc +++ b/server/tpm2_initializer_impl.cc @@ -23,6 +23,7 @@ #include <trunks/trunks_factory_impl.h> #include "tpm_manager/common/local_data.pb.h" +#include "tpm_manager/common/tpm_manager_constants.h" #include "tpm_manager/server/openssl_crypto_util_impl.h" namespace { @@ -56,10 +57,9 @@ bool Tpm2InitializerImpl::InitializeTpm() { VLOG(1) << "Tpm already owned."; return true; } - // First we read the local data. If the |owned_by_this_install| flag is set, - // either we did not finish removing owner dependencies or TakeOwnership - // failed. In either case, we want to retake ownership with the same - // passwords. + // First we read the local data. If we did not finish removing owner + // dependencies or if TakeOwnership failed, we want to retake ownership + // with the same passwords. LocalData local_data; if (!local_data_store_->Read(&local_data)) { LOG(ERROR) << "Error reading local data."; @@ -68,9 +68,8 @@ bool Tpm2InitializerImpl::InitializeTpm() { std::string owner_password; std::string endorsement_password; std::string lockout_password; - // TODO(usanghi): Use owner dependency information rather than the - // |owned_by_this_install| flag here. - if (local_data.owned_by_this_install()) { + // If there are valid owner dependencies, we need to reuse the old passwords. + if (local_data.owner_dependency_size() > 0) { owner_password.assign(local_data.owner_password()); endorsement_password.assign(local_data.endorsement_password()); lockout_password.assign(local_data.lockout_password()); @@ -90,11 +89,13 @@ bool Tpm2InitializerImpl::InitializeTpm() { } // We write the passwords to disk, in case there is an error while taking // ownership. - local_data.set_owned_by_this_install(true); + local_data.clear_owner_dependency(); + for (auto dependency: kInitialTpmOwnerDependencies) { + local_data.add_owner_dependency(dependency); + } local_data.set_owner_password(owner_password); local_data.set_endorsement_password(endorsement_password); local_data.set_lockout_password(lockout_password); - // TODO(usanghi): Add ownership dependencies here. if (!local_data_store_->Write(local_data)) { LOG(ERROR) << "Error saving local data."; return false; diff --git a/server/tpm2_initializer_test.cc b/server/tpm2_initializer_test.cc index b962606..a46f27d 100644 --- a/server/tpm2_initializer_test.cc +++ b/server/tpm2_initializer_test.cc @@ -23,6 +23,7 @@ #include <trunks/mock_tpm_utility.h> #include <trunks/trunks_factory_for_test.h> +#include "tpm_manager/common/tpm_manager_constants.h" #include "tpm_manager/server/mock_local_data_store.h" #include "tpm_manager/server/mock_openssl_crypto_util.h" #include "tpm_manager/server/mock_tpm_status.h" @@ -107,7 +108,6 @@ TEST_F(Tpm2InitializerTest, InitializeTpmSuccess) { std::string endorsement_password; std::string lockout_password; LocalData local_data; - local_data.set_owned_by_this_install(false); EXPECT_CALL(mock_data_store_, Read(_)) .WillOnce(DoAll(SetArgPointee<0>(local_data), Return(true))); @@ -126,7 +126,7 @@ TEST_F(Tpm2InitializerTest, InitializeTpmSuccessAfterError) { std::string endorsement_password("endorsement"); std::string lockout_password("lockout"); LocalData local_data; - local_data.set_owned_by_this_install(true); + local_data.add_owner_dependency(kTestDependency); local_data.set_owner_password(owner_password); local_data.set_endorsement_password(endorsement_password); local_data.set_lockout_password(lockout_password); @@ -136,7 +136,8 @@ TEST_F(Tpm2InitializerTest, InitializeTpmSuccessAfterError) { EXPECT_CALL(mock_data_store_, Write(_)) .WillOnce(DoAll(SaveArg<0>(&local_data), Return(true))); - EXPECT_EQ(true, local_data.owned_by_this_install()); + EXPECT_EQ(1, local_data.owner_dependency_size()); + EXPECT_EQ(kTestDependency, local_data.owner_dependency(0)); EXPECT_EQ(owner_password, local_data.owner_password()); EXPECT_EQ(endorsement_password, local_data.endorsement_password()); EXPECT_EQ(lockout_password, local_data.lockout_password()); diff --git a/server/tpm_initializer_impl.cc b/server/tpm_initializer_impl.cc index 55fd38b..4e72eb0 100644 --- a/server/tpm_initializer_impl.cc +++ b/server/tpm_initializer_impl.cc @@ -24,6 +24,7 @@ #include "tpm_manager/server/local_data_store.h" #include "tpm_manager/server/tpm_connection.h" +#include "tpm_manager/common/tpm_manager_constants.h" #include "tpm_manager/server/tpm_status.h" #include "tpm_manager/server/tpm_util.h" @@ -63,9 +64,11 @@ bool TpmInitializerImpl::InitializeTpm() { return false; } LocalData local_data; - local_data.set_owned_by_this_install(true); + local_data.clear_owner_dependency(); + for (auto value: kInitialTpmOwnerDependencies) { + local_data.add_owner_dependency(value); + } local_data.set_owner_password(owner_password); - // TODO(usanghi): Add ownership dependencies here. if (!local_data_store_->Write(local_data)) { LOG(ERROR) << "Error saving local data."; return false; |