commit ae73ca248e0e03f5e236484328e8112097f98651
author Edman Anjos <edmanp@google.com> Tue Sep 08 13:06:15 2020 +0000
committer Gerrit Code Review <noreply-gerritcodereview@google.com> Tue Sep 08 13:06:15 2020 +0000
tree eac53ae52b007b8bde186dd4f423e353d56092ff
parent 74cdafdfd144c35a63103d1905778c9ddab027ae
parent ba9b7d32a7934f80eced3112857d84b84809d8f3

Merge "Model KeyStore security level in keymaster worker"

keystore/KeyStore.cpp

diff --git a/keystore/KeyStore.cpp b/keystore/KeyStore.cpp
index 7545397..1f80899 100644
--- a/keystore/KeyStore.cpp
+++ b/keystore/KeyStore.cpp
@@ -60,8 +60,8 @@
                   "KmasterDevices and KeymasterWorkers must have the same size");
     for (size_t i = 0; i < kmDevices.size(); ++i) {
         if (kmDevices[SecurityLevel(i)]) {
-            mKmDevices[SecurityLevel(i)] =
-                std::make_shared<KeymasterWorker>(kmDevices[SecurityLevel(i)], this);
+            mKmDevices[SecurityLevel(i)] = std::make_shared<KeymasterWorker>(
+                kmDevices[SecurityLevel(i)], this, SecurityLevel(i));
         }
     }
 }

keystore/keymaster_worker.cpp

diff --git a/keystore/keymaster_worker.cpp b/keystore/keymaster_worker.cpp
index 7481a1e..cbb184c 100644
--- a/keystore/keymaster_worker.cpp
+++ b/keystore/keymaster_worker.cpp
@@ -83,8 +83,10 @@
     }
 }
 
-KeymasterWorker::KeymasterWorker(sp<Keymaster> keymasterDevice, KeyStore* keyStore)
-    : keymasterDevice_(std::move(keymasterDevice)), operationMap_(keyStore), keyStore_(keyStore) {
+KeymasterWorker::KeymasterWorker(sp<Keymaster> keymasterDevice, KeyStore* keyStore,
+                                 SecurityLevel internalSecurityLevel)
+    : keymasterDevice_(std::move(keymasterDevice)), operationMap_(keyStore), keyStore_(keyStore),
+      internalSecurityLevel_(internalSecurityLevel) {
     // make sure that hal version is cached.
     if (keymasterDevice_) keymasterDevice_->halVersion();
 }
@@ -821,7 +823,7 @@
             outCharacteristics = keyCharacteristics;
 
             Blob keyBlob(&hidlKeyBlob[0], hidlKeyBlob.size(), nullptr, 0, ::TYPE_KEYMASTER_10);
-            keyBlob.setSecurityLevel(securityLevel);
+            keyBlob.setSecurityLevel(internalSecurityLevel_);
             keyBlob.setCriticalToDeviceEncryption(flags &
                                                   KEYSTORE_FLAG_CRITICAL_TO_DEVICE_ENCRYPTION);
             if (isAuthenticationBound(keyParams) && !keyBlob.isCriticalToDeviceEncryption()) {
@@ -929,7 +931,7 @@
             outCharacteristics = keyCharacteristics;
 
             Blob keyBlob(&hidlKeyBlob[0], hidlKeyBlob.size(), nullptr, 0, ::TYPE_KEYMASTER_10);
-            keyBlob.setSecurityLevel(securityLevel);
+            keyBlob.setSecurityLevel(internalSecurityLevel_);
             keyBlob.setCriticalToDeviceEncryption(flags &
                                                   KEYSTORE_FLAG_CRITICAL_TO_DEVICE_ENCRYPTION);
             if (isAuthenticationBound(keyParams) && !keyBlob.isCriticalToDeviceEncryption()) {
@@ -1004,8 +1006,6 @@
                         CAPTURE_MOVE(worker_cb)]() mutable {
         auto hidlWrappingKey = blob2hidlVec(wrappingBlob);
 
-        SecurityLevel securityLevel = keymasterDevice_->halVersion().securityLevel;
-
         KeyCharacteristics outCharacteristics;
         KeyStoreServiceReturnCode error;
 
@@ -1019,7 +1019,7 @@
             outCharacteristics = keyCharacteristics;
 
             Blob keyBlob(hidlKeyBlob.data(), hidlKeyBlob.size(), nullptr, 0, ::TYPE_KEYMASTER_10);
-            keyBlob.setSecurityLevel(securityLevel);
+            keyBlob.setSecurityLevel(internalSecurityLevel_);
             if (isAuthenticationBound(keyCharacteristics.hardwareEnforced)) {
                 keyBlob.setSuperEncrypted(true);
             }

keystore/keymaster_worker.h

diff --git a/keystore/keymaster_worker.h b/keystore/keymaster_worker.h
index f11af29..fbd52b4 100644
--- a/keystore/keymaster_worker.h
+++ b/keystore/keymaster_worker.h
@@ -135,6 +135,19 @@
     OperationMap operationMap_;
     KeyStore* keyStore_;
 
+    /**
+     * Models the security level of this worker internal to KeyStore.
+     *
+     * When the device has only a software Keymaster, KeyStore will set it on the TEE slot and
+     * instantiate a new in-process software Keymaster. In that case there is a mismatch between the
+     * security level used by KeyStore and what is reported from the HAL. This represents the level
+     * used internally by KeyStore.
+     *
+     * This value is used to associate blobs to the corresponding Keymaster backend. It does not
+     * indicate an actual Keymaster HAL security level and should never be exposed to users.
+     */
+    SecurityLevel internalSecurityLevel_;
+
     template <typename KMFn, typename ErrorType, typename... Args, size_t... I>
     void unwrap_tuple(KMFn kmfn, std::function<void(ErrorType)> cb,
                       const std::tuple<Args...>& tuple, std::index_sequence<I...>) {
@@ -200,7 +213,8 @@
                                          hidl_vec<KeyParameter>* params);
 
   public:
-    KeymasterWorker(sp<Keymaster> keymasterDevice, KeyStore* keyStore);
+    KeymasterWorker(sp<Keymaster> keymasterDevice, KeyStore* keyStore,
+                    SecurityLevel internalSecurityLevel);
 
     void logIfKeymasterVendorError(ErrorCode ec) const;