From c7889d1191ee74d0cec14afcd9696c7c1e4f44f8 Mon Sep 17 00:00:00 2001 From: Shawn Willden Date: Thu, 28 Jan 2016 07:56:53 +0000 Subject: Revert "Add attestation support to SoftKeymaster." This reverts commit fc3cafd487e69c84d83444e1d129d0ab131c4e3d. Change-Id: I1fb38db044c4039be04d1f75fb89ca9a6404321f --- android_keymaster_test.cpp | 126 +--------------------------- android_keymaster_test_utils.cpp | 9 -- android_keymaster_test_utils.h | 5 +- include/keymaster/android_keymaster_utils.h | 18 ++-- include/keymaster/soft_keymaster_device.h | 4 - soft_keymaster_device.cpp | 50 +---------- 6 files changed, 13 insertions(+), 199 deletions(-) diff --git a/android_keymaster_test.cpp b/android_keymaster_test.cpp index b6f5064..8966c04 100644 --- a/android_keymaster_test.cpp +++ b/android_keymaster_test.cpp @@ -28,16 +28,14 @@ #include #include "android_keymaster_test_utils.h" -#include "attestation_record.h" #include "keymaster0_engine.h" #include "openssl_utils.h" using std::ifstream; using std::istreambuf_iterator; -using std::ofstream; using std::string; -using std::unique_ptr; using std::vector; +using std::unique_ptr; extern "C" { int __android_log_print(int prio, const char* tag, const char* fmt); @@ -3446,128 +3444,6 @@ TEST_P(Keymaster0AdapterTest, OldHwKeymaster0RsaBlobGetCharacteristics) { EXPECT_EQ(1, GetParam()->keymaster0_calls()); } -typedef Keymaster2Test AttestationTest; -INSTANTIATE_TEST_CASE_P(AndroidKeymasterTest, AttestationTest, test_params); - -static X509* parse_cert_blob(const keymaster_blob_t& blob) { - const uint8_t* p = blob.data; - return d2i_X509(nullptr, &p, blob.data_length); -} - -static bool verify_chain(const keymaster_cert_chain_t& chain) { - for (size_t i = 0; i < chain.entry_count - 1; ++i) { - keymaster_blob_t& key_cert_blob = chain.entries[i]; - keymaster_blob_t& signing_cert_blob = chain.entries[i + 1]; - - X509_Ptr key_cert(parse_cert_blob(key_cert_blob)); - X509_Ptr signing_cert(parse_cert_blob(signing_cert_blob)); - EXPECT_TRUE(!!key_cert.get() && !!signing_cert.get()); - if (!key_cert.get() || !signing_cert.get()) - return false; - - EVP_PKEY_Ptr signing_pubkey(X509_get_pubkey(signing_cert.get())); - EXPECT_TRUE(!!signing_pubkey.get()); - if (!signing_pubkey.get()) - return false; - - EXPECT_EQ(1, X509_verify(key_cert.get(), signing_pubkey.get())) - << "Verification of certificate " << i << " failed"; - } - - return true; -} - -// Extract attestation record from cert. Returned object is still part of cert; don't free it -// separately. -static ASN1_OCTET_STRING* get_attestation_record(X509* certificate) { - ASN1_OBJECT_Ptr oid(OBJ_txt2obj(kAttestionRecordOid, 1 /* dotted string format */)); - EXPECT_TRUE(!!oid.get()); - if (!oid.get()) - return nullptr; - - int location = X509_get_ext_by_OBJ(certificate, oid.get(), -1 /* search from beginning */); - EXPECT_NE(-1, location); - if (location == -1) - return nullptr; - - X509_EXTENSION* attest_rec_ext = X509_get_ext(certificate, location); - EXPECT_TRUE(!!attest_rec_ext); - if (!attest_rec_ext) - return nullptr; - - ASN1_OCTET_STRING* attest_rec = X509_EXTENSION_get_data(attest_rec_ext); - EXPECT_TRUE(!!attest_rec); - return attest_rec; -} - -static bool verify_attestation_record(AuthorizationSet expected_sw_enforced, - AuthorizationSet expected_tee_enforced, - const keymaster_blob_t& attestation_cert) { - - X509_Ptr cert(parse_cert_blob(attestation_cert)); - EXPECT_TRUE(!!cert.get()); - if (!cert.get()) - return false; - - ASN1_OCTET_STRING* attest_rec = get_attestation_record(cert.get()); - EXPECT_TRUE(!!attest_rec); - if (!attest_rec) - return false; - - AuthorizationSet att_sw_enforced; - AuthorizationSet att_tee_enforced; - EXPECT_EQ(KM_ERROR_OK, parse_attestation_record(attest_rec->data, attest_rec->length, - &att_sw_enforced, &att_tee_enforced)); - - // Add TAG_USER_ID to the attestation sw-enforced list, because user IDs are not included in - // attestations, since they're meaningless off-device. - uint32_t user_id; - if (expected_sw_enforced.GetTagValue(TAG_USER_ID, &user_id)) - att_sw_enforced.push_back(TAG_USER_ID, user_id); - if (expected_tee_enforced.GetTagValue(TAG_USER_ID, &user_id)) - att_tee_enforced.push_back(TAG_USER_ID, user_id); - - att_sw_enforced.Sort(); - expected_sw_enforced.Sort(); - EXPECT_EQ(expected_sw_enforced, att_sw_enforced); - - att_tee_enforced.Sort(); - expected_tee_enforced.Sort(); - EXPECT_EQ(expected_tee_enforced, att_tee_enforced); - - return true; -} - -TEST_P(AttestationTest, RsaSignedWithRsa) { - ASSERT_EQ(KM_ERROR_OK, GenerateKey(AuthorizationSetBuilder() - .RsaSigningKey(256, 3) - .Digest(KM_DIGEST_NONE) - .Padding(KM_PAD_NONE))); - - keymaster_cert_chain_t cert_chain; - EXPECT_EQ(KM_ERROR_OK, AttestKey(KM_ALGORITHM_RSA, &cert_chain)); - EXPECT_EQ(3U, cert_chain.entry_count); - EXPECT_TRUE(verify_chain(cert_chain)); - EXPECT_TRUE(verify_attestation_record(sw_enforced(), hw_enforced(), cert_chain.entries[0])); - - keymaster_free_cert_chain(&cert_chain); -} - -TEST_P(AttestationTest, RsaSignedWithEc) { - ASSERT_EQ(KM_ERROR_OK, GenerateKey(AuthorizationSetBuilder() - .RsaSigningKey(256, 3) - .Digest(KM_DIGEST_NONE) - .Padding(KM_PAD_NONE))); - - keymaster_cert_chain_t cert_chain; - EXPECT_EQ(KM_ERROR_OK, AttestKey(KM_ALGORITHM_EC, &cert_chain)); - EXPECT_EQ(3U, cert_chain.entry_count); - EXPECT_TRUE(verify_chain(cert_chain)); - EXPECT_TRUE(verify_attestation_record(sw_enforced(), hw_enforced(), cert_chain.entries[0])); - - keymaster_free_cert_chain(&cert_chain); -} - TEST(SoftKeymasterWrapperTest, CheckKeymaster2Device) { // Make a good fake device, and wrap it. SoftKeymasterDevice* good_fake(new SoftKeymasterDevice(new TestKeymasterContext)); diff --git a/android_keymaster_test_utils.cpp b/android_keymaster_test_utils.cpp index bfe68ff..f62dd66 100644 --- a/android_keymaster_test_utils.cpp +++ b/android_keymaster_test_utils.cpp @@ -328,15 +328,6 @@ keymaster_error_t Keymaster2Test::AbortOperation() { return device()->abort(device(), op_handle_); } -keymaster_error_t Keymaster2Test::AttestKey(keymaster_algorithm_t algorithm, - keymaster_cert_chain_t* cert_chain) { - AuthorizationSet attest_params( - AuthorizationSetBuilder().Authorization(TAG_ALGORITHM, algorithm)); - attest_params.push_back(UserAuthParams()); - attest_params.push_back(ClientParams()); - return device()->attest_key(device(), &blob_, &attest_params, cert_chain); -} - string Keymaster2Test::ProcessMessage(keymaster_purpose_t purpose, const string& message) { EXPECT_EQ(KM_ERROR_OK, BeginOperation(purpose, client_params(), NULL /* output_params */)); diff --git a/android_keymaster_test_utils.h b/android_keymaster_test_utils.h index cba1467..d1acec0 100644 --- a/android_keymaster_test_utils.h +++ b/android_keymaster_test_utils.h @@ -144,7 +144,7 @@ inline std::string make_string(const uint8_t* data, size_t length) { return std::string(reinterpret_cast(data), length); } -template std::string make_string(const uint8_t (&a)[N]) { +template std::string make_string(const uint8_t(&a)[N]) { return make_string(a, N); } @@ -208,8 +208,6 @@ class Keymaster2Test : public testing::TestWithParam { keymaster_error_t AbortOperation(); - keymaster_error_t AttestKey(keymaster_algorithm_t algorithm, keymaster_cert_chain_t* chain); - keymaster_error_t GetVersion(uint8_t* major, uint8_t* minor, uint8_t* subminor); std::string ProcessMessage(keymaster_purpose_t purpose, const std::string& message); @@ -452,6 +450,7 @@ struct Keymaster0CountingWrapper : public keymaster0_device_t { int counter_; }; + /** * This function takes a keymaster1_device_t and wraps it in an adapter that supports only * KM_DIGEST_SHA_2_256. diff --git a/include/keymaster/android_keymaster_utils.h b/include/keymaster/android_keymaster_utils.h index 9f0cf6a..1964f1f 100644 --- a/include/keymaster/android_keymaster_utils.h +++ b/include/keymaster/android_keymaster_utils.h @@ -50,14 +50,14 @@ inline int64_t java_time(time_t time) { /** * Return the size in bytes of the array \p a. */ -template inline size_t array_size(const T (&a)[N]) { +template inline size_t array_size(const T(&a)[N]) { return sizeof(a); } /** * Return the number of elements in array \p a. */ -template inline size_t array_length(const T (&)[N]) { +template inline size_t array_length(const T(&)[N]) { return N; } @@ -78,7 +78,7 @@ template inline T* dup_array(const T* a, size_t n) { * responsibility. Note that the dup is necessarily returned as a pointer, so size is lost. Call * array_length() on the original array to discover the size. */ -template inline T* dup_array(const T (&a)[N]) { +template inline T* dup_array(const T(&a)[N]) { return dup_array(a, N); } @@ -91,7 +91,7 @@ uint8_t* dup_buffer(const void* buf, size_t size); /** * Copy the contents of array \p arr to \p dest. */ -template inline void copy_array(const T (&arr)[N], T* dest) { +template inline void copy_array(const T(&arr)[N], T* dest) { for (size_t i = 0; i < N; ++i) dest[i] = arr[i]; } @@ -101,7 +101,7 @@ template inline void copy_array(const T (&arr)[N], T* des * early-exit, meaning that it should not be used in contexts where timing analysis attacks could be * a concern. */ -template inline bool array_contains(const T (&a)[N], T val) { +template inline bool array_contains(const T(&a)[N], T val) { for (size_t i = 0; i < N; ++i) { if (a[i] == val) { return true; @@ -144,9 +144,10 @@ class Eraser { template explicit Eraser(T* t); template - explicit Eraser(T& t) : buf_(reinterpret_cast(&t)), size_(sizeof(t)) {} + explicit Eraser(T& t) + : buf_(reinterpret_cast(&t)), size_(sizeof(t)) {} - template explicit Eraser(uint8_t (&arr)[N]) : buf_(arr), size_(N) {} + template explicit Eraser(uint8_t(&arr)[N]) : buf_(arr), size_(N) {} Eraser(void* buf, size_t size) : buf_(static_cast(buf)), size_(size) {} ~Eraser() { memset_s(buf_, 0, size_); } @@ -175,9 +176,6 @@ template class ArrayWrapper { T* begin_; T* end_; }; -template ArrayWrapper array_range(T* begin, size_t length) { - return ArrayWrapper(begin, length); -} /** * Convert any unsigned integer from network to host order. We implement this here rather than diff --git a/include/keymaster/soft_keymaster_device.h b/include/keymaster/soft_keymaster_device.h index e9e325d..c39b326 100644 --- a/include/keymaster/soft_keymaster_device.h +++ b/include/keymaster/soft_keymaster_device.h @@ -200,10 +200,6 @@ class SoftKeymasterDevice { const keymaster_blob_t* client_id, const keymaster_blob_t* app_data, keymaster_blob_t* export_data); - static keymaster_error_t attest_key(const keymaster2_device_t* dev, - const keymaster_key_blob_t* key_to_attest, - const keymaster_key_param_set_t* attest_params, - keymaster_cert_chain_t* cert_chain); static keymaster_error_t delete_key(const keymaster2_device_t* dev, const keymaster_key_blob_t* key); static keymaster_error_t delete_all_keys(const keymaster2_device_t* dev); diff --git a/soft_keymaster_device.cpp b/soft_keymaster_device.cpp index cbeaec7..cff3ebb 100644 --- a/soft_keymaster_device.cpp +++ b/soft_keymaster_device.cpp @@ -280,8 +280,8 @@ void SoftKeymasterDevice::initialize_device_struct(uint32_t flags) { km2_device_.get_key_characteristics = get_key_characteristics; km2_device_.import_key = import_key; km2_device_.export_key = export_key; - km2_device_.agree_key = nullptr; // TODO(swillden) Implement ECDH - km2_device_.attest_key = attest_key; + km2_device_.agree_key = nullptr; // TODO(swillden) Implement ECDH + km2_device_.attest_key = nullptr; // TODO(swillden) Implement attestation km2_device_.upgrade_key = nullptr; // TODO(swillden) Implement upgrade km2_device_.delete_key = delete_key; km2_device_.delete_all_keys = delete_all_keys; @@ -977,52 +977,6 @@ keymaster_error_t SoftKeymasterDevice::export_key(const keymaster2_device_t* dev export_data); } -/* static */ -keymaster_error_t SoftKeymasterDevice::attest_key(const keymaster2_device_t* dev, - const keymaster_key_blob_t* key_to_attest, - const keymaster_key_param_set_t* attest_params, - keymaster_cert_chain_t* cert_chain) { - if (!dev || !key_to_attest || !attest_params || !cert_chain) - return KM_ERROR_UNEXPECTED_NULL_POINTER; - - cert_chain->entry_count = 0; - cert_chain->entries = nullptr; - - AttestKeyRequest request; - request.SetKeyMaterial(*key_to_attest); - request.attest_params.Reinitialize(*attest_params); - - AttestKeyResponse response; - convert_device(dev)->impl_->AttestKey(request, &response); - if (response.error != KM_ERROR_OK) - return response.error; - - // Allocate and clear storage for cert_chain. - keymaster_cert_chain_t& rsp_chain = response.certificate_chain; - cert_chain->entries = reinterpret_cast( - malloc(rsp_chain.entry_count * sizeof(*cert_chain->entries))); - if (!cert_chain->entries) - return KM_ERROR_MEMORY_ALLOCATION_FAILED; - cert_chain->entry_count = rsp_chain.entry_count; - for (keymaster_blob_t& entry : array_range(cert_chain->entries, cert_chain->entry_count)) - entry = {}; - - // Copy cert_chain contents - size_t i = 0; - for (keymaster_blob_t& entry : array_range(rsp_chain.entries, rsp_chain.entry_count)) { - cert_chain->entries[i].data = reinterpret_cast(malloc(entry.data_length)); - if (!cert_chain->entries[i].data) { - keymaster_free_cert_chain(cert_chain); - return KM_ERROR_MEMORY_ALLOCATION_FAILED; - } - cert_chain->entries[i].data_length = entry.data_length; - memcpy(const_cast(cert_chain->entries[i].data), entry.data, entry.data_length); - ++i; - } - - return KM_ERROR_OK; -} - /* static */ keymaster_error_t SoftKeymasterDevice::delete_key(const keymaster1_device_t* dev, const keymaster_key_blob_t* key) { -- cgit v1.2.3