diff --git a/Cargo.lock b/Cargo.lock index 03dc93ab69f..f5f882550db 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -83,6 +83,12 @@ dependencies = [ "term", ] +[[package]] +name = "assert_matches" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b34d609dfbaf33d6889b2b7106d3ca345eacad44200913df5ba02bfd31d2ba9" + [[package]] name = "async-bb8-diesel" version = "0.1.0" @@ -352,6 +358,7 @@ dependencies = [ name = "bootstore" version = "0.1.0" dependencies = [ + "assert_matches", "bcs", "bincode", "derive_more", diff --git a/bootstore/Cargo.toml b/bootstore/Cargo.toml index 6ef8366391c..e19d2df5250 100644 --- a/bootstore/Cargo.toml +++ b/bootstore/Cargo.toml @@ -30,5 +30,6 @@ uuid = { version = "1.1.0", features = [ "serde", "v4" ] } vsss-rs = { version = "2.0.0", default-features = false, features = ["std"] } [dev-dependencies] +assert_matches = "1.5.0" bincode = "1.3.3" omicron-test-utils = { path = "../test-utils" } diff --git a/bootstore/src/db/error.rs b/bootstore/src/db/error.rs new file mode 100644 index 00000000000..34f2b76ae22 --- /dev/null +++ b/bootstore/src/db/error.rs @@ -0,0 +1,67 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! DB related errors + +use serde::{Deserialize, Serialize}; +use uuid::Uuid; + +#[derive(thiserror::Error, Debug, Clone, PartialEq, Serialize, Deserialize)] +pub enum Error { + #[error("Failed to open db connection to {path}: {err}")] + DbOpen { path: String, err: String }, + + #[error("Diesel/sqlite error: {err}")] + Diesel { err: String }, + + #[error("BCS serialization error: {err}")] + Bcs { err: String }, + + #[error("Failed to parse string as UUID: {uuidstr} - {err}")] + ParseUuid { uuidstr: String, err: String }, + + // Temporary until the using code is written + #[allow(dead_code)] + #[error("Share commit for {epoch} does not match prepare")] + CommitHashMismatch { epoch: i32 }, + + #[error("No shares have been committed")] + NoSharesCommitted, + + #[error("DB invariant violated: {0}")] + DbInvariant(String), + + #[error("Already initialized with rack uuid: {0}")] + AlreadyInitialized(Uuid), + + #[error( + "Tried to Prepare a KeyShare with epoch {epoch}, but found one \ + with later epoch {stored_epoch}" + )] + OldKeySharePrepare { epoch: i32, stored_epoch: i32 }, + + #[error("A distinct key share already exists for epoch {epoch}")] + KeyShareAlreadyExists { epoch: i32 }, + + #[error("Rack UUID mismatch: Expected: {expected}, Actual: {actual:?}")] + RackUuidMismatch { expected: Uuid, actual: Option }, + + #[error("Rack not initialized")] + RackNotInitialized, + + #[error("Key share for epoch {epoch} not committed")] + KeyShareNotCommitted { epoch: i32 }, +} + +impl From for Error { + fn from(err: diesel::result::Error) -> Self { + Error::Diesel { err: err.to_string() } + } +} + +impl From for Error { + fn from(err: bcs::Error) -> Self { + Error::Bcs { err: err.to_string() } + } +} diff --git a/bootstore/src/db/macros.rs b/bootstore/src/db/macros.rs index 3cb7cb48221..e471f1589a4 100644 --- a/bootstore/src/db/macros.rs +++ b/bootstore/src/db/macros.rs @@ -4,12 +4,16 @@ //! Macros used to create ToSql and FromSql impls required by Diesel -/// Shamelessly stolen from buildomat/common/src/db.rs +/// Shamelessly cribbed and mutated from buildomat/common/src/db.rs /// Thanks @jmc macro_rules! bcs_new_type { ($name:ident, $mytype:ty) => { #[derive( - Clone, Debug, FromSqlRow, diesel::expression::AsExpression, + Clone, + PartialEq, + Debug, + FromSqlRow, + diesel::expression::AsExpression, )] #[diesel(sql_type = diesel::sql_types::Binary)] pub struct $name(pub $mytype); diff --git a/bootstore/src/db/mod.rs b/bootstore/src/db/mod.rs index a0a326021c2..3dcd8ca5d1f 100644 --- a/bootstore/src/db/mod.rs +++ b/bootstore/src/db/mod.rs @@ -4,8 +4,9 @@ //! Database layer for the bootstore +mod error; mod macros; -mod models; +pub(crate) mod models; mod schema; use diesel::connection::SimpleConnection; @@ -13,50 +14,35 @@ use diesel::prelude::*; use diesel::SqliteConnection; use slog::Logger; use slog::{info, o}; +use uuid::Uuid; use crate::trust_quorum::SerializableShareDistribution; +pub(crate) use error::Error; use models::KeyShare; use models::Sha3_256Digest; use models::Share; -use sha3::{Digest, Sha3_256}; - -#[derive(thiserror::Error, Debug)] -pub enum Error { - #[error("Failed to open db connection to {path}: {err}")] - DbOpen { path: String, err: ConnectionError }, - - #[error(transparent)] - Db(#[from] diesel::result::Error), - - #[error(transparent)] - Bcs(#[from] bcs::Error), - - // Temporary until the using code is written - #[allow(dead_code)] - #[error("Share commit for {epoch} does not match prepare")] - CommitHashMismatch { epoch: i32 }, -} pub struct Db { - // Temporary until the using code is written - #[allow(dead_code)] log: Logger, - // Temporary until the using code is written - #[allow(dead_code)] + conn: SqliteConnection, } // Temporary until the using code is written #[allow(dead_code)] impl Db { - pub fn open(log: &Logger, path: &str) -> Result { + /// Initialize the database + /// + /// Create tables if they don't exist and set pragmas + pub fn init(log: &Logger, path: &str) -> Result { let schema = include_str!("./schema.sql"); let log = log.new(o!( "component" => "BootstoreDb" )); info!(log, "opening database {:?}", path); - let mut c = SqliteConnection::establish(path) - .map_err(|err| Error::DbOpen { path: path.into(), err })?; + let mut c = SqliteConnection::establish(path).map_err(|err| { + Error::DbOpen { path: path.into(), err: err.to_string() } + })?; // Enable foreign key processing, which is off by default. Without // enabling this, there is no referential integrity check between @@ -74,43 +60,113 @@ impl Db { diesel::sql_query("PRAGMA synchronous = 'FULL'").execute(&mut c)?; // Create tables - c.batch_execute(&schema)?; + c.batch_execute(schema)?; Ok(Db { log, conn: c }) } + /// Write a KeyShare for epoch a along with the rack UUID + /// + /// The share must be committed with [`commit_share`] + pub fn initialize( + &mut self, + rack_uuid: &Uuid, + share_distribution: SerializableShareDistribution, + ) -> Result<(), Error> { + self.conn.immediate_transaction(|tx| { + if is_initialized(tx)? { + // If the rack is initialized, a rack uuid must exist + let uuid = get_rack_uuid(tx)?.unwrap(); + return Err(Error::AlreadyInitialized(uuid)); + } + match initialize_rack_uuid(tx, rack_uuid)? { + Some(old_uuid) => { + info!(self.log, "Re-Initializing Rack: Old UUID: {old_uuid}, New UUID: {rack_uuid}"); + update_prepare_for_epoch_0(tx, share_distribution) + + }, + None => { + info!(self.log, "Initializing Rack for the first time with UUID: {rack_uuid}"); + insert_prepare(tx, 0, share_distribution) + } + } + + }) + } + + /// Write an uncommitted `KeyShare` into the database. + /// + /// This command is idempotent. + /// + /// The rules for inserting a KeyShare are: + /// 1. A KeyShare for the given epoch does not exist unless it is identical + /// 2. A KeyShare for a later epoch does not exist + /// + /// Calling this method with an epoch of 0 is a programmer error so we + /// assert. pub fn prepare_share( &mut self, + rack_uuid: &Uuid, epoch: i32, - share: SerializableShareDistribution, + share_distribution: SerializableShareDistribution, ) -> Result<(), Error> { - info!(self.log, "Writing key share prepare for {epoch} to the Db"); + assert_ne!(0, epoch); use schema::key_shares::dsl; - // We save the digest so we don't have to deserialize and recompute most of the time. - // We'd only want to do that for a consistency check occasionally. - let val = bcs::to_bytes(&share)?; - let share_digest = - sprockets_common::Sha3_256Digest(Sha3_256::digest(&val).into()) - .into(); - let prepare = KeyShare { - epoch, - share: Share(share), - share_digest, - committed: false, - }; - diesel::insert_into(dsl::key_shares) - .values(&prepare) - .execute(&mut self.conn)?; - Ok(()) + let prepare = KeyShare::new(epoch, share_distribution)?; + self.conn.immediate_transaction(|tx| { + // Has the rack been initialized? + if !is_initialized(tx)? { + return Err(Error::RackNotInitialized); + } + + // Does the rack_uuid match what's stored? + validate_rack_uuid(tx, rack_uuid)?; + + // Check for idempotence + if let Some(stored_key_share) = dsl::key_shares + .filter(dsl::epoch.eq(epoch)) + .get_result::(tx) + .optional()? + { + if prepare == stored_key_share { + return Ok(()); + } else { + return Err(Error::KeyShareAlreadyExists { epoch }); + } + } + + // We don't allow shares for old epochs + if let Some(stored_epoch) = dsl::key_shares + .select(dsl::epoch) + .filter(dsl::epoch.gt(epoch)) + .get_result::(tx) + .optional()? + { + return Err(Error::OldKeySharePrepare { epoch, stored_epoch }); + } + + info!( + self.log, + "Writing key share prepare for epoch {epoch} to the Db" + ); + diesel::insert_into(dsl::key_shares) + .values(&prepare) + .execute(tx)?; + Ok(()) + }) } pub fn commit_share( &mut self, + rack_uuid: &Uuid, epoch: i32, digest: sprockets_common::Sha3_256Digest, ) -> Result<(), Error> { use schema::key_shares::dsl; self.conn.immediate_transaction(|tx| { + // Does the rack_uuid match what's stored? + validate_rack_uuid(tx, rack_uuid)?; + // We only want to commit if the share digest of the commit is the // same as that of the prepare. let prepare_digest = dsl::key_shares @@ -128,16 +184,135 @@ impl Db { Ok(()) }) } + + pub fn get_committed_share(&mut self, epoch: i32) -> Result { + use schema::key_shares::dsl; + let share = dsl::key_shares + .select(dsl::share) + .filter(dsl::epoch.eq(epoch)) + .filter(dsl::committed.eq(true)) + .get_result::(&mut self.conn) + .optional()?; + + match share { + Some(share) => Ok(share), + None => Err(Error::KeyShareNotCommitted { epoch }), + } + } +} + +// +// ---------- DB HELPER METHODS +// + +// Insert an uncommitted key share for a given epoch. This should be called +// inside a transaction by the caller so that the given checks can be +// performed. +fn insert_prepare( + tx: &mut SqliteConnection, + epoch: i32, + share_distribution: SerializableShareDistribution, +) -> Result<(), Error> { + use schema::key_shares::dsl; + let prepare = KeyShare::new(epoch, share_distribution)?; + diesel::insert_into(dsl::key_shares).values(&prepare).execute(tx)?; + Ok(()) +} + +// During rack initialization we set the rack UUID. +// +// We only allow rewriting this UUID when the rack is not initialized, +// and so we only call this from the `initialize` method. +// +// Since we only allow a single row, we just delete the existing rows +// and insert the new one. +// +// Return the old rack UUID if one exists +fn initialize_rack_uuid( + tx: &mut SqliteConnection, + new_uuid: &Uuid, +) -> Result, Error> { + use schema::rack::dsl; + let old_uuid = get_rack_uuid(tx)?; + diesel::delete(dsl::rack).execute(tx)?; + diesel::insert_into(dsl::rack) + .values(dsl::uuid.eq(new_uuid.to_string())) + .execute(tx)?; + Ok(old_uuid) +} + +// Overwrite a share for epoch 0 during rack initialization +// +// This method should only be called from initialize. +fn update_prepare_for_epoch_0( + tx: &mut SqliteConnection, + share_distribution: SerializableShareDistribution, +) -> Result<(), Error> { + use schema::key_shares::dsl; + let prepare = KeyShare::new(0, share_distribution)?; + diesel::update(dsl::key_shares).set(prepare).execute(tx)?; + Ok(()) +} + +/// Return true if there is a commit for epoch 0, false otherwise +fn is_initialized(tx: &mut SqliteConnection) -> Result { + use schema::key_shares::dsl; + Ok(dsl::key_shares + .select(dsl::epoch) + .filter(dsl::epoch.eq(0)) + .filter(dsl::committed.eq(true)) + .get_result::(tx) + .optional()? + .is_some()) +} + +/// Return `Ok(())` if the persisted rack uuid matches `uuid`, or +/// `Err(Error::RackUuidMismatch{..})` otherwise. +fn validate_rack_uuid( + tx: &mut SqliteConnection, + rack_uuid: &Uuid, +) -> Result<(), Error> { + let stored_rack_uuid = get_rack_uuid(tx)?; + if Some(rack_uuid) != stored_rack_uuid.as_ref() { + return Err(Error::RackUuidMismatch { + expected: *rack_uuid, + actual: stored_rack_uuid, + }); + } + Ok(()) +} + +pub fn get_rack_uuid(tx: &mut SqliteConnection) -> Result, Error> { + use schema::rack::dsl; + + dsl::rack.select(dsl::uuid).get_result::(tx).optional()?.map_or( + Ok(None), + |uuidstr| { + Uuid::parse_str(&uuidstr).map(Some).map_err(|err| { + Error::ParseUuid { uuidstr, err: err.to_string() } + }) + }, + ) } #[cfg(test)] -mod tests { +pub mod tests { use super::*; use crate::trust_quorum::{RackSecret, ShareDistribution}; + use assert_matches::assert_matches; use omicron_test_utils::dev::test_setup_log; + use sha3::{Digest, Sha3_256}; + + // This is solely so we can test the non-member functions with a properly + // initialized DB + impl Db { + fn get_conn(&mut self) -> &mut SqliteConnection { + &mut self.conn + } + } // TODO: Fill in with actual member certs - fn new_shares() -> Vec { + pub fn new_shares() -> Vec { let member_device_id_certs = vec![]; let rack_secret_threshold = 3; let total_shares = 5; @@ -160,15 +335,16 @@ mod tests { fn simple_prepare_insert_and_query() { use schema::key_shares::dsl; let logctx = test_setup_log("test_db"); - let mut db = Db::open(&logctx.log, ":memory:").unwrap(); + let mut db = Db::init(&logctx.log, ":memory:").unwrap(); let shares = new_shares(); let epoch = 0; let expected: SerializableShareDistribution = shares[0].clone().into(); - db.prepare_share(epoch, expected.clone()).unwrap(); + let conn = db.get_conn(); + insert_prepare(conn, epoch, expected.clone()).unwrap(); let (share, committed) = dsl::key_shares .select((dsl::share, dsl::committed)) .filter(dsl::epoch.eq(epoch)) - .get_result::<(Share, bool)>(&mut db.conn) + .get_result::<(Share, bool)>(conn) .unwrap(); assert_eq!(share.0, expected); assert_eq!(committed, false); @@ -178,24 +354,24 @@ mod tests { #[test] fn commit_fails_without_corresponding_prepare() { let logctx = test_setup_log("test_db"); - let mut db = Db::open(&logctx.log, ":memory:").unwrap(); + let mut db = Db::init(&logctx.log, ":memory:").unwrap(); let epoch = 0; let digest = sprockets_common::Sha3_256Digest::default(); - let err = db.commit_share(epoch, digest).unwrap_err(); - assert!(matches!(err, Error::Db(diesel::result::Error::NotFound))); + db.commit_share(&Uuid::new_v4(), epoch, digest).unwrap_err(); logctx.cleanup_successful(); } #[test] fn commit_fails_with_invalid_hash() { let logctx = test_setup_log("test_db"); - let mut db = Db::open(&logctx.log, ":memory:").unwrap(); + let mut db = Db::init(&logctx.log, ":memory:").unwrap(); let shares = new_shares(); let epoch = 0; let expected: SerializableShareDistribution = shares[0].clone().into(); - db.prepare_share(epoch, expected.clone()).unwrap(); + let rack_uuid = Uuid::new_v4(); + db.initialize(&rack_uuid, expected.clone()).unwrap(); let digest = sprockets_common::Sha3_256Digest::default(); - let err = db.commit_share(epoch, digest).unwrap_err(); + let err = db.commit_share(&rack_uuid, epoch, digest).unwrap_err(); assert!(matches!(err, Error::CommitHashMismatch { epoch: _ })); logctx.cleanup_successful(); } @@ -203,26 +379,49 @@ mod tests { #[test] fn commit_succeeds_with_correct_hash() { let logctx = test_setup_log("test_db"); - let mut db = Db::open(&logctx.log, ":memory:").unwrap(); + let mut db = Db::init(&logctx.log, ":memory:").unwrap(); let shares = new_shares(); let epoch = 0; let expected: SerializableShareDistribution = shares[0].clone().into(); - db.prepare_share(epoch, expected.clone()).unwrap(); + let rack_uuid = Uuid::new_v4(); + db.initialize(&rack_uuid, expected.clone()).unwrap(); let val = bcs::to_bytes(&expected).unwrap(); let digest = sprockets_common::Sha3_256Digest(Sha3_256::digest(&val).into()) .into(); - assert!(db.commit_share(epoch, digest).is_ok()); + assert!(db.commit_share(&rack_uuid, epoch, digest).is_ok()); // Ensure `committed = true` use schema::key_shares::dsl; let committed = dsl::key_shares .select(dsl::committed) .filter(dsl::epoch.eq(epoch)) - .get_result::(&mut db.conn) + .get_result::(db.get_conn()) .unwrap(); assert_eq!(true, committed); logctx.cleanup_successful(); } + + #[test] + fn ensure_db_trigger_fires_for_more_than_one_row_in_rack_table() { + let logctx = test_setup_log("test_db"); + let mut db = Db::init(&logctx.log, ":memory:").unwrap(); + use schema::rack::dsl; + // One insert succeeds. + diesel::insert_into(dsl::rack) + .values(dsl::uuid.eq(Uuid::new_v4().to_string())) + .execute(db.get_conn()) + .unwrap(); + + // A second fails + let err = diesel::insert_into(dsl::rack) + .values(dsl::uuid.eq(Uuid::new_v4().to_string())) + .execute(db.get_conn()) + .unwrap_err(); + assert_matches!(err, diesel::result::Error::DatabaseError(_, s) => { + assert_eq!("maximum one rack", s.message()); + }); + logctx.cleanup_successful(); + } } diff --git a/bootstore/src/db/models.rs b/bootstore/src/db/models.rs index f567b35a8f5..e8a7a966909 100644 --- a/bootstore/src/db/models.rs +++ b/bootstore/src/db/models.rs @@ -8,10 +8,12 @@ use diesel::deserialize::FromSql; use diesel::prelude::*; use diesel::serialize::ToSql; use diesel::FromSqlRow; +use sha3::{Digest, Sha3_256}; use super::macros::array_new_type; use super::macros::bcs_new_type; use super::schema::*; +use super::Error; use crate::trust_quorum::SerializableShareDistribution; bcs_new_type!(Share, SerializableShareDistribution); @@ -19,7 +21,10 @@ bcs_new_type!(Share, SerializableShareDistribution); /// When a [`KeyShareParepare`] message arrives it is stored in a [`KeyShare`] /// When a [`KeyShareCommit`] message arrives the `committed` field/column is /// set to true. -#[derive(Debug, Queryable, Insertable)] +#[derive( + Debug, PartialEq, Queryable, Insertable, Identifiable, AsChangeset, +)] +#[diesel(primary_key(epoch))] pub struct KeyShare { pub epoch: i32, pub share: Share, @@ -27,14 +32,47 @@ pub struct KeyShare { pub committed: bool, } -// A chacha20poly1305 secret encrypted by a chacha20poly1305 secret key -// derived from the rack secret for the given epoch with the given salt -// -// The epoch informs which rack secret should be used to derive the -// encryptiong key used to encrypt this root secret. -// -// TODO-security: We probably don't want to log even the encrypted secret, but -// it's likely useful for debugging right now. +impl KeyShare { + pub fn new( + epoch: i32, + share_distribution: SerializableShareDistribution, + ) -> Result { + // We save the digest so we don't have to deserialize and recompute most of the time. + // We'd only want to do that for a consistency check occasionally. + let share_digest = + Self::share_distribution_digest(&share_distribution)?; + Ok(KeyShare { + epoch, + share: Share(share_distribution), + share_digest, + committed: false, + }) + } + + pub fn share_distribution_digest( + sd: &SerializableShareDistribution, + ) -> Result { + let val = bcs::to_bytes(&sd)?; + Ok(sprockets_common::Sha3_256Digest(Sha3_256::digest(&val).into()) + .into()) + } +} + +/// Information about the rack +#[derive(Debug, Queryable, Insertable)] +#[diesel(table_name = rack)] +pub struct Rack { + pub uuid: String, +} + +/// A chacha20poly1305 secret encrypted by a chacha20poly1305 secret key +/// derived from the rack secret for the given epoch with the given salt +/// +/// The epoch informs which rack secret should be used to derive the +/// encryption key used to encrypt this root secret. +/// +/// TODO-security: We probably don't want to log even the encrypted secret, but +/// it's likely useful for debugging right now. #[derive(Debug, Queryable, Insertable)] pub struct EncryptedRootSecret { /// The epoch of the rack secret rotation or rack reconfiguration @@ -71,3 +109,9 @@ impl From for Sha3_256Digest { Sha3_256Digest(digest.0) } } + +impl From for sprockets_common::Sha3_256Digest { + fn from(digest: Sha3_256Digest) -> Self { + sprockets_common::Sha3_256Digest(digest.0) + } +} diff --git a/bootstore/src/db/schema.rs b/bootstore/src/db/schema.rs index dbf72dcbc6c..172e79d6ed1 100644 --- a/bootstore/src/db/schema.rs +++ b/bootstore/src/db/schema.rs @@ -13,6 +13,12 @@ table! { } } +table! { + rack(uuid) { + uuid -> Text, + } +} + table! { encrypted_root_secrets(epoch) { epoch -> Integer, diff --git a/bootstore/src/db/schema.sql b/bootstore/src/db/schema.sql index 4b0a0257e6b..bf74d917e82 100644 --- a/bootstore/src/db/schema.sql +++ b/bootstore/src/db/schema.sql @@ -7,6 +7,12 @@ CREATE TABLE IF NOT EXISTS key_shares ( PRIMARY KEY (epoch) ); +CREATE TABLE IF NOT EXISTS rack ( + uuid TEXT NOT NULL, + + PRIMARY KEY (uuid) +); + CREATE TABLE IF NOT EXISTS encrypted_root_secrets ( epoch INTEGER NOT NULL, salt BLOB NOT NULL, @@ -16,3 +22,13 @@ CREATE TABLE IF NOT EXISTS encrypted_root_secrets ( PRIMARY KEY (epoch) FOREIGN KEY (epoch) REFERENCES key_share_prepares (epoch) ); + +-- Ensure there is no more than one rack row +-- We rollback the whole transaction in the case of a constraint +-- violation, since we have broken an invariant and should not proceed. +CREATE TRIGGER ensure_rack_contains_at_most_one_row +BEFORE INSERT ON rack +WHEN (SELECT COUNT(*) FROM rack) >= 1 +BEGIN + SELECT RAISE(ROLLBACK, 'maximum one rack'); +END; diff --git a/bootstore/src/messages.rs b/bootstore/src/messages.rs index 95a9023cd67..6006a557f80 100644 --- a/bootstore/src/messages.rs +++ b/bootstore/src/messages.rs @@ -9,6 +9,7 @@ use serde::{Deserialize, Serialize}; use uuid::Uuid; use vsss_rs::Share; +use crate::db; use crate::trust_quorum::SerializableShareDistribution; /// A request sent to a [`Node`] from another [`Node`] or a [`Coordinator`]. @@ -64,7 +65,11 @@ pub enum NodeOp { /// A request from a [`Coordinator`] for the Commit phase of a /// rekey or reconfiguration - KeyShareCommit { rack_uuid: Uuid, epoch: i32 }, + KeyShareCommit { + rack_uuid: Uuid, + epoch: i32, + prepare_share_distribution_digest: sprockets_common::Sha3_256Digest, + }, } /// A response from a [`Node`] to another [`Node`] or a [`Coordinator`] @@ -82,13 +87,14 @@ pub enum NodeOpResult { /// A key share for a given epoch as requested by [`PeerRequest::GetShare`] Share { epoch: i32, share: Share }, - /// An ack for the most recent coordinator message + /// An ack for the most recent coordinator message, where there is no data + /// to return CoordinatorAck, } /// Errors returned inside a [`NodeOpResult`] #[derive( - Debug, Clone, PartialEq, From, Serialize, Deserialize, thiserror::Error, + Debug, PartialEq, Clone, From, Serialize, Deserialize, thiserror::Error, )] pub enum NodeError { #[error("Version {0} messages are unsupported.")] @@ -106,8 +112,17 @@ pub enum NodeError { AlreadyInitialized { rack_uuid: Uuid }, #[error( - "No corresponding key share prepare for this commit: rack UUID: + "No corresponding key share prepare for this commit: rack UUID: \ {rack_uuid}, epoch: {epoch}" )] MissingKeySharePrepare { rack_uuid: Uuid, epoch: i32 }, + + #[error("DB error: {0}")] + Db(db::Error), + + #[error( + "'KeySharePrepare' messages are not allowed for epoch 0. \ +Please send an 'Initialize' message" + )] + KeySharePrepareForEpoch0, } diff --git a/bootstore/src/node.rs b/bootstore/src/node.rs index 183cb5c46bb..f0c7edbdfcf 100644 --- a/bootstore/src/node.rs +++ b/bootstore/src/node.rs @@ -8,7 +8,8 @@ //! This allows easier testing of clusters and failure situations. use slog::Logger; -use sprockets_host::Ed25519Certificate; +use sprockets_common::Sha3_256Digest; +//use sprockets_host::Ed25519Certificate; use uuid::Uuid; use crate::db::Db; @@ -20,8 +21,8 @@ pub struct Config { log: Logger, db_path: String, // TODO: This will live inside the certificate eventually - _serial_number: String, - _device_id_cert: Ed25519Certificate, + //_serial_number: String, + //_device_id_cert: Ed25519Certificate, } /// A node of the bootstore @@ -45,7 +46,7 @@ pub struct Node { impl Node { /// Create a new Node pub fn new(config: Config) -> Node { - let db = Db::open(&config.log, &config.db_path).unwrap(); + let db = Db::init(&config.log, &config.db_path).unwrap(); Node { config, db } } @@ -63,54 +64,229 @@ impl Node { let result = match req.op { NodeOp::GetShare { epoch } => self.handle_get_share(epoch), NodeOp::Initialize { rack_uuid, share_distribution } => { - self.handle_initialize(rack_uuid, share_distribution) + self.handle_initialize(&rack_uuid, share_distribution) } NodeOp::KeySharePrepare { rack_uuid, epoch, share_distribution, } => self.handle_key_share_prepare( - rack_uuid, + &rack_uuid, epoch, share_distribution, ), - NodeOp::KeyShareCommit { rack_uuid, epoch } => { - self.handle_key_share_commit(rack_uuid, epoch) - } + NodeOp::KeyShareCommit { + rack_uuid, + epoch, + prepare_share_distribution_digest, + } => self.handle_key_share_commit( + &rack_uuid, + epoch, + prepare_share_distribution_digest, + ), }; NodeResponse { version: req.version, id: req.id, result } } + // Handle `GetShare` messages from another node fn handle_get_share( &mut self, - _epoch: i32, + epoch: i32, ) -> Result { - unimplemented!(); + let share = self.db.get_committed_share(epoch)?; + Ok(NodeOpResult::Share { epoch, share: share.0.share }) } + // Handle `Initialize` messages from the coordinator fn handle_initialize( &mut self, - _rack_uuid: Uuid, - _share_distribution: SerializableShareDistribution, + rack_uuid: &Uuid, + share_distribution: SerializableShareDistribution, ) -> Result { - unimplemented!(); + self.db.initialize(rack_uuid, share_distribution)?; + Ok(NodeOpResult::CoordinatorAck) } + // Handle `KeySharePrepare` messages from the coordinator fn handle_key_share_prepare( &mut self, - _rack_uuid: Uuid, - _epoch: i32, - _share_distribution: SerializableShareDistribution, + rack_uuid: &Uuid, + epoch: i32, + share_distribution: SerializableShareDistribution, ) -> Result { - unimplemented!(); + if epoch == 0 { + return Err(NodeError::KeySharePrepareForEpoch0); + } + + self.db.prepare_share(rack_uuid, epoch, share_distribution)?; + + Ok(NodeOpResult::CoordinatorAck) } + // Handle `KeyShareCommit` messages from the coordinator fn handle_key_share_commit( &mut self, - _rack_uuid: Uuid, - _epoch: i32, + rack_uuid: &Uuid, + epoch: i32, + prepare_shared_distribution_digest: Sha3_256Digest, ) -> Result { - unimplemented!(); + self.db.commit_share( + rack_uuid, + epoch, + prepare_shared_distribution_digest, + )?; + + Ok(NodeOpResult::CoordinatorAck) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::db; + use crate::db::models::KeyShare; + use crate::db::tests::new_shares; + use crate::trust_quorum::ShareDistribution; + use omicron_test_utils::dev::test_setup_log; + use omicron_test_utils::dev::LogContext; + use uuid::Uuid; + + fn setup() -> (LogContext, Node, Vec) { + let logctx = test_setup_log("test_db"); + let config = + Config { log: logctx.log.clone(), db_path: ":memory:".to_string() }; + let node = Node::new(config); + (logctx, node, new_shares()) + } + + #[test] + fn initialize_can_run_again_if_epoch_0_is_not_committed() { + let (logctx, mut node, share_distributions) = setup(); + let sd: SerializableShareDistribution = + share_distributions[0].clone().into(); + let expected = Ok(NodeOpResult::CoordinatorAck); + assert_eq!( + expected, + node.handle_initialize(&Uuid::new_v4(), sd.clone()) + ); + // We can re-initialize with a new uuid + let rack_uuid = Uuid::new_v4(); + assert_eq!(expected, node.handle_initialize(&rack_uuid, sd.clone())); + + // We can re-initialize with the same uuid + assert_eq!(expected, node.handle_initialize(&rack_uuid, sd.clone())); + + // Committing the key share for epoch 0 means we cannot initialize again + let epoch = 0; + let prepare = KeyShare::new(epoch, sd.clone()).unwrap(); + node.db + .commit_share(&rack_uuid, epoch, prepare.share_digest.into()) + .unwrap(); + + let expected = + Err(NodeError::Db(db::Error::AlreadyInitialized(rack_uuid))); + assert_eq!(expected, node.handle_initialize(&rack_uuid, sd)); + + logctx.cleanup_successful(); + } + + #[test] + fn initialize_and_reconfigure() { + let (logctx, mut node, share_distributions) = setup(); + let sd: SerializableShareDistribution = + share_distributions[0].clone().into(); + let rack_uuid = Uuid::new_v4(); + let ok_ack = Ok(NodeOpResult::CoordinatorAck); + let sd_digest = + KeyShare::share_distribution_digest(&sd).unwrap().into(); + let epoch = 0; + + // Successful rack initialization - The initialize request is the prepare + // Then we commit it. In the future, we will also prepare and commit a plan + // as well as a share distribution. In that case we will likely have a separate + // `InitializeCommit` message and will likely change the `Initialize` message to + // `InitializePrepare`. + assert_eq!(ok_ack, node.handle_initialize(&rack_uuid, sd)); + assert_eq!( + ok_ack, + node.handle_key_share_commit(&rack_uuid, epoch, sd_digest) + ); + + // Let's simulate a successful reconfiguration + let sd: SerializableShareDistribution = new_shares()[0].clone().into(); + let sd_digest = + KeyShare::share_distribution_digest(&sd).unwrap().into(); + let epoch = 1; + assert_eq!( + ok_ack, + node.handle_key_share_prepare(&rack_uuid, epoch, sd) + ); + assert_eq!( + ok_ack, + node.handle_key_share_commit(&rack_uuid, epoch, sd_digest) + ); + + logctx.cleanup_successful(); + } + + #[test] + fn cannot_prepare_if_not_initialized() { + let (logctx, mut node, share_distributions) = setup(); + let sd: SerializableShareDistribution = + share_distributions[0].clone().into(); + let rack_uuid = Uuid::new_v4(); + let epoch = 1; + + // We don't have even a prepared key share at epoch 0 yet + assert_eq!( + Err(NodeError::Db(db::Error::RackNotInitialized)), + node.handle_key_share_prepare(&rack_uuid, epoch, sd.clone()) + ); + + // Create a prepare, but don't commit it + let ok_ack = Ok(NodeOpResult::CoordinatorAck); + assert_eq!(ok_ack, node.handle_initialize(&rack_uuid, sd.clone())); + + // Try (and fail) to issue a prepare for epoch 1 again. The actual contents of the + // share distribution doesn't matter. + assert_eq!( + Err(NodeError::Db(db::Error::RackNotInitialized)), + node.handle_key_share_prepare(&rack_uuid, epoch, sd.clone()) + ); + + logctx.cleanup_successful(); + } + + #[test] + fn cannot_prepare_with_bad_rack_uuid() { + let (logctx, mut node, share_distributions) = setup(); + let sd: SerializableShareDistribution = + share_distributions[0].clone().into(); + let rack_uuid = Uuid::new_v4(); + let ok_ack = Ok(NodeOpResult::CoordinatorAck); + let sd_digest = + KeyShare::share_distribution_digest(&sd).unwrap().into(); + let epoch = 0; + + // Successful rack initialization + assert_eq!(ok_ack, node.handle_initialize(&rack_uuid, sd.clone())); + assert_eq!( + ok_ack, + node.handle_key_share_commit(&rack_uuid, epoch, sd_digest) + ); + + // Attempt to prepare with an invalid rack uuid + let bad_uuid = Uuid::new_v4(); + let epoch = 1; + assert_eq!( + Err(NodeError::Db(db::Error::RackUuidMismatch { + expected: bad_uuid, + actual: Some(rack_uuid) + })), + node.handle_key_share_prepare(&bad_uuid, epoch, sd.clone()) + ); + + logctx.cleanup_successful(); } } diff --git a/test-utils/src/dev/mod.rs b/test-utils/src/dev/mod.rs index 45954cd4942..1e6c7e80c44 100644 --- a/test-utils/src/dev/mod.rs +++ b/test-utils/src/dev/mod.rs @@ -11,7 +11,7 @@ pub mod poll; pub mod test_cmds; use anyhow::Context; -use dropshot::test_util::LogContext; +pub use dropshot::test_util::LogContext; use dropshot::ConfigLogging; use dropshot::ConfigLoggingIfExists; use dropshot::ConfigLoggingLevel;