diff options
author | Martijn Coenen <maco@google.com> | 2021-07-27 13:40:23 +0200 |
---|---|---|
committer | Martijn Coenen <maco@google.com> | 2021-07-28 13:51:49 +0200 |
commit | 22a13c378a86f458f70ef4f58e31573b4aa55f5b (patch) | |
tree | db81f960a92723215b92a4087cd6d98d03f02a23 | |
parent | 4791ea8bed6db8bfbd80f662e6344a25de18d4e7 (diff) | |
download | security-22a13c378a86f458f70ef4f58e31573b4aa55f5b.tar.gz |
Don't mark odsign as oneshot.
If odsign is marked as oneshot, and it crashes (eg due to a coding
error), the device will not boot completely, because init keeps waiting
for the odsign.key.done / odsign.verification.done properties. So
instead, we don't mark it as oneshot, but stop the service manually in
the exit paths of the code. This ensures that if a bad OTA / module
update causes odsign to crash, we will automatically start it again; if
it crashes repeatedly, apexd will detect this, and roll back any module
update.
In the good path, there's no difference - odsign will run just once and
be stopped.
Bug: 194334176
Test: manually make odsign crash; inspect output
Change-Id: I7015f291888d6b8066e4c526a7e8cf3c9c7ea618
Merged-In: I7015f291888d6b8066e4c526a7e8cf3c9c7ea618
-rw-r--r-- | ondevice-signing/odsign.rc | 5 | ||||
-rw-r--r-- | ondevice-signing/odsign_main.cpp | 11 |
2 files changed, 13 insertions, 3 deletions
diff --git a/ondevice-signing/odsign.rc b/ondevice-signing/odsign.rc index 044bae7b..de09fc0e 100644 --- a/ondevice-signing/odsign.rc +++ b/ondevice-signing/odsign.rc @@ -2,5 +2,8 @@ service odsign /system/bin/odsign class core user root group system - oneshot disabled # does not start with the core class + +# Note that odsign is not oneshot, but stopped manually when it exits. This +# ensures that if odsign crashes during a module update, apexd will detect +# those crashes and roll back the update. diff --git a/ondevice-signing/odsign_main.cpp b/ondevice-signing/odsign_main.cpp index 0991704b..c4433406 100644 --- a/ondevice-signing/odsign_main.cpp +++ b/ondevice-signing/odsign_main.cpp @@ -64,6 +64,8 @@ static const char* kOdsignVerificationStatusProp = "odsign.verification.success" static const char* kOdsignVerificationStatusValid = "1"; static const char* kOdsignVerificationStatusError = "0"; +static const char* kStopServiceProp = "ctl.stop"; + Result<void> verifyExistingCert(const SigningKey& key) { if (access(kSigningKeyCert.c_str(), F_OK) < 0) { return ErrnoError() << "Key certificate not found: " << kSigningKeyCert; @@ -288,8 +290,10 @@ int main(int /* argc */, char** /* argv */) { // 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); + SetProperty(kOdsignVerificationDoneProp, "1"); + // Tell init it shouldn't try to restart us - see odsign.rc + SetProperty(kStopServiceProp, "odsign"); }; auto scope_guard = android::base::make_scope_guard(errorScopeGuard); @@ -385,7 +389,10 @@ int main(int /* argc */, char** /* argv */) { // 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); + SetProperty(kOdsignVerificationDoneProp, "1"); + + // Tell init it shouldn't try to restart us - see odsign.rc + SetProperty(kStopServiceProp, "odsign"); return 0; } |