Superencrypt authentication-bound keys.
This CL causes keystore to automatically encrypt all newly-created
keymaster key blobs which are authentication-bound. This appears on its
face to be pointless, since the sensitive key material in the key blobs
is already encrypted by the Trusted Execution Environment. It's not
pointless because this adds a cryptographic dependency on the user's
password, including any strengthening performed by
LockSettingService... which may include the use of a separate hardware
trusted module, separate from (and presumably more secure than) the TEE.
A better solution is planned for the next release, but that requires
changes to Gatekeeper and Keymaster. This superencryption will be
removed when that work is done.
Note that the encryption method used by keystore is weak. A separate CL will
replace the weak method with a proper authenticated encryption.
(cherry picked from commit 07aebe73053df12c21c7481a93146bd76add7fbd)
Test: Manual testing.
Bug: 35849499
Change-Id: I0c4910ea24b97bc8046f3d114bfb336670d03321
diff --git a/keystore/keystore.cpp b/keystore/keystore.cpp
index 32667e0..bc2e0dc 100644
--- a/keystore/keystore.cpp
+++ b/keystore/keystore.cpp
@@ -175,13 +175,28 @@
Blob blob;
ResponseCode rc = get(filename, &blob, ::TYPE_ANY, userId);
- /* get can fail if the blob is encrypted and the state is
- * not unlocked, only skip deleting blobs that were loaded and
- * who are not encrypted. If there are blobs we fail to read for
- * other reasons err on the safe side and delete them since we
- * can't tell if they're encrypted.
- */
- shouldDelete = !(rc == ResponseCode::NO_ERROR && !blob.isEncrypted());
+ switch (rc) {
+ case ResponseCode::SYSTEM_ERROR:
+ case ResponseCode::VALUE_CORRUPTED:
+ // If we can't read blobs, delete them.
+ shouldDelete = true;
+ break;
+
+ case ResponseCode::NO_ERROR:
+ case ResponseCode::LOCKED:
+ // Delete encrypted blobs but keep unencrypted blobs and super-encrypted blobs. We
+ // need to keep super-encrypted blobs so we can report that the user is
+ // unauthenticated if a caller tries to use them, rather than reporting that they
+ // don't exist.
+ shouldDelete = blob.isEncrypted();
+ break;
+
+ default:
+ ALOGE("Got unexpected return code %d from KeyStore::get()", rc);
+ // This shouldn't happen. To be on the safe side, delete it.
+ shouldDelete = true;
+ break;
+ }
}
if (shouldDelete) {
del(filename, ::TYPE_ANY, userId);
@@ -272,8 +287,7 @@
importKey(keyBlob->getValue(), keyBlob->getLength(), filename, userId,
keyBlob->isEncrypted() ? KEYSTORE_FLAG_ENCRYPTED : KEYSTORE_FLAG_NONE);
- // The HAL allowed the import, reget the key to have the "fresh"
- // version.
+ // The HAL allowed the import, reget the key to have the "fresh" version.
if (imported == ResponseCode::NO_ERROR) {
rc = get(filename, keyBlob, TYPE_KEY_PAIR, userId);
}