diff --git a/askar-storage/src/any.rs b/askar-storage/src/any.rs index 1bee07cf..c1e4e5dc 100644 --- a/askar-storage/src/any.rs +++ b/askar-storage/src/any.rs @@ -263,7 +263,7 @@ impl<'a> ManageBackend<'a> for &'a str { let opts = self.into_options()?; debug!("Open store with options: {:?}", &opts); - match opts.schema.as_ref() { + match opts.scheme.as_ref() { #[cfg(feature = "postgres")] "postgres" => { let opts = postgres::PostgresStoreOptions::new(opts)?; @@ -281,7 +281,7 @@ impl<'a> ManageBackend<'a> for &'a str { _ => Err(err_msg!( Unsupported, "Unsupported backend: {}", - &opts.schema + &opts.scheme )), } }) @@ -298,7 +298,7 @@ impl<'a> ManageBackend<'a> for &'a str { let opts = self.into_options()?; debug!("Provision store with options: {:?}", &opts); - match opts.schema.as_ref() { + match opts.scheme.as_ref() { #[cfg(feature = "postgres")] "postgres" => { let opts = postgres::PostgresStoreOptions::new(opts)?; @@ -316,7 +316,7 @@ impl<'a> ManageBackend<'a> for &'a str { _ => Err(err_msg!( Unsupported, "Unsupported backend: {}", - &opts.schema + &opts.scheme )), } }) @@ -327,7 +327,7 @@ impl<'a> ManageBackend<'a> for &'a str { let opts = self.into_options()?; debug!("Remove store with options: {:?}", &opts); - match opts.schema.as_ref() { + match opts.scheme.as_ref() { #[cfg(feature = "postgres")] "postgres" => { let opts = postgres::PostgresStoreOptions::new(opts)?; @@ -343,7 +343,7 @@ impl<'a> ManageBackend<'a> for &'a str { _ => Err(err_msg!( Unsupported, "Unsupported backend: {}", - &opts.schema + &opts.scheme )), } }) diff --git a/askar-storage/src/backend/postgres/provision.rs b/askar-storage/src/backend/postgres/provision.rs index c55cbd04..77d43fd4 100644 --- a/askar-storage/src/backend/postgres/provision.rs +++ b/askar-storage/src/backend/postgres/provision.rs @@ -36,6 +36,8 @@ pub struct PostgresStoreOptions { pub(crate) admin_uri: String, pub(crate) host: String, pub(crate) name: String, + pub(crate) username: String, + pub(crate) schema: Option, } impl PostgresStoreOptions { @@ -73,8 +75,13 @@ impl PostgresStoreOptions { } else { DEFAULT_MIN_CONNECTIONS }; + let schema = opts.query.remove("schema"); let admin_acct = opts.query.remove("admin_account"); let admin_pass = opts.query.remove("admin_password"); + let username = match opts.user.as_ref() { + "" => "postgres".to_owned(), + a => a.to_owned(), + }; let uri = opts.clone().into_uri(); if admin_acct.is_some() || admin_pass.is_some() { if let Some(admin_acct) = admin_acct { @@ -90,12 +97,11 @@ impl PostgresStoreOptions { return Err(err_msg!(Input, "Missing database name")); } let name = path[1..].to_string(); - if name.find(|c| c == '"' || c == '\0').is_some() { - return Err(err_msg!( - Input, - "Invalid character in database name: '\"' and '\\0' are disallowed" - )); + if let Some(schema) = schema.as_ref() { + _validate_ident(schema, "schema")?; } + _validate_ident(&name, "database")?; + _validate_ident(&username, "username")?; // admin user selects the default database opts.path = Cow::Borrowed("/postgres"); Ok(Self { @@ -107,6 +113,8 @@ impl PostgresStoreOptions { admin_uri: opts.into_uri(), host, name, + username, + schema, }) } @@ -119,6 +127,10 @@ impl PostgresStoreOptions { .log_statements(log::LevelFilter::Debug) .log_slow_statements(log::LevelFilter::Debug, Default::default()); } + if let Some(s) = self.schema.as_ref() { + // NB: schema is a validated identifier + conn_opts = conn_opts.options([("search_path", s)]); + } PgPoolOptions::default() .acquire_timeout(self.connect_timeout) .idle_timeout(self.idle_timeout) @@ -136,15 +148,18 @@ impl PostgresStoreOptions { Err(SqlxError::Database(db_err)) if db_err.code() == Some(Cow::Borrowed("3D000")) => { // error 3D000 is INVALID CATALOG NAME in postgres, // this indicates that the database does not exist - let mut admin_conn = PgConnection::connect(self.admin_uri.as_ref()).await?; - // any character except NUL is allowed in an identifier. - // double quotes must be escaped, but we just disallow those - let create_q = format!("CREATE DATABASE \"{}\"", self.name); - match sqlx::query(&create_q) - .persistent(false) - .execute(&mut admin_conn) + let mut admin_conn = PgConnection::connect(self.admin_uri.as_ref()) .await - { + .map_err(err_map!( + Backend, + "Error creating admin connection to database" + ))?; + // self.name and self.username are validated identifiers + let create_q = format!( + "CREATE DATABASE \"{}\" OWNER \"{}\"", + self.name, self.username + ); + match admin_conn.execute(create_q.as_str()).await { Ok(_) => (), Err(SqlxError::Database(db_err)) if db_err.code() == Some(Cow::Borrowed("23505")) @@ -181,26 +196,43 @@ impl PostgresStoreOptions { if recreate { // remove expected tables reset_db(&mut txn).await?; - } else if sqlx::query_scalar::<_, i64>( - "SELECT COUNT(*) FROM information_schema.tables - WHERE table_schema='public' AND table_name='config'", - ) - .fetch_one(txn.as_mut()) - .await? - == 1 - { - // proceed to open, will fail if the version doesn't match - return open_db( - conn_pool, - Some(method), - pass_key, - profile, - self.host, - self.name, - ) - .await; + } else { + // check for presence of config table + let count = if let Some(schema) = self.schema.as_ref() { + sqlx::query_scalar::<_, i64>( + "SELECT COUNT(*) FROM information_schema.tables + WHERE table_schema=?1 AND table_name='config'", + ) + .persistent(false) + .bind(schema) + .fetch_one(txn.as_mut()) + .await + .map_err(err_map!(Backend, "Error checking for existing store"))? + } else { + sqlx::query_scalar::<_, i64>( + "SELECT COUNT(*) FROM information_schema.tables + WHERE table_schema=ANY (CURRENT_SCHEMAS(false)) AND table_name='config'", + ) + .persistent(false) + .fetch_one(txn.as_mut()) + .await + .map_err(err_map!(Backend, "Error checking for existing store"))? + }; + if count > 0 { + // proceed to open, will fail if the version doesn't match + return open_db( + conn_pool, + Some(method), + pass_key, + profile, + self.host, + self.name, + ) + .await; + } } - // else: no 'config' table, assume empty database + + // no 'config' table, assume empty database let (profile_key, enc_profile_key, store_key, store_key_ref) = unblock({ let pass_key = pass_key.into_owned(); @@ -208,7 +240,14 @@ impl PostgresStoreOptions { }) .await?; let default_profile = profile.unwrap_or_else(random_profile_name); - let profile_id = init_db(txn, &default_profile, store_key_ref, enc_profile_key).await?; + let profile_id = init_db( + txn, + &default_profile, + store_key_ref, + enc_profile_key, + self.schema.as_ref().unwrap_or(&self.username), + ) + .await?; let mut key_cache = KeyCache::new(store_key); key_cache.add_profile_mut(default_profile.clone(), profile_id, profile_key); @@ -235,22 +274,23 @@ impl PostgresStoreOptions { // this indicates that the database does not exist Err(err_msg!(NotFound, "The requested database was not found")) } - Err(e) => Err(e.into()), + Err(err) => Err(err_msg!(Backend, "Error connecting to database pool").with_cause(err)), }?; open_db(pool, method, pass_key, profile, self.host, self.name).await } /// Remove an existing Postgres store defined by these configuration options pub async fn remove(self) -> Result { - let mut admin_conn = PgConnection::connect(self.admin_uri.as_ref()).await?; + let mut admin_conn = PgConnection::connect(self.admin_uri.as_ref()) + .await + .map_err(err_map!( + Backend, + "Error creating admin connection to database" + ))?; // any character except NUL is allowed in an identifier. // double quotes must be escaped, but we just disallow those let drop_q = format!("DROP DATABASE \"{}\"", self.name); - match sqlx::query(&drop_q) - .persistent(false) - .execute(&mut admin_conn) - .await - { + match admin_conn.execute(drop_q.as_str()).await { Ok(_) => Ok(true), Err(SqlxError::Database(db_err)) if db_err.code() == Some(Cow::Borrowed("3D000")) => { // invalid catalog name is raised if the database does not exist @@ -295,25 +335,28 @@ pub(crate) async fn init_db<'t>( profile_name: &str, store_key_ref: String, enc_profile_key: Vec, + schema: &str, ) -> Result { txn.execute( - " - CREATE TABLE config ( + format!(r#" + CREATE SCHEMA IF NOT EXISTS "{schema}"; + + CREATE TABLE "{schema}".config ( name TEXT NOT NULL, value TEXT, PRIMARY KEY(name) ); - CREATE TABLE profiles ( + CREATE TABLE "{schema}".profiles ( id BIGSERIAL, name TEXT NOT NULL, reference TEXT NULL, profile_key BYTEA NULL, PRIMARY KEY(id) ); - CREATE UNIQUE INDEX ix_profile_name ON profiles(name); + CREATE UNIQUE INDEX ix_profile_name ON "{schema}".profiles(name); - CREATE TABLE items ( + CREATE TABLE "{schema}".items ( id BIGSERIAL, profile_id BIGINT NOT NULL, kind SMALLINT NOT NULL, @@ -322,27 +365,28 @@ pub(crate) async fn init_db<'t>( value BYTEA NOT NULL, expiry TIMESTAMP NULL, PRIMARY KEY(id), - FOREIGN KEY(profile_id) REFERENCES profiles(id) + FOREIGN KEY(profile_id) REFERENCES "{schema}".profiles(id) ON DELETE CASCADE ON UPDATE CASCADE ); - CREATE UNIQUE INDEX ix_items_uniq ON items(profile_id, kind, category, name); + CREATE UNIQUE INDEX ix_items_uniq ON "{schema}".items(profile_id, kind, category, name); - CREATE TABLE items_tags ( + CREATE TABLE "{schema}".items_tags ( id BIGSERIAL, item_id BIGINT NOT NULL, name BYTEA NOT NULL, value BYTEA NOT NULL, plaintext SMALLINT NOT NULL, PRIMARY KEY(id), - FOREIGN KEY(item_id) REFERENCES items(id) + FOREIGN KEY(item_id) REFERENCES "{schema}".items(id) ON DELETE CASCADE ON UPDATE CASCADE ); - CREATE INDEX ix_items_tags_item_id ON items_tags(item_id); - CREATE INDEX ix_items_tags_name_enc ON items_tags(name, SUBSTR(value, 1, 12)) include (item_id) WHERE plaintext=0; - CREATE INDEX ix_items_tags_name_plain ON items_tags(name, value) include (item_id) WHERE plaintext=1; - ", + CREATE INDEX ix_items_tags_item_id ON "{schema}".items_tags(item_id); + CREATE INDEX ix_items_tags_name_enc ON "{schema}".items_tags(name, SUBSTR(value, 1, 12)) INCLUDE (item_id) WHERE plaintext=0; + CREATE INDEX ix_items_tags_name_plain ON "{schema}".items_tags(name, value) INCLUDE (item_id) WHERE plaintext=1; + "#).as_str(), ) - .await?; + .await + .map_err(err_map!(Backend, "Error creating database tables"))?; sqlx::query( "INSERT INTO config (name, value) VALUES @@ -354,14 +398,16 @@ pub(crate) async fn init_db<'t>( .bind(profile_name) .bind(store_key_ref) .execute(txn.as_mut()) - .await?; + .await + .map_err(err_map!(Backend, "Error inserting configuration"))?; let profile_id = sqlx::query_scalar("INSERT INTO profiles (name, profile_key) VALUES ($1, $2) RETURNING id") .bind(profile_name) .bind(enc_profile_key) .fetch_one(txn.as_mut()) - .await?; + .await + .map_err(err_map!(Backend, "Error inserting default profile"))?; txn.commit().await?; @@ -399,7 +445,8 @@ pub(crate) async fn open_db( WHERE name IN ('default_profile', 'key', 'version')"#, ) .fetch_all(conn.as_mut()) - .await?; + .await + .map_err(err_map!(Backend, "Error fetching store configuration"))?; for row in config { match row.try_get(0)? { "default_profile" => { @@ -453,6 +500,22 @@ pub(crate) async fn open_db( )) } +/// Validate a postgres identifier. +/// Any character except NUL is allowed in an identifier. Double quotes must be escaped, +/// but we just disallow those instead. +fn _validate_ident(ident: &str, name: &str) -> Result<(), Error> { + if ident.is_empty() { + Err(err_msg!(Input, "{name} identifier is empty")) + } else if ident.find(|c| c == '"' || c == '\0').is_some() { + Err(err_msg!( + Input, + "Invalid character in {name} identifier: '\"' and '\\0' are disallowed" + )) + } else { + Ok(()) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/askar-storage/src/backend/postgres/test_db.rs b/askar-storage/src/backend/postgres/test_db.rs index 4890b99f..c13d07a6 100644 --- a/askar-storage/src/backend/postgres/test_db.rs +++ b/askar-storage/src/backend/postgres/test_db.rs @@ -66,8 +66,14 @@ impl TestDB { reset_db(&mut init_txn).await?; // create tables and add default profile - let profile_id = - init_db(init_txn, &default_profile, store_key_ref, enc_profile_key).await?; + let profile_id = init_db( + init_txn, + &default_profile, + store_key_ref, + enc_profile_key, + &opts.username, + ) + .await?; let mut key_cache = KeyCache::new(store_key); key_cache.add_profile_mut(default_profile.clone(), profile_id, profile_key); diff --git a/askar-storage/src/backend/sqlite/provision.rs b/askar-storage/src/backend/sqlite/provision.rs index 8ad123c9..2779ea41 100644 --- a/askar-storage/src/backend/sqlite/provision.rs +++ b/askar-storage/src/backend/sqlite/provision.rs @@ -164,14 +164,18 @@ impl SqliteStoreOptions { if recreate && !self.in_memory { try_remove_file(self.path.to_string()).await?; } - let conn_pool = self.pool(true).await?; + let conn_pool = self + .pool(true) + .await + .map_err(err_map!(Backend, "Error creating database pool"))?; if !recreate && sqlx::query_scalar::<_, i64>( "SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name='config'", ) .fetch_one(&conn_pool) - .await? + .await + .map_err(err_map!(Backend, "Error checking for existing store"))? == 1 { return open_db( @@ -213,7 +217,8 @@ impl SqliteStoreOptions { "The requested database path was not found" )) } else { - Err(SqlxError::Database(db_err).into()) + Err(err_msg!(Backend, "Error connecting to database pool") + .with_cause(SqlxError::Database(db_err))) } } Err(err) => Err(err.into()), @@ -347,7 +352,7 @@ async fn init_db( .bind(store_key_ref) .bind(enc_profile_key) .execute(conn.as_mut()) - .await?; + .await.map_err(err_map!(Backend, "Error creating database tables"))?; let mut key_cache = KeyCache::new(store_key); @@ -355,7 +360,8 @@ async fn init_db( .persistent(false) .bind(profile_name) .fetch_one(conn.as_mut()) - .await?; + .await + .map_err(err_map!(Backend, "Error checking for existing profile"))?; key_cache.add_profile_mut(profile_name.to_string(), row.try_get(0)?, profile_key); Ok(key_cache) @@ -378,7 +384,8 @@ async fn open_db( WHERE name IN ("default_profile", "key", "version")"#, ) .fetch_all(conn.as_mut()) - .await?; + .await + .map_err(err_map!(Backend, "Error fetching store configuration"))?; for row in config { match row.try_get(0)? { "default_profile" => { diff --git a/askar-storage/src/options.rs b/askar-storage/src/options.rs index 9b3f739c..564576c4 100644 --- a/askar-storage/src/options.rs +++ b/askar-storage/src/options.rs @@ -9,7 +9,7 @@ use crate::error::Error; /// Parsed representation of database connection URI pub struct Options<'a> { /// The URI schema - pub schema: Cow<'a, str>, + pub scheme: Cow<'a, str>, /// The authenticating user name pub user: Cow<'a, str>, /// The authenticating user password @@ -30,19 +30,19 @@ impl<'a> Options<'a> { let mut fragment_and_remain = uri.splitn(2, '#'); let uri = fragment_and_remain.next().unwrap_or_default(); let fragment = percent_decode(fragment_and_remain.next().unwrap_or_default()); - let mut schema_and_remain = uri.splitn(2, ':'); - let schema = schema_and_remain.next().unwrap_or_default(); + let mut scheme_and_remain = uri.splitn(2, ':'); + let scheme = scheme_and_remain.next().unwrap_or_default(); - let (schema, host_and_query) = if let Some(remain) = schema_and_remain.next() { - if schema.is_empty() { + let (scheme, host_and_query) = if let Some(remain) = scheme_and_remain.next() { + if scheme.is_empty() { ("", uri) } else { - (schema, remain.trim_start_matches("//")) + (scheme, remain.trim_start_matches("//")) } } else { ("", uri) }; - let schema = percent_decode(schema); + let scheme = percent_decode(scheme); let mut host_and_query = host_and_query.splitn(2, '?'); let (user, password, host) = { @@ -82,7 +82,7 @@ impl<'a> Options<'a> { password, host, path, - schema, + scheme, query, fragment, }) @@ -91,8 +91,8 @@ impl<'a> Options<'a> { /// Convert an options structure back into a string pub fn into_uri(self) -> String { let mut uri = String::new(); - if !self.schema.is_empty() { - percent_encode_into(&mut uri, &self.schema); + if !self.scheme.is_empty() { + percent_encode_into(&mut uri, &self.scheme); uri.push_str("://"); } if !self.user.is_empty() || !self.password.is_empty() { @@ -160,14 +160,14 @@ mod tests { #[test] fn options_basic() { - let opts = Options::parse_uri("schema://user%2E:pass@host/dbname?a+1=b#frag").unwrap(); + let opts = Options::parse_uri("scheme://user%2E:pass@host/dbname?a+1=b#frag").unwrap(); let bs = Cow::Borrowed; assert_eq!( opts, Options { user: bs("user."), password: bs("pass"), - schema: bs("schema"), + scheme: bs("scheme"), host: bs("host"), path: bs("/dbname"), query: HashMap::from_iter(vec![("a 1".to_owned(), "b".to_owned())]), @@ -184,7 +184,7 @@ mod tests { Options { user: Default::default(), password: Default::default(), - schema: Default::default(), + scheme: Default::default(), host: Cow::Borrowed("dbname"), path: Cow::Borrowed("/path"), query: HashMap::from_iter(vec![("a".to_owned(), "".to_owned())]),