Apply authorization checks only during begin().
Bug: 22077675
Change-Id: I29f4a913abc485b5b80cae5eb1eb9914fa6802a5
diff --git a/include/keymaster/keymaster_enforcement.h b/include/keymaster/keymaster_enforcement.h
index 5b5d33b..a0fccdf 100644
--- a/include/keymaster/keymaster_enforcement.h
+++ b/include/keymaster/keymaster_enforcement.h
@@ -59,6 +59,37 @@
bool is_begin_operation);
/**
+ * Iterates through the authorization set and returns the corresponding keymaster error. Will
+ * return KM_ERROR_OK if all criteria is met for the given purpose in the authorization set with
+ * the given operation params. Used for encrypt, decrypt sign, and verify.
+ */
+ keymaster_error_t AuthorizeBegin(const keymaster_purpose_t purpose, const km_id_t keyid,
+ const AuthorizationSet& auth_set,
+ const AuthorizationSet& operation_params);
+
+ /**
+ * Iterates through the authorization set and returns the corresponding keymaster error. Will
+ * return KM_ERROR_OK if all criteria is met for the given purpose in the authorization set with
+ * the given operation params and handle. Used for encrypt, decrypt sign, and verify.
+ */
+ keymaster_error_t AuthorizeUpdate(const AuthorizationSet& auth_set,
+ const AuthorizationSet& operation_params,
+ keymaster_operation_handle_t op_handle) {
+ return AuthorizeUpdateOrFinish(auth_set, operation_params, op_handle);
+ }
+
+ /**
+ * Iterates through the authorization set and returns the corresponding keymaster error. Will
+ * return KM_ERROR_OK if all criteria is met for the given purpose in the authorization set with
+ * the given operation params and handle. Used for encrypt, decrypt sign, and verify.
+ */
+ keymaster_error_t AuthorizeFinish(const AuthorizationSet& auth_set,
+ const AuthorizationSet& operation_params,
+ keymaster_operation_handle_t op_handle) {
+ return AuthorizeUpdateOrFinish(auth_set, operation_params, op_handle);
+ }
+
+ /**
* Creates a key ID for use in subsequent calls to AuthorizeOperation. Clients needn't use this
* method of creating key IDs, as long as they use something consistent and unique. This method
* hashes the key blob.
@@ -116,6 +147,10 @@
virtual bool ValidateTokenSignature(const hw_auth_token_t& token) const = 0;
private:
+ keymaster_error_t AuthorizeUpdateOrFinish(const AuthorizationSet& auth_set,
+ const AuthorizationSet& operation_params,
+ keymaster_operation_handle_t op_handle);
+
bool MinTimeBetweenOpsPassed(uint32_t min_time_between, const km_id_t keyid);
bool MaxUsesPerBootNotExceeded(const km_id_t keyid, uint32_t max_uses);
bool AuthTokenMatches(const AuthorizationSet& auth_set,
diff --git a/keymaster_enforcement.cpp b/keymaster_enforcement.cpp
index aac5d12..8273cf5 100644
--- a/keymaster_enforcement.cpp
+++ b/keymaster_enforcement.cpp
@@ -65,19 +65,70 @@
return purpose == KM_PURPOSE_DECRYPT || purpose == KM_PURPOSE_VERIFY;
}
-inline bool can_skip_authentication(bool is_begin_operation, bool is_auth_per_op_key) {
- // Durign begin with auth-per-op keys, we don't require authentication because it can't be
- // performed until after begin returns the operation handle used to for the authentication
- // challenge.
- return is_begin_operation && is_auth_per_op_key;
-}
-
keymaster_error_t KeymasterEnforcement::AuthorizeOperation(const keymaster_purpose_t purpose,
const km_id_t keyid,
const AuthorizationSet& auth_set,
const AuthorizationSet& operation_params,
keymaster_operation_handle_t op_handle,
bool is_begin_operation) {
+ if (is_begin_operation)
+ return AuthorizeBegin(purpose, keyid, auth_set, operation_params);
+ else
+ return AuthorizeUpdateOrFinish(auth_set, operation_params, op_handle);
+}
+
+// For update and finish the only thing to check is user authentication, and then only if it's not
+// timeout-based.
+keymaster_error_t
+KeymasterEnforcement::AuthorizeUpdateOrFinish(const AuthorizationSet& auth_set,
+ const AuthorizationSet& operation_params,
+ keymaster_operation_handle_t op_handle) {
+ int auth_type_index = -1;
+ for (size_t pos = 0; pos < auth_set.size(); ++pos) {
+ switch (auth_set[pos].tag) {
+ case KM_TAG_NO_AUTH_REQUIRED:
+ case KM_TAG_AUTH_TIMEOUT:
+ // If no auth is required or if auth is timeout-based, we have nothing to check.
+ return KM_ERROR_OK;
+
+ case KM_TAG_USER_AUTH_TYPE:
+ auth_type_index = pos;
+ break;
+
+ default:
+ break;
+ }
+ }
+
+ // Note that at this point we should be able to assume that authentication is required, because
+ // authentication is required if KM_TAG_NO_AUTH_REQUIRED is absent. However, there are legacy
+ // keys which have no authentication-related tags, so we assume that absence is equivalent to
+ // presence of KM_TAG_NO_AUTH_REQUIRED.
+ //
+ // So, if we found KM_TAG_USER_AUTH_TYPE or if we find KM_TAG_USER_SECURE_ID then authentication
+ // is required. If we find neither, then we assume authentication is not required and return
+ // success.
+ bool authentication_required = (auth_type_index != -1);
+ for (auto& param : auth_set) {
+ if (param.tag == KM_TAG_USER_SECURE_ID) {
+ authentication_required = true;
+ int auth_timeout_index = -1;
+ if (AuthTokenMatches(auth_set, operation_params, param.long_integer, auth_type_index,
+ auth_timeout_index, op_handle, false /* is_begin_operation */))
+ return KM_ERROR_OK;
+ }
+ }
+
+ if (authentication_required)
+ return KM_ERROR_KEY_USER_NOT_AUTHENTICATED;
+
+ return KM_ERROR_OK;
+}
+
+keymaster_error_t KeymasterEnforcement::AuthorizeBegin(const keymaster_purpose_t purpose,
+ const km_id_t keyid,
+ const AuthorizationSet& auth_set,
+ const AuthorizationSet& operation_params) {
// Find some entries that may be needed to handle KM_TAG_USER_SECURE_ID
int auth_timeout_index = -1;
int auth_type_index = -1;
@@ -150,12 +201,13 @@
if (no_auth_required_index != -1) {
// Key has both KM_TAG_USER_SECURE_ID and KM_TAG_NO_AUTH_REQUIRED
return KM_ERROR_INVALID_KEY_BLOB;
- } else if (!can_skip_authentication(is_begin_operation, auth_timeout_index == -1) ||
- operation_params.find(KM_TAG_AUTH_TOKEN) != -1) {
+ }
+
+ if (auth_timeout_index != -1) {
authentication_required = true;
if (AuthTokenMatches(auth_set, operation_params, param.long_integer,
- auth_type_index, auth_timeout_index, op_handle,
- is_begin_operation))
+ auth_type_index, auth_timeout_index, 0 /* op_handle */,
+ true /* is_begin_operation */))
auth_token_matched = true;
}
break;
diff --git a/keymaster_enforcement_test.cpp b/keymaster_enforcement_test.cpp
index ef8ae16..c91d7e8 100644
--- a/keymaster_enforcement_test.cpp
+++ b/keymaster_enforcement_test.cpp
@@ -36,7 +36,7 @@
const AuthorizationSet& auth_set) {
AuthorizationSet empty_set;
return KeymasterEnforcement::AuthorizeOperation(
- purpose, keyid, auth_set, empty_set, 0 /* op_handle */, false /* is_begin_operation */);
+ purpose, keyid, auth_set, empty_set, 0 /* op_handle */, true /* is_begin_operation */);
}
using KeymasterEnforcement::AuthorizeOperation;
@@ -432,16 +432,16 @@
EXPECT_EQ(KM_ERROR_OK,
kmen.AuthorizeOperation(KM_PURPOSE_ENCRYPT, key_id, caller_nonce, begin_params,
- 0 /* challenge */, false /* is_begin_operation */));
+ 0 /* challenge */, true /* is_begin_operation */));
EXPECT_EQ(KM_ERROR_OK,
kmen.AuthorizeOperation(KM_PURPOSE_DECRYPT, key_id, caller_nonce, begin_params,
- 0 /* challenge */, false /* is_begin_operation */));
+ 0 /* challenge */, true /* is_begin_operation */));
EXPECT_EQ(KM_ERROR_CALLER_NONCE_PROHIBITED,
kmen.AuthorizeOperation(KM_PURPOSE_ENCRYPT, key_id, no_caller_nonce, begin_params,
- 0 /* challenge */, false /* is_begin_operation */));
+ 0 /* challenge */, true /* is_begin_operation */));
EXPECT_EQ(KM_ERROR_OK,
kmen.AuthorizeOperation(KM_PURPOSE_DECRYPT, key_id, no_caller_nonce, begin_params,
- 0 /* challenge */, false /* is_begin_operation */));
+ 0 /* challenge */, true /* is_begin_operation */));
}
TEST_F(KeymasterBaseTest, TestBootloaderOnly) {
@@ -752,8 +752,8 @@
kmen.AuthorizeOperation(KM_PURPOSE_SIGN, key_id, auth_set, op_params, token.challenge,
true /* is_begin_operation */));
- // And later (though begin would fail, so there wouldn't be a later).
- EXPECT_EQ(KM_ERROR_KEY_USER_NOT_AUTHENTICATED,
+ // Later we don't check (though begin would fail, so there wouldn't be a later).
+ EXPECT_EQ(KM_ERROR_OK,
kmen.AuthorizeOperation(KM_PURPOSE_SIGN, key_id, auth_set, op_params, token.challenge,
false /* is_begin_operation */));
}
