Merge "Keystore 2.0: Protect libselinux against concurrent access."
diff --git a/keystore2/src/database.rs b/keystore2/src/database.rs
index 4ab4258..073799b 100644
--- a/keystore2/src/database.rs
+++ b/keystore2/src/database.rs
@@ -46,7 +46,7 @@
 use crate::impl_metadata; // This is in db_utils.rs
 use crate::key_parameter::{KeyParameter, Tag};
 use crate::permission::KeyPermSet;
-use crate::utils::{get_current_time_in_seconds, watchdog as wd, AID_USER_OFFSET};
+use crate::utils::{get_current_time_in_milliseconds, watchdog as wd, AID_USER_OFFSET};
 use crate::{
     db_utils::{self, SqlField},
     gc::Gc,
@@ -737,29 +737,24 @@
 }
 
 /// Database representation of the monotonic time retrieved from the system call clock_gettime with
-/// CLOCK_MONOTONIC_RAW. Stores monotonic time as i64 in seconds.
+/// CLOCK_MONOTONIC_RAW. Stores monotonic time as i64 in milliseconds.
 #[derive(Debug, Copy, Clone, Default, Eq, PartialEq, Ord, PartialOrd)]
 pub struct MonotonicRawTime(i64);
 
 impl MonotonicRawTime {
     /// Constructs a new MonotonicRawTime
     pub fn now() -> Self {
-        Self(get_current_time_in_seconds())
+        Self(get_current_time_in_milliseconds())
     }
 
-    /// Constructs a new MonotonicRawTime from a given number of seconds.
-    pub fn from_secs(val: i64) -> Self {
-        Self(val)
+    /// Returns the value of MonotonicRawTime in milliseconds as i64
+    pub fn milliseconds(&self) -> i64 {
+        self.0
     }
 
     /// Returns the integer value of MonotonicRawTime as i64
     pub fn seconds(&self) -> i64 {
-        self.0
-    }
-
-    /// Returns the value of MonotonicRawTime in milli seconds as i64
-    pub fn milli_seconds(&self) -> i64 {
-        self.0 * 1000
+        self.0 / 1000
     }
 
     /// Like i64::checked_sub.
@@ -785,6 +780,7 @@
 #[derive(Clone)]
 pub struct AuthTokenEntry {
     auth_token: HardwareAuthToken,
+    // Time received in milliseconds
     time_received: MonotonicRawTime,
 }
 
@@ -833,6 +829,20 @@
     /// Name of the file that holds the cross-boot persistent database.
     pub const PERSISTENT_DB_FILENAME: &'static str = &"persistent.sqlite";
 
+    /// Set write-ahead logging mode on the persistent database found in `db_root`.
+    pub fn set_wal_mode(db_root: &Path) -> Result<()> {
+        let path = Self::make_persistent_path(&db_root)?;
+        let conn =
+            Connection::open(path).context("In KeystoreDB::set_wal_mode: Failed to open DB")?;
+        let mode: String = conn
+            .pragma_update_and_check(None, "journal_mode", &"WAL", |row| row.get(0))
+            .context("In KeystoreDB::set_wal_mode: Failed to set journal_mode")?;
+        match mode.as_str() {
+            "wal" => Ok(()),
+            _ => Err(anyhow!("Unable to set WAL mode, db is still in {} mode.", mode)),
+        }
+    }
+
     /// This will create a new database connection connecting the two
     /// files persistent.sqlite and perboot.sqlite in the given directory.
     /// It also attempts to initialize all of the tables.
@@ -841,18 +851,8 @@
     pub fn new(db_root: &Path, gc: Option<Arc<Gc>>) -> Result<Self> {
         let _wp = wd::watch_millis("KeystoreDB::new", 500);
 
-        // Build the path to the sqlite file.
-        let mut persistent_path = db_root.to_path_buf();
-        persistent_path.push(Self::PERSISTENT_DB_FILENAME);
-
-        // Now convert them to strings prefixed with "file:"
-        let mut persistent_path_str = "file:".to_owned();
-        persistent_path_str.push_str(&persistent_path.to_string_lossy());
-
-        let conn = Self::make_connection(&persistent_path_str)?;
-
-        // On busy fail Immediately. It is unlikely to succeed given a bug in sqlite.
-        conn.busy_handler(None).context("In KeystoreDB::new: Failed to set busy handler.")?;
+        let persistent_path = Self::make_persistent_path(&db_root)?;
+        let conn = Self::make_connection(&persistent_path)?;
 
         let mut db = Self { conn, gc, perboot: perboot::PERBOOT_DB.clone() };
         db.with_transaction(TransactionBehavior::Immediate, |tx| {
@@ -971,6 +971,18 @@
         Ok(())
     }
 
+    fn make_persistent_path(db_root: &Path) -> Result<String> {
+        // Build the path to the sqlite file.
+        let mut persistent_path = db_root.to_path_buf();
+        persistent_path.push(Self::PERSISTENT_DB_FILENAME);
+
+        // Now convert them to strings prefixed with "file:"
+        let mut persistent_path_str = "file:".to_owned();
+        persistent_path_str.push_str(&persistent_path.to_string_lossy());
+
+        Ok(persistent_path_str)
+    }
+
     fn make_connection(persistent_file: &str) -> Result<Connection> {
         let conn =
             Connection::open_in_memory().context("Failed to initialize SQLite connection.")?;
@@ -1413,18 +1425,6 @@
         .context("In get_or_create_key_with.")
     }
 
-    /// SQLite3 seems to hold a shared mutex while running the busy handler when
-    /// waiting for the database file to become available. This makes it
-    /// impossible to successfully recover from a locked database when the
-    /// transaction holding the device busy is in the same process on a
-    /// different connection. As a result the busy handler has to time out and
-    /// fail in order to make progress.
-    ///
-    /// Instead, we set the busy handler to None (return immediately). And catch
-    /// Busy and Locked errors (the latter occur on in memory databases with
-    /// shared cache, e.g., the per-boot database.) and restart the transaction
-    /// after a grace period of half a millisecond.
-    ///
     /// Creates a transaction with the given behavior and executes f with the new transaction.
     /// The transaction is committed only if f returns Ok and retried if DatabaseBusy
     /// or DatabaseLocked is encountered.
@@ -2395,11 +2395,12 @@
                 let mut stmt = tx
                     .prepare(
                         "SELECT keyentryid, access_vector FROM persistent.grant
-                            WHERE grantee = ? AND id = ?;",
+                            WHERE grantee = ? AND id = ? AND
+                            (SELECT state FROM persistent.keyentry WHERE id = keyentryid) = ?;",
                     )
                     .context("Domain::GRANT prepare statement failed")?;
                 let mut rows = stmt
-                    .query(params![caller_uid as i64, key.nspace])
+                    .query(params![caller_uid as i64, key.nspace, KeyLifeCycle::Live])
                     .context("Domain:Grant: query failed.")?;
                 let (key_id, access_vector): (i64, i32) =
                     db_utils::with_rows_extract_one(&mut rows, |row| {
@@ -3179,6 +3180,7 @@
     use android_hardware_security_secureclock::aidl::android::hardware::security::secureclock::{
         Timestamp::Timestamp,
     };
+    use rusqlite::DatabaseName::Attached;
     use rusqlite::NO_PARAMS;
     use rusqlite::TransactionBehavior;
     use std::cell::RefCell;
@@ -5201,7 +5203,7 @@
         let tx2 = db.conn.transaction_with_behavior(TransactionBehavior::Immediate)?;
         tx2.commit()?;
         let last_off_body_2 = db.get_last_off_body();
-        assert!(last_off_body_1.seconds() < last_off_body_2.seconds());
+        assert!(last_off_body_1 < last_off_body_2);
         Ok(())
     }
 
@@ -5487,4 +5489,26 @@
         assert_eq!(db.find_auth_token_entry(|_| true).unwrap().0.auth_token.mac, b"mac2".to_vec());
         Ok(())
     }
+
+    #[test]
+    fn test_set_wal_mode() -> Result<()> {
+        let temp_dir = TempDir::new("test_set_wal_mode")?;
+        let mut db = KeystoreDB::new(temp_dir.path(), None)?;
+        let mode: String =
+            db.conn.pragma_query_value(Some(Attached("persistent")), "journal_mode", |row| {
+                row.get(0)
+            })?;
+        assert_eq!(mode, "delete");
+        db.conn.close().expect("Close didn't work");
+
+        KeystoreDB::set_wal_mode(temp_dir.path())?;
+
+        db = KeystoreDB::new(temp_dir.path(), None)?;
+        let mode: String =
+            db.conn.pragma_query_value(Some(Attached("persistent")), "journal_mode", |row| {
+                row.get(0)
+            })?;
+        assert_eq!(mode, "wal");
+        Ok(())
+    }
 }
diff --git a/keystore2/src/enforcements.rs b/keystore2/src/enforcements.rs
index 8bf2090..29a3f0b 100644
--- a/keystore2/src/enforcements.rs
+++ b/keystore2/src/enforcements.rs
@@ -824,12 +824,12 @@
         } else {
             // Filter the matching auth tokens by age.
             if auth_token_max_age_millis != 0 {
-                let now_in_millis = MonotonicRawTime::now().milli_seconds();
+                let now_in_millis = MonotonicRawTime::now();
                 let result = Self::find_auth_token(|auth_token_entry: &AuthTokenEntry| {
                     let token_valid = now_in_millis
-                        .checked_sub(auth_token_entry.time_received().milli_seconds())
+                        .checked_sub(&auth_token_entry.time_received())
                         .map_or(false, |token_age_in_millis| {
-                            auth_token_max_age_millis > token_age_in_millis
+                            auth_token_max_age_millis > token_age_in_millis.milliseconds()
                         });
                     token_valid && auth_token_entry.satisfies(&sids, auth_type)
                 });
diff --git a/keystore2/src/globals.rs b/keystore2/src/globals.rs
index 8d39b22..89114a6 100644
--- a/keystore2/src/globals.rs
+++ b/keystore2/src/globals.rs
@@ -39,11 +39,12 @@
 use binder::FromIBinder;
 use keystore2_vintf::get_aidl_instances;
 use lazy_static::lazy_static;
-use std::sync::{Arc, Mutex};
+use std::sync::{Arc, Mutex, RwLock};
 use std::{cell::RefCell, sync::Once};
 use std::{collections::HashMap, path::Path, path::PathBuf};
 
 static DB_INIT: Once = Once::new();
+static DB_SET_WAL_MODE: Once = Once::new();
 
 /// Open a connection to the Keystore 2.0 database. This is called during the initialization of
 /// the thread local DB field. It should never be called directly. The first time this is called
@@ -54,11 +55,16 @@
 /// is run only once, as long as the ASYNC_TASK instance is the same. So only one additional
 /// database connection is created for the garbage collector worker.
 pub fn create_thread_local_db() -> KeystoreDB {
-    let mut db = KeystoreDB::new(
-        &DB_PATH.lock().expect("Could not get the database directory."),
-        Some(GC.clone()),
-    )
-    .expect("Failed to open database.");
+    let db_path = DB_PATH.read().expect("Could not get the database directory.");
+
+    DB_SET_WAL_MODE.call_once(|| {
+        log::info!("Setting Keystore 2.0 database to WAL mode first time since boot.");
+        KeystoreDB::set_wal_mode(&db_path)
+            .expect("In create_thread_local_db: Could not set WAL mode.");
+    });
+
+    let mut db = KeystoreDB::new(&db_path, Some(GC.clone())).expect("Failed to open database.");
+
     DB_INIT.call_once(|| {
         log::info!("Touching Keystore 2.0 database for this first time since boot.");
         db.insert_last_off_body(MonotonicRawTime::now());
@@ -139,7 +145,7 @@
 
 lazy_static! {
     /// The path where keystore stores all its keys.
-    pub static ref DB_PATH: Mutex<PathBuf> = Mutex::new(
+    pub static ref DB_PATH: RwLock<PathBuf> = RwLock::new(
         Path::new("/data/misc/keystore").to_path_buf());
     /// Runtime database of unwrapped super keys.
     pub static ref SUPER_KEY: Arc<SuperKeyManager> = Default::default();
@@ -157,7 +163,7 @@
     /// LegacyBlobLoader is initialized and exists globally.
     /// The same directory used by the database is used by the LegacyBlobLoader as well.
     pub static ref LEGACY_BLOB_LOADER: Arc<LegacyBlobLoader> = Arc::new(LegacyBlobLoader::new(
-        &DB_PATH.lock().expect("Could not get the database path for legacy blob loader.")));
+        &DB_PATH.read().expect("Could not get the database path for legacy blob loader.")));
     /// Legacy migrator. Atomically migrates legacy blobs to the database.
     pub static ref LEGACY_MIGRATOR: Arc<LegacyMigrator> =
         Arc::new(LegacyMigrator::new(Arc::new(Default::default())));
@@ -173,7 +179,7 @@
                 map_km_error(km_dev.deleteKey(&*blob))
                     .context("In invalidate key closure: Trying to invalidate key blob.")
             }),
-            KeystoreDB::new(&DB_PATH.lock().expect("Could not get the database directory."), None)
+            KeystoreDB::new(&DB_PATH.read().expect("Could not get the database directory."), None)
                 .expect("Failed to open database."),
             SUPER_KEY.clone(),
         )
diff --git a/keystore2/src/keystore2_main.rs b/keystore2/src/keystore2_main.rs
index 29330a6..53461da 100644
--- a/keystore2/src/keystore2_main.rs
+++ b/keystore2/src/keystore2_main.rs
@@ -56,7 +56,7 @@
     // For the ground truth check the service startup rule for init (typically in keystore2.rc).
     let id_rotation_state = if let Some(dir) = args.next() {
         let db_path = Path::new(&dir);
-        *keystore2::globals::DB_PATH.lock().expect("Could not lock DB_PATH.") =
+        *keystore2::globals::DB_PATH.write().expect("Could not lock DB_PATH.") =
             db_path.to_path_buf();
         IdRotationState::new(&db_path)
     } else {
@@ -121,7 +121,7 @@
     }
 
     let vpnprofilestore = VpnProfileStore::new_native_binder(
-        &keystore2::globals::DB_PATH.lock().expect("Could not get DB_PATH."),
+        &keystore2::globals::DB_PATH.read().expect("Could not get DB_PATH."),
     );
     binder::add_service(VPNPROFILESTORE_SERVICE_NAME, vpnprofilestore.as_binder()).unwrap_or_else(
         |e| {
diff --git a/keystore2/src/utils.rs b/keystore2/src/utils.rs
index 10865ae..a110c64 100644
--- a/keystore2/src/utils.rs
+++ b/keystore2/src/utils.rs
@@ -185,18 +185,15 @@
     parameters.into_iter().map(|p| p.into_authorization()).collect()
 }
 
-/// This returns the current time (in seconds) as an instance of a monotonic clock, by invoking the
-/// system call since Rust does not support getting monotonic time instance as an integer.
-pub fn get_current_time_in_seconds() -> i64 {
+/// This returns the current time (in milliseconds) as an instance of a monotonic clock,
+/// by invoking the system call since Rust does not support getting monotonic time instance
+/// as an integer.
+pub fn get_current_time_in_milliseconds() -> i64 {
     let mut current_time = libc::timespec { tv_sec: 0, tv_nsec: 0 };
     // Following unsafe block includes one system call to get monotonic time.
     // Therefore, it is not considered harmful.
     unsafe { libc::clock_gettime(libc::CLOCK_MONOTONIC_RAW, &mut current_time) };
-    // It is safe to unwrap here because try_from() returns std::convert::Infallible, which is
-    // defined to be an error that can never happen (i.e. the result is always ok).
-    // This suppresses the compiler's complaint about converting tv_sec to i64 in method
-    // get_current_time_in_seconds.
-    current_time.tv_sec as i64
+    current_time.tv_sec as i64 * 1000 + (current_time.tv_nsec as i64 / 1_000_000)
 }
 
 /// Converts a response code as returned by the Android Protected Confirmation HIDL compatibility
diff --git a/keystore2/vpnprofilestore/lib.rs b/keystore2/vpnprofilestore/lib.rs
index 8b3bc2b..df2731a 100644
--- a/keystore2/vpnprofilestore/lib.rs
+++ b/keystore2/vpnprofilestore/lib.rs
@@ -22,7 +22,7 @@
     BinderFeatures, ExceptionCode, Result as BinderResult, Status as BinderStatus, Strong,
     ThreadState,
 };
-use anyhow::{Context, Result};
+use anyhow::{anyhow, Context, Result};
 use keystore2::{async_task::AsyncTask, legacy_blob::LegacyBlobLoader, utils::watchdog as wd};
 use rusqlite::{
     params, Connection, OptionalExtension, Transaction, TransactionBehavior, NO_PARAMS,
@@ -30,25 +30,42 @@
 use std::{
     collections::HashSet,
     path::{Path, PathBuf},
+    sync::Once,
 };
 
+static DB_SET_WAL_MODE: Once = Once::new();
+
 struct DB {
     conn: Connection,
 }
 
 impl DB {
     fn new(db_file: &Path) -> Result<Self> {
+        DB_SET_WAL_MODE.call_once(|| {
+            log::info!("Setting VpnProfileStore database to WAL mode first time since boot.");
+            Self::set_wal_mode(&db_file).expect("In vpnprofilestore: Could not set WAL mode.");
+        });
+
         let mut db = Self {
             conn: Connection::open(db_file).context("Failed to initialize SQLite connection.")?,
         };
 
-        // On busy fail Immediately. It is unlikely to succeed given a bug in sqlite.
-        db.conn.busy_handler(None).context("Failed to set busy handler.")?;
-
         db.init_tables().context("Trying to initialize vpnstore db.")?;
         Ok(db)
     }
 
+    fn set_wal_mode(db_file: &Path) -> Result<()> {
+        let conn = Connection::open(db_file)
+            .context("In VpnProfileStore set_wal_mode: Failed to open DB.")?;
+        let mode: String = conn
+            .pragma_update_and_check(None, "journal_mode", &"WAL", |row| row.get(0))
+            .context("In VpnProfileStore set_wal_mode: Failed to set journal_mode")?;
+        match mode.as_str() {
+            "wal" => Ok(()),
+            _ => Err(anyhow!("Unable to set WAL mode, db is still in {} mode.", mode)),
+        }
+    }
+
     fn with_transaction<T, F>(&mut self, behavior: TransactionBehavior, f: F) -> Result<T>
     where
         F: Fn(&Transaction) -> Result<T>,
@@ -470,6 +487,9 @@
         const PROFILE_COUNT: u32 = 5000u32;
         const PROFILE_DB_COUNT: u32 = 5000u32;
 
+        let mode: String = db.conn.pragma_query_value(None, "journal_mode", |row| row.get(0))?;
+        assert_eq!(mode, "wal");
+
         let mut actual_profile_count = PROFILE_COUNT;
         // First insert PROFILE_COUNT profiles.
         for count in 0..PROFILE_COUNT {