Merge "Handle instance files in odsign."
diff --git a/ondevice-signing/CertUtils.cpp b/ondevice-signing/CertUtils.cpp
index 8d4f273..d67bea6 100644
--- a/ondevice-signing/CertUtils.cpp
+++ b/ondevice-signing/CertUtils.cpp
@@ -369,20 +369,6 @@
     return result;
 }
 
-Result<std::vector<uint8_t>> extractRsaPublicKeyFromX509(const std::vector<uint8_t>& derCert) {
-    auto derCertBytes = derCert.data();
-    bssl::UniquePtr<X509> decoded_cert(d2i_X509(nullptr, &derCertBytes, derCert.size()));
-    if (decoded_cert.get() == nullptr) {
-        return Error() << "Failed to decode X509 certificate.";
-    }
-    bssl::UniquePtr<EVP_PKEY> decoded_pkey(X509_get_pubkey(decoded_cert.get()));
-    if (decoded_pkey == nullptr) {
-        return Error() << "Failed to extract public key from x509 cert";
-    }
-
-    return extractRsaPublicKey(decoded_pkey.get());
-}
-
 Result<CertInfo> verifyAndExtractCertInfoFromX509(const std::string& path,
                                                   const std::vector<uint8_t>& publicKey) {
     auto public_key = modulusToRsaPkey(publicKey);
diff --git a/ondevice-signing/CertUtils.h b/ondevice-signing/CertUtils.h
index 1ed4c06..fe703fa 100644
--- a/ondevice-signing/CertUtils.h
+++ b/ondevice-signing/CertUtils.h
@@ -60,9 +60,6 @@
 extractPublicKeyFromSubjectPublicKeyInfo(const std::vector<uint8_t>& subjectKeyInfo);
 android::base::Result<std::vector<uint8_t>> extractPublicKeyFromX509(const std::string& path);
 
-android::base::Result<std::vector<uint8_t>>
-extractRsaPublicKeyFromX509(const std::vector<uint8_t>& x509);
-
 android::base::Result<CertInfo>
 verifyAndExtractCertInfoFromX509(const std::string& path, const std::vector<uint8_t>& publicKey);
 
diff --git a/ondevice-signing/FakeCompOs.cpp b/ondevice-signing/FakeCompOs.cpp
index cd54e28..596d6e2 100644
--- a/ondevice-signing/FakeCompOs.cpp
+++ b/ondevice-signing/FakeCompOs.cpp
@@ -53,7 +53,8 @@
 // TODO: Allocate a namespace for CompOS
 const int64_t kCompOsNamespace = 101;
 
-Result<std::unique_ptr<FakeCompOs>> FakeCompOs::newInstance() {
+Result<std::unique_ptr<FakeCompOs>>
+FakeCompOs::startInstance(const std::string& /*instanceImagePath*/) {
     std::unique_ptr<FakeCompOs> compOs(new FakeCompOs);
     auto init = compOs->initialize();
     if (init.ok()) {
@@ -88,69 +89,6 @@
     return {};
 }
 
-Result<FakeCompOs::KeyData> FakeCompOs::generateKey() const {
-    std::vector<KeyParameter> params;
-
-    KeyParameter algo;
-    algo.tag = Tag::ALGORITHM;
-    algo.value = KeyParameterValue::make<KeyParameterValue::algorithm>(Algorithm::RSA);
-    params.push_back(algo);
-
-    KeyParameter key_size;
-    key_size.tag = Tag::KEY_SIZE;
-    key_size.value = KeyParameterValue::make<KeyParameterValue::integer>(kRsaKeySize);
-    params.push_back(key_size);
-
-    KeyParameter digest;
-    digest.tag = Tag::DIGEST;
-    digest.value = KeyParameterValue::make<KeyParameterValue::digest>(Digest::SHA_2_256);
-    params.push_back(digest);
-
-    KeyParameter padding;
-    padding.tag = Tag::PADDING;
-    padding.value =
-        KeyParameterValue::make<KeyParameterValue::paddingMode>(PaddingMode::RSA_PKCS1_1_5_SIGN);
-    params.push_back(padding);
-
-    KeyParameter exponent;
-    exponent.tag = Tag::RSA_PUBLIC_EXPONENT;
-    exponent.value = KeyParameterValue::make<KeyParameterValue::longInteger>(kRsaKeyExponent);
-    params.push_back(exponent);
-
-    KeyParameter purpose;
-    purpose.tag = Tag::PURPOSE;
-    purpose.value = KeyParameterValue::make<KeyParameterValue::keyPurpose>(KeyPurpose::SIGN);
-    params.push_back(purpose);
-
-    KeyParameter auth;
-    auth.tag = Tag::NO_AUTH_REQUIRED;
-    auth.value = KeyParameterValue::make<KeyParameterValue::boolValue>(true);
-    params.push_back(auth);
-
-    KeyDescriptor descriptor;
-    descriptor.domain = Domain::BLOB;
-    descriptor.nspace = kCompOsNamespace;
-
-    KeyMetadata metadata;
-    auto status = mSecurityLevel->generateKey(descriptor, {}, params, 0, {}, &metadata);
-    if (!status.isOk()) {
-        return Error() << "Failed to generate key";
-    }
-
-    auto& cert = metadata.certificate;
-    if (!cert) {
-        return Error() << "No certificate.";
-    }
-
-    auto& blob = metadata.key.blob;
-    if (!blob) {
-        return Error() << "No blob.";
-    }
-
-    KeyData key_data{std::move(metadata.certificate.value()), std::move(metadata.key.blob.value())};
-    return key_data;
-}
-
 Result<FakeCompOs::ByteVector> FakeCompOs::signData(const ByteVector& keyBlob,
                                                     const ByteVector& data) const {
     KeyDescriptor descriptor;
diff --git a/ondevice-signing/FakeCompOs.h b/ondevice-signing/FakeCompOs.h
index eb1a8dd..6c7a445 100644
--- a/ondevice-signing/FakeCompOs.h
+++ b/ondevice-signing/FakeCompOs.h
@@ -34,14 +34,9 @@
 
   public:
     using ByteVector = std::vector<uint8_t>;
-    struct KeyData {
-        ByteVector cert;
-        ByteVector blob;
-    };
 
-    static android::base::Result<std::unique_ptr<FakeCompOs>> newInstance();
-
-    android::base::Result<KeyData> generateKey() const;
+    static android::base::Result<std::unique_ptr<FakeCompOs>>
+    startInstance(const std::string& instanceImagePath);
 
     android::base::Result<void> loadAndVerifyKey(const ByteVector& keyBlob,
                                                  const ByteVector& publicKey) const;
diff --git a/ondevice-signing/odsign_main.cpp b/ondevice-signing/odsign_main.cpp
index 699049e..bba39b8 100644
--- a/ondevice-signing/odsign_main.cpp
+++ b/ondevice-signing/odsign_main.cpp
@@ -51,36 +51,35 @@
 
 const std::string kArtArtifactsDir = "/data/misc/apexdata/com.android.art/dalvik-cache";
 
-static const char* kOdrefreshPath = "/apex/com.android.art/bin/odrefresh";
+constexpr const char* kOdrefreshPath = "/apex/com.android.art/bin/odrefresh";
 
-static const char* kFsVerityProcPath = "/proc/sys/fs/verity";
+constexpr const char* kFsVerityProcPath = "/proc/sys/fs/verity";
 
-static const bool kForceCompilation = false;
-static const bool kUseCompOs = false;  // STOPSHIP if true
+constexpr bool kForceCompilation = false;
+constexpr bool kUseCompOs = false;  // STOPSHIP if true
 
-static const char* kVirtApexPath = "/apex/com.android.virt";
+constexpr const char* kCompOsApexPath = "/apex/com.android.compos";
 const std::string kCompOsCert = "/data/misc/odsign/compos_key.cert";
-const std::string kCompOsPublicKey = "/data/misc/odsign/compos_key.pubkey";
-const std::string kCompOsKeyBlob = "/data/misc/odsign/compos_key.blob";
+const std::string kCompOsPublicKey = "/data/misc/apexdata/com.android.compos/compos_key.pubkey";
+const std::string kCompOsKeyBlob = "/data/misc/apexdata/com.android.compos/compos_key.blob";
+const std::string kCompOsInstance = "/data/misc/apexdata/com.android.compos/compos_instance.img";
+
 const std::string kCompOsPendingPublicKey =
     "/data/misc/apexdata/com.android.compos/compos_pending_key.pubkey";
 const std::string kCompOsPendingKeyBlob =
     "/data/misc/apexdata/com.android.compos/compos_pending_key.blob";
+const std::string kCompOsPendingInstance =
+    "/data/misc/apexdata/com.android.compos/compos_pending_instance.img";
 const std::string kCompOsPendingArtifactsDir = "/data/misc/apexdata/com.android.art/compos-pending";
 
-static const char* kOdsignVerificationDoneProp = "odsign.verification.done";
-static const char* kOdsignKeyDoneProp = "odsign.key.done";
+constexpr const char* kOdsignVerificationDoneProp = "odsign.verification.done";
+constexpr const char* kOdsignKeyDoneProp = "odsign.key.done";
 
-static const char* kOdsignVerificationStatusProp = "odsign.verification.success";
-static const char* kOdsignVerificationStatusValid = "1";
-static const char* kOdsignVerificationStatusError = "0";
+constexpr const char* kOdsignVerificationStatusProp = "odsign.verification.success";
+constexpr const char* kOdsignVerificationStatusValid = "1";
+constexpr const char* kOdsignVerificationStatusError = "0";
 
-static const char* kStopServiceProp = "ctl.stop";
-
-static void writeBytesToFile(const std::vector<uint8_t>& bytes, const std::string& path) {
-    std::string str(bytes.begin(), bytes.end());
-    android::base::WriteStringToFile(str, path);
-}
+constexpr const char* kStopServiceProp = "ctl.stop";
 
 static std::vector<uint8_t> readBytesFromFile(const std::string& path) {
     std::string str;
@@ -88,6 +87,16 @@
     return std::vector<uint8_t>(str.begin(), str.end());
 }
 
+static bool rename(const std::string& from, const std::string& to) {
+    std::error_code ec;
+    std::filesystem::rename(from, to, ec);
+    if (ec) {
+        LOG(ERROR) << "Can't rename " << from << " to " << to << ": " << ec.message();
+        return false;
+    }
+    return true;
+}
+
 static int removeDirectory(const std::string& directory) {
     std::error_code ec;
     auto num_removed = std::filesystem::remove_all(directory, ec);
@@ -131,7 +140,7 @@
 }
 
 bool compOsPresent() {
-    return access(kVirtApexPath, F_OK) == 0;
+    return access(kCompOsApexPath, F_OK) == 0;
 }
 
 Result<void> verifyExistingRootCert(const SigningKey& key) {
@@ -194,38 +203,60 @@
     return existingCertInfo.value().subjectRsaPublicKey;
 }
 
-Result<std::vector<uint8_t>> verifyOrGenerateCompOsKey(const SigningKey& signingKey) {
-    std::unique_ptr<FakeCompOs> compOs;
-    std::vector<uint8_t> keyBlob;
+// Attempt to start a CompOS VM from the given instance image and then get it to
+// verify the public key & key blob.  Returns the RsaPublicKey bytes if
+// successful, an empty vector if any of the files are not present, or an error
+// otherwise.
+Result<std::vector<uint8_t>> loadAndVerifyCompOsKey(const std::string& instanceFile,
+                                                    const std::string& publicKeyFile,
+                                                    const std::string& keyBlobFile) {
+    if (access(instanceFile.c_str(), F_OK) != 0 || access(publicKeyFile.c_str(), F_OK) != 0 ||
+        access(keyBlobFile.c_str(), F_OK) != 0) {
+        return {};
+    }
+
+    auto compOsStatus = FakeCompOs::startInstance(instanceFile);
+    if (!compOsStatus.ok()) {
+        return Error() << "Failed to start CompOs instance " << instanceFile << ": "
+                       << compOsStatus.error();
+    }
+    auto& compOs = compOsStatus.value();
+
+    auto publicKey = readBytesFromFile(publicKeyFile);
+    auto keyBlob = readBytesFromFile(keyBlobFile);
+    auto response = compOs->loadAndVerifyKey(keyBlob, publicKey);
+    if (!response.ok()) {
+        return response.error();
+    }
+
+    return publicKey;
+}
+
+Result<std::vector<uint8_t>> verifyCompOsKey(const SigningKey& signingKey) {
     std::vector<uint8_t> publicKey;
-    bool new_key = true;
 
     // If a pending key has been generated we don't know if it is the correct
-    // one for the current CompOS VM, so we need to start it and ask it.
-    if (access(kCompOsPendingPublicKey.c_str(), F_OK) == 0 &&
-        access(kCompOsPendingKeyBlob.c_str(), F_OK) == 0) {
-        auto compOsStatus = FakeCompOs::newInstance();
-        if (!compOsStatus.ok()) {
-            return Error() << "Failed to start CompOs: " << compOsStatus.error();
-        }
-        compOs = std::move(compOsStatus.value());
-
-        auto pendingKeyBlob = readBytesFromFile(kCompOsPendingKeyBlob);
-        auto pendingPublicKey = readBytesFromFile(kCompOsPendingPublicKey);
-
-        auto response = compOs->loadAndVerifyKey(pendingKeyBlob, pendingPublicKey);
-        if (response.ok()) {
+    // one for the pending CompOS VM, so we need to start it and ask it.
+    auto pendingPublicKey = loadAndVerifyCompOsKey(kCompOsPendingInstance, kCompOsPendingPublicKey,
+                                                   kCompOsPendingKeyBlob);
+    if (pendingPublicKey.ok()) {
+        if (!pendingPublicKey->empty()) {
             LOG(INFO) << "Verified pending CompOs key";
-            keyBlob = std::move(pendingKeyBlob);
-            publicKey = std::move(pendingPublicKey);
-        } else {
-            LOG(WARNING) << "Failed to verify pending CompOs key: " << response.error();
-            // And fall through to looking at the current key.
+
+            if (rename(kCompOsPendingInstance, kCompOsInstance) &&
+                rename(kCompOsPendingPublicKey, kCompOsPublicKey) &&
+                rename(kCompOsPendingKeyBlob, kCompOsKeyBlob)) {
+                publicKey = std::move(*pendingPublicKey);
+            }
         }
-        // Whether they're good or bad, we've finished with these files.
-        unlink(kCompOsPendingKeyBlob.c_str());
-        unlink(kCompOsPendingPublicKey.c_str());
+    } else {
+        LOG(WARNING) << "Failed to verify pending CompOs key: " << pendingPublicKey.error();
+        // And fall through to dealing with any current key.
     }
+    // Whether good or bad, we've finished with these files.
+    unlink(kCompOsPendingInstance.c_str());
+    unlink(kCompOsPendingKeyBlob.c_str());
+    unlink(kCompOsPendingPublicKey.c_str());
 
     if (publicKey.empty()) {
         // Alternatively if we signed a cert for the key on a previous boot, then we
@@ -239,61 +270,31 @@
         }
     }
 
-    if (compOs == nullptr) {
-        auto compOsStatus = FakeCompOs::newInstance();
-        if (!compOsStatus.ok()) {
-            return Error() << "Failed to start CompOs: " << compOsStatus.error();
-        }
-        compOs = std::move(compOsStatus.value());
-    }
-
     // Otherwise, if there is an existing key that we haven't signed yet, then we can sign it
     // now if CompOS confirms it's OK.
     if (publicKey.empty()) {
-        if (access(kCompOsPublicKey.c_str(), F_OK) == 0 &&
-            access(kCompOsKeyBlob.c_str(), F_OK) == 0) {
-            auto currentKeyBlob = readBytesFromFile(kCompOsKeyBlob);
-            auto currentPublicKey = readBytesFromFile(kCompOsPublicKey);
-
-            auto response = compOs->loadAndVerifyKey(currentKeyBlob, currentPublicKey);
-            if (response.ok()) {
+        auto currentPublicKey =
+            loadAndVerifyCompOsKey(kCompOsInstance, kCompOsPublicKey, kCompOsKeyBlob);
+        if (currentPublicKey.ok()) {
+            if (!currentPublicKey->empty()) {
                 LOG(INFO) << "Verified existing CompOs key";
-                keyBlob = std::move(currentKeyBlob);
-                publicKey = std::move(currentPublicKey);
-                new_key = false;
-            } else {
-                LOG(WARNING) << "Failed to verify existing CompOs key: " << response.error();
+                publicKey = std::move(*currentPublicKey);
             }
+        } else {
+            LOG(WARNING) << "Failed to verify existing CompOs key: " << currentPublicKey.error();
+            // Delete so we won't try again on next boot.
+            unlink(kCompOsInstance.c_str());
+            unlink(kCompOsKeyBlob.c_str());
+            unlink(kCompOsPublicKey.c_str());
         }
     }
 
-    // If all else has failed we need to ask CompOS to generate a new key.
     if (publicKey.empty()) {
-        auto keyData = compOs->generateKey();
-        if (!keyData.ok()) {
-            return Error() << "Failed to generate key: " << keyData.error();
-        }
-        auto publicKeyStatus = extractRsaPublicKeyFromX509(keyData.value().cert);
-        if (!publicKeyStatus.ok()) {
-            return Error() << "Failed to extract CompOs public key" << publicKeyStatus.error();
-        }
-
-        LOG(INFO) << "Generated new CompOs key";
-
-        keyBlob = std::move(keyData.value().blob);
-        publicKey = std::move(publicKeyStatus.value());
+        return Error() << "No valid CompOs key present.";
     }
 
-    // We've finished with CompOs now, let it exit.
-    compOs.reset();
-
-    // One way or another we now have a valid key pair. Persist the data for
-    // CompOS, and a cert so we can simplify the checks on subsequent boots.
-
-    if (new_key) {
-        writeBytesToFile(keyBlob, kCompOsKeyBlob);
-        writeBytesToFile(publicKey, kCompOsPublicKey);
-    }
+    // One way or another we now have a valid key pair. Persist a certificate so
+    // we can simplify the checks on subsequent boots.
 
     auto signFunction = [&](const std::string& to_be_signed) {
         return signingKey.sign(to_be_signed);
@@ -463,7 +464,7 @@
 }
 
 Result<std::vector<uint8_t>> addCompOsCertToFsVerityKeyring(const SigningKey& signingKey) {
-    auto publicKey = verifyOrGenerateCompOsKey(signingKey);
+    auto publicKey = verifyCompOsKey(signingKey);
     if (!publicKey.ok()) {
         return publicKey.error();
     }
@@ -501,14 +502,11 @@
     // No useful current artifacts, lets see if the CompOs ones are ok
     LOG(INFO) << "Current artifacts are out of date, switching to pending artifacts";
     removeDirectory(kArtArtifactsDir);
-    std::error_code ec;
-    std::filesystem::rename(kCompOsPendingArtifactsDir, kArtArtifactsDir, ec);
-    if (ec) {
-        LOG(ERROR) << "Can't rename " << kCompOsPendingArtifactsDir << " to " << kArtArtifactsDir
-                   << ": " << ec.message();
+    if (!rename(kCompOsPendingArtifactsDir, kArtArtifactsDir)) {
         removeDirectory(kCompOsPendingArtifactsDir);
         return art::odrefresh::ExitCode::kCompilationRequired;
     }
+
     // TODO: Make sure that we check here that the contents of the artifacts
     // correspond to their filenames (and extensions) - the CompOs signatures
     // can't guarantee that.
@@ -609,7 +607,7 @@
     if (supportsCompOs) {
         auto compos_key = addCompOsCertToFsVerityKeyring(*key);
         if (!compos_key.ok()) {
-            LOG(ERROR) << compos_key.error();
+            LOG(WARNING) << compos_key.error();
         } else {
             odrefresh_status =
                 checkCompOsPendingArtifacts(compos_key.value(), *key, &digests_verified);