On-device signing: use properties to indicate state.
We want to make parts of the on-device signing daemon asynchronous to
optimize boot time; but in order to not keep the current keystore boot
stage open for too long, odsign will be divided in 2 stages:
1. stage where the signing key is available
2. stage where the signing key is no longer available
Work done in stage 1 is:
1) Retrieving the signing key and verifying its properties
2) Inserting the public key cert in the fs-verity keyring
3) Retrieving odsign metadata and verifying its integrity
4) Generating new compilation artifacts and sign them, if needed
Work done in phase 2 is:
1) Verify existing compilation artifacts, and delete them if invalid
One consequence of this work split is that if we determine existing
artifacts are invalid in phase 2 (eg, the signature doesn't match, or a
file is not in fs-verity), we no longer have access to the key in order
to be able to sign new files. So in those cases, we just delete the
files and boot without artifacts, and will try again on the next boot.
This shouldn't happen unless the artifacts become corrupted somehow.
Reaching stage 2 is indicated by setting the odsign.status property to
'key_use_done'.
Finally, we use the odsign.verification_success property to indicate
whether the artifacts in data were successfully verified; this is
another condition ART can use to determine whether it should use these
artifacts. This covers malicious cases where there are modified
artifacts on /data, and the odsign daemon is prevented from deleting
them. Through the prop, it can still convey that they should not be
used.
Bug: 165630556
Test: inspect properties on boot
Change-Id: Ic0c75cb90c15627a6666741acd5249ddea80c7be
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;
}