commit 040c2a46918fc7b7a39a07459a6f68fc8f7fa86d
author nagendra modadugu <ngm@google.com> Fri Jun 08 17:46:36 2018 -0700
committer Nagendra Modadugu <ngm@google.com> Sat Jun 09 01:20:36 2018 +0000
tree 249245f36b57277d0b9a82298911ef5f94916478
parent 6b47ae0b131fb70c8e8db8ba937a36a81d36ef44

keymaster: skip length check in ImportWrappedKey

The wrapped material length had a hard-coded check
to AES-256.  This results in imports of other key
sizes and algorithms failing.

Pulling out the algorithm from the ASN.1 will need
some refactoring, so just skip the check for now.
The device will check for correctness anyway.

Change-Id: I5c20a53cb68544a73d897e1bff63910783fb9397

hals/keymaster/KeymasterDevice.cpp

diff --git a/hals/keymaster/KeymasterDevice.cpp b/hals/keymaster/KeymasterDevice.cpp
index fad6580..651b4dc 100644
--- a/hals/keymaster/KeymasterDevice.cpp
+++ b/hals/keymaster/KeymasterDevice.cpp
@@ -743,7 +743,7 @@
     // TODO: does keystore chunk stream data?  To what quantum?
     if (input.size() > KM_MAX_PROTO_FIELD_SIZE) {
         LOG(ERROR) << "Excess input length: " << input.size()
-                   << "max allowed: " << KM_MAX_PROTO_FIELD_SIZE;
+                   << " max allowed: " << KM_MAX_PROTO_FIELD_SIZE;
         if (this->abort(operationHandle) != ErrorCode::OK) {
             LOG(ERROR) << "abort( " << operationHandle
                        << ") failed";
@@ -826,7 +826,7 @@
 
     if (input.size() > KM_MAX_PROTO_FIELD_SIZE) {
         LOG(ERROR) << "Excess input length: " << input.size()
-                   << "max allowed: " << KM_MAX_PROTO_FIELD_SIZE;
+                   << " max allowed: " << KM_MAX_PROTO_FIELD_SIZE;
         if (this->abort(operationHandle) != ErrorCode::OK) {
             LOG(ERROR) << "abort( " << operationHandle
                        << ") failed";

hals/keymaster/import_wrapped_key.cpp

diff --git a/hals/keymaster/import_wrapped_key.cpp b/hals/keymaster/import_wrapped_key.cpp
index 1d474d3..84be481 100644
--- a/hals/keymaster/import_wrapped_key.cpp
+++ b/hals/keymaster/import_wrapped_key.cpp
@@ -56,7 +56,10 @@
 #define KM_WRAPPER_FORMAT_VERSION    0
 #define KM_WRAPPER_GCM_IV_SIZE       12
 #define KM_WRAPPER_GCM_TAG_SIZE      16
-#define KM_WRAPPER_WRAPPED_KEY_SIZE  32
+#define KM_WRAPPER_WRAPPED_AES_KEY_SIZE  32
+#define KM_WRAPPER_WRAPPED_DES_KEY_SIZE  24
+// TODO: update max once PKCS8 support is introduced.
+#define KM_WRAPPER_WRAPPED_MAX_KEY_SIZE  32
 #define KM_TAG_MASK                  0x0FFFFFFF
 
 // BoringSSL helpers.
@@ -459,13 +462,16 @@
         LOG(ERROR) << "Failed to parse secure key";
         return ErrorCode::INVALID_ARGUMENT;
     }
-    if (CBS_len(&secureKey) != KM_WRAPPER_WRAPPED_KEY_SIZE) {
-        LOG(ERROR) << "Secure key len, expected: "
-                   << KM_WRAPPER_WRAPPED_KEY_SIZE
+
+    // TODO: check that the wrapped key size matches the algorithm.
+    if (CBS_len(&secureKey) > KM_WRAPPER_WRAPPED_MAX_KEY_SIZE) {
+        LOG(ERROR) << "Secure key len exceeded: "
+                   << KM_WRAPPER_WRAPPED_MAX_KEY_SIZE
                    << " got: "
                    << CBS_len(&secureKey);
         return ErrorCode::INVALID_ARGUMENT;
     }
+
     if (!CBS_get_asn1(&child, &tag, CBS_ASN1_OCTETSTRING)) {
         LOG(ERROR) << "Failed to parse gcm tag";
         return ErrorCode::INVALID_ARGUMENT;