Merge "Fix ill-formed certificate request"
diff --git a/keystore2/Android.bp b/keystore2/Android.bp
index 0069f95..64636ac 100644
--- a/keystore2/Android.bp
+++ b/keystore2/Android.bp
@@ -46,7 +46,6 @@
         "libkeystore2_crypto_rust",
         "libkeystore2_km_compat",
         "libkeystore2_selinux",
-        "libkeystore2_system_property-rust",
         "libkeystore2_vintf_rust",
         "liblazy_static",
         "liblibc",
@@ -55,6 +54,7 @@
         "liblog_rust",
         "librand",
         "librusqlite",
+        "libsystem_properties-rust",
         "libthiserror",
     ],
     shared_libs: [
diff --git a/keystore2/src/database.rs b/keystore2/src/database.rs
index 33c2c32..0081bb7 100644
--- a/keystore2/src/database.rs
+++ b/keystore2/src/database.rs
@@ -4966,10 +4966,7 @@
                 Ok(KeyEntryRow {
                     id: row.get(0)?,
                     key_type: row.get(1)?,
-                    domain: match row.get(2)? {
-                        Some(i) => Some(Domain(i)),
-                        None => None,
-                    },
+                    domain: row.get::<_, Option<_>>(2)?.map(Domain),
                     namespace: row.get(3)?,
                     alias: row.get(4)?,
                     state: row.get(5)?,
diff --git a/keystore2/src/metrics_store.rs b/keystore2/src/metrics_store.rs
index 32067b9..4634da1 100644
--- a/keystore2/src/metrics_store.rs
+++ b/keystore2/src/metrics_store.rs
@@ -45,11 +45,11 @@
     Storage::Storage as MetricsStorage,
 };
 use anyhow::{Context, Result};
-use keystore2_system_property::{write, PropertyWatcher, PropertyWatcherError};
 use lazy_static::lazy_static;
 use std::collections::HashMap;
 use std::sync::Mutex;
 use std::time::{Duration, SystemTime, UNIX_EPOCH};
+use system_properties::PropertyWatcherError;
 
 // Note: Crash events are recorded at keystore restarts, based on the assumption that keystore only
 // gets restarted after a crash, during a boot cycle.
@@ -626,7 +626,8 @@
         }
     };
 
-    if let Err(e) = write(KEYSTORE_CRASH_COUNT_PROPERTY, &new_count.to_string()) {
+    if let Err(e) = system_properties::write(KEYSTORE_CRASH_COUNT_PROPERTY, &new_count.to_string())
+    {
         log::error!(
             concat!(
                 "In update_keystore_crash_sysprop:: ",
@@ -639,12 +640,10 @@
 
 /// Read the system property: keystore.crash_count.
 pub fn read_keystore_crash_count() -> Result<i32> {
-    let mut prop_reader = PropertyWatcher::new("keystore.crash_count").context(concat!(
-        "In read_keystore_crash_count: Failed to create reader a PropertyWatcher."
-    ))?;
-    prop_reader
-        .read(|_n, v| v.parse::<i32>().map_err(std::convert::Into::into))
-        .context("In read_keystore_crash_count: Failed to read the existing system property.")
+    system_properties::read("keystore.crash_count")
+        .context("In read_keystore_crash_count: Failed read property.")?
+        .parse::<i32>()
+        .map_err(std::convert::Into::into)
 }
 
 /// Enum defining the bit position for each padding mode. Since padding mode can be repeatable, it
diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs
index b66c778..e0eabe1 100644
--- a/keystore2/src/security_level.rs
+++ b/keystore2/src/security_level.rs
@@ -907,7 +907,7 @@
                         "In convert_storage_key_to_ephemeral: calling convertStorageKeyToEphemeral (2)",
                         500,
                     );
-                    map_km_error(km_dev.convertStorageKeyToEphemeral(key_blob))
+                    map_km_error(km_dev.convertStorageKeyToEphemeral(&upgraded_blob))
                 }
                     .context(concat!(
                         "In convert_storage_key_to_ephemeral: ",
diff --git a/keystore2/src/super_key.rs b/keystore2/src/super_key.rs
index e02d9bc..17718da 100644
--- a/keystore2/src/super_key.rs
+++ b/keystore2/src/super_key.rs
@@ -46,13 +46,13 @@
     aes_gcm_decrypt, aes_gcm_encrypt, generate_aes256_key, generate_salt, Password, ZVec,
     AES_256_KEY_LENGTH,
 };
-use keystore2_system_property::PropertyWatcher;
 use std::{
     collections::HashMap,
     sync::Arc,
     sync::{Mutex, Weak},
 };
 use std::{convert::TryFrom, ops::Deref};
+use system_properties::PropertyWatcher;
 
 const MAX_MAX_BOOT_LEVEL: usize = 1_000_000_000;
 /// Allow up to 15 seconds between the user unlocking using a biometric, and the auth
@@ -126,10 +126,8 @@
     fn from_metadata(metadata: &BlobMetaData) -> Option<Self> {
         if let Some(EncryptedBy::KeyId(key_id)) = metadata.encrypted_by() {
             Some(SuperKeyIdentifier::DatabaseId(*key_id))
-        } else if let Some(boot_level) = metadata.max_boot_level() {
-            Some(SuperKeyIdentifier::BootLevel(*boot_level))
         } else {
-            None
+            metadata.max_boot_level().map(|boot_level| SuperKeyIdentifier::BootLevel(*boot_level))
         }
     }
 
diff --git a/keystore2/system_property/Android.bp b/keystore2/system_property/Android.bp
deleted file mode 100644
index 773804d..0000000
--- a/keystore2/system_property/Android.bp
+++ /dev/null
@@ -1,53 +0,0 @@
-// Copyright 2021, The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package {
-    // See: http://go/android-license-faq
-    // A large-scale-change added 'default_applicable_licenses' to import
-    // all of the 'license_kinds' from "system_security_license"
-    // to get the below license kinds:
-    //   SPDX-license-identifier-Apache-2.0
-    default_applicable_licenses: ["system_security_license"],
-}
-
-rust_bindgen {
-    name: "libkeystore2_system_property_bindgen",
-    wrapper_src: "system_property_bindgen.hpp",
-    crate_name: "keystore2_system_property_bindgen",
-    source_stem: "bindings",
-
-    bindgen_flags: [
-        "--size_t-is-usize",
-        "--allowlist-function=__system_property_find",
-        "--allowlist-function=__system_property_read_callback",
-        "--allowlist-function=__system_property_set",
-        "--allowlist-function=__system_property_wait",
-    ],
-}
-
-rust_library {
-    name: "libkeystore2_system_property-rust",
-    crate_name: "keystore2_system_property",
-    srcs: [
-        "lib.rs",
-    ],
-    rustlibs: [
-        "libanyhow",
-        "libkeystore2_system_property_bindgen",
-        "libthiserror",
-    ],
-    shared_libs: [
-        "libbase",
-    ],
-}
diff --git a/keystore2/system_property/lib.rs b/keystore2/system_property/lib.rs
deleted file mode 100644
index b993c87..0000000
--- a/keystore2/system_property/lib.rs
+++ /dev/null
@@ -1,217 +0,0 @@
-// Copyright 2021, The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-//! This crate provides the PropertyWatcher type, which watches for changes
-//! in Android system properties.
-
-use anyhow::{anyhow, Context, Result as AnyhowResult};
-use keystore2_system_property_bindgen::prop_info as PropInfo;
-use std::os::raw::c_char;
-use std::ptr::null;
-use std::{
-    ffi::{c_void, CStr, CString},
-    str::Utf8Error,
-};
-use thiserror::Error;
-
-/// Errors this crate can generate
-#[derive(Error, Debug)]
-pub enum PropertyWatcherError {
-    /// We can't watch for a property whose name contains a NUL character.
-    #[error("Cannot convert name to C string")]
-    BadNameError(#[from] std::ffi::NulError),
-    /// We can only watch for properties that exist when the watcher is created.
-    #[error("System property is absent")]
-    SystemPropertyAbsent,
-    /// __system_property_wait timed out despite being given no timeout.
-    #[error("Wait failed")]
-    WaitFailed,
-    /// read callback was not called
-    #[error("__system_property_read_callback did not call callback")]
-    ReadCallbackNotCalled,
-    /// read callback gave us a NULL pointer
-    #[error("__system_property_read_callback gave us a NULL pointer instead of a string")]
-    MissingCString,
-    /// read callback gave us a bad C string
-    #[error("__system_property_read_callback gave us a non-UTF8 C string")]
-    BadCString(#[from] Utf8Error),
-    /// read callback returned an error
-    #[error("Callback failed")]
-    CallbackError(#[from] anyhow::Error),
-    /// Failure in setting the system property
-    #[error("__system_property_set failed.")]
-    SetPropertyFailed,
-}
-
-/// Result type specific for this crate.
-pub type Result<T> = std::result::Result<T, PropertyWatcherError>;
-
-/// PropertyWatcher takes the name of an Android system property such
-/// as `keystore.boot_level`; it can report the current value of this
-/// property, or wait for it to change.
-pub struct PropertyWatcher {
-    prop_name: CString,
-    prop_info: *const PropInfo,
-    serial: keystore2_system_property_bindgen::__uint32_t,
-}
-
-impl PropertyWatcher {
-    /// Create a PropertyWatcher for the named system property.
-    pub fn new(name: &str) -> Result<Self> {
-        Ok(Self { prop_name: CString::new(name)?, prop_info: null(), serial: 0 })
-    }
-
-    // Lazy-initializing accessor for self.prop_info.
-    fn get_prop_info(&mut self) -> Option<*const PropInfo> {
-        if self.prop_info.is_null() {
-            // Unsafe required for FFI call. Input and output are both const.
-            // The returned pointer is valid for the lifetime of the program.
-            self.prop_info = unsafe {
-                keystore2_system_property_bindgen::__system_property_find(self.prop_name.as_ptr())
-            };
-        }
-        if self.prop_info.is_null() {
-            None
-        } else {
-            Some(self.prop_info)
-        }
-    }
-
-    fn read_raw(prop_info: *const PropInfo, mut f: impl FnOnce(Option<&CStr>, Option<&CStr>)) {
-        // Unsafe function converts values passed to us by
-        // __system_property_read_callback to Rust form
-        // and pass them to inner callback.
-        unsafe extern "C" fn callback(
-            res_p: *mut c_void,
-            name: *const c_char,
-            value: *const c_char,
-            _: keystore2_system_property_bindgen::__uint32_t,
-        ) {
-            let name = if name.is_null() { None } else { Some(CStr::from_ptr(name)) };
-            let value = if value.is_null() { None } else { Some(CStr::from_ptr(value)) };
-            let f = &mut *res_p.cast::<&mut dyn FnMut(Option<&CStr>, Option<&CStr>)>();
-            f(name, value);
-        }
-
-        let mut f: &mut dyn FnOnce(Option<&CStr>, Option<&CStr>) = &mut f;
-
-        // Unsafe block for FFI call. We convert the FnOnce
-        // to a void pointer, and unwrap it in our callback.
-        unsafe {
-            keystore2_system_property_bindgen::__system_property_read_callback(
-                prop_info,
-                Some(callback),
-                &mut f as *mut _ as *mut c_void,
-            )
-        }
-    }
-
-    /// Call the passed function, passing it the name and current value
-    /// of this system property. See documentation for
-    /// `__system_property_read_callback` for details.
-    /// Returns an error if the property is empty or doesn't exist.
-    pub fn read<T, F>(&mut self, f: F) -> Result<T>
-    where
-        F: FnOnce(&str, &str) -> anyhow::Result<T>,
-    {
-        let prop_info = self.get_prop_info().ok_or(PropertyWatcherError::SystemPropertyAbsent)?;
-        let mut result = Err(PropertyWatcherError::ReadCallbackNotCalled);
-        Self::read_raw(prop_info, |name, value| {
-            // use a wrapping closure as an erzatz try block.
-            result = (|| {
-                let name = name.ok_or(PropertyWatcherError::MissingCString)?.to_str()?;
-                let value = value.ok_or(PropertyWatcherError::MissingCString)?.to_str()?;
-                f(name, value).map_err(PropertyWatcherError::CallbackError)
-            })()
-        });
-        result
-    }
-
-    // Waits for the property that self is watching to be created. Returns immediately if the
-    // property already exists.
-    fn wait_for_property_creation(&mut self) -> Result<()> {
-        let mut global_serial = 0;
-        loop {
-            match self.get_prop_info() {
-                Some(_) => return Ok(()),
-                None => {
-                    // Unsafe call for FFI. The function modifies only global_serial, and has
-                    // no side-effects.
-                    if !unsafe {
-                        // Wait for a global serial number change, then try again. On success,
-                        // the function will update global_serial with the last version seen.
-                        keystore2_system_property_bindgen::__system_property_wait(
-                            null(),
-                            global_serial,
-                            &mut global_serial,
-                            null(),
-                        )
-                    } {
-                        return Err(PropertyWatcherError::WaitFailed);
-                    }
-                }
-            }
-        }
-    }
-
-    /// Wait for the system property to change. This
-    /// records the serial number of the last change, so
-    /// race conditions are avoided.
-    pub fn wait(&mut self) -> Result<()> {
-        // If the property is null, then wait for it to be created. Subsequent waits will
-        // skip this step and wait for our specific property to change.
-        if self.prop_info.is_null() {
-            return self.wait_for_property_creation();
-        }
-
-        let mut new_serial = self.serial;
-        // Unsafe block to call __system_property_wait.
-        // All arguments are private to PropertyWatcher so we
-        // can be confident they are valid.
-        if !unsafe {
-            keystore2_system_property_bindgen::__system_property_wait(
-                self.prop_info,
-                self.serial,
-                &mut new_serial,
-                null(),
-            )
-        } {
-            return Err(PropertyWatcherError::WaitFailed);
-        }
-        self.serial = new_serial;
-        Ok(())
-    }
-}
-
-/// Writes a system property.
-pub fn write(name: &str, value: &str) -> AnyhowResult<()> {
-    if
-    // Unsafe required for FFI call. Input and output are both const and valid strings.
-    unsafe {
-        // If successful, __system_property_set returns 0, otherwise, returns -1.
-        keystore2_system_property_bindgen::__system_property_set(
-            CString::new(name)
-                .context("In keystore2::system_property::write: Construction CString from name.")?
-                .as_ptr(),
-            CString::new(value)
-                .context("In keystore2::system_property::write: Constructing CString from value.")?
-                .as_ptr(),
-        )
-    } == 0
-    {
-        Ok(())
-    } else {
-        Err(anyhow!(PropertyWatcherError::SetPropertyFailed))
-    }
-}
diff --git a/keystore2/system_property/system_property_bindgen.hpp b/keystore2/system_property/system_property_bindgen.hpp
deleted file mode 100644
index e3c1ade..0000000
--- a/keystore2/system_property/system_property_bindgen.hpp
+++ /dev/null
@@ -1,18 +0,0 @@
-/*
- * Copyright (C) 2021 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-#pragma once
-
-#include "sys/system_properties.h"
diff --git a/ondevice-signing/VerityUtils.cpp b/ondevice-signing/VerityUtils.cpp
index e58de68..2beb7eb 100644
--- a/ondevice-signing/VerityUtils.cpp
+++ b/ondevice-signing/VerityUtils.cpp
@@ -83,7 +83,12 @@
     if (ret < 0) {
         return ErrnoError() << "Failed to compute fs-verity digest";
     }
-    std::vector<uint8_t> digestVector(&digest->digest[0], &digest->digest[32]);
+    int expected_digest_size = libfsverity_get_digest_size(FS_VERITY_HASH_ALG_SHA256);
+    if (digest->digest_size != expected_digest_size) {
+        return Error() << "Digest does not have expected size: " << expected_digest_size
+                       << " actual: " << digest->digest_size;
+    }
+    std::vector<uint8_t> digestVector(&digest->digest[0], &digest->digest[expected_digest_size]);
     free(digest);
     return digestVector;
 }
@@ -111,7 +116,7 @@
 
 template <typename T>
 static trailing_unique_ptr<T> makeUniqueWithTrailingData(size_t trailing_data_size) {
-    uint8_t* memory = new uint8_t[sizeof(T*) + trailing_data_size];
+    uint8_t* memory = new uint8_t[sizeof(T) + trailing_data_size];
     T* ptr = new (memory) T;
     return trailing_unique_ptr<T>{ptr};
 }