Merge changes from topic "verbose-vendor-logging" am: b6069dcb3c
am: bc57549c90
Change-Id: Ifb53b5e715b1e66a0af4afee54be9133e4c3b198
diff --git a/keystore/blob.cpp b/keystore/blob.cpp
index d86958c..69cbabb 100644
--- a/keystore/blob.cpp
+++ b/keystore/blob.cpp
@@ -36,6 +36,7 @@
#include <string>
#include <android-base/logging.h>
+#include <android-base/unique_fd.h>
namespace {
@@ -341,22 +342,35 @@
size_t fileLength = offsetof(blobv3, value) + dataLength + rawBlob->info;
- int out =
- TEMP_FAILURE_RETRY(open(filename.c_str(), O_WRONLY | O_TRUNC | O_CREAT, S_IRUSR | S_IWUSR));
- if (out < 0) {
- ALOGW("could not open file: %s: %s", filename.c_str(), strerror(errno));
+ char tmpFileName[] = ".tmpXXXXXX";
+ {
+ android::base::unique_fd out(TEMP_FAILURE_RETRY(mkstemp(tmpFileName)));
+ if (out < 0) {
+ LOG(ERROR) << "could not open temp file: " << tmpFileName
+ << " for writing blob file: " << filename.c_str()
+ << " because: " << strerror(errno);
+ return ResponseCode::SYSTEM_ERROR;
+ }
+
+ const size_t writtenBytes =
+ writeFully(out, reinterpret_cast<uint8_t*>(rawBlob), fileLength);
+
+ if (writtenBytes != fileLength) {
+ LOG(ERROR) << "blob not fully written " << writtenBytes << " != " << fileLength;
+ unlink(tmpFileName);
+ return ResponseCode::SYSTEM_ERROR;
+ }
+ }
+
+ if (rename(tmpFileName, filename.c_str()) == -1) {
+ LOG(ERROR) << "could not rename blob file to " << filename
+ << " because: " << strerror(errno);
+ unlink(tmpFileName);
return ResponseCode::SYSTEM_ERROR;
}
- const size_t writtenBytes = writeFully(out, reinterpret_cast<uint8_t*>(rawBlob), fileLength);
- if (close(out) != 0) {
- return ResponseCode::SYSTEM_ERROR;
- }
- if (writtenBytes != fileLength) {
- ALOGW("blob not fully written %zu != %zu", writtenBytes, fileLength);
- unlink(filename.c_str());
- return ResponseCode::SYSTEM_ERROR;
- }
+ fsyncDirectory(getContainingDirectory(filename));
+
return ResponseCode::NO_ERROR;
}
diff --git a/keystore/keymaster_worker.cpp b/keystore/keymaster_worker.cpp
index 922ef0a..23a0023 100644
--- a/keystore/keymaster_worker.cpp
+++ b/keystore/keymaster_worker.cpp
@@ -22,6 +22,10 @@
#include <android-base/logging.h>
+#include <log/log_event_list.h>
+
+#include <private/android_logger.h>
+
#include "KeyStore.h"
#include "keymaster_enforcement.h"
@@ -74,6 +78,26 @@
keymasterDevice_->logIfKeymasterVendorError(ec);
}
+void KeymasterWorker::deleteOldKeyOnUpgrade(const LockedKeyBlobEntry& blobfile, Blob keyBlob) {
+ // if we got the blob successfully, we try and delete it from the keymaster device
+ auto& dev = keymasterDevice_;
+ uid_t uid = blobfile->uid();
+ const auto& alias = blobfile->alias();
+
+ if (keyBlob.getType() == ::TYPE_KEYMASTER_10) {
+ auto ret = KS_HANDLE_HIDL_ERROR(dev, dev->deleteKey(blob2hidlVec(keyBlob)));
+ // A device doesn't have to implement delete_key.
+ bool success = ret == ErrorCode::OK || ret == ErrorCode::UNIMPLEMENTED;
+ if (__android_log_security()) {
+ android_log_event_list(SEC_TAG_KEY_DESTROYED)
+ << int32_t(success) << alias << int32_t(uid) << LOG_ID_SECURITY;
+ }
+ if (!success) {
+ LOG(ERROR) << "Keymaster delete for key " << alias << " of uid " << uid << " failed";
+ }
+ }
+}
+
std::tuple<KeyStoreServiceReturnCode, Blob>
KeymasterWorker::upgradeKeyBlob(const LockedKeyBlobEntry& lockedEntry,
const AuthorizationSet& params) {
@@ -111,12 +135,6 @@
return;
}
- error = keyStore_->del(lockedEntry);
- if (!error.isOk()) {
- ALOGI("upgradeKeyBlob keystore->del failed %d", error.getErrorCode());
- return;
- }
-
Blob newBlob(&upgradedKeyBlob[0], upgradedKeyBlob.size(), nullptr /* info */,
0 /* infoLength */, ::TYPE_KEYMASTER_10);
newBlob.setSecurityLevel(blob.getSecurityLevel());
@@ -129,6 +147,8 @@
ALOGI("upgradeKeyBlob keystore->put failed %d", error.getErrorCode());
return;
}
+
+ deleteOldKeyOnUpgrade(lockedEntry, std::move(blob));
blob = std::move(newBlob);
};
diff --git a/keystore/keymaster_worker.h b/keystore/keymaster_worker.h
index e1a1c02..2c72c80 100644
--- a/keystore/keymaster_worker.h
+++ b/keystore/keymaster_worker.h
@@ -175,6 +175,8 @@
unwrap_tuple(kmfn, std::move(cb), tuple, std::index_sequence_for<Args...>{});
});
}
+
+ void deleteOldKeyOnUpgrade(const LockedKeyBlobEntry& blobfile, Blob keyBlob);
std::tuple<KeyStoreServiceReturnCode, Blob>
upgradeKeyBlob(const LockedKeyBlobEntry& lockedEntry, const AuthorizationSet& params);
std::tuple<KeyStoreServiceReturnCode, KeyCharacteristics, Blob, Blob>
diff --git a/keystore/keystore_utils.cpp b/keystore/keystore_utils.cpp
index 78056d6..f0f6098 100644
--- a/keystore/keystore_utils.cpp
+++ b/keystore/keystore_utils.cpp
@@ -31,6 +31,9 @@
#include <keystore/keymaster_types.h>
#include <keystore/keystore_client.h>
+#include <android-base/logging.h>
+#include <android-base/unique_fd.h>
+
#include "blob.h"
size_t readFully(int fd, uint8_t* data, size_t size) {
@@ -64,6 +67,44 @@
return size;
}
+std::string getContainingDirectory(const std::string& filename) {
+ std::string containing_dir;
+ size_t last_pos;
+ size_t pos = std::string::npos;
+
+ __builtin_add_overflow(filename.size(), -1, &last_pos);
+
+ // strip all trailing '/'
+ while ((pos = filename.find_last_of('/', last_pos)) == last_pos && pos != 0) {
+ --last_pos;
+ }
+
+ if (pos == 0) {
+ containing_dir = "/";
+ } else if (pos == std::string::npos) {
+ containing_dir = ".";
+ } else {
+ containing_dir = filename.substr(0, pos);
+ }
+
+ return containing_dir;
+}
+
+void fsyncDirectory(const std::string& path) {
+ android::base::unique_fd dir_fd(TEMP_FAILURE_RETRY(open(path.c_str(), O_DIRECTORY | O_RDONLY)));
+
+ if (dir_fd < 0) {
+ LOG(WARNING) << "Could not open dir: " << path << " error: " << strerror(errno);
+ return;
+ }
+
+ if (TEMP_FAILURE_RETRY(fsync(dir_fd)) == -1) {
+ LOG(WARNING) << "Failed to fsync the directory " << path << " error: " << strerror(errno);
+ }
+
+ return;
+}
+
void add_legacy_key_authorizations(int keyType, keystore::AuthorizationSet* params) {
using namespace keystore;
params->push_back(TAG_PURPOSE, KeyPurpose::SIGN);
diff --git a/keystore/keystore_utils.h b/keystore/keystore_utils.h
index 3bc9c01..380eb4e 100644
--- a/keystore/keystore_utils.h
+++ b/keystore/keystore_utils.h
@@ -18,6 +18,7 @@
#define KEYSTORE_KEYSTORE_UTILS_H_
#include <cstdint>
+#include <string>
#include <vector>
#include <openssl/evp.h>
@@ -29,6 +30,8 @@
size_t readFully(int fd, uint8_t* data, size_t size);
size_t writeFully(int fd, uint8_t* data, size_t size);
+std::string getContainingDirectory(const std::string& filename);
+void fsyncDirectory(const std::string& path);
void add_legacy_key_authorizations(int keyType, keystore::AuthorizationSet* params);