NIAP: Log key integrity failure to audit log.
Logs key integrity violation in two cases:
1. software-detected corruption of key blob.
2. keymaster operation returning INVALID_KEY_BLOB
Changed AES_gcm_decrypt to return VALUE_CORRUPTED on decryption errors
to be consistent with digest check for older version blob.
Bug: 70886042
Test: manual, by patching some bytes in the blob.
Test: cts-tradefed run cts -m CtsKeystoreTestCases
Change-Id: Ic8f6b7a2a49aee01253b429644af409e568d7deb
diff --git a/keystore/KeyStore.cpp b/keystore/KeyStore.cpp
index 0efc4a3..f197d91 100644
--- a/keystore/KeyStore.cpp
+++ b/keystore/KeyStore.cpp
@@ -26,6 +26,7 @@
#include <utils/String16.h>
#include <utils/String8.h>
+#include <android-base/scopeguard.h>
#include <android/hardware/keymaster/3.0/IKeymasterDevice.h>
#include <android/security/IKeystoreService.h>
#include <log/log_event_list.h>
@@ -36,14 +37,6 @@
#include "permissions.h"
#include <keystore/keystore_hidl_support.h>
-namespace {
-
-// Tags for audit logging. Be careful and don't log sensitive data.
-// Should be in sync with frameworks/base/core/java/android/app/admin/SecurityLogTags.logtags
-constexpr int SEC_TAG_KEY_DESTROYED = 210026;
-
-} // anonymous namespace
-
namespace keystore {
const char* KeyStore::kOldMasterKey = ".masterkey";
@@ -305,10 +298,19 @@
userState->setState(STATE_LOCKED);
}
+static void maybeLogKeyIntegrityViolation(const char* filename, const BlobType type);
+
ResponseCode KeyStore::get(const char* filename, Blob* keyBlob, const BlobType type, uid_t userId) {
UserState* userState = getUserState(userId);
- ResponseCode rc =
- keyBlob->readBlob(filename, userState->getEncryptionKey(), userState->getState());
+ ResponseCode rc;
+
+ auto logOnScopeExit = android::base::make_scope_guard([&] {
+ if (rc == ResponseCode::VALUE_CORRUPTED) {
+ maybeLogKeyIntegrityViolation(filename, type);
+ }
+ });
+
+ rc = keyBlob->readBlob(filename, userState->getEncryptionKey(), userState->getState());
if (rc != ResponseCode::NO_ERROR) {
return rc;
}
@@ -837,4 +839,16 @@
return upgraded;
}
+static void maybeLogKeyIntegrityViolation(const char* filename, const BlobType type) {
+ if (!__android_log_security() || (type != TYPE_KEY_PAIR && type != TYPE_KEYMASTER_10)) return;
+
+ auto uidAlias = filename2UidAlias(filename);
+ uid_t uid = -1;
+ std::string alias;
+
+ if (uidAlias.isOk()) std::tie(uid, alias) = std::move(uidAlias).value();
+
+ log_key_integrity_violation(alias.c_str(), uid);
+}
+
} // namespace keystore
diff --git a/keystore/blob.cpp b/keystore/blob.cpp
index aa1ae37..d21c691 100644
--- a/keystore/blob.cpp
+++ b/keystore/blob.cpp
@@ -114,13 +114,13 @@
out_pos += out_len;
if (!EVP_DecryptFinal_ex(ctx.get(), out_pos, &out_len)) {
ALOGD("Failed to decrypt blob; ciphertext or tag is likely corrupted");
- return ResponseCode::SYSTEM_ERROR;
+ return ResponseCode::VALUE_CORRUPTED;
}
out_pos += out_len;
if (out_pos - out_tmp.get() != static_cast<ssize_t>(len)) {
ALOGD("Encrypted plaintext is the wrong size, expected %zu, got %zd", len,
out_pos - out_tmp.get());
- return ResponseCode::SYSTEM_ERROR;
+ return ResponseCode::VALUE_CORRUPTED;
}
std::copy(out_tmp.get(), out_pos, out);
diff --git a/keystore/key_store_service.cpp b/keystore/key_store_service.cpp
index 89c31a5..d59966f 100644
--- a/keystore/key_store_service.cpp
+++ b/keystore/key_store_service.cpp
@@ -64,11 +64,6 @@
constexpr double kIdRotationPeriod = 30 * 24 * 60 * 60; /* Thirty days, in seconds */
const char* kTimestampFilePath = "timestamp";
-// Tags for audit logging. Be careful and don't log sensitive data.
-// Should be in sync with frameworks/base/core/java/android/app/admin/SecurityLogTags.logtags
-constexpr int SEC_TAG_AUTH_KEY_GENERATED = 210024;
-constexpr int SEC_TAG_KEY_IMPORTED = 210025;
-
struct BIGNUM_Delete {
void operator()(BIGNUM* p) const { BN_free(p); }
};
@@ -916,6 +911,9 @@
auto hidlCb = [&](ErrorCode ret, const KeyCharacteristics& keyCharacteristics) {
error = ret;
if (!error.isOk()) {
+ if (error == ErrorCode::INVALID_KEY_BLOB) {
+ log_key_integrity_violation(name8, targetUid);
+ }
return;
}
*outCharacteristics =
@@ -1100,6 +1098,9 @@
auto hidlCb = [&](ErrorCode ret, const ::android::hardware::hidl_vec<uint8_t>& keyMaterial) {
result->resultCode = ret;
if (!result->resultCode.isOk()) {
+ if (result->resultCode == ErrorCode::INVALID_KEY_BLOB) {
+ log_key_integrity_violation(name8, targetUid);
+ }
return;
}
result->exportData = keyMaterial;
@@ -1262,6 +1263,9 @@
uint64_t operationHandle) {
result->resultCode = ret;
if (!result->resultCode.isOk()) {
+ if (result->resultCode == ErrorCode::INVALID_KEY_BLOB) {
+ log_key_integrity_violation(name8, targetUid);
+ }
return;
}
result->handle = operationHandle;
@@ -2151,6 +2155,9 @@
auto hidlCb = [&](ErrorCode ret, const ::std::vector<uint8_t>& upgradedKeyBlob) {
error = ret;
if (!error.isOk()) {
+ if (error == ErrorCode::INVALID_KEY_BLOB) {
+ log_key_integrity_violation(name8, uid);
+ }
return;
}
diff --git a/keystore/keystore_utils.cpp b/keystore/keystore_utils.cpp
index 3da3791..e5ae29a 100644
--- a/keystore/keystore_utils.cpp
+++ b/keystore/keystore_utils.cpp
@@ -24,6 +24,9 @@
#include <cutils/log.h>
#include <private/android_filesystem_config.h>
+#include <private/android_logger.h>
+
+#include <log/log_event_list.h>
#include <keystore/keymaster_types.h>
#include <keystore/keystore_client.h>
@@ -95,6 +98,12 @@
return uid / AID_USER;
}
+void log_key_integrity_violation(const char* name, uid_t uid) {
+ if (!__android_log_security()) return;
+ android_log_event_list(SEC_TAG_KEY_INTEGRITY_VIOLATION)
+ << name << int32_t(uid) << LOG_ID_SECURITY;
+}
+
namespace keystore {
hidl_vec<uint8_t> blob2hidlVec(const Blob& blob) {
diff --git a/keystore/keystore_utils.h b/keystore/keystore_utils.h
index f1211de..3bc9c01 100644
--- a/keystore/keystore_utils.h
+++ b/keystore/keystore_utils.h
@@ -56,6 +56,15 @@
class Blob;
+// Tags for audit logging. Be careful and don't log sensitive data.
+// Should be in sync with frameworks/base/core/java/android/app/admin/SecurityLogTags.logtags
+constexpr int SEC_TAG_KEY_DESTROYED = 210026;
+constexpr int SEC_TAG_KEY_INTEGRITY_VIOLATION = 210032;
+constexpr int SEC_TAG_AUTH_KEY_GENERATED = 210024;
+constexpr int SEC_TAG_KEY_IMPORTED = 210025;
+
+void log_key_integrity_violation(const char* name, uid_t uid);
+
namespace keystore {
hidl_vec<uint8_t> blob2hidlVec(const Blob& blob);