cryptohome: Enhanced handling of corrupted attestation database in migration.
Database migration process will now try to re-create PCR0/1 quote if
they are not found in the database.
BUG=chromium:899932
TEST=USE=asan FEATURES=test emerge-chell cryptohome
Change-Id: Ic2c77141427ad7136253ab481f405c8f242d1187
Reviewed-on: https://chromium-review.googlesource.com/1350568
Commit-Ready: John Chen <zuan@chromium.org>
Tested-by: John Chen <zuan@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>
(cherry picked from commit 97981945a083079767c5b3f3ba4eb6c40dec15a4)
Reviewed-on: https://chromium-review.googlesource.com/c/1369446
Reviewed-by: John Chen <zuan@chromium.org>
Reviewed-by: Louis Collard <louiscollard@chromium.org>
Commit-Queue: John Chen <zuan@chromium.org>
diff --git a/attestation/server/attestation_service.h b/attestation/server/attestation_service.h
index fac2a6a..e7ee491 100644
--- a/attestation/server/attestation_service.h
+++ b/attestation/server/attestation_service.h
@@ -224,6 +224,7 @@
// Migrates identity date in |database_| if needed. Returns true if the
// migration was needed and successful.
+ // Note that this function is not multithread safe.
bool MigrateIdentityData();
// Shutdown to be run on the worker thread.
diff --git a/cryptohome/attestation.cc b/cryptohome/attestation.cc
index 85c1b78..f05b3e4 100644
--- a/cryptohome/attestation.cc
+++ b/cryptohome/attestation.cc
@@ -623,41 +623,6 @@
return -1;
}
- // Quote PCR0.
- SecureBlob external_data;
- if (!tpm_->GetRandomDataSecureBlob(kQuoteExternalDataSize, &external_data)) {
- LOG(ERROR) << __func__ << ": GetRandomDataSecureBlob failed.";
- return -1;
- }
- Blob quoted_pcr_value0;
- SecureBlob quoted_data0;
- SecureBlob quote0;
- if (!tpm_->QuotePCR(0,
- identity_key_blob,
- external_data,
- "ed_pcr_value0,
- "ed_data0,
- "e0)) {
- LOG(ERROR) << "Attestation: Failed to generate PCR0 quote for identity "
- << identity << ".";
- return -1;
- }
-
- // Quote PCR1.
- Blob quoted_pcr_value1;
- SecureBlob quoted_data1;
- SecureBlob quote1;
- if (!tpm_->QuotePCR(1,
- identity_key_blob,
- external_data,
- "ed_pcr_value1,
- "ed_data1,
- "e1)) {
- LOG(ERROR) << "Attestation: Failed to generate PCR1 quote for identity "
- << identity << ".";
- return -1;
- }
-
// This only needs to be done once when we haven't stored credentials yet.
TPMCredentials* credentials_pb = database_pb_.mutable_credentials();
if (!credentials_pb->has_endorsement_credential()) {
@@ -699,28 +664,27 @@
// Store PCR quotes in the identity.
auto* map = identity_data->mutable_pcr_quotes();
- Quote quote_pb0;
- quote_pb0.set_quote(quote0.data(), quote0.size());
- quote_pb0.set_quoted_data(quoted_data0.data(), quoted_data0.size());
- quote_pb0.set_quoted_pcr_value(BlobToString(quoted_pcr_value0));
- auto in0 = map->insert(QuoteMap::value_type(0, quote_pb0));
- if (!in0.second) {
- LOG(ERROR) << "Attestation: Failed to store PCR0 quote for identity "
- << identity << ".";
- return -1;
- }
+ // Quote PCR0 and PCR1
+ for (int i = 0; i <= 1; i++) {
+ Quote quote_pb;
+ if (!CreatePCRQuote(i, identity_key_blob, "e_pb)) {
+ // Note that if in the future we regularly uses multiple AIK
+ // (i.e. identity key), then we'll need to print error messages here to
+ // indicate which one of the identity key failed the PCR quote process.
+ return -1;
+ }
- Quote quote_pb1;
- quote_pb1.set_quote(quote1.data(), quote1.size());
- quote_pb1.set_quoted_data(quoted_data1.data(), quoted_data1.size());
- quote_pb1.set_quoted_pcr_value(BlobToString(quoted_pcr_value1));
- quote_pb1.set_pcr_source_hint(platform_->GetHardwareID());
+ // PCR1 quote needs to have the source hint
+ if (i == 1) {
+ quote_pb.set_pcr_source_hint(platform_->GetHardwareID());
+ }
- auto in1 = map->insert(QuoteMap::value_type(1, quote_pb1));
- if (!in1.second) {
- LOG(ERROR) << "Attestation: Failed to store PCR1 quote for identity "
- << identity << ".";
- return -1;
+ auto in = map->insert(QuoteMap::value_type(i, quote_pb));
+ if (!in.second) {
+ LOG(ERROR) << "Attestation: Failed to store PCR" << i << " quote for "
+ << "identity " << identity << ".";
+ return -1;
+ }
}
// Return the index of the newly created identity.
@@ -1837,79 +1801,93 @@
return false;
}
- bool error = false;
-
// The identity we're creating will have the next index in identities.
LOG(INFO) << "Attestation: Migrating existing identity into identity "
<< database_pb_.identities().size() << ".";
- AttestationDatabase::Identity* identity_data =
- database_pb_.mutable_identities()->Add();
- identity_data->set_features(IDENTITY_FEATURE_ENTERPRISE_ENROLLMENT_ID);
+ AttestationDatabase::Identity identity_data;
+ identity_data.set_features(IDENTITY_FEATURE_ENTERPRISE_ENROLLMENT_ID);
if (database_pb_.has_identity_binding()) {
- identity_data->mutable_identity_binding()->CopyFrom(
+ identity_data.mutable_identity_binding()->CopyFrom(
database_pb_.identity_binding());
} else {
LOG(ERROR) << "Attestation: Identity Key Binding not found, not migrating.";
- error = true;
+ return false;
}
if (database_pb_.has_identity_key()) {
- identity_data->mutable_identity_key()->CopyFrom(
+ identity_data.mutable_identity_key()->CopyFrom(
database_pb_.identity_key());
- identity_data->mutable_identity_key()->clear_identity_credential();
- if (database_pb_.identity_key().has_identity_credential()) {
- // Create an identity certificate for this identity and the default PCA.
- AttestationDatabase::IdentityCertificate identity_certificate;
- identity_certificate.set_identity(kFirstIdentity);
- identity_certificate.set_aca(kDefaultPCA);
- identity_certificate.set_identity_credential(
- database_pb_.identity_key().identity_credential());
- auto* map = database_pb_.mutable_identity_certificates();
- auto in = map->insert(IdentityCertificateMap::value_type(
- kDefaultPCA, identity_certificate));
- if (!in.second) {
- LOG(ERROR) << "Attestation: Could not migrate existing identity.";
- error = true;
- }
- }
- if (database_pb_.identity_key().has_enrollment_id()) {
- database_pb_.set_enrollment_id(
- database_pb_.identity_key().enrollment_id());
- }
+ identity_data.mutable_identity_key()->clear_identity_credential();
+ // Migration for the other part of the identity_key is moved to the end of
+ // the function, this is so that we don't have to rollback too many things
+ // when there's an error.
} else {
LOG(ERROR) << "Attestation: Identity Key not found, not migrating.";
- error = true;
+ return false;
}
- if (database_pb_.has_pcr0_quote()) {
- auto in = identity_data->mutable_pcr_quotes()->insert(
- QuoteMap::value_type(0, database_pb_.pcr0_quote()));
+ for (int i = 0; i <= 1; i++) {
+ Quote quote_pb;
+ if (i == 0 && database_pb_.has_pcr0_quote()) {
+ quote_pb.CopyFrom(database_pb_.pcr0_quote());
+ } else if (i == 1 && database_pb_.has_pcr1_quote()) {
+ quote_pb.CopyFrom(database_pb_.pcr1_quote());
+ } else {
+ // Attempt to generate the missing PCR quote.
+ SecureBlob identity_key_blob(
+ identity_data.identity_key().identity_key_blob());
+ if (CreatePCRQuote(i, identity_key_blob, "e_pb)) {
+ if (i == 1) {
+ quote_pb.set_pcr_source_hint(platform_->GetHardwareID());
+ }
+ LOG(WARNING) << "Attestation: Regenerated missing PCR" << i
+ << " quote during migration.";
+ } else {
+ LOG(ERROR) << "Attestation: Failed to regenerate missing PCR"
+ << i << " quote during migration.";
+ return false;
+ }
+ }
+
+ // If we arrive here, quote_pb is definitely filled.
+ auto in = identity_data.mutable_pcr_quotes()->insert(
+ QuoteMap::value_type(i, quote_pb));
if (!in.second) {
LOG(ERROR) << "Attestation: Could not migrate existing identity.";
- error = true;
+ return false;
}
- } else {
- LOG(ERROR) << "Attestation: Missing PCR0 quote in existing database.";
- error = true;
}
- if (database_pb_.has_pcr1_quote()) {
- auto in = identity_data->mutable_pcr_quotes()->insert(
- QuoteMap::value_type(1, database_pb_.pcr1_quote()));
+
+ // Migrate the other part of identity_key, note that since we are here,
+ // identity_key is guaranteed to exist.
+ if (database_pb_.identity_key().has_identity_credential()) {
+ // Create an identity certificate for this identity and the default PCA.
+ AttestationDatabase::IdentityCertificate identity_certificate;
+ identity_certificate.set_identity(kFirstIdentity);
+ identity_certificate.set_aca(kDefaultPCA);
+ identity_certificate.set_identity_credential(
+ database_pb_.identity_key().identity_credential());
+
+ auto* map = database_pb_.mutable_identity_certificates();
+ auto in = map->insert(IdentityCertificateMap::value_type(
+ kDefaultPCA, identity_certificate));
if (!in.second) {
LOG(ERROR) << "Attestation: Could not migrate existing identity.";
- error = true;
+ return false;
}
- } else {
- LOG(ERROR) << "Attestation: Missing PCR1 quote in existing database.";
- error = true;
+ }
+ if (database_pb_.identity_key().has_enrollment_id()) {
+ database_pb_.set_enrollment_id(
+ database_pb_.identity_key().enrollment_id());
}
- if (error) {
- database_pb_.mutable_identities()->RemoveLast();
- database_pb_.mutable_identity_certificates()->erase(kDefaultPCA);
- }
+ // If we are here, we can be sure that identity_data actually holds a
+ // valid identity object, so we save it back into the DB.
+ AttestationDatabase::Identity* new_identity_data =
+ database_pb_.mutable_identities()->Add();
+ new_identity_data->CopyFrom(identity_data);
- return !error;
+ return true;
}
void Attestation::ClearDatabase() {
@@ -2693,6 +2671,35 @@
}
}
+bool Attestation::CreatePCRQuote(
+ uint32_t pcr_index,
+ const SecureBlob& identity_key_blob,
+ Quote* output) {
+ SecureBlob external_data(kQuoteExternalDataSize);
+ CryptoLib::GetSecureRandom(external_data.data(), kQuoteExternalDataSize);
+
+ Blob quoted_pcr_value;
+ SecureBlob quoted_data;
+ SecureBlob quote;
+
+ if (!tpm_->QuotePCR(pcr_index,
+ identity_key_blob,
+ external_data,
+ "ed_pcr_value,
+ "ed_data,
+ "e)) {
+ LOG(ERROR) << "Attestation: Failed to generate PCR" << pcr_index
+ << " quote.";
+ return false;
+ }
+
+ output->set_quote(quote.data(), quote.size());
+ output->set_quoted_data(quoted_data.data(), quoted_data.size());
+ output->set_quoted_pcr_value(BlobToString(quoted_pcr_value));
+
+ return true;
+}
+
bool Attestation::SendPCARequestAndBlock(PCAType pca_type,
PCARequestType request_type,
const brillo::SecureBlob& request,
diff --git a/cryptohome/attestation.h b/cryptohome/attestation.h
index b8160e3..3512587 100644
--- a/cryptohome/attestation.h
+++ b/cryptohome/attestation.h
@@ -816,6 +816,11 @@
// a fallback if the device firmware does not already do this.
void ExtendPCR1IfClear();
+ // Quote the given PCR and fill the result in the Quote object
+ bool CreatePCRQuote(uint32_t pcr_index,
+ const brillo::SecureBlob& identity_key_blob,
+ Quote* output);
+
// Creates a PCA URL for the given |pca_type| and |request_type|.
std::string GetPCAURL(PCAType pca_type, PCARequestType request_type) const;
diff --git a/cryptohome/attestation_unittest.cc b/cryptohome/attestation_unittest.cc
index 295265a..bb774de 100644
--- a/cryptohome/attestation_unittest.cc
+++ b/cryptohome/attestation_unittest.cc
@@ -709,16 +709,29 @@
default_encrypted_endorsement_credential,
db.credentials().default_encrypted_endorsement_credential()));
- // The default identity could not be copied from the deprecated database.
+ // The default identity is copied from the deprecated database after
+ // re-generating the PCR0 quote.
// The deprecated fields have not been cleared so that older code can still
// use the database.
- ASSERT_TRUE(db.identities().empty());
+ ASSERT_EQ(1, db.identities().size());
ASSERT_EQ("identity_binding", db.identity_binding().identity_binding());
ASSERT_EQ("identity_public_key", db.identity_binding().identity_public_key());
EXPECT_EQ("pcr1_quote", db.pcr1_quote().quote());
+ // Check the migrated identity after re-generating the PCR0 quote is
+ // correct.
+ const AttestationDatabase::Identity& default_identity_data =
+ db.identities().Get(Attestation::kDefaultPCA);
+
+ ASSERT_EQ("identity_binding",
+ default_identity_data.identity_binding().identity_binding());
+ ASSERT_EQ("identity_public_key",
+ default_identity_data.identity_binding().identity_public_key());
+ ASSERT_EQ("pcr1_quote", default_identity_data.pcr_quotes().at(1).quote());
+ ASSERT_TRUE(default_identity_data.pcr_quotes().at(0).has_quote());
+
// There is no identity certificate since there is no identity.
- ASSERT_TRUE(db.identity_certificates().empty());
+ ASSERT_EQ(db.identity_certificates().size(), 1);
}
TEST_F(AttestationBaseTest,
