Keystore 2.0: Revise database.
* Make grants persistent.
* Moved some utility functions to db_utils.rs.
* KeystoreDB::new() now takes a path argument that indicates where the
the database files are to be expected.
* A new test module test::utils.rs introduces TempDir which creates a
new temporary directory that is cleaned up with all of its content
when the TempDir opject is dropped.
Test: keystore2_test
Change-Id: I056c404fd9d592ddc8b531394b1c6cc17a0fd736
diff --git a/keystore2/src/database.rs b/keystore2/src/database.rs
index 45d561a..a2cd6cd 100644
--- a/keystore2/src/database.rs
+++ b/keystore2/src/database.rs
@@ -41,6 +41,7 @@
//! from the database module these functions take permission check
//! callbacks.
+use crate::db_utils;
use crate::error::{Error as KsError, ResponseCode};
use crate::key_parameter::{KeyParameter, SqlField, Tag};
use crate::permission::KeyPermSet;
@@ -56,11 +57,12 @@
use rand::prelude::random;
use rusqlite::{
params, types::FromSql, types::FromSqlResult, types::ToSqlOutput, types::ValueRef, Connection,
- OptionalExtension, Row, Rows, ToSql, Transaction, TransactionBehavior, NO_PARAMS,
+ OptionalExtension, ToSql, Transaction, TransactionBehavior, NO_PARAMS,
};
use std::{
collections::HashSet,
- sync::{Condvar, Mutex, Once},
+ path::Path,
+ sync::{Condvar, Mutex},
};
#[cfg(test)]
use tests::random;
@@ -236,8 +238,6 @@
}
}
-static INIT_TABLES: Once = Once::new();
-
/// KeystoreDB wraps a connection to an SQLite database and tracks its
/// ownership. It also implements all of Keystore 2.0's database functionality.
pub struct KeystoreDB {
@@ -246,15 +246,26 @@
impl KeystoreDB {
/// This will create a new database connection connecting the two
- /// files persistent.sqlite and perboot.sqlite in the current working
- /// directory, which is usually `/data/misc/keystore/`.
- /// It also attempts to initialize all of the tables on the first instantiation
- /// per service startup. KeystoreDB cannot be used by multiple threads.
+ /// files persistent.sqlite and perboot.sqlite in the given directory.
+ /// It also attempts to initialize all of the tables.
+ /// KeystoreDB cannot be used by multiple threads.
/// Each thread should open their own connection using `thread_local!`.
- pub fn new() -> Result<Self> {
- let conn = Self::make_connection("file:persistent.sqlite", "file:perboot.sqlite")?;
+ pub fn new(db_root: &Path) -> Result<Self> {
+ // Build the path to the sqlite files.
+ let mut persistent_path = db_root.to_path_buf();
+ persistent_path.push("persistent.sqlite");
+ let mut perboot_path = db_root.to_path_buf();
+ perboot_path.push("perboot.sqlite");
- INIT_TABLES.call_once(|| Self::init_tables(&conn).expect("Failed to initialize tables."));
+ // 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 mut perboot_path_str = "file:".to_owned();
+ perboot_path_str.push_str(&perboot_path.to_string_lossy());
+
+ let conn = Self::make_connection(&persistent_path_str, &perboot_path_str)?;
+
+ Self::init_tables(&conn)?;
Ok(Self { conn })
}
@@ -298,14 +309,8 @@
)
.context("Failed to initialize \"keyparameter\" table.")?;
- // TODO only drop the perboot table if we start up for the first time per boot.
- // Right now this is done once per startup which will lose some information
- // upon a crash.
- // Note: This is no regression with respect to the legacy Keystore.
- conn.execute("DROP TABLE IF EXISTS perboot.grant;", NO_PARAMS)
- .context("Failed to drop perboot.grant table")?;
conn.execute(
- "CREATE TABLE perboot.grant (
+ "CREATE TABLE IF NOT EXISTS persistent.grant (
id INTEGER UNIQUE,
grantee INTEGER,
keyentryid INTEGER,
@@ -478,7 +483,7 @@
let mut rows = stmt
.query(params![key.domain.0 as u32, key.nspace, alias])
.context("In load_key_entry_id: Failed to read from keyentry table.")?;
- Self::with_rows_extract_one(&mut rows, |row| {
+ db_utils::with_rows_extract_one(&mut rows, |row| {
row.map_or_else(|| Err(KsError::Rc(ResponseCode::KEY_NOT_FOUND)), Ok)?
.get(0)
.context("Failed to unpack id.")
@@ -527,7 +532,7 @@
Domain::GRANT => {
let mut stmt = tx
.prepare(
- "SELECT keyentryid, access_vector FROM perboot.grant
+ "SELECT keyentryid, access_vector FROM persistent.grant
WHERE grantee = ? AND id = ?;",
)
.context("Domain::GRANT prepare statement failed")?;
@@ -535,7 +540,7 @@
.query(params![caller_uid as i64, key.nspace])
.context("Domain:Grant: query failed.")?;
let (key_id, access_vector): (i64, i32) =
- Self::with_rows_extract_one(&mut rows, |row| {
+ db_utils::with_rows_extract_one(&mut rows, |row| {
let r =
row.map_or_else(|| Err(KsError::Rc(ResponseCode::KEY_NOT_FOUND)), Ok)?;
Ok((
@@ -560,7 +565,7 @@
let mut rows =
stmt.query(params![key.nspace]).context("Domain::KEY_ID: query failed.")?;
let (domain, namespace): (Domain, i64) =
- Self::with_rows_extract_one(&mut rows, |row| {
+ db_utils::with_rows_extract_one(&mut rows, |row| {
let r =
row.map_or_else(|| Err(KsError::Rc(ResponseCode::KEY_NOT_FOUND)), Ok)?;
Ok((
@@ -599,7 +604,7 @@
let mut km_blob: Option<Vec<u8>> = None;
let mut cert_blob: Option<Vec<u8>> = None;
let mut cert_chain_blob: Option<Vec<u8>> = None;
- Self::with_rows_extract_all(&mut rows, |row| {
+ db_utils::with_rows_extract_all(&mut rows, |row| {
let sub_type: SubComponentType =
row.get(2).context("Failed to extract subcomponent_type.")?;
match (sub_type, load_bits.load_public()) {
@@ -640,7 +645,7 @@
let mut rows =
stmt.query(params![key_id]).context("In load_key_parameters: query failed.")?;
- Self::with_rows_extract_all(&mut rows, |row| {
+ db_utils::with_rows_extract_all(&mut rows, |row| {
let tag = Tag(row.get(0).context("Failed to read tag.")?);
let sec_level = SecurityLevel(row.get(2).context("Failed to read sec_level.")?);
parameters.push(
@@ -795,7 +800,7 @@
let grant_id = if let Some(grant_id) = tx
.query_row(
- "SELECT id FROM perboot.grant
+ "SELECT id FROM persistent.grant
WHERE keyentryid = ? AND grantee = ?;",
params![key_id, grantee_uid],
|row| row.get(0),
@@ -804,7 +809,7 @@
.context("In grant: Failed get optional existing grant id.")?
{
tx.execute(
- "UPDATE perboot.grant
+ "UPDATE persistent.grant
SET access_vector = ?
WHERE id = ?;",
params![i32::from(access_vector), grant_id],
@@ -814,7 +819,7 @@
} else {
Self::insert_with_retry(|id| {
tx.execute(
- "INSERT INTO perboot.grant (id, grantee, keyentryid, access_vector)
+ "INSERT INTO persistent.grant (id, grantee, keyentryid, access_vector)
VALUES (?, ?, ?, ?);",
params![id, grantee_uid, key_id, i32::from(access_vector)],
)
@@ -850,7 +855,7 @@
check_permission(&access_key_descriptor).context("In grant: check_permission failed.")?;
tx.execute(
- "DELETE FROM perboot.grant
+ "DELETE FROM persistent.grant
WHERE keyentryid = ? AND grantee = ?;",
params![key_id, grantee_uid],
)
@@ -883,41 +888,6 @@
}
}
}
-
- // Takes Rows as returned by a query call on prepared statement.
- // Extracts exactly one row with the `row_extractor` and fails if more
- // rows are available.
- // If no row was found, `None` is passed to the `row_extractor`.
- // This allows the row extractor to decide on an error condition or
- // a different default behavior.
- fn with_rows_extract_one<'a, T, F>(rows: &mut Rows<'a>, row_extractor: F) -> Result<T>
- where
- F: FnOnce(Option<&Row<'a>>) -> Result<T>,
- {
- let result =
- row_extractor(rows.next().context("with_rows_extract_one: Failed to unpack row.")?);
-
- rows.next()
- .context("In with_rows_extract_one: Failed to unpack unexpected row.")?
- .map_or_else(|| Ok(()), |_| Err(KsError::sys()))
- .context("In with_rows_extract_one: Unexpected row.")?;
-
- result
- }
-
- fn with_rows_extract_all<'a, F>(rows: &mut Rows<'a>, mut row_extractor: F) -> Result<()>
- where
- F: FnMut(&Row<'a>) -> Result<()>,
- {
- loop {
- match rows.next().context("In with_rows_extract_all: Failed to unpack row")? {
- Some(row) => {
- row_extractor(&row).context("In with_rows_extract_all.")?;
- }
- None => break Ok(()),
- }
- }
- }
}
#[cfg(test)]
@@ -930,15 +900,13 @@
};
use crate::key_perm_set;
use crate::permission::{KeyPerm, KeyPermSet};
+ use crate::test::utils::TempDir;
use rusqlite::NO_PARAMS;
use std::cell::RefCell;
use std::sync::atomic::{AtomicU8, Ordering};
use std::sync::Arc;
use std::thread;
- static PERSISTENT_TEST_SQL: &str = "/data/local/tmp/persistent.sqlite";
- static PERBOOT_TEST_SQL: &str = "/data/local/tmp/perboot.sqlite";
-
fn new_test_db() -> Result<KeystoreDB> {
let conn = KeystoreDB::make_connection("file::memory:", "file::memory:")?;
@@ -946,13 +914,6 @@
Ok(KeystoreDB { conn })
}
- fn new_test_db_with_persistent_file() -> Result<KeystoreDB> {
- let conn = KeystoreDB::make_connection(PERSISTENT_TEST_SQL, PERBOOT_TEST_SQL)?;
-
- KeystoreDB::init_tables(&conn).context("Failed to initialize tables.")?;
- Ok(KeystoreDB { conn })
- }
-
// Ensure that we're using the "injected" random function, not the real one.
#[test]
fn test_mocked_random() {
@@ -976,44 +937,30 @@
.prepare("SELECT name from persistent.sqlite_master WHERE type='table' ORDER BY name;")?
.query_map(params![], |row| row.get(0))?
.collect::<rusqlite::Result<Vec<String>>>()?;
- assert_eq!(tables.len(), 3);
+ assert_eq!(tables.len(), 4);
assert_eq!(tables[0], "blobentry");
- assert_eq!(tables[1], "keyentry");
- assert_eq!(tables[2], "keyparameter");
+ assert_eq!(tables[1], "grant");
+ assert_eq!(tables[2], "keyentry");
+ assert_eq!(tables[3], "keyparameter");
let tables = db
.conn
.prepare("SELECT name from perboot.sqlite_master WHERE type='table' ORDER BY name;")?
.query_map(params![], |row| row.get(0))?
.collect::<rusqlite::Result<Vec<String>>>()?;
- assert_eq!(tables.len(), 1);
- assert_eq!(tables[0], "grant");
- Ok(())
- }
-
- #[test]
- fn test_no_persistence_for_tests() -> Result<()> {
- let db = new_test_db()?;
-
- db.create_key_entry(Domain::APP, 100)?;
- let entries = get_keyentry(&db)?;
- assert_eq!(entries.len(), 1);
- let db = new_test_db()?;
-
- let entries = get_keyentry(&db)?;
- assert_eq!(entries.len(), 0);
+ assert_eq!(tables.len(), 0);
Ok(())
}
#[test]
fn test_persistence_for_files() -> Result<()> {
- let _file_guard_persistent = TempFile { filename: PERSISTENT_TEST_SQL };
- let _file_guard_perboot = TempFile { filename: PERBOOT_TEST_SQL };
- let db = new_test_db_with_persistent_file()?;
+ let temp_dir = TempDir::new("persistent_db_test")?;
+ let db = KeystoreDB::new(temp_dir.path())?;
db.create_key_entry(Domain::APP, 100)?;
let entries = get_keyentry(&db)?;
assert_eq!(entries.len(), 1);
- let db = new_test_db_with_persistent_file()?;
+
+ let db = KeystoreDB::new(temp_dir.path())?;
let entries_new = get_keyentry(&db)?;
assert_eq!(entries, entries_new);
@@ -1230,7 +1177,7 @@
// Limiting scope of stmt, because it borrows db.
let mut stmt = db
.conn
- .prepare("SELECT id, grantee, keyentryid, access_vector FROM perboot.grant;")?;
+ .prepare("SELECT id, grantee, keyentryid, access_vector FROM persistent.grant;")?;
let mut rows =
stmt.query_map::<(i64, u32, i64, KeyPermSet), _, _>(NO_PARAMS, |row| {
Ok((
@@ -1438,22 +1385,12 @@
static KEY_LOCK_TEST_ALIAS: &str = "my super duper locked key";
- static KEY_LOCK_TEST_SQL: &str = "/data/local/tmp/persistent_key_lock.sqlite";
- static KEY_LOCK_PERBOOT_TEST_SQL: &str = "/data/local/tmp/perboot_key_lock.sqlite";
-
- fn new_test_db_with_persistent_file_key_lock() -> Result<KeystoreDB> {
- let conn = KeystoreDB::make_connection(KEY_LOCK_TEST_SQL, KEY_LOCK_PERBOOT_TEST_SQL)?;
-
- KeystoreDB::init_tables(&conn).context("Failed to initialize tables.")?;
- Ok(KeystoreDB { conn })
- }
-
#[test]
fn test_insert_and_load_full_keyentry_domain_app_concurrently() -> Result<()> {
let handle = {
- let _file_guard_persistent = Arc::new(TempFile { filename: KEY_LOCK_TEST_SQL });
- let _file_guard_perboot = Arc::new(TempFile { filename: KEY_LOCK_PERBOOT_TEST_SQL });
- let mut db = new_test_db_with_persistent_file_key_lock()?;
+ let temp_dir = Arc::new(TempDir::new("id_lock_test")?);
+ let temp_dir_clone = temp_dir.clone();
+ let mut db = KeystoreDB::new(temp_dir.path())?;
let key_id = make_test_key_entry(&mut db, Domain::APP, 33, KEY_LOCK_TEST_ALIAS)
.context("test_insert_and_load_full_keyentry_domain_app")?
.0;
@@ -1490,9 +1427,8 @@
// of `state` from 1 to 2, despite having a whole second to overtake
// the primary thread.
let handle = thread::spawn(move || {
- let _file_a = _file_guard_persistent;
- let _file_b = _file_guard_perboot;
- let mut db = new_test_db_with_persistent_file_key_lock().unwrap();
+ let temp_dir = temp_dir_clone;
+ let mut db = KeystoreDB::new(temp_dir.path()).unwrap();
assert!(db
.load_key_entry(
KeyDescriptor {
@@ -1846,8 +1782,9 @@
}
fn debug_dump_grant_table(db: &mut KeystoreDB) -> Result<()> {
- let mut stmt =
- db.conn.prepare("SELECT id, grantee, keyentryid, access_vector FROM perboot.grant;")?;
+ let mut stmt = db
+ .conn
+ .prepare("SELECT id, grantee, keyentryid, access_vector FROM persistent.grant;")?;
let rows = stmt.query_map::<(i64, i64, i64, i64), _, _>(NO_PARAMS, |row| {
Ok((row.get(0)?, row.get(1)?, row.get(2)?, row.get(3)?))
})?;
@@ -1860,18 +1797,6 @@
Ok(())
}
- // A class that deletes a file when it is dropped.
- // TODO: If we ever add a crate that does this, we can use it instead.
- struct TempFile {
- filename: &'static str,
- }
-
- impl Drop for TempFile {
- fn drop(&mut self) {
- std::fs::remove_file(self.filename).expect("Cannot delete temporary file");
- }
- }
-
// Use a custom random number generator that repeats each number once.
// This allows us to test repeated elements.