Key upgrading for FDE.

Correctly handle a key upgrade error from keymaster by upgrading the
FDE RSA key and writing the new key blob to disk.

Bug: 69792304
Test: Roll back PLATFORM_SECURITY_PATCH a month, wipe and reboot, roll
      forwards again, check logs with and without this patch.
Change-Id: I220d2dd4e3d791f636e9bc5f063064cecbf1b88a

Keymaster.cpp

@@ -206,71 +206,83 @@ int keymaster_compatibility_cryptfs_scrypt() {
     return dev.isSecure();
 }
 
-int keymaster_create_key_for_cryptfs_scrypt(uint32_t rsa_key_size,
+static bool write_string_to_buf(const std::string& towrite, uint8_t* buffer, uint32_t buffer_size,
-                                            uint64_t rsa_exponent,
+                                uint32_t* out_size) {
-                                            uint32_t ratelimit,
+    if (!buffer || !out_size) {
-                                            uint8_t* key_buffer,
+        LOG(ERROR) << "Missing target pointers";
-                                            uint32_t key_buffer_size,
+        return false;
-                                            uint32_t* key_out_size)
-{
-    Keymaster dev;
-    std::string key;
-    if (!dev) {
-        LOG(ERROR) << "Failed to initiate keymaster session";
-        return -1;
     }
-    if (!key_buffer || !key_out_size) {
+    *out_size = towrite.size();
-        LOG(ERROR) << __FILE__ << ":" << __LINE__ << ":Invalid argument";
+    if (buffer_size < towrite.size()) {
-        return -1;
+        LOG(ERROR) << "Buffer too small " << buffer_size << " < " << towrite.size();
+        return false;
     }
+    memset(buffer, '\0', buffer_size);
+    std::copy(towrite.begin(), towrite.end(), buffer);
+    return true;
+}
+
+static AuthorizationSet keyParams(uint32_t rsa_key_size, uint64_t rsa_exponent, uint32_t ratelimit) {
+    return AuthorizationSetBuilder()
+        .Authorization(TAG_ALGORITHM, Algorithm::RSA)
+        .Authorization(TAG_KEY_SIZE, rsa_key_size)
+        .Authorization(TAG_RSA_PUBLIC_EXPONENT, rsa_exponent)
+        .Authorization(TAG_PURPOSE, KeyPurpose::SIGN)
+        .Authorization(TAG_PADDING, PaddingMode::NONE)
+        .Authorization(TAG_DIGEST, Digest::NONE)
+        .Authorization(TAG_BLOB_USAGE_REQUIREMENTS, KeyBlobUsageRequirements::STANDALONE)
+        .Authorization(TAG_NO_AUTH_REQUIRED)
+        .Authorization(TAG_MIN_SECONDS_BETWEEN_OPS, ratelimit);
+}
+
+int keymaster_create_key_for_cryptfs_scrypt(uint32_t rsa_key_size, uint64_t rsa_exponent,
+                                            uint32_t ratelimit, uint8_t* key_buffer,
+                                            uint32_t key_buffer_size, uint32_t* key_out_size) {
     if (key_out_size) {
         *key_out_size = 0;
     }
-
-    auto paramBuilder = AuthorizationSetBuilder()
-                            .Authorization(TAG_ALGORITHM, Algorithm::RSA)
-                            .Authorization(TAG_KEY_SIZE, rsa_key_size)
-                            .Authorization(TAG_RSA_PUBLIC_EXPONENT, rsa_exponent)
-                            .Authorization(TAG_PURPOSE, KeyPurpose::SIGN)
-                            .Authorization(TAG_PADDING, PaddingMode::NONE)
-                            .Authorization(TAG_DIGEST, Digest::NONE)
-                            .Authorization(TAG_BLOB_USAGE_REQUIREMENTS,
-                                    KeyBlobUsageRequirements::STANDALONE)
-                            .Authorization(TAG_NO_AUTH_REQUIRED)
-                            .Authorization(TAG_MIN_SECONDS_BETWEEN_OPS, ratelimit);
-
-    if (!dev.generateKey(paramBuilder, &key)) {
-        return -1;
-    }
-
-    if (key_out_size) {
-        *key_out_size = key.size();
-    }
-
-    if (key_buffer_size < key.size()) {
-        return -1;
-    }
-
-    std::copy(key.data(), key.data() + key.size(), key_buffer);
-    return 0;
-}
-
-int keymaster_sign_object_for_cryptfs_scrypt(const uint8_t* key_blob,
-                                             size_t key_blob_size,
-                                             uint32_t ratelimit,
-                                             const uint8_t* object,
-                                             const size_t object_size,
-                                             uint8_t** signature_buffer,
-                                             size_t* signature_buffer_size)
-{
     Keymaster dev;
     if (!dev) {
         LOG(ERROR) << "Failed to initiate keymaster session";
         return -1;
     }
+    std::string key;
+    if (!dev.generateKey(keyParams(rsa_key_size, rsa_exponent, ratelimit), &key)) return -1;
+    if (!write_string_to_buf(key, key_buffer, key_buffer_size, key_out_size)) return -1;
+    return 0;
+}
+
+int keymaster_upgrade_key_for_cryptfs_scrypt(uint32_t rsa_key_size, uint64_t rsa_exponent,
+                                             uint32_t ratelimit, const uint8_t* key_blob,
+                                             size_t key_blob_size, uint8_t* key_buffer,
+                                             uint32_t key_buffer_size, uint32_t* key_out_size) {
+    if (key_out_size) {
+        *key_out_size = 0;
+    }
+    Keymaster dev;
+    if (!dev) {
+        LOG(ERROR) << "Failed to initiate keymaster session";
+        return -1;
+    }
+    std::string old_key(reinterpret_cast<const char*>(key_blob), key_blob_size);
+    std::string new_key;
+    if (!dev.upgradeKey(old_key, keyParams(rsa_key_size, rsa_exponent, ratelimit), &new_key))
+        return -1;
+    if (!write_string_to_buf(new_key, key_buffer, key_buffer_size, key_out_size)) return -1;
+    return 0;
+}
+
+KeymasterSignResult keymaster_sign_object_for_cryptfs_scrypt(
+    const uint8_t* key_blob, size_t key_blob_size, uint32_t ratelimit, const uint8_t* object,
+    const size_t object_size, uint8_t** signature_buffer, size_t* signature_buffer_size) {
+    Keymaster dev;
+    if (!dev) {
+        LOG(ERROR) << "Failed to initiate keymaster session";
+        return KeymasterSignResult::error;
+    }
     if (!key_blob || !object || !signature_buffer || !signature_buffer_size) {
         LOG(ERROR) << __FILE__ << ":" << __LINE__ << ":Invalid argument";
-        return -1;
+        return KeymasterSignResult::error;
     }
 
     AuthorizationSet outParams;
@@ -291,28 +303,33 @@ int keymaster_sign_object_for_cryptfs_scrypt(const uint8_t* key_blob,
         } else break;
     }
 
+    if (op.errorCode() == ErrorCode::KEY_REQUIRES_UPGRADE) {
+        LOG(ERROR) << "Keymaster key requires upgrade";
+        return KeymasterSignResult::upgrade;
+    }
+
     if (op.errorCode() != ErrorCode::OK) {
         LOG(ERROR) << "Error starting keymaster signature transaction: " << int32_t(op.errorCode());
-        return -1;
+        return KeymasterSignResult::error;
     }
 
     if (!op.updateCompletely(input, &output)) {
         LOG(ERROR) << "Error sending data to keymaster signature transaction: "
                    << uint32_t(op.errorCode());
-        return -1;
+        return KeymasterSignResult::error;
     }
 
     if (!op.finish(&output)) {
         LOG(ERROR) << "Error finalizing keymaster signature transaction: " << int32_t(op.errorCode());
-        return -1;
+        return KeymasterSignResult::error;
     }
 
     *signature_buffer = reinterpret_cast<uint8_t*>(malloc(output.size()));
     if (*signature_buffer == nullptr) {
         LOG(ERROR) << "Error allocation buffer for keymaster signature";
-        return -1;
+        return KeymasterSignResult::error;
     }
     *signature_buffer_size = output.size();
     std::copy(output.data(), output.data() + output.size(), *signature_buffer);
-    return 0;
+    return KeymasterSignResult::ok;
 }

Keymaster.h

@@ -127,6 +127,14 @@ class Keymaster {
  */
 __BEGIN_DECLS
 
+/* Return values for keymaster_sign_object_for_cryptfs_scrypt */
+
+enum class KeymasterSignResult {
+    ok = 0,
+    error = -1,
+    upgrade = -2,
+};
+
 int keymaster_compatibility_cryptfs_scrypt();
 int keymaster_create_key_for_cryptfs_scrypt(uint32_t rsa_key_size,
                                             uint64_t rsa_exponent,
@@ -135,13 +143,14 @@ int keymaster_create_key_for_cryptfs_scrypt(uint32_t rsa_key_size,
                                             uint32_t key_buffer_size,
                                             uint32_t* key_out_size);
 
-int keymaster_sign_object_for_cryptfs_scrypt(const uint8_t* key_blob,
+int keymaster_upgrade_key_for_cryptfs_scrypt(uint32_t rsa_key_size, uint64_t rsa_exponent,
-                                             size_t key_blob_size,
+                                             uint32_t ratelimit, const uint8_t* key_blob,
-                                             uint32_t ratelimit,
+                                             size_t key_blob_size, uint8_t* key_buffer,
-                                             const uint8_t* object,
+                                             uint32_t key_buffer_size, uint32_t* key_out_size);
-                                             const size_t object_size,
+
-                                             uint8_t** signature_buffer,
+KeymasterSignResult keymaster_sign_object_for_cryptfs_scrypt(
-                                             size_t* signature_buffer_size);
+    const uint8_t* key_blob, size_t key_blob_size, uint32_t ratelimit, const uint8_t* object,
+    const size_t object_size, uint8_t** signature_buffer, size_t* signature_buffer_size);
 
 __END_DECLS
 

cryptfs.cpp

@@ -96,6 +96,8 @@ extern "C" {
 #define RETRY_MOUNT_ATTEMPTS 10
 #define RETRY_MOUNT_DELAY_SECONDS 1
 
+static int put_crypt_ftr_and_key(struct crypt_mnt_ftr* crypt_ftr);
+
 static unsigned char saved_master_key[KEY_LEN_BYTES];
 static char *saved_mount_point;
 static int  master_key_saved = 0;
@@ -120,7 +122,7 @@ static int keymaster_create_key(struct crypt_mnt_ftr *ftr)
             &ftr->keymaster_blob_size);
     if (rc) {
         if (ftr->keymaster_blob_size > KEYMASTER_BLOB_SIZE) {
-            SLOGE("Keymaster key blob to large)");
+            SLOGE("Keymaster key blob too large");
             ftr->keymaster_blob_size = 0;
         }
         SLOGE("Failed to generate keypair");
@@ -169,8 +171,31 @@ static int keymaster_sign_object(struct crypt_mnt_ftr *ftr,
             SLOGE("Unknown KDF type %d", ftr->kdf_type);
             return -1;
     }
-    return keymaster_sign_object_for_cryptfs_scrypt(ftr->keymaster_blob, ftr->keymaster_blob_size,
+    for (;;) {
-            KEYMASTER_CRYPTFS_RATE_LIMIT, to_sign, to_sign_size, signature, signature_size);
+        auto result = keymaster_sign_object_for_cryptfs_scrypt(
+            ftr->keymaster_blob, ftr->keymaster_blob_size, KEYMASTER_CRYPTFS_RATE_LIMIT, to_sign,
+            to_sign_size, signature, signature_size);
+        switch (result) {
+            case KeymasterSignResult::ok:
+                return 0;
+            case KeymasterSignResult::upgrade:
+                break;
+            default:
+                return -1;
+        }
+        SLOGD("Upgrading key");
+        if (keymaster_upgrade_key_for_cryptfs_scrypt(
+                RSA_KEY_SIZE, RSA_EXPONENT, KEYMASTER_CRYPTFS_RATE_LIMIT, ftr->keymaster_blob,
+                ftr->keymaster_blob_size, ftr->keymaster_blob, KEYMASTER_BLOB_SIZE,
+                &ftr->keymaster_blob_size) != 0) {
+            SLOGE("Failed to upgrade key");
+            return -1;
+        }
+        if (put_crypt_ftr_and_key(ftr) != 0) {
+            SLOGE("Failed to write upgraded key to disk");
+        }
+        SLOGD("Key upgraded successfully");
+    }
 }
 
 /* Store password when userdata is successfully decrypted and mounted.

tests/CryptfsScryptHidlizationEquivalence_test.cpp

@@ -431,8 +431,11 @@ static int keymaster_sign_object_new(struct crypt_mnt_ftr *ftr,
             SLOGE("Unknown KDF type %d", ftr->kdf_type);
             return -1;
     }
-    return keymaster_sign_object_for_cryptfs_scrypt(ftr->keymaster_blob, ftr->keymaster_blob_size,
+    if (keymaster_sign_object_for_cryptfs_scrypt(
-            KEYMASTER_CRYPTFS_RATE_LIMIT, to_sign, to_sign_size, signature, signature_size);
+            ftr->keymaster_blob, ftr->keymaster_blob_size, KEYMASTER_CRYPTFS_RATE_LIMIT, to_sign,
+            to_sign_size, signature, signature_size) != KeymasterSignResult::ok)
+        return -1;
+    return 0;
 }
 
 namespace android {