Move cert chain management out of AttestKeyResponse
Bug: 171846199
Test: keymaster_tests
Change-Id: I32c6f84ac45ed1a80229408b680523e80bfa0a2e
diff --git a/android_keymaster/android_keymaster.cpp b/android_keymaster/android_keymaster.cpp
index 2351b1a..fb46064 100644
--- a/android_keymaster/android_keymaster.cpp
+++ b/android_keymaster/android_keymaster.cpp
@@ -205,8 +205,7 @@
void AndroidKeymaster::GenerateKey(const GenerateKeyRequest& request,
GenerateKeyResponse* response) {
- if (response == nullptr)
- return;
+ if (response == nullptr) return;
keymaster_algorithm_t algorithm;
const KeyFactory* factory = nullptr;
@@ -224,8 +223,7 @@
response->unenforced.Clear();
response->error = factory->GenerateKey(request.key_description, &key_blob,
&response->enforced, &response->unenforced);
- if (response->error == KM_ERROR_OK)
- response->key_blob = key_blob.release();
+ if (response->error == KM_ERROR_OK) response->key_blob = key_blob.release();
}
}
@@ -381,9 +379,8 @@
key->sw_enforced().push_back(TAG_ATTESTATION_APPLICATION_ID, attestation_application_id);
}
- CertificateChain certchain =
+ response->certificate_chain =
context_->GenerateAttestation(*key, request.attest_params, &response->error);
- if (response->error == KM_ERROR_OK) response->certificate_chain = certchain.release();
}
void AndroidKeymaster::UpgradeKey(const UpgradeKeyRequest& request, UpgradeKeyResponse* response) {
@@ -397,8 +394,7 @@
}
void AndroidKeymaster::ImportKey(const ImportKeyRequest& request, ImportKeyResponse* response) {
- if (response == nullptr)
- return;
+ if (response == nullptr) return;
keymaster_algorithm_t algorithm;
const KeyFactory* factory = nullptr;
@@ -412,8 +408,7 @@
response->error = factory->ImportKey(request.key_description, request.key_format,
KeymasterKeyBlob(key_material), &key_blob,
&response->enforced, &response->unenforced);
- if (response->error == KM_ERROR_OK)
- response->key_blob = key_blob.release();
+ if (response->error == KM_ERROR_OK) response->key_blob = key_blob.release();
}
}
diff --git a/android_keymaster/android_keymaster_messages.cpp b/android_keymaster/android_keymaster_messages.cpp
index 5378912..41e9574 100644
--- a/android_keymaster/android_keymaster_messages.cpp
+++ b/android_keymaster/android_keymaster_messages.cpp
@@ -70,6 +70,50 @@
return true;
}
+/*
+ * Helper functions for working with certificate chains.
+ */
+const size_t kMaxChainEntryCount = 10;
+
+size_t chain_size(const keymaster_cert_chain_t& certificate_chain) {
+ size_t result = sizeof(uint32_t); /* certificate_chain.entry_count */
+ for (size_t i = 0; i < certificate_chain.entry_count; ++i) {
+ result += sizeof(uint32_t); /* certificate_chain.entries[i].data_length */
+ result += certificate_chain.entries[i].data_length;
+ }
+ return result;
+}
+
+uint8_t* serialize_chain(const keymaster_cert_chain_t& certificate_chain, uint8_t* buf,
+ const uint8_t* end) {
+ buf = append_uint32_to_buf(buf, end, certificate_chain.entry_count);
+ for (size_t i = 0; i < certificate_chain.entry_count; ++i) {
+ buf = append_size_and_data_to_buf(buf, end, certificate_chain.entries[i].data,
+ certificate_chain.entries[i].data_length);
+ }
+ return buf;
+}
+
+CertificateChain deserialize_chain(const uint8_t** buf_ptr, const uint8_t* end) {
+ size_t entry_count;
+ if (!copy_uint32_from_buf(buf_ptr, end, &entry_count) || entry_count > kMaxChainEntryCount) {
+ return {};
+ }
+
+ CertificateChain certificate_chain(entry_count);
+ if (!certificate_chain.entries) return {};
+
+ for (size_t i = 0; i < certificate_chain.entry_count; ++i) {
+ UniquePtr<uint8_t[]> data;
+ size_t data_length;
+ if (!copy_size_and_data_from_buf(buf_ptr, end, &data_length, &data)) return {};
+ certificate_chain.entries[i].data = data.release();
+ certificate_chain.entries[i].data_length = data_length;
+ }
+
+ return certificate_chain;
+}
+
} // namespace
size_t KeymasterResponse::SerializedSize() const {
@@ -464,65 +508,17 @@
return deserialize_key_blob(&key_blob, buf_ptr, end) && attest_params.Deserialize(buf_ptr, end);
}
-AttestKeyResponse::~AttestKeyResponse() {
- for (size_t i = 0; i < certificate_chain.entry_count; ++i)
- delete[] certificate_chain.entries[i].data;
- delete[] certificate_chain.entries;
-}
-
-const size_t kMaxChainEntryCount = 10;
-bool AttestKeyResponse::AllocateChain(size_t entry_count) {
- if (entry_count > kMaxChainEntryCount) return false;
-
- if (certificate_chain.entries) {
- for (size_t i = 0; i < certificate_chain.entry_count; ++i)
- delete[] certificate_chain.entries[i].data;
- delete[] certificate_chain.entries;
- }
-
- certificate_chain.entry_count = entry_count;
- certificate_chain.entries = new (std::nothrow) keymaster_blob_t[entry_count];
- if (!certificate_chain.entries) {
- certificate_chain.entry_count = 0;
- return false;
- }
-
- memset(certificate_chain.entries, 0, sizeof(certificate_chain.entries[0]) * entry_count);
- return true;
-}
-
size_t AttestKeyResponse::NonErrorSerializedSize() const {
- size_t result = sizeof(uint32_t); /* certificate_chain.entry_count */
- for (size_t i = 0; i < certificate_chain.entry_count; ++i) {
- result += sizeof(uint32_t); /* certificate_chain.entries[i].data_length */
- result += certificate_chain.entries[i].data_length;
- }
- return result;
+ return chain_size(certificate_chain);
}
uint8_t* AttestKeyResponse::NonErrorSerialize(uint8_t* buf, const uint8_t* end) const {
- buf = append_uint32_to_buf(buf, end, certificate_chain.entry_count);
- for (size_t i = 0; i < certificate_chain.entry_count; ++i) {
- buf = append_size_and_data_to_buf(buf, end, certificate_chain.entries[i].data,
- certificate_chain.entries[i].data_length);
- }
- return buf;
+ return serialize_chain(certificate_chain, buf, end);
}
bool AttestKeyResponse::NonErrorDeserialize(const uint8_t** buf_ptr, const uint8_t* end) {
- size_t entry_count;
- if (!copy_uint32_from_buf(buf_ptr, end, &entry_count) || !AllocateChain(entry_count))
- return false;
-
- for (size_t i = 0; i < certificate_chain.entry_count; ++i) {
- UniquePtr<uint8_t[]> data;
- size_t data_length;
- if (!copy_size_and_data_from_buf(buf_ptr, end, &data_length, &data)) return false;
- certificate_chain.entries[i].data = data.release();
- certificate_chain.entries[i].data_length = data_length;
- }
-
- return true;
+ certificate_chain = deserialize_chain(buf_ptr, end);
+ return !!certificate_chain.entries;
}
UpgradeKeyRequest::~UpgradeKeyRequest() {
diff --git a/include/keymaster/android_keymaster_messages.h b/include/keymaster/android_keymaster_messages.h
index aa0d84e..f6ecd1b 100644
--- a/include/keymaster/android_keymaster_messages.h
+++ b/include/keymaster/android_keymaster_messages.h
@@ -624,19 +624,13 @@
};
struct AttestKeyResponse : public KeymasterResponse {
- explicit AttestKeyResponse(int32_t ver = kDefaultMessageVersion) : KeymasterResponse(ver) {
- certificate_chain.entry_count = 0;
- certificate_chain.entries = nullptr;
- }
- ~AttestKeyResponse();
-
- bool AllocateChain(size_t entry_count);
+ explicit AttestKeyResponse(int32_t ver = kDefaultMessageVersion) : KeymasterResponse(ver) {}
size_t NonErrorSerializedSize() const override;
uint8_t* NonErrorSerialize(uint8_t* buf, const uint8_t* end) const override;
bool NonErrorDeserialize(const uint8_t** buf_ptr, const uint8_t* end) override;
- keymaster_cert_chain_t certificate_chain;
+ CertificateChain certificate_chain;
};
struct UpgradeKeyRequest : public KeymasterMessage {
diff --git a/tests/android_keymaster_messages_test.cpp b/tests/android_keymaster_messages_test.cpp
index c8bb8bb..da07569 100644
--- a/tests/android_keymaster_messages_test.cpp
+++ b/tests/android_keymaster_messages_test.cpp
@@ -576,7 +576,8 @@
for (int ver = 0; ver <= kMaxMessageVersion; ++ver) {
AttestKeyResponse msg(ver);
msg.error = KM_ERROR_OK;
- EXPECT_TRUE(msg.AllocateChain(3));
+ msg.certificate_chain = CertificateChain(3);
+ EXPECT_TRUE(!!msg.certificate_chain.entries);
msg.certificate_chain.entries[0] = {dup_buffer("foo", 3), 3};
msg.certificate_chain.entries[1] = {dup_buffer("bar", 3), 3};
msg.certificate_chain.entries[2] = {dup_buffer("baz", 3), 3};
