Truncate too-long digests for keymaster 0 ECDSA sign operations
BoringSSL doesn't pre-truncate too-long digests before calling the ECDSA
sign operation via the ENGINE interface, and TrustyKeymaster is picky
about accepting them. This means that trying to sign a message with,
say, a 256-bit key and a 384-bit hash fails on Volantis.
This CL also corrects an error in get_supported_digests for ECDSA, which
was advertising support for MD5. BoringSSL doesn't support ECDSA with
MD5 and we're not offering it in the JCA API, so the solution is simply
not to advertise it and to return a better error code if it's requested
anyway.
Bug: 22355708
Change-Id: Iba2dad6953db7eda23951760b734f499a13c5191
diff --git a/android_keymaster_test.cpp b/android_keymaster_test.cpp
index cb120a6..2ee8147 100644
--- a/android_keymaster_test.cpp
+++ b/android_keymaster_test.cpp
@@ -46,6 +46,10 @@
StdoutLogger logger;
+template <typename T> vector<T> make_vector(const T* array, size_t len) {
+ return vector<T>(array, array + len);
+}
+
class TestKeymasterEnforcement : public KeymasterEnforcement {
public:
TestKeymasterEnforcement() : KeymasterEnforcement(3, 3) {}
@@ -221,10 +225,9 @@
ASSERT_EQ(KM_ERROR_OK, device()->get_supported_digests(device(), KM_ALGORITHM_EC,
KM_PURPOSE_SIGN, &digests, &len));
- EXPECT_TRUE(
- ResponseContains({KM_DIGEST_NONE, KM_DIGEST_MD5, KM_DIGEST_SHA1, KM_DIGEST_SHA_2_224,
- KM_DIGEST_SHA_2_256, KM_DIGEST_SHA_2_384, KM_DIGEST_SHA_2_512},
- digests, len));
+ EXPECT_TRUE(ResponseContains({KM_DIGEST_NONE, KM_DIGEST_SHA1, KM_DIGEST_SHA_2_224,
+ KM_DIGEST_SHA_2_256, KM_DIGEST_SHA_2_384, KM_DIGEST_SHA_2_512},
+ digests, len));
free(digests);
EXPECT_EQ(KM_ERROR_UNSUPPORTED_PURPOSE,
@@ -717,6 +720,34 @@
EXPECT_EQ(2, GetParam()->keymaster0_calls());
}
+TEST_P(SigningOperationsTest, EcsdaAllSizesAndHashes) {
+ size_t len;
+ keymaster_digest_t* digest_arr;
+ ASSERT_EQ(KM_ERROR_OK, device()->get_supported_digests(device(), KM_ALGORITHM_EC,
+ KM_PURPOSE_SIGN, &digest_arr, &len));
+ vector<int> key_sizes = {224, 256, 384, 521};
+ vector<keymaster_digest_t> digests = make_vector(digest_arr, len);
+ free(digest_arr);
+
+ for (int key_size : key_sizes) {
+ for (keymaster_digest_t digest : digests) {
+ ASSERT_EQ(
+ KM_ERROR_OK,
+ GenerateKey(AuthorizationSetBuilder().EcdsaSigningKey(key_size).Digest(digest)));
+
+ string message(1024, 'a');
+ string signature;
+ if (digest == KM_DIGEST_NONE)
+ message.resize(key_size / 8);
+ SignMessage(message, &signature, digest);
+ }
+ }
+
+ if (GetParam()->algorithm_in_hardware(KM_ALGORITHM_EC))
+ EXPECT_EQ(digests.size() * key_sizes.size() * 3,
+ static_cast<size_t>(GetParam()->keymaster0_calls()));
+}
+
TEST_P(SigningOperationsTest, AesEcbSign) {
ASSERT_EQ(KM_ERROR_OK,
GenerateKey(AuthorizationSetBuilder().AesEncryptionKey(128).Authorization(
@@ -1233,10 +1264,6 @@
EXPECT_EQ(4, GetParam()->keymaster0_calls());
}
-template <typename T> vector<T> make_vector(const T* array, size_t len) {
- return vector<T>(array, array + len);
-}
-
TEST_P(VerificationOperationsTest, RsaAllDigestAndPadCombinations) {
// Get all supported digests and padding modes.
size_t digests_len;
diff --git a/ecdsa_operation.cpp b/ecdsa_operation.cpp
index 4c7b96f..54af6b2 100644
--- a/ecdsa_operation.cpp
+++ b/ecdsa_operation.cpp
@@ -24,9 +24,9 @@
namespace keymaster {
-static const keymaster_digest_t supported_digests[] = {
- KM_DIGEST_NONE, KM_DIGEST_MD5, KM_DIGEST_SHA1, KM_DIGEST_SHA_2_224,
- KM_DIGEST_SHA_2_256, KM_DIGEST_SHA_2_384, KM_DIGEST_SHA_2_512};
+static const keymaster_digest_t supported_digests[] = {KM_DIGEST_NONE, KM_DIGEST_SHA1,
+ KM_DIGEST_SHA_2_224, KM_DIGEST_SHA_2_256,
+ KM_DIGEST_SHA_2_384, KM_DIGEST_SHA_2_512};
Operation* EcdsaOperationFactory::CreateOperation(const Key& key,
const AuthorizationSet& begin_params,
@@ -70,8 +70,7 @@
case KM_DIGEST_NONE:
return KM_ERROR_OK;
case KM_DIGEST_MD5:
- digest_algorithm_ = EVP_md5();
- return KM_ERROR_OK;
+ return KM_ERROR_UNSUPPORTED_DIGEST;
case KM_DIGEST_SHA1:
digest_algorithm_ = EVP_sha1();
return KM_ERROR_OK;
diff --git a/keymaster0_engine.cpp b/keymaster0_engine.cpp
index 4ee5f9c..d7ad709 100644
--- a/keymaster0_engine.cpp
+++ b/keymaster0_engine.cpp
@@ -25,6 +25,7 @@
#include "keymaster/android_keymaster_utils.h"
+#include <openssl/bn.h>
#include <openssl/ec_key.h>
#include <openssl/ecdsa.h>
@@ -166,7 +167,7 @@
if (!key_material_copy)
return nullptr;
- unique_ptr<keymaster_key_blob_t> blob_copy(new (std::nothrow) keymaster_key_blob_t);
+ unique_ptr<keymaster_key_blob_t> blob_copy(new (std::nothrow) keymaster_key_blob_t);
if (!blob_copy.get())
return nullptr;
blob_copy->key_material_size = key_data_size;
@@ -345,6 +346,17 @@
return 1;
}
+static size_t ec_group_size_bits(EC_KEY* ec_key) {
+ const EC_GROUP* group = EC_KEY_get0_group(ec_key);
+ unique_ptr<BN_CTX, BN_CTX_Delete> bn_ctx(BN_CTX_new());
+ unique_ptr<BIGNUM, BIGNUM_Delete> order(BN_new());
+ if (!EC_GROUP_get_order(group, order.get(), bn_ctx.get())) {
+ ALOGE("Failed to get EC group order");
+ return 0;
+ }
+ return BN_num_bits(order.get());
+}
+
int Keymaster0Engine::EcdsaSign(const uint8_t* digest, size_t digest_len, uint8_t* sig,
unsigned int* sig_len, EC_KEY* ec_key) const {
const keymaster_key_blob_t* key_blob = EcKeyToBlob(ec_key);
@@ -353,6 +365,11 @@
return 0;
}
+ // Truncate digest if it's too long
+ size_t max_input_len = (ec_group_size_bits(ec_key) + 7) / 8;
+ if (digest_len > max_input_len)
+ digest_len = max_input_len;
+
keymaster_ec_sign_params_t sign_params = {DIGEST_NONE};
unique_ptr<uint8_t[], Malloc_Delete> signature;
size_t signature_length;