TQ: Introduce tqdb (#8801)

This PR builds on #8753 

This is a hefty PR, but it's not as bad as it looks. Over 4k lines of it
is in the example log file in the second commit. There's also some moved
and unmodified code that I'll point out.

This PR introduces a new test tool for the trust-quorum protocol:
tqdb. tqdb is a repl that takes event traces produced by the `cluster`
proptest and uses them for deterministic replay of actions against test
state.

The test state includes a "universe" of real protocol nodes, a fake
nexus, and fake networks. The proptest and debugging state is shared and
contained in the `trust-quorum-test-utils`.

The debugger allows a variety of functionality including stepping
through individual events, setting breakpoints, snapshotting and diffing
states and viewing the event log itself.

The purpose of tqdb is twofold:

  1. Allow for debugging of failed proptests. This is non-trivial in
     some cases, even with shrunken tests, because the generated
     actions are high-level and are all generated up front. The actual
     operations such as reconfigurations are derived from these high
     level random generations  in conjunction with the current state
     of the system. Therefore the set of failing generated actions
     doesn't really tell you much. You have to look at the logs, and
     the assertion that fired and reason about it with incomplete
     information. Now, for each concrete action taken, we record the
     event in a log. In the case of a failure an event log can be
     loaded into tqdb, with a breakpoint set right before the failure. A
     snapshot of the state can be taken, and then the failing event can
     be applied. The diff will tell you what changed and allow you to
     inspect the actual state of the system. Full visibility into your
     failure is now possible.

 2. The trust quorum protocol is non-trivial. Tqdb allows developers
    to see in detail how the protocol behaves and understand what is
    happening in certain situations. Event logs can be created by hand
    (or script) for particularly interesting scenarios and then run
    through tqdb.

In order to get the diff functionality to work as I wanted, I had to
implement `Eq` for types that implemented `subtle::ConstantTimeEq` in
both `gfss` (our secret sharing library), and `trust-quorum` crates.
However the safety in terms of the compiler breaking the constant
time guarantees is unknown. Therefore, a feature flag was added
such that only `test-utils` and `tqdb` crates are able to use these
implementations. They are not used in the production codebase. Feature
unification is not at play here because neither `test-utils` or `tqdb`
are part of the product.

Author: andrewjstone

Cargo.lock

@@ -141,6 +141,8 @@ members = [
     "test-utils",
     "trust-quorum",
     "trust-quorum/gfss",
+    "trust-quorum/test-utils",
+    "trust-quorum/tqdb",
     "typed-rng",
     "update-common",
     "update-engine",
@@ -298,6 +300,8 @@ default-members = [
     "sp-sim",
     "trust-quorum",
     "trust-quorum/gfss",
+    "trust-quorum/test-utils",
+    "trust-quorum/tqdb",
     "test-utils",
     "typed-rng",
     "update-common",
@@ -460,6 +464,8 @@ gateway-test-utils = { path = "gateway-test-utils" }
 gateway-types = { path = "gateway-types" }
 gethostname = "0.5.0"
 gfss = { path = "trust-quorum/gfss" }
+trust-quorum = { path = "trust-quorum" }
+trust-quorum-test-utils = { path = "trust-quorum/test-utils" }
 glob = "0.3.2"
 guppy = "0.17.20"
 headers = "0.4.1"

Cargo.toml

@@ -15,7 +15,7 @@ use iddqd::IdOrdMap;
 use indent_write::fmt::IndentWriter;
 use internal_dns_types::diff::DnsDiff;
 use itertools::Itertools;
-use log_capture::LogCapture;
+pub use log_capture::LogCapture;
 use nexus_inventory::CollectionBuilder;
 use nexus_reconfigurator_blippy::Blippy;
 use nexus_reconfigurator_blippy::BlippyReportSortKey;

dev-tools/reconfigurator-cli/src/lib.rs

@@ -9,6 +9,7 @@ use anyhow::anyhow;
 use anyhow::bail;
 use camino::Utf8Path;
 use clap::Parser;
+use reedline::Prompt;
 use reedline::Reedline;
 use reedline::Signal;
 use std::fs::File;
@@ -110,13 +111,24 @@ pub fn run_repl_from_file<C: Parser>(
 pub fn run_repl_on_stdin<C: Parser>(
     run_one: &mut dyn FnMut(C) -> anyhow::Result<Option<String>>,
 ) -> anyhow::Result<()> {
-    let mut ed = Reedline::create();
+    let ed = Reedline::create();
     let prompt = reedline::DefaultPrompt::new(
         reedline::DefaultPromptSegment::Empty,
         reedline::DefaultPromptSegment::Empty,
     );
+    run_repl_on_stdin_customized(ed, &prompt, run_one)
+}
+
+/// Runs a REPL using stdin/stdout with a customized `Reedline` and `Prompt`
+///
+/// See docs for [`run_repl_on_stdin`]
+pub fn run_repl_on_stdin_customized<C: Parser>(
+    mut ed: Reedline,
+    prompt: &dyn Prompt,
+    run_one: &mut dyn FnMut(C) -> anyhow::Result<Option<String>>,
+) -> anyhow::Result<()> {
     loop {
-        match ed.read_line(&prompt) {
+        match ed.read_line(prompt) {
             Ok(Signal::Success(buffer)) => {
                 // Strip everything after '#' as a comment.
                 let entry = match buffer.split_once('#') {

dev-tools/repl-utils/src/lib.rs

@@ -8,6 +8,7 @@ license = "MPL-2.0"
 workspace = true
 
 [dependencies]
+anyhow.workspace = true
 bcs.workspace = true
 bootstore.workspace = true
 camino.workspace = true
@@ -36,6 +37,18 @@ omicron-workspace-hack.workspace = true
 
 [dev-dependencies]
 assert_matches.workspace = true
+dropshot.workspace = true
 omicron-test-utils.workspace = true
 proptest.workspace = true
+serde_json.workspace = true
 test-strategy.workspace = true
+trust-quorum-test-utils.workspace = true
+
+[features]
+# Impl `PartialEq` and `Eq` for types implementing `subtle::ConstantTimeEq` when
+# this feature is enabled.
+# 
+# This is of unknown risk. The rust compiler may obviate the security of using
+# subtle when we do this. On the other hand its very useful for testing and
+# debugging outside of production.
+danger_partial_eq_ct_wrapper = ["gfss/danger_partial_eq_ct_wrapper"]

trust-quorum/Cargo.toml

@@ -21,3 +21,14 @@ omicron-workspace-hack.workspace = true
 [dev-dependencies]
 proptest.workspace = true
 test-strategy.workspace = true
+
+[features]
+
+
+# Impl `PartialEq` and `Eq` for types implementing `subtle::ConstantTimeEq` when
+# this feature is enabled.
+# 
+# This is of unknown risk. The rust compiler may obviate the security of using
+# subtle when we do this. On the other hand its very useful for testing and
+# debugging outside of production.
+danger_partial_eq_ct_wrapper = []

trust-quorum/gfss/Cargo.toml

@@ -32,7 +32,7 @@ use zeroize::Zeroize;
 
 /// An element in a finite field of prime power 2^8
 ///
-/// We explicitly don't enable the equality operators to prevent ourselves from
+/// We explicitly don't derive the equality operators to prevent ourselves from
 /// accidentally using those instead of the constant time ones.
 #[repr(transparent)]
 #[derive(Debug, Clone, Copy, Zeroize, Serialize, Deserialize)]
@@ -120,6 +120,15 @@ impl ConstantTimeEq for Gf256 {
     }
 }
 
+#[cfg(feature = "danger_partial_eq_ct_wrapper")]
+impl PartialEq for Gf256 {
+    fn eq(&self, other: &Self) -> bool {
+        self.ct_eq(&other).into()
+    }
+}
+#[cfg(feature = "danger_partial_eq_ct_wrapper")]
+impl Eq for Gf256 {}
+
 impl Add for Gf256 {
     type Output = Self;
 

trust-quorum/gfss/src/gf256.rs

@@ -137,6 +137,16 @@ impl Share {
     }
 }
 
+#[cfg(feature = "danger_partial_eq_ct_wrapper")]
+impl PartialEq for Share {
+    fn eq(&self, other: &Self) -> bool {
+        self.x_coordinate == other.x_coordinate
+            && self.y_coordinates == other.y_coordinates
+    }
+}
+#[cfg(feature = "danger_partial_eq_ct_wrapper")]
+impl Eq for Share {}
+
 impl std::fmt::Debug for Share {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("KeyShareGf256").finish()

trust-quorum/gfss/src/shamir.rs

@@ -8,17 +8,17 @@
 //! share for that configuration it must collect a threshold of key shares from
 //! other nodes so  that it can compute its own key share.
 
-use crate::crypto::Sha3_256Digest;
 use crate::{
     Alarm, Configuration, Epoch, NodeHandlerCtx, PeerMsgKind, PlatformId,
 };
 use gfss::gf256::Gf256;
 use gfss::shamir::{self, Share};
-use slog::{Logger, error, o, warn};
+use slog::{Logger, error, o};
 use std::collections::BTreeMap;
 
 /// In memory state that tracks retrieval of key shares in order to compute
 /// this node's key share for a given configuration.
+#[derive(Debug, Clone)]
 pub struct KeyShareComputer {
     log: Logger,
 
@@ -28,6 +28,17 @@ pub struct KeyShareComputer {
     collected_shares: BTreeMap<PlatformId, Share>,
 }
 
+#[cfg(feature = "danger_partial_eq_ct_wrapper")]
+impl PartialEq for KeyShareComputer {
+    fn eq(&self, other: &Self) -> bool {
+        self.config == other.config
+            && self.collected_shares == other.collected_shares
+    }
+}
+
+#[cfg(feature = "danger_partial_eq_ct_wrapper")]
+impl Eq for KeyShareComputer {}
+
 impl KeyShareComputer {
     pub fn new(
         log: &Logger,
@@ -54,7 +65,9 @@ impl KeyShareComputer {
         ctx: &mut impl NodeHandlerCtx,
         peer: PlatformId,
     ) {
-        if !self.collected_shares.contains_key(&peer) {
+        if self.config.members.contains_key(&peer)
+            && !self.collected_shares.contains_key(&peer)
+        {
             ctx.send(peer, PeerMsgKind::GetShare(self.config.epoch));
         }
     }
@@ -70,55 +83,29 @@ impl KeyShareComputer {
         epoch: Epoch,
         share: Share,
     ) -> bool {
-        // Are we trying to retrieve shares for `epoch`?
-        if epoch != self.config.epoch {
-            warn!(
-                self.log,
-                "Received Share from node with wrong epoch";
-                "received_epoch" => %epoch,
-                "from" => %from
-            );
-            return false;
-        }
-
-        // Is the sender a member of the configuration `epoch`?
-        // Was the sender a member of the configuration at `old_epoch`?
-        let Some(expected_digest) = self.config.members.get(&from) else {
-            warn!(
-                self.log,
-                "Received Share from unexpected node";
-                "epoch" => %epoch,
-                "from" => %from
-            );
+        if !crate::validate_share(&self.log, &self.config, &from, epoch, &share)
+        {
+            // Logging done inside `validate_share`
             return false;
         };
 
-        // Does the share hash match what we expect?
-        let mut digest = Sha3_256Digest::default();
-        share.digest::<sha3::Sha3_256>(&mut digest.0);
-        if digest != *expected_digest {
-            error!(
-                self.log,
-                "Received share with invalid digest";
-                "epoch" => %epoch,
-                "from" => %from
-            );
-            return false;
-        }
-
         // A valid share was received. Is it new?
         if self.collected_shares.insert(from, share).is_some() {
             return false;
         }
 
-        // Do we have enough shares to computer our rack share?
+        // Do we have enough shares to compute our rack share?
         if self.collected_shares.len() < self.config.threshold.0 as usize {
             return false;
         }
 
+        // Share indices are assigned according the configuration membership's
+        // key order, when the configuration is constructed.
+        //
         // What index are we in the configuration? This is our "x-coordinate"
         // for our key share calculation. We always start indexing from 1, since
         // 0 is the rack secret.
+        //
         let index =
             self.config.members.keys().position(|id| id == ctx.platform_id());
 

trust-quorum/src/compute_key_share.rs

@@ -7,6 +7,7 @@
 use crate::crypto::{EncryptedRackSecrets, RackSecret, Sha3_256Digest};
 use crate::validators::ValidatedReconfigureMsg;
 use crate::{Epoch, PlatformId, Threshold};
+use daft::Diffable;
 use gfss::shamir::{Share, SplitError};
 use iddqd::{IdOrdItem, id_upcast};
 use omicron_uuid_kinds::RackUuid;
@@ -31,7 +32,15 @@ pub enum ConfigurationError {
 ///
 /// Only valid for non-lrtq configurations
 #[derive(
-    Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize,
+    Debug,
+    Clone,
+    PartialEq,
+    Eq,
+    PartialOrd,
+    Ord,
+    Serialize,
+    Deserialize,
+    Diffable,
 )]
 pub struct Configuration {
     /// Unique Id of the rack