commit e23a2c91145e2294915e5d0cc5d7591c1aa82aca
author Shawn Willden <swillden@google.com> Mon Jul 06 15:52:45 2015 -0600
committer Shawn Willden <swillden@google.com> Mon Jul 06 16:34:49 2015 -0600
tree 4bfb734245a378f89723c07f229a40088d949b54
parent 5532a085818bdf27ede33c9199024b86023e5961

Fix enforcement of block mode and MAC length on AES ops

Bug: 22301168
Change-Id: I54b4efffa1786b08704dd6e785360870f155ed80

aes_operation.cpp

diff --git a/aes_operation.cpp b/aes_operation.cpp
index face408..bf18041 100644
--- a/aes_operation.cpp
+++ b/aes_operation.cpp
@@ -70,23 +70,30 @@
     if (!begin_params.GetTagValue(TAG_BLOCK_MODE, &block_mode)) {
         LOG_E("%d block modes specified in begin params", begin_params.GetTagCount(TAG_BLOCK_MODE));
         *error = KM_ERROR_UNSUPPORTED_BLOCK_MODE;
+        return nullptr;
     } else if (!supported(block_mode)) {
         LOG_E("Block mode %d not supported", block_mode);
         *error = KM_ERROR_UNSUPPORTED_BLOCK_MODE;
+        return nullptr;
     } else if (!key.authorizations().Contains(TAG_BLOCK_MODE, block_mode)) {
         LOG_E("Block mode %d was specified, but not authorized by key", block_mode);
         *error = KM_ERROR_INCOMPATIBLE_BLOCK_MODE;
+        return nullptr;
     }
 
     size_t tag_length = 0;
     if (block_mode == KM_MODE_GCM) {
         uint32_t tag_length_bits;
-        if (!begin_params.GetTagValue(TAG_MAC_LENGTH, &tag_length_bits))
+        if (!begin_params.GetTagValue(TAG_MAC_LENGTH, &tag_length_bits)) {
             *error = KM_ERROR_MISSING_MAC_LENGTH;
+            return nullptr;
+        }
         tag_length = tag_length_bits / 8;
         if (tag_length_bits % 8 != 0 || tag_length > GCM_MAX_TAG_LENGTH ||
-            tag_length < GCM_MIN_TAG_LENGTH)
+            tag_length < GCM_MIN_TAG_LENGTH) {
             *error = KM_ERROR_UNSUPPORTED_MAC_LENGTH;
+            return nullptr;
+        }
     }
 
     keymaster_padding_t padding;
@@ -95,14 +102,12 @@
     if (!allows_padding(block_mode) && padding != KM_PAD_NONE) {
         LOG_E("Mode does not support padding", 0);
         *error = KM_ERROR_INCOMPATIBLE_PADDING_MODE;
+        return nullptr;
     }
 
     bool caller_nonce = key.authorizations().GetTagValue(TAG_CALLER_NONCE);
 
-    if (*error != KM_ERROR_OK)
-        return nullptr;
-
-    Operation* op = NULL;
+    Operation* op = nullptr;
     switch (purpose()) {
     case KM_PURPOSE_ENCRYPT:
         op = new (std::nothrow)
@@ -116,7 +121,7 @@
         break;
     default:
         *error = KM_ERROR_UNSUPPORTED_PURPOSE;
-        return NULL;
+        return nullptr;
     }
 
     if (!op)

android_keymaster_test.cpp

diff --git a/android_keymaster_test.cpp b/android_keymaster_test.cpp
index 341183b..cb120a6 100644
--- a/android_keymaster_test.cpp
+++ b/android_keymaster_test.cpp
@@ -2076,6 +2076,21 @@
     EXPECT_EQ(0, GetParam()->keymaster0_calls());
 }
 
+TEST_P(EncryptionOperationsTest, AesEcbNotAuthorized) {
+    ASSERT_EQ(KM_ERROR_OK, GenerateKey(AuthorizationSetBuilder()
+                                           .AesEncryptionKey(128)
+                                           .Authorization(TAG_BLOCK_MODE, KM_MODE_CBC)
+                                           .Padding(KM_PAD_NONE)));
+    // Two-block message.
+    string message = "12345678901234567890123456789012";
+    AuthorizationSet begin_params(client_params());
+    begin_params.push_back(TAG_BLOCK_MODE, KM_MODE_ECB);
+    begin_params.push_back(TAG_PADDING, KM_PAD_NONE);
+    EXPECT_EQ(KM_ERROR_INCOMPATIBLE_BLOCK_MODE, BeginOperation(KM_PURPOSE_ENCRYPT, begin_params));
+
+    EXPECT_EQ(0, GetParam()->keymaster0_calls());
+}
+
 TEST_P(EncryptionOperationsTest, AesEcbNoPaddingWrongInputSize) {
     ASSERT_EQ(KM_ERROR_OK, GenerateKey(AuthorizationSetBuilder()
                                            .AesEncryptionKey(128)