From b2024e03492bffb2a0c7a21e77bef6da8d1f9777 Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Mon, 15 Mar 2021 12:44:36 -0700
Subject: [PATCH] KeyStorage: improve logging for key generation

The error messages that are printed when probing for rollback resistance
support on a device that doesn't support rollback-resistant keys can
make it sound like something is going wrong.  Print a WARNING message
afterwards to try to make it clear what is going on.  Also adjust or add
DEBUG messages when starting to generate each key so that it's easier to
distinguish the log messages for different key generation operations.

Bug: 182815123
Test: boot on device that doesn't support rollback-resistant keys and
      check log.
Change-Id: I37a13eb5c1e839fb94581f3e7ec1cd8da0263d2b
---
 KeyStorage.cpp | 40 +++++++++++++++++++++++-----------------
 KeyUtil.cpp    |  2 ++
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/KeyStorage.cpp b/KeyStorage.cpp
index 457bb66..356d556 100644
--- a/KeyStorage.cpp
+++ b/KeyStorage.cpp
@@ -133,17 +133,33 @@ static void hashWithPrefix(char const* prefix, const std::string& tohash, std::s
     SHA512_Final(reinterpret_cast<uint8_t*>(&(*res)[0]), &c);
 }
 
-static bool generateKeymasterKey(Keymaster& keymaster, const KeyAuthentication& auth,
-                                 const std::string& appId, std::string* key) {
+// Generates a keymaster key, using rollback resistance if supported.
+static bool generateKeymasterKey(Keymaster& keymaster,
+                                 const km::AuthorizationSetBuilder& paramBuilder,
+                                 std::string* key) {
+    auto paramsWithRollback = paramBuilder;
+    paramsWithRollback.Authorization(km::TAG_ROLLBACK_RESISTANCE);
+
+    if (!keymaster.generateKey(paramsWithRollback, key)) {
+        LOG(WARNING) << "Failed to generate rollback-resistant key.  This is expected if keymaster "
+                        "doesn't support rollback resistance.  Falling back to "
+                        "non-rollback-resistant key.";
+        if (!keymaster.generateKey(paramBuilder, key)) return false;
+    }
+    return true;
+}
+
+static bool generateKeyStorageKey(Keymaster& keymaster, const KeyAuthentication& auth,
+                                  const std::string& appId, std::string* key) {
     auto paramBuilder = km::AuthorizationSetBuilder()
                             .AesEncryptionKey(AES_KEY_BYTES * 8)
                             .GcmModeMinMacLen(GCM_MAC_BYTES * 8)
                             .Authorization(km::TAG_APPLICATION_ID, km::support::blob2hidlVec(appId));
     if (auth.token.empty()) {
-        LOG(DEBUG) << "Creating key that doesn't need auth token";
+        LOG(DEBUG) << "Generating \"key storage\" key that doesn't need auth token";
         paramBuilder.Authorization(km::TAG_NO_AUTH_REQUIRED);
     } else {
-        LOG(DEBUG) << "Auth token required for key";
+        LOG(DEBUG) << "Generating \"key storage\" key that needs auth token";
         if (auth.token.size() != sizeof(hw_auth_token_t)) {
             LOG(ERROR) << "Auth token should be " << sizeof(hw_auth_token_t) << " bytes, was "
                        << auth.token.size() << " bytes";
@@ -155,13 +171,7 @@ static bool generateKeymasterKey(Keymaster& keymaster, const KeyAuthentication&
         paramBuilder.Authorization(km::TAG_USER_AUTH_TYPE, km::HardwareAuthenticatorType::PASSWORD);
         paramBuilder.Authorization(km::TAG_AUTH_TIMEOUT, AUTH_TIMEOUT);
     }
-
-    auto paramsWithRollback = paramBuilder;
-    paramsWithRollback.Authorization(km::TAG_ROLLBACK_RESISTANCE);
-
-    // Generate rollback-resistant key if possible.
-    return keymaster.generateKey(paramsWithRollback, key) ||
-           keymaster.generateKey(paramBuilder, key);
+    return generateKeymasterKey(keymaster, paramBuilder, key);
 }
 
 bool generateWrappedStorageKey(KeyBuffer* key) {
@@ -170,11 +180,7 @@ bool generateWrappedStorageKey(KeyBuffer* key) {
     std::string key_temp;
     auto paramBuilder = km::AuthorizationSetBuilder().AesEncryptionKey(AES_KEY_BYTES * 8);
     paramBuilder.Authorization(km::TAG_STORAGE_KEY);
-    auto paramsWithRollback = paramBuilder;
-    paramsWithRollback.Authorization(km::TAG_ROLLBACK_RESISTANCE);
-    if (!keymaster.generateKey(paramsWithRollback, &key_temp)) {
-        if (!keymaster.generateKey(paramBuilder, &key_temp)) return false;
-    }
+    if (!generateKeymasterKey(keymaster, paramBuilder, &key_temp)) return false;
     *key = KeyBuffer(key_temp.size());
     memcpy(reinterpret_cast<void*>(key->data()), key_temp.c_str(), key->size());
     return true;
@@ -631,7 +637,7 @@ bool storeKey(const std::string& dir, const KeyAuthentication& auth, const KeyBu
         Keymaster keymaster;
         if (!keymaster) return false;
         std::string kmKey;
-        if (!generateKeymasterKey(keymaster, auth, appId, &kmKey)) return false;
+        if (!generateKeyStorageKey(keymaster, auth, appId, &kmKey)) return false;
         if (!writeStringToFile(kmKey, dir + "/" + kFn_keymaster_key_blob)) return false;
         km::AuthorizationSet keyParams;
         km::HardwareAuthToken authToken;
diff --git a/KeyUtil.cpp b/KeyUtil.cpp
index 9d01e1e..886054e 100644
--- a/KeyUtil.cpp
+++ b/KeyUtil.cpp
@@ -57,8 +57,10 @@ bool generateStorageKey(const KeyGeneration& gen, KeyBuffer* key) {
             LOG(ERROR) << "Cannot generate a wrapped key " << gen.keysize << " bytes long";
             return false;
         }
+        LOG(DEBUG) << "Generating wrapped storage key";
         return generateWrappedStorageKey(key);
     } else {
+        LOG(DEBUG) << "Generating standard storage key";
         return randomKey(gen.keysize, key);
     }
 }