Support Keymaster2 with lots of clever template logic :)

am: 0323afd

* commit '0323afd69d82ce900d520f4611f56e6c06fc08a1':
  Support Keymaster2 with lots of clever template logic :)

Keymaster.cpp

@@ -17,10 +17,104 @@
 #include "Keymaster.h"
 
 #include <android-base/logging.h>
+#include <hardware/hardware.h>
+#include <hardware/keymaster1.h>
+#include <hardware/keymaster2.h>
 
 namespace android {
 namespace vold {
 
+class IKeymasterDevice {
+  public:
+    IKeymasterDevice() {}
+    virtual ~IKeymasterDevice() {}
+    virtual keymaster_error_t generate_key(const keymaster_key_param_set_t* params,
+                                           keymaster_key_blob_t* key_blob) const = 0;
+    virtual keymaster_error_t delete_key(const keymaster_key_blob_t* key) const = 0;
+    virtual keymaster_error_t begin(keymaster_purpose_t purpose, const keymaster_key_blob_t* key,
+                                    const keymaster_key_param_set_t* in_params,
+                                    keymaster_key_param_set_t* out_params,
+                                    keymaster_operation_handle_t* operation_handle) const = 0;
+    virtual keymaster_error_t update(keymaster_operation_handle_t operation_handle,
+                                     const keymaster_key_param_set_t* in_params,
+                                     const keymaster_blob_t* input, size_t* input_consumed,
+                                     keymaster_key_param_set_t* out_params,
+                                     keymaster_blob_t* output) const = 0;
+    virtual keymaster_error_t finish(keymaster_operation_handle_t operation_handle,
+                                     const keymaster_key_param_set_t* in_params,
+                                     const keymaster_blob_t* signature,
+                                     keymaster_key_param_set_t* out_params,
+                                     keymaster_blob_t* output) const = 0;
+    virtual keymaster_error_t abort(keymaster_operation_handle_t operation_handle) const = 0;
+
+  protected:
+    DISALLOW_COPY_AND_ASSIGN(IKeymasterDevice);
+};
+
+template <typename T> class KeymasterDevice : public IKeymasterDevice {
+  public:
+    KeymasterDevice(T* d) : mDevice{d} {}
+    keymaster_error_t generate_key(const keymaster_key_param_set_t* params,
+                                   keymaster_key_blob_t* key_blob) const override final {
+        return mDevice->generate_key(mDevice, params, key_blob, nullptr);
+    }
+    keymaster_error_t delete_key(const keymaster_key_blob_t* key) const override final {
+        if (mDevice->delete_key == nullptr) return KM_ERROR_OK;
+        return mDevice->delete_key(mDevice, key);
+    }
+    keymaster_error_t begin(keymaster_purpose_t purpose, const keymaster_key_blob_t* key,
+                            const keymaster_key_param_set_t* in_params,
+                            keymaster_key_param_set_t* out_params,
+                            keymaster_operation_handle_t* operation_handle) const override final {
+        return mDevice->begin(mDevice, purpose, key, in_params, out_params, operation_handle);
+    }
+    keymaster_error_t update(keymaster_operation_handle_t operation_handle,
+                             const keymaster_key_param_set_t* in_params,
+                             const keymaster_blob_t* input, size_t* input_consumed,
+                             keymaster_key_param_set_t* out_params,
+                             keymaster_blob_t* output) const override final {
+        return mDevice->update(mDevice, operation_handle, in_params, input, input_consumed,
+                               out_params, output);
+    }
+    keymaster_error_t abort(keymaster_operation_handle_t operation_handle) const override final {
+        return mDevice->abort(mDevice, operation_handle);
+    }
+
+  protected:
+    T* const mDevice;
+};
+
+class Keymaster1Device : public KeymasterDevice<keymaster1_device_t> {
+  public:
+    Keymaster1Device(keymaster1_device_t* d) : KeymasterDevice<keymaster1_device_t>{d} {}
+    ~Keymaster1Device() override final { keymaster1_close(mDevice); }
+    keymaster_error_t finish(keymaster_operation_handle_t operation_handle,
+                             const keymaster_key_param_set_t* in_params,
+                             const keymaster_blob_t* signature,
+                             keymaster_key_param_set_t* out_params,
+                             keymaster_blob_t* output) const override final {
+        return mDevice->finish(mDevice, operation_handle, in_params, signature, out_params, output);
+    }
+};
+
+class Keymaster2Device : public KeymasterDevice<keymaster2_device_t> {
+  public:
+    Keymaster2Device(keymaster2_device_t* d) : KeymasterDevice<keymaster2_device_t>{d} {}
+    ~Keymaster2Device() override final { keymaster2_close(mDevice); }
+    keymaster_error_t finish(keymaster_operation_handle_t operation_handle,
+                             const keymaster_key_param_set_t* in_params,
+                             const keymaster_blob_t* signature,
+                             keymaster_key_param_set_t* out_params,
+                             keymaster_blob_t* output) const override final {
+        return mDevice->finish(mDevice, operation_handle, in_params, nullptr, signature, out_params,
+                               output);
+    }
+};
+
+KeymasterOperation::~KeymasterOperation() {
+    if (mDevice) mDevice->abort(mOpHandle);
+}
+
 bool KeymasterOperation::updateCompletely(const std::string& input, std::string* output) {
     output->clear();
     auto it = input.begin();
@@ -29,8 +123,8 @@ bool KeymasterOperation::updateCompletely(const std::string& input, std::string*
         keymaster_blob_t inputBlob{reinterpret_cast<const uint8_t*>(&*it), toRead};
         keymaster_blob_t outputBlob;
         size_t inputConsumed;
-        auto error = mDevice->update(mDevice, mOpHandle, nullptr, &inputBlob, &inputConsumed,
+        auto error =
-                                     nullptr, &outputBlob);
+            mDevice->update(mOpHandle, nullptr, &inputBlob, &inputConsumed, nullptr, &outputBlob);
         if (error != KM_ERROR_OK) {
             LOG(ERROR) << "update failed, code " << error;
             mDevice = nullptr;
@@ -49,7 +143,7 @@ bool KeymasterOperation::updateCompletely(const std::string& input, std::string*
 }
 
 bool KeymasterOperation::finish() {
-    auto error = mDevice->finish(mDevice, mOpHandle, nullptr, nullptr, nullptr, nullptr);
+    auto error = mDevice->finish(mOpHandle, nullptr, nullptr, nullptr, nullptr);
     mDevice = nullptr;
     if (error != KM_ERROR_OK) {
         LOG(ERROR) << "finish failed, code " << error;
@@ -60,7 +154,7 @@ bool KeymasterOperation::finish() {
 
 bool KeymasterOperation::finishWithOutput(std::string* output) {
     keymaster_blob_t outputBlob;
-    auto error = mDevice->finish(mDevice, mOpHandle, nullptr, nullptr, nullptr, &outputBlob);
+    auto error = mDevice->finish(mOpHandle, nullptr, nullptr, nullptr, &outputBlob);
     mDevice = nullptr;
     if (error != KM_ERROR_OK) {
         LOG(ERROR) << "finish failed, code " << error;
@@ -79,22 +173,31 @@ Keymaster::Keymaster() {
         LOG(ERROR) << "hw_get_module_by_class returned " << ret;
         return;
     }
-    // TODO: This will need to be updated to support keymaster2.
+    if (module->module_api_version == KEYMASTER_MODULE_API_VERSION_1_0) {
-    if (module->module_api_version != KEYMASTER_MODULE_API_VERSION_1_0) {
+        keymaster1_device_t* device;
+        ret = keymaster1_open(module, &device);
+        if (ret != 0) {
+            LOG(ERROR) << "keymaster1_open returned " << ret;
+            return;
+        }
+        mDevice = std::make_shared<Keymaster1Device>(device);
+    } else if (module->module_api_version == KEYMASTER_MODULE_API_VERSION_2_0) {
+        keymaster2_device_t* device;
+        ret = keymaster2_open(module, &device);
+        if (ret != 0) {
+            LOG(ERROR) << "keymaster2_open returned " << ret;
+            return;
+        }
+        mDevice = std::make_shared<Keymaster2Device>(device);
+    } else {
         LOG(ERROR) << "module_api_version is " << module->module_api_version;
         return;
     }
-    ret = keymaster1_open(module, &mDevice);
-    if (ret != 0) {
-        LOG(ERROR) << "keymaster1_open returned " << ret;
-        mDevice = nullptr;
-        return;
-    }
 }
 
 bool Keymaster::generateKey(const keymaster::AuthorizationSet& inParams, std::string* key) {
     keymaster_key_blob_t keyBlob;
-    auto error = mDevice->generate_key(mDevice, &inParams, &keyBlob, nullptr);
+    auto error = mDevice->generate_key(&inParams, &keyBlob);
     if (error != KM_ERROR_OK) {
         LOG(ERROR) << "generate_key failed, code " << error;
         return false;
@@ -105,9 +208,8 @@ bool Keymaster::generateKey(const keymaster::AuthorizationSet& inParams, std::st
 }
 
 bool Keymaster::deleteKey(const std::string& key) {
-    if (mDevice->delete_key == nullptr) return true;
     keymaster_key_blob_t keyBlob{reinterpret_cast<const uint8_t*>(key.data()), key.size()};
-    auto error = mDevice->delete_key(mDevice, &keyBlob);
+    auto error = mDevice->delete_key(&keyBlob);
     if (error != KM_ERROR_OK) {
         LOG(ERROR) << "delete_key failed, code " << error;
         return false;
@@ -121,7 +223,7 @@ KeymasterOperation Keymaster::begin(keymaster_purpose_t purpose, const std::stri
     keymaster_key_blob_t keyBlob{reinterpret_cast<const uint8_t*>(key.data()), key.size()};
     keymaster_operation_handle_t mOpHandle;
     keymaster_key_param_set_t outParams_set;
-    auto error = mDevice->begin(mDevice, purpose, &keyBlob, &inParams, &outParams_set, &mOpHandle);
+    auto error = mDevice->begin(purpose, &keyBlob, &inParams, &outParams_set, &mOpHandle);
     if (error != KM_ERROR_OK) {
         LOG(ERROR) << "begin failed, code " << error;
         return KeymasterOperation(nullptr, mOpHandle);
@@ -136,7 +238,7 @@ KeymasterOperation Keymaster::begin(keymaster_purpose_t purpose, const std::stri
                                     const keymaster::AuthorizationSet& inParams) {
     keymaster_key_blob_t keyBlob{reinterpret_cast<const uint8_t*>(key.data()), key.size()};
     keymaster_operation_handle_t mOpHandle;
-    auto error = mDevice->begin(mDevice, purpose, &keyBlob, &inParams, nullptr, &mOpHandle);
+    auto error = mDevice->begin(purpose, &keyBlob, &inParams, nullptr, &mOpHandle);
     if (error != KM_ERROR_OK) {
         LOG(ERROR) << "begin failed, code " << error;
         return KeymasterOperation(nullptr, mOpHandle);

Keymaster.h

@@ -14,13 +14,12 @@
  * limitations under the License.
  */
 
-#ifndef ANDROID_VOLD_KEYMASTER1_H
+#ifndef ANDROID_VOLD_KEYMASTER_H
-#define ANDROID_VOLD_KEYMASTER1_H
+#define ANDROID_VOLD_KEYMASTER_H
 
+#include <memory>
 #include <string>
-
+#include <utility>
-#include <hardware/hardware.h>
-#include <hardware/keymaster1.h>
 
 #include <keymaster/authorization_set.h>
 
@@ -29,19 +28,21 @@ namespace vold {
 
 using namespace keymaster;
 
-// C++ wrappers to the keymaster1 C interface.
+// C++ wrappers to the Keymaster C interface.
 // This is tailored to the needs of KeyStorage, but could be extended to be
 // a more general interface.
 
+// Class that wraps a keymaster1_device_t or keymaster2_device_t and provides methods
+// they have in common. Also closes the device on destruction.
+class IKeymasterDevice;
+
 // Wrapper for a keymaster_operation_handle_t representing an
 // ongoing Keymaster operation.  Aborts the operation
 // in the destructor if it is unfinished. Methods log failures
 // to LOG(ERROR).
 class KeymasterOperation {
   public:
-    ~KeymasterOperation() {
+    ~KeymasterOperation();
-        if (mDevice) mDevice->abort(mDevice, mOpHandle);
-    }
     // Is this instance valid? This is false if creation fails, and becomes
     // false on finish or if an update fails.
     explicit operator bool() { return mDevice != nullptr; }
@@ -54,28 +55,24 @@ class KeymasterOperation {
     bool finishWithOutput(std::string* output);
     // Move constructor
     KeymasterOperation(KeymasterOperation&& rhs) {
-        mOpHandle = rhs.mOpHandle;
+        mOpHandle = std::move(rhs.mOpHandle);
-        mDevice = rhs.mDevice;
+        mDevice = std::move(rhs.mDevice);
-        rhs.mDevice = nullptr;
     }
 
   private:
-    KeymasterOperation(keymaster1_device_t* d, keymaster_operation_handle_t h)
+    KeymasterOperation(std::shared_ptr<IKeymasterDevice> d, keymaster_operation_handle_t h)
         : mDevice{d}, mOpHandle{h} {}
-    keymaster1_device_t* mDevice;
+    std::shared_ptr<IKeymasterDevice> mDevice;
     keymaster_operation_handle_t mOpHandle;
     DISALLOW_COPY_AND_ASSIGN(KeymasterOperation);
     friend class Keymaster;
 };
 
-// Wrapper for a keymaster1_device_t representing an open connection
+// Wrapper for a Keymaster device for methods that start a KeymasterOperation or are not
-// to the keymaster, which is closed in the destructor.
+// part of one.
 class Keymaster {
   public:
     Keymaster();
-    ~Keymaster() {
-        if (mDevice) keymaster1_close(mDevice);
-    }
     // false if we failed to open the keymaster device.
     explicit operator bool() { return mDevice != nullptr; }
     // Generate a key in the keymaster from the given params.
@@ -90,7 +87,7 @@ class Keymaster {
                              const AuthorizationSet& inParams);
 
   private:
-    keymaster1_device_t* mDevice;
+    std::shared_ptr<IKeymasterDevice> mDevice;
     DISALLOW_COPY_AND_ASSIGN(Keymaster);
 };