Merge changes from topic "odsign_prop"
* changes:
On-device signing: use properties to indicate state.
On-device signing: some more error checking.
diff --git a/ondevice-signing/VerityUtils.cpp b/ondevice-signing/VerityUtils.cpp
index ff7de7e..cab92e2 100644
--- a/ondevice-signing/VerityUtils.cpp
+++ b/ondevice-signing/VerityUtils.cpp
@@ -74,8 +74,14 @@
Result<std::vector<uint8_t>> createDigest(const std::string& path) {
struct stat filestat;
unique_fd fd(TEMP_FAILURE_RETRY(open(path.c_str(), O_RDONLY | O_CLOEXEC)));
+ if (fd < 0) {
+ return ErrnoError() << "Failed to open " << path;
+ }
- stat(path.c_str(), &filestat);
+ int ret = stat(path.c_str(), &filestat);
+ if (ret < 0) {
+ return ErrnoError() << "Failed to stat " << path;
+ }
struct libfsverity_merkle_tree_params params = {
.version = 1,
.hash_algorithm = FS_VERITY_HASH_ALG_SHA256,
@@ -84,9 +90,13 @@
};
struct libfsverity_digest* digest;
- libfsverity_compute_digest(&fd, &read_callback, ¶ms, &digest);
-
- return std::vector<uint8_t>(&digest->digest[0], &digest->digest[32]);
+ ret = libfsverity_compute_digest(&fd, &read_callback, ¶ms, &digest);
+ if (ret < 0) {
+ return ErrnoError() << "Failed to compute fs-verity digest for " << path;
+ }
+ std::vector<uint8_t> digestVector(&digest->digest[0], &digest->digest[32]);
+ free(digest);
+ return digestVector;
}
namespace {
diff --git a/ondevice-signing/odsign_main.cpp b/ondevice-signing/odsign_main.cpp
index eeef868..58e49ec 100644
--- a/ondevice-signing/odsign_main.cpp
+++ b/ondevice-signing/odsign_main.cpp
@@ -26,6 +26,7 @@
#include <android-base/file.h>
#include <android-base/logging.h>
+#include <android-base/properties.h>
#include <android-base/scopeguard.h>
#include <logwrap/logwrap.h>
@@ -39,6 +40,7 @@
using android::base::ErrnoError;
using android::base::Error;
using android::base::Result;
+using android::base::SetProperty;
using OdsignInfo = ::odsign::proto::OdsignInfo;
@@ -56,6 +58,13 @@
static const bool kForceCompilation = false;
static const bool kUseKeystore = false;
+static const char* kOdsignVerificationDoneProp = "odsign.verification.done";
+static const char* kOdsignKeyDoneProp = "odsign.key.done";
+
+static const char* kOdsignVerificationStatusProp = "odsign.verification.success";
+static const char* kOdsignVerificationStatusValid = "1";
+static const char* kOdsignVerificationStatusError = "0";
+
Result<void> verifyExistingCert(const SigningKey& key) {
if (access(kSigningKeyCert.c_str(), F_OK) < 0) {
return ErrnoError() << "Key certificate not found: " << kSigningKeyCert;
@@ -237,23 +246,62 @@
return {};
}
-int main(int /* argc */, char** /* argv */) {
- auto removeArtifacts = []() -> std::uintmax_t {
- std::error_code ec;
- auto num_removed = std::filesystem::remove_all(kArtArtifactsDir, ec);
- if (ec) {
- // TODO can't remove artifacts, signal Zygote shouldn't use them
- LOG(ERROR) << "Can't remove " << kArtArtifactsDir << ": " << ec.message();
- return 0;
- } else {
- if (num_removed > 0) {
- LOG(INFO) << "Removed " << num_removed << " entries from " << kArtArtifactsDir;
- }
- return num_removed;
+static int removeArtifacts() {
+ std::error_code ec;
+ auto num_removed = std::filesystem::remove_all(kArtArtifactsDir, ec);
+ if (ec) {
+ LOG(ERROR) << "Can't remove " << kArtArtifactsDir << ": " << ec.message();
+ return 0;
+ } else {
+ if (num_removed > 0) {
+ LOG(INFO) << "Removed " << num_removed << " entries from " << kArtArtifactsDir;
}
+ return num_removed;
+ }
+}
+
+static Result<void> verifyArtifacts(const SigningKey& key, bool supportsFsVerity) {
+ auto signInfo = getOdsignInfo(key);
+ // Tell init we're done with the key; this is a boot time optimization
+ // in particular for the no fs-verity case, where we need to do a
+ // costly verification. If the files haven't been tampered with, which
+ // should be the common path, the verification will succeed, and we won't
+ // need the key anymore. If it turns out the artifacts are invalid (eg not
+ // in fs-verity) or the hash doesn't match, we won't be able to generate
+ // new artifacts without the key, so in those cases, remove the artifacts,
+ // and use JIT zygote for the current boot. We should recover automatically
+ // by the next boot.
+ SetProperty(kOdsignKeyDoneProp, "1");
+ if (!signInfo.ok()) {
+ return Error() << signInfo.error().message();
+ }
+ std::map<std::string, std::string> trusted_digests(signInfo->file_hashes().begin(),
+ signInfo->file_hashes().end());
+ Result<void> integrityStatus;
+
+ if (supportsFsVerity) {
+ integrityStatus = verifyIntegrityFsVerity(trusted_digests);
+ } else {
+ integrityStatus = verifyIntegrityNoFsVerity(trusted_digests);
+ }
+ if (!integrityStatus.ok()) {
+ return Error() << integrityStatus.error().message();
+ }
+
+ return {};
+}
+
+int main(int /* argc */, char** /* argv */) {
+ auto errorScopeGuard = []() {
+ // In case we hit any error, remove the artifacts and tell Zygote not to use anything
+ removeArtifacts();
+ // Tell init we don't need to use our key anymore
+ SetProperty(kOdsignKeyDoneProp, "1");
+ // Tell init we're done with verification, and that it was an error
+ SetProperty(kOdsignVerificationDoneProp, "1");
+ SetProperty(kOdsignVerificationStatusProp, kOdsignVerificationStatusError);
};
- // Make sure we delete the artifacts in all early (error) exit paths
- auto scope_guard = android::base::make_scope_guard(removeArtifacts);
+ auto scope_guard = android::base::make_scope_guard(errorScopeGuard);
SigningKey* key;
if (kUseKeystore) {
@@ -301,35 +349,25 @@
}
}
- auto signInfo = getOdsignInfo(*key);
- if (!signInfo.ok()) {
- int num_removed = removeArtifacts();
- // Only a warning if there were artifacts to begin with, which suggests tampering or
- // corruption
- if (num_removed > 0) {
- LOG(WARNING) << signInfo.error().message();
- }
- } else {
- std::map<std::string, std::string> trusted_digests(signInfo->file_hashes().begin(),
- signInfo->file_hashes().end());
- Result<void> integrityStatus;
-
- if (supportsFsVerity) {
- integrityStatus = verifyIntegrityFsVerity(trusted_digests);
- } else {
- integrityStatus = verifyIntegrityNoFsVerity(trusted_digests);
- }
- if (!integrityStatus.ok()) {
- LOG(WARNING) << integrityStatus.error().message() << ", removing " << kArtArtifactsDir;
- removeArtifacts();
- }
- }
-
// Ask ART whether it considers the artifacts valid
LOG(INFO) << "Asking odrefresh to verify artifacts (if present)...";
bool artifactsValid = validateArtifacts();
LOG(INFO) << "odrefresh said they are " << (artifactsValid ? "VALID" : "INVALID");
+ // A post-condition of validating artifacts is that if the ones on /system
+ // are used, kArtArtifactsDir is removed. Conversely, if kArtArtifactsDir
+ // exists, those are artifacts that will be used, and we should verify them.
+ int err = access(kArtArtifactsDir.c_str(), F_OK);
+ // If we receive any error other than ENOENT, be suspicious
+ bool artifactsPresent = (err == 0) || (err < 0 && errno != ENOENT);
+ if (artifactsPresent) {
+ auto verificationResult = verifyArtifacts(*key, supportsFsVerity);
+ if (!verificationResult.ok()) {
+ LOG(ERROR) << verificationResult.error().message();
+ return -1;
+ }
+ }
+
if (!artifactsValid || kForceCompilation) {
LOG(INFO) << "Starting compilation... ";
bool ret = compileArtifacts(kForceCompilation);
@@ -355,10 +393,13 @@
}
}
- // TODO we want to make sure Zygote only picks up the artifacts if we deemed
- // everything was ok here. We could use a sysprop, or some other mechanism?
LOG(INFO) << "On-device signing done.";
scope_guard.Disable();
+ // At this point, we're done with the key for sure
+ SetProperty(kOdsignKeyDoneProp, "1");
+ // And we did a successful verification
+ SetProperty(kOdsignVerificationDoneProp, "1");
+ SetProperty(kOdsignVerificationStatusProp, kOdsignVerificationStatusValid);
return 0;
}