commit 9a79ce98d7c5ebb44065767fb7c2280c523a3ead
author Alan Stokes <alanstokes@google.com> Thu Nov 25 11:47:54 2021 +0000
committer Alan Stokes <alanstokes@google.com> Thu Nov 25 15:08:31 2021 +0000
tree 20d973aaa05cedfb08f07401fbaa21a9e1c3fc81
parent 7858101d4b954fa2029c6e9d2ef2a04654031e3f

Make verify_key easier to test

Various minor improvements:
- Better logging on failure
- Log briefly on success
- Enable debug mode for the VM to be turned on
- Allow the test instance to be verified

And a few changes aiming at better readability.

Bug: 186126194
Test: compos_verify_key --debug --instance test
Change-Id: I45a90dd9a8852066077cf31659373f77c8b6ad6c

compos/verify_key/verify_key.rs

diff --git a/compos/verify_key/verify_key.rs b/compos/verify_key/verify_key.rs
index 0a9d36b..945acb4 100644
--- a/compos/verify_key/verify_key.rs
+++ b/compos/verify_key/verify_key.rs
@@ -22,7 +22,7 @@
 use compos_common::compos_client::{VmInstance, VmParameters};
 use compos_common::{
     COMPOS_DATA_ROOT, CURRENT_INSTANCE_DIR, INSTANCE_IMAGE_FILE, PENDING_INSTANCE_DIR,
-    PRIVATE_KEY_BLOB_FILE, PUBLIC_KEY_FILE,
+    PRIVATE_KEY_BLOB_FILE, PUBLIC_KEY_FILE, TEST_INSTANCE_DIR,
 };
 use std::fs::{self, File};
 use std::io::Read;
@@ -30,41 +30,54 @@
 
 const MAX_FILE_SIZE_BYTES: u64 = 8 * 1024;
 
-fn main() -> Result<()> {
+fn main() {
     android_logger::init_once(
         android_logger::Config::default()
             .with_tag("compos_verify_key")
             .with_min_level(log::Level::Info),
     );
 
+    if let Err(e) = try_main() {
+        log::error!("{:?}", e);
+        std::process::exit(-1)
+    }
+}
+
+fn try_main() -> Result<()> {
     let matches = clap::App::new("compos_verify_key")
         .arg(
             clap::Arg::with_name("instance")
                 .long("instance")
                 .takes_value(true)
                 .required(true)
-                .possible_values(&["pending", "current"]),
+                .possible_values(&["pending", "current", "test"]),
         )
+        .arg(clap::Arg::with_name("debug").long("debug"))
         .get_matches();
-    let do_pending = matches.value_of("instance").unwrap() == "pending";
 
-    let instance_dir: PathBuf =
-        [COMPOS_DATA_ROOT, if do_pending { PENDING_INSTANCE_DIR } else { CURRENT_INSTANCE_DIR }]
-            .iter()
-            .collect();
+    let debug_mode = matches.is_present("debug");
+    let (promote_if_valid, instance_dir) = match matches.value_of("instance").unwrap() {
+        "pending" => (true, PENDING_INSTANCE_DIR),
+        "current" => (false, CURRENT_INSTANCE_DIR),
+        "test" => (false, TEST_INSTANCE_DIR),
+        _ => unreachable!("Unexpected instance name"),
+    };
+
+    let instance_dir: PathBuf = [COMPOS_DATA_ROOT, instance_dir].iter().collect();
 
     if !instance_dir.is_dir() {
-        bail!("{} is not a directory", instance_dir.display());
+        bail!("{:?} is not a directory", instance_dir);
     }
 
     // We need to start the thread pool to be able to receive Binder callbacks
     ProcessState::start_thread_pool();
 
-    let result = verify(&instance_dir).and_then(|_| {
-        if do_pending {
-            // If the pending instance is ok, then it must actually match the current system state,
+    let result = verify(debug_mode, &instance_dir).and_then(|_| {
+        log::info!("Verified {:?}", instance_dir);
+        if promote_if_valid {
+            // If the instance is ok, then it must actually match the current system state,
             // so we promote it to current.
-            log::info!("Promoting pending to current");
+            log::info!("Promoting to current");
             promote_to_current(&instance_dir)
         } else {
             Ok(())
@@ -73,7 +86,7 @@
 
     if result.is_err() {
         // This is best efforts, and we still want to report the original error as our result
-        log::info!("Removing {}", instance_dir.display());
+        log::info!("Removing {:?}", instance_dir);
         if let Err(e) = fs::remove_dir_all(&instance_dir) {
             log::warn!("Failed to remove directory: {}", e);
         }
@@ -82,7 +95,7 @@
     result
 }
 
-fn verify(instance_dir: &Path) -> Result<()> {
+fn verify(debug_mode: bool, instance_dir: &Path) -> Result<()> {
     let blob = instance_dir.join(PRIVATE_KEY_BLOB_FILE);
     let public_key = instance_dir.join(PUBLIC_KEY_FILE);
     let instance_image = instance_dir.join(INSTANCE_IMAGE_FILE);
@@ -93,7 +106,7 @@
 
     let virtualization_service = VmInstance::connect_to_virtualization_service()?;
     let vm_instance =
-        VmInstance::start(&*virtualization_service, instance_image, &VmParameters::default())?;
+        VmInstance::start(&*virtualization_service, instance_image, &VmParameters { debug_mode })?;
     let service = vm_instance.get_service()?;
 
     let result = service.verifySigningKey(&blob, &public_key).context("Verifying signing key")?;
@@ -105,6 +118,17 @@
     Ok(())
 }
 
+fn promote_to_current(instance_dir: &Path) -> Result<()> {
+    let current_dir: PathBuf = [COMPOS_DATA_ROOT, CURRENT_INSTANCE_DIR].iter().collect();
+
+    // This may fail if the directory doesn't exist - which is fine, we only care about the rename
+    // succeeding.
+    let _ = fs::remove_dir_all(&current_dir);
+
+    fs::rename(&instance_dir, &current_dir).context("Unable to promote instance to current")?;
+    Ok(())
+}
+
 fn read_small_file(file: PathBuf) -> Result<Vec<u8>> {
     let mut file = File::open(file)?;
     if file.metadata()?.len() > MAX_FILE_SIZE_BYTES {
@@ -114,15 +138,3 @@
     file.read_to_end(&mut data)?;
     Ok(data)
 }
-
-fn promote_to_current(instance_dir: &Path) -> Result<()> {
-    let current_dir: PathBuf = [COMPOS_DATA_ROOT, CURRENT_INSTANCE_DIR].iter().collect();
-
-    // This may fail if the directory doesn't exist - which is fine, we only care about the rename
-    // succeeding.
-    let _ = fs::remove_dir_all(&current_dir);
-
-    fs::rename(&instance_dir, &current_dir)
-        .context("Unable to promote pending instance to current")?;
-    Ok(())
-}