Merge "Fix pass-through of deletion on wrapped KM0 and KM1."
diff --git a/android_keymaster_test.cpp b/android_keymaster_test.cpp
index 5fb3699..dd8ec0b 100644
--- a/android_keymaster_test.cpp
+++ b/android_keymaster_test.cpp
@@ -427,8 +427,10 @@
EXPECT_TRUE(contains(crypto_params, TAG_RSA_PUBLIC_EXPONENT, 3));
EXPECT_FALSE(contains(non_crypto_params, TAG_RSA_PUBLIC_EXPONENT, 3));
+ EXPECT_EQ(KM_ERROR_OK, DeleteKey());
+
if (GetParam()->algorithm_in_km0_hardware(KM_ALGORITHM_RSA))
- EXPECT_EQ(1, GetParam()->keymaster0_calls());
+ EXPECT_EQ(2, GetParam()->keymaster0_calls());
}
TEST_P(NewKeyGeneration, RsaDefaultSize) {
@@ -813,7 +815,6 @@
string output;
ASSERT_EQ(KM_ERROR_INVALID_ARGUMENT, FinishOperation(&output));
-
if (GetParam()->algorithm_in_km0_hardware(KM_ALGORITHM_RSA))
EXPECT_EQ(3, GetParam()->keymaster0_calls());
}
diff --git a/android_keymaster_test_utils.cpp b/android_keymaster_test_utils.cpp
index 3f058c5..9deba77 100644
--- a/android_keymaster_test_utils.cpp
+++ b/android_keymaster_test_utils.cpp
@@ -180,6 +180,10 @@
return device()->generate_key(device(), ¶ms, &blob_, &characteristics_);
}
+keymaster_error_t Keymaster1Test::DeleteKey() {
+ return device()->delete_key(device(), &blob_);
+}
+
keymaster_error_t Keymaster1Test::ImportKey(const AuthorizationSetBuilder& builder,
keymaster_key_format_t format,
const string& key_material) {
diff --git a/android_keymaster_test_utils.h b/android_keymaster_test_utils.h
index c7177ea..bd4fbdb 100644
--- a/android_keymaster_test_utils.h
+++ b/android_keymaster_test_utils.h
@@ -176,6 +176,8 @@
keymaster_error_t GenerateKey(const AuthorizationSetBuilder& builder);
+ keymaster_error_t DeleteKey();
+
keymaster_error_t ImportKey(const AuthorizationSetBuilder& builder,
keymaster_key_format_t format, const std::string& key_material);
@@ -405,6 +407,8 @@
static int counting_delete_keypair(const struct keymaster0_device* dev, const uint8_t* key_blob,
const size_t key_blob_length) {
increment(dev);
+ if (key_blob && key_blob_length > 0)
+ EXPECT_EQ('Q', *key_blob);
if (device(dev)->delete_keypair) {
std::unique_ptr<uint8_t[]> dup_blob(unmunge_blob(key_blob, key_blob_length));
return device(dev)->delete_keypair(device(dev), dup_blob.get(), key_blob_length);
diff --git a/include/keymaster/soft_keymaster_context.h b/include/keymaster/soft_keymaster_context.h
index dac376a..413117a 100644
--- a/include/keymaster/soft_keymaster_context.h
+++ b/include/keymaster/soft_keymaster_context.h
@@ -66,6 +66,8 @@
const AuthorizationSet& additional_params,
KeymasterKeyBlob* key_material, AuthorizationSet* hw_enforced,
AuthorizationSet* sw_enforced) const override;
+ keymaster_error_t DeleteKey(const KeymasterKeyBlob& blob) const override;
+ keymaster_error_t DeleteAllKeys() const override;
keymaster_error_t AddRngEntropy(const uint8_t* buf, size_t length) const override;
keymaster_error_t GenerateRandom(uint8_t* buf, size_t length) const override;
diff --git a/integrity_assured_key_blob.cpp b/integrity_assured_key_blob.cpp
index 83ca432..0cb2de0 100644
--- a/integrity_assured_key_blob.cpp
+++ b/integrity_assured_key_blob.cpp
@@ -50,9 +50,9 @@
};
static keymaster_error_t ComputeHmac(const uint8_t* serialized_data, size_t serialized_data_size,
- const AuthorizationSet& hidden, uint8_t hmac[HMAC_SIZE]) {
+ const AuthorizationSet& hidden, uint8_t hmac[HMAC_SIZE]) {
size_t hidden_bytes_size = hidden.SerializedSize();
- UniquePtr<uint8_t[]> hidden_bytes(new (std::nothrow) uint8_t[hidden_bytes_size]);
+ UniquePtr<uint8_t[]> hidden_bytes(new (std::nothrow) uint8_t[hidden_bytes_size]);
if (!hidden_bytes.get())
return KM_ERROR_MEMORY_ALLOCATION_FAILED;
hidden.Serialize(hidden_bytes.get(), hidden_bytes.get() + hidden_bytes_size);
@@ -108,7 +108,7 @@
const uint8_t* p = key_blob.begin();
const uint8_t* end = key_blob.end();
- if (p > end || p + HMAC_SIZE > end)
+ if (p > end || p + HMAC_SIZE > end)
return KM_ERROR_INVALID_KEY_BLOB;
uint8_t computed_hmac[HMAC_SIZE];
@@ -120,6 +120,20 @@
if (CRYPTO_memcmp(key_blob.end() - HMAC_SIZE, computed_hmac, HMAC_SIZE) != 0)
return KM_ERROR_INVALID_KEY_BLOB;
+ return DeserializeIntegrityAssuredBlob_NoHmacCheck(key_blob, key_material, hw_enforced,
+ sw_enforced);
+}
+
+keymaster_error_t DeserializeIntegrityAssuredBlob_NoHmacCheck(const KeymasterKeyBlob& key_blob,
+ KeymasterKeyBlob* key_material,
+ AuthorizationSet* hw_enforced,
+ AuthorizationSet* sw_enforced) {
+ const uint8_t* p = key_blob.begin();
+ const uint8_t* end = key_blob.end() - HMAC_SIZE;
+
+ if (p > end)
+ return KM_ERROR_INVALID_KEY_BLOB;
+
if (*p != BLOB_VERSION)
return KM_ERROR_INVALID_KEY_BLOB;
++p;
diff --git a/integrity_assured_key_blob.h b/integrity_assured_key_blob.h
index e709314..6239a8a 100644
--- a/integrity_assured_key_blob.h
+++ b/integrity_assured_key_blob.h
@@ -37,6 +37,11 @@
AuthorizationSet* hw_enforced,
AuthorizationSet* sw_enforced);
+keymaster_error_t DeserializeIntegrityAssuredBlob_NoHmacCheck(const KeymasterKeyBlob& key_blob,
+ KeymasterKeyBlob* key_material,
+ AuthorizationSet* hw_enforced,
+ AuthorizationSet* sw_enforced);
+
} // namespace keymaster;
#endif // SYSTEM_KEYMASTER_INTEGRITY_ASSURED_KEY_BLOB_
diff --git a/keymaster0_engine.cpp b/keymaster0_engine.cpp
index d752ae2..bcd64e5 100644
--- a/keymaster0_engine.cpp
+++ b/keymaster0_engine.cpp
@@ -147,6 +147,19 @@
return true;
}
+bool Keymaster0Engine::DeleteKey(const KeymasterKeyBlob& blob) const {
+ if (!keymaster0_device_->delete_keypair)
+ return true;
+ return (keymaster0_device_->delete_keypair(keymaster0_device_, blob.key_material,
+ blob.key_material_size) == 0);
+}
+
+bool Keymaster0Engine::DeleteAllKeys() const {
+ if (!keymaster0_device_->delete_all)
+ return true;
+ return (keymaster0_device_->delete_all(keymaster0_device_) == 0);
+}
+
static keymaster_key_blob_t* duplicate_blob(const uint8_t* key_data, size_t key_data_size) {
unique_ptr<uint8_t[]> key_material_copy(dup_buffer(key_data, key_data_size));
if (!key_material_copy)
diff --git a/keymaster0_engine.h b/keymaster0_engine.h
index 71f550b..8c4970b 100644
--- a/keymaster0_engine.h
+++ b/keymaster0_engine.h
@@ -50,6 +50,8 @@
bool ImportKey(keymaster_key_format_t key_format, const KeymasterKeyBlob& to_import,
KeymasterKeyBlob* imported_key_material) const;
+ bool DeleteKey(const KeymasterKeyBlob& blob) const;
+ bool DeleteAllKeys() const;
RSA* BlobToRsaKey(const KeymasterKeyBlob& blob) const;
EC_KEY* BlobToEcKey(const KeymasterKeyBlob& blob) const;
diff --git a/keymaster1_engine.cpp b/keymaster1_engine.cpp
index 6b40022..bec3007 100644
--- a/keymaster1_engine.cpp
+++ b/keymaster1_engine.cpp
@@ -121,6 +121,18 @@
return error;
}
+keymaster_error_t Keymaster1Engine::DeleteKey(const KeymasterKeyBlob& blob) const {
+ if (!keymaster1_device_->delete_key)
+ return KM_ERROR_OK;
+ return keymaster1_device_->delete_key(keymaster1_device_, &blob);
+}
+
+keymaster_error_t Keymaster1Engine::DeleteAllKeys() const {
+ if (!keymaster1_device_->delete_all_keys)
+ return KM_ERROR_OK;
+ return keymaster1_device_->delete_all_keys(keymaster1_device_);
+}
+
RSA* Keymaster1Engine::BuildRsaKey(const KeymasterKeyBlob& blob,
const AuthorizationSet& additional_params,
keymaster_error_t* error) const {
diff --git a/keymaster1_engine.h b/keymaster1_engine.h
index 9d696df..a94332e 100644
--- a/keymaster1_engine.h
+++ b/keymaster1_engine.h
@@ -52,6 +52,8 @@
const KeymasterKeyBlob& input_key_material,
KeymasterKeyBlob* output_key_blob, AuthorizationSet* hw_enforced,
AuthorizationSet* sw_enforced) const;
+ keymaster_error_t DeleteKey(const KeymasterKeyBlob& blob) const;
+ keymaster_error_t DeleteAllKeys() const;
struct KeyData {
KeyData(const KeymasterKeyBlob& blob, const AuthorizationSet& params)
diff --git a/soft_keymaster_context.cpp b/soft_keymaster_context.cpp
index 1b1fa39..a703b4b 100644
--- a/soft_keymaster_context.cpp
+++ b/soft_keymaster_context.cpp
@@ -357,6 +357,59 @@
return KM_ERROR_INVALID_KEY_BLOB;
}
+keymaster_error_t SoftKeymasterContext::DeleteKey(const KeymasterKeyBlob& blob) const {
+ if (km1_engine_) {
+ keymaster_error_t error = km1_engine_->DeleteKey(blob);
+ if (error == KM_ERROR_INVALID_KEY_BLOB) {
+ // Note that we succeed on invalid blob, because it probably just indicates that the
+ // blob is a software blob, not a hardware blob.
+ error = KM_ERROR_OK;
+ }
+ return error;
+ }
+
+ if (km0_engine_) {
+ // This could be a keymaster0 hardware key, and it could be either raw or encapsulated in an
+ // integrity-assured blob. If it's integrity-assured, we can't validate it strongly,
+ // because we don't have the necessary additional_params data. However, the probability
+ // that anything other than an integrity-assured blob would have all of the structure
+ // required to decode as a valid blob is low -- unless it's maliciously-constructed, but the
+ // deserializer should be proof against bad data, as should the keymaster0 hardware.
+ //
+ // Thus, we first try to parse it as integrity-assured. If that works, we pass the result
+ // to the underlying hardware. If not, we pass blob unmodified to the underlying hardware.
+ KeymasterKeyBlob key_material;
+ AuthorizationSet hw_enforced, sw_enforced;
+ keymaster_error_t error = DeserializeIntegrityAssuredBlob_NoHmacCheck(
+ blob, &key_material, &hw_enforced, &sw_enforced);
+ if (error == KM_ERROR_OK && km0_engine_->DeleteKey(key_material))
+ return KM_ERROR_OK;
+
+ km0_engine_->DeleteKey(blob);
+
+ // We succeed unconditionally at this point, even if delete failed. Failure indicates that
+ // either the blob is a software blob (which we can't distinguish with certainty without
+ // additional_params) or because it is a hardware blob and the hardware failed. In the
+ // first case, there is no error. In the second case, the client can't do anything to fix
+ // it anyway, so it's not too harmful to simply swallow the error. This is not ideal, but
+ // it's the least-bad alternative.
+ return KM_ERROR_OK;
+ }
+
+ // Nothing to do for software-only contexts.
+ return KM_ERROR_OK;
+}
+
+keymaster_error_t SoftKeymasterContext::DeleteAllKeys() const {
+ if (km1_engine_)
+ return km1_engine_->DeleteAllKeys();
+
+ if (km0_engine_ && !km0_engine_->DeleteAllKeys())
+ return KM_ERROR_UNKNOWN_ERROR;
+
+ return KM_ERROR_OK;
+}
+
keymaster_error_t SoftKeymasterContext::AddRngEntropy(const uint8_t* buf, size_t length) const {
RAND_add(buf, length, 0 /* Don't assume any entropy is added to the pool. */);
return KM_ERROR_OK;
diff --git a/soft_keymaster_device.cpp b/soft_keymaster_device.cpp
index 4858eaf..9d8fb90 100644
--- a/soft_keymaster_device.cpp
+++ b/soft_keymaster_device.cpp
@@ -17,11 +17,11 @@
#include <keymaster/soft_keymaster_device.h>
#include <assert.h>
+#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
-#include <stddef.h>
#include <algorithm>
@@ -763,16 +763,8 @@
if (!dev || !key || !key->key_material)
return KM_ERROR_UNEXPECTED_NULL_POINTER;
- const keymaster1_device_t* km1_dev = convert_device(dev)->wrapped_km1_device_;
- if (km1_dev && km1_dev->delete_key)
- return km1_dev->delete_key(km1_dev, key);
-
- const keymaster0_device_t* km0_dev = convert_device(dev)->wrapped_km0_device_;
- if (km0_dev && km0_dev->delete_keypair)
- if (km0_dev->delete_keypair(km0_dev, key->key_material, key->key_material_size) < 0)
- return KM_ERROR_UNKNOWN_ERROR;
-
- return KM_ERROR_OK;
+ KeymasterKeyBlob blob(*key);
+ return convert_device(dev)->context_->DeleteKey(blob);
}
/* static */
@@ -780,16 +772,7 @@
if (!dev)
return KM_ERROR_UNEXPECTED_NULL_POINTER;
- const keymaster1_device_t* km1_dev = convert_device(dev)->wrapped_km1_device_;
- if (km1_dev && km1_dev->delete_all_keys)
- return km1_dev->delete_all_keys(km1_dev);
-
- const keymaster0_device_t* km0_dev = convert_device(dev)->wrapped_km0_device_;
- if (km0_dev && km0_dev->delete_all)
- if (km0_dev->delete_all(km0_dev) < 0)
- return KM_ERROR_UNKNOWN_ERROR;
-
- return KM_ERROR_OK;
+ return convert_device(dev)->context_->DeleteAllKeys();
}
static bool FindAlgorithm(const keymaster_key_param_set_t& params,
