Fix handling of auth-per-op keys and software digesting
When keystore is using a keymaster1 hardware device that does not
implement all digest algorithms (as allowed by the KM1 spec), keystore
does digesting in software and uses the underlying keymaster1 hardware
to perform the core cryptographic operation.
When auth-per-operation keys (i.e. fingerprint-bound keys) are used, a
keymaster operation is created and associated with an "operation
handle" (64-bit integer). This handle is embedded in the authentication
token generated by the fingerprint matcher, which is what "unlocks" the
key for that one operation.
When those two situations are combined, the SoftKeymasterDevice which
wraps the hardware was caching the keymaster-generated operation handle
for use in completing the operation, but generating its own operation
handle which it returned to keystore. So the software layer's operation
handle got embedded in the auth token and when that auth token was
presented to the hardware, the hardware refuse to accept it, since it
did not contain the hardware's operation handle.
The fix is to have the software wrapper use the underlying hardware's
operation handle.
Bug: 65286954
Test: Manually tested with app linked on above bug
Change-Id: I320c5d03911942e873680ba0d7ea91044920e936
diff --git a/android_keymaster.cpp b/android_keymaster.cpp
index 568cfa4..687a5ca 100644
--- a/android_keymaster.cpp
+++ b/android_keymaster.cpp
@@ -283,7 +283,7 @@
return;
operation->SetAuthorizations(key->authorizations());
- response->error = operation_table_->Add(operation.release(), &response->op_handle);
+ response->error = operation_table_->Add(operation.release(), *context_, &response->op_handle);
}
void AndroidKeymaster::UpdateOperation(const UpdateOperationRequest& request,
diff --git a/ecdsa_keymaster1_operation.h b/ecdsa_keymaster1_operation.h
index 6045686..2066639 100644
--- a/ecdsa_keymaster1_operation.h
+++ b/ecdsa_keymaster1_operation.h
@@ -43,7 +43,9 @@
keymaster_error_t GetError(EVP_PKEY* ecdsa_key);
- protected:
+ keymaster_operation_handle_t GetOperationHandle() const { return operation_handle_; }
+
+ private:
keymaster_purpose_t purpose_;
keymaster_operation_handle_t operation_handle_;
const Keymaster1Engine* engine_;
@@ -90,6 +92,12 @@
return super::Abort();
}
+ keymaster_error_t CreateOperationHandle(const KeymasterContext& /* context */,
+ keymaster_operation_handle_t* op_handle) override {
+ *op_handle = wrapped_operation_.GetOperationHandle();
+ return KM_ERROR_OK;
+ }
+
private:
EcdsaKeymaster1WrappedOperation wrapped_operation_;
};
diff --git a/operation.cpp b/operation.cpp
index 410c9aa..cb46b45 100644
--- a/operation.cpp
+++ b/operation.cpp
@@ -17,6 +17,7 @@
#include "operation.h"
#include <keymaster/authorization_set.h>
+#include <keymaster/keymaster_context.h>
#include "key.h"
@@ -134,6 +135,11 @@
return true;
}
+keymaster_error_t Operation::CreateOperationHandle(const KeymasterContext& context,
+ keymaster_operation_handle_t* op_handle) {
+ return context.GenerateRandom(reinterpret_cast<uint8_t*>(op_handle), sizeof(*op_handle));
+}
+
keymaster_error_t Operation::UpdateForFinish(const AuthorizationSet& input_params,
const Buffer& input) {
if (!input_params.empty() || input.available_read()) {
diff --git a/operation.h b/operation.h
index aadc406..1415f04 100644
--- a/operation.h
+++ b/operation.h
@@ -30,6 +30,7 @@
class AuthorizationSet;
class Key;
+class KeymasterContext;
class Operation;
class OperationFactory {
@@ -109,8 +110,10 @@
const Buffer& signature, AuthorizationSet* output_params,
Buffer* output) = 0;
virtual keymaster_error_t Abort() = 0;
+ virtual keymaster_error_t CreateOperationHandle(const KeymasterContext& context,
+ keymaster_operation_handle_t* op_handle);
-protected:
+ protected:
// Helper function for implementing Finish() methods that need to call Update() to process
// input, but don't expect any output.
keymaster_error_t UpdateForFinish(const AuthorizationSet& input_params, const Buffer& input);
diff --git a/operation_table.cpp b/operation_table.cpp
index 533f754..cf907c8 100644
--- a/operation_table.cpp
+++ b/operation_table.cpp
@@ -31,7 +31,7 @@
handle = 0;
}
-keymaster_error_t OperationTable::Add(Operation* operation,
+keymaster_error_t OperationTable::Add(Operation* operation, const KeymasterContext& context,
keymaster_operation_handle_t* op_handle) {
if (!table_.get()) {
table_.reset(new (std::nothrow) Entry[table_size_]);
@@ -40,8 +40,9 @@
}
UniquePtr<Operation> op(operation);
- if (RAND_bytes(reinterpret_cast<uint8_t*>(op_handle), sizeof(*op_handle)) != 1)
- return TranslateLastOpenSslError();
+ keymaster_error_t error = operation->CreateOperationHandle(context, op_handle);
+ if (error != KM_ERROR_OK)
+ return error;
if (*op_handle == 0) {
// Statistically this is vanishingly unlikely, which means if it ever happens in practice,
// it indicates a broken RNG.
diff --git a/operation_table.h b/operation_table.h
index 0f3f096..3ac8506 100644
--- a/operation_table.h
+++ b/operation_table.h
@@ -23,6 +23,7 @@
namespace keymaster {
+class KeymasterContext;
class Operation;
class OperationTable {
@@ -39,7 +40,8 @@
Operation* operation;
};
- keymaster_error_t Add(Operation* operation, keymaster_operation_handle_t* op_handle);
+ keymaster_error_t Add(Operation* operation, const KeymasterContext& context,
+ keymaster_operation_handle_t* op_handle);
Operation* Find(keymaster_operation_handle_t op_handle);
bool Delete(keymaster_operation_handle_t);
diff --git a/rsa_keymaster1_operation.h b/rsa_keymaster1_operation.h
index 30123f0..1cfa8be 100644
--- a/rsa_keymaster1_operation.h
+++ b/rsa_keymaster1_operation.h
@@ -43,7 +43,9 @@
keymaster_error_t GetError(EVP_PKEY* rsa_key);
- protected:
+ keymaster_operation_handle_t GetOperationHandle() const { return operation_handle_; }
+
+ private:
keymaster_purpose_t purpose_;
keymaster_operation_handle_t operation_handle_;
const Keymaster1Engine* engine_;
@@ -90,6 +92,12 @@
return super::Abort();
}
+ keymaster_error_t CreateOperationHandle(const KeymasterContext& /* context */,
+ keymaster_operation_handle_t* op_handle) override {
+ *op_handle = wrapped_operation_.GetOperationHandle();
+ return KM_ERROR_OK;
+ }
+
private:
RsaKeymaster1WrappedOperation wrapped_operation_;
};
