Merge branch 'dir-munger-v2' into 'main'

Implement a directory munger to simulate pathological cases in arti-testing (v2)

Closes #397

See merge request !442

Cargo.lock

@@ -211,6 +211,10 @@ dependencies = [
  "rlimit",
  "serde",
  "tokio",
+ "tor-checkable",
+ "tor-dirmgr",
+ "tor-error",
+ "tor-netdoc",
  "tor-rtcompat",
  "tracing",
  "tracing-appender",
@@ -3515,6 +3519,8 @@ dependencies = [
  "tor-error",
  "tor-llcrypto",
  "tor-protover",
+ "visibility",
+ "visible",
  "weak-table",
 ]
 
@@ -3866,6 +3872,27 @@ version = "0.9.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f"
 
+[[package]]
+name = "visibility"
+version = "0.0.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8881d5cc0ae34e3db2f1de5af81e5117a420d2f937506c2dc20d6f4cfb069051"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
+[[package]]
+name = "visible"
+version = "0.0.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a044005fd5c0fc1ebd79c622e5606431c6b879a6a19acafb754be9926a2de73e"
+dependencies = [
+ "quote",
+ "syn",
+]
+
 [[package]]
 name = "void"
 version = "1.0.2"

crates/arti-testing/Cargo.toml

@@ -15,8 +15,19 @@ publish = false
 
 [dependencies]
 arti = { package = "arti", path = "../arti", version = "0.1.0" }
-arti-client = { package = "arti-client", path = "../arti-client", version = "0.1.0" }
+arti-client = { package = "arti-client", path = "../arti-client", version = "0.1.0", features = [
+    "dirfilter",
+] }
+tor-dirmgr = { package = "tor-dirmgr", path = "../tor-dirmgr", version = "0.1.0", features = [
+    "dirfilter",
+] }
+tor-netdoc = { package = "tor-netdoc", path = "../tor-netdoc", version = "0.1.0", features = [
+    "experimental-api",
+    "dangerous-expose-struct-fields",
+] }
+tor-checkable = { path = "../tor-checkable", version = "0.1.0", features = ["experimental-api"] }
 tor-rtcompat = { path = "../tor-rtcompat", version = "0.1.0" }
+tor-error = { path = "../tor-error", version = "0.1.0" }
 arti-config = { path = "../arti-config", version = "0.1.0" }
 
 anyhow = "1.0.23"

crates/arti-testing/src/config.rs

@@ -91,6 +91,13 @@ pub(crate) fn parse_cmdline() -> Result<Job> {
                 .value_name("SECS")
                 .global(true),
         )
+        .arg(
+            Arg::with_name("dir-filter")
+                .long("dir-filter")
+                .takes_value(true)
+                .value_name("FILTER_NAME")
+                .global(true),
+        )
         .subcommand(
             SubCommand::with_name("connect")
                 .about("Try to bootstrap and connect to an address")
@@ -169,6 +176,12 @@ pub(crate) fn parse_cmdline() -> Result<Job> {
         }
     };
 
+    let dir_filter = matches
+        .value_of("dir-filter")
+        .map(crate::dirfilter::new_filter)
+        .transpose()?
+        .unwrap_or_else(crate::dirfilter::nil_filter);
+
     let action = if let Some(_m) = matches.subcommand_matches("bootstrap") {
         Action::Bootstrap
     } else if let Some(matches) = matches.subcommand_matches("connect") {
@@ -191,6 +204,7 @@ pub(crate) fn parse_cmdline() -> Result<Job> {
         config,
         timeout,
         tcp_breakage,
+        dir_filter,
         console_log,
         expectation,
     })

crates/arti-testing/src/dirfilter.rs

+//! Support for modifying directories in various ways in order to cause
+//! different kinds of network failure.
+
+use anyhow::{anyhow, Result};
+use rand::Rng;
+use std::sync::{Arc, Mutex};
+use tor_dirmgr::filter::DirFilter;
+use tor_netdoc::{
+    doc::{
+        microdesc::Microdesc,
+        netstatus::{RouterStatus, UncheckedMdConsensus},
+    },
+    types::{family::RelayFamily, policy::PortPolicy},
+};
+
+/// Return a new directory filter as configured by a specified string.
+pub(crate) fn new_filter(s: &str) -> Result<Arc<dyn DirFilter + 'static>> {
+    Ok(match s {
+        "replace-onion-keys" => Arc::new(ReplaceOnionKeysFilter::default()),
+        "one-big-family" => Arc::new(OneBigFamilyFilter::default()),
+        "no-exit-ports" => Arc::new(NoExitPortsFilter::default()),
+        "bad-signatures" => Arc::new(BadSignaturesFilter::default()),
+        "non-existent-signing-keys" => Arc::new(NonexistentSigningKeysFilter::default()),
+        "bad-microdesc-digests" => Arc::new(BadMicrodescDigestsFilter::default()),
+        _ => {
+            return Err(anyhow!(
+                "Unrecognized filter. Options are: 
+    replace-onion-keys, one-big-family, no-exit-ports, bad-signatures,
+    non-existent-signing-keys, bad-microdesc-digests."
+            ));
+        }
+    })
+}
+
+/// A filter that doesn't do anything.
+///
+/// We define this so we can set a filter unconditionally and simplify our code a
+/// little.
+#[derive(Debug)]
+struct NilFilter;
+impl DirFilter for NilFilter {}
+
+/// Return a filter that doesn't do anything.
+pub(crate) fn nil_filter() -> Arc<dyn DirFilter + 'static> {
+    Arc::new(NilFilter)
+}
+
+/// A filter to replace onion keys with junk.
+///
+/// Doing this means that all CREATE2 attempts via ntor will fail.  (If any were
+/// to succeed, they'd fail when they try to extend.)
+#[derive(Debug, Default)]
+struct ReplaceOnionKeysFilter;
+
+impl DirFilter for ReplaceOnionKeysFilter {
+    fn filter_md(&self, mut md: Microdesc) -> tor_dirmgr::Result<Microdesc> {
+        let junk_key: [u8; 32] = rand::thread_rng().gen();
+        md.ntor_onion_key = junk_key.into();
+        Ok(md)
+    }
+}
+
+/// A filter to put all relays into a family with one another.
+///
+/// This filter will prevent the client from generating any mult-hop circuits,
+/// since they'll all violate our path constraints.
+#[derive(Debug, Default)]
+struct OneBigFamilyFilter {
+    /// The family we're going to put all the microdescs into.  We set this to
+    /// contain all the identities, every time we load a consensus.
+    ///
+    /// (This filter won't do a very good job of ensuring consistency between
+    /// this family and the MDs we attach it to, but that's okay for the kind of
+    /// testing we want to do.)
+    new_family: Mutex<Arc<RelayFamily>>,
+}
+
+impl DirFilter for OneBigFamilyFilter {
+    fn filter_consensus(
+        &self,
+        consensus: UncheckedMdConsensus,
+    ) -> tor_dirmgr::Result<UncheckedMdConsensus> {
+        let mut new_family = RelayFamily::new();
+        for r in consensus.dangerously_peek().consensus.relays() {
+            new_family.push(*r.rsa_identity());
+        }
+
+        *self.new_family.lock().expect("poisoned lock") = Arc::new(new_family);
+
+        Ok(consensus)
+    }
+
+    fn filter_md(&self, mut md: Microdesc) -> tor_dirmgr::Result<Microdesc> {
+        let big_family = self.new_family.lock().expect("poisoned lock").clone();
+        md.family = big_family;
+        Ok(md)
+    }
+}
+
+/// A filter to remove all exit policies.
+///
+/// With this change, any attempt to build a circuit connecting for to an
+/// address will fail, since no exit will appear to support it.
+#[derive(Debug)]
+struct NoExitPortsFilter {
+    /// A "reject all ports" policy.
+    reject_all: Arc<PortPolicy>,
+}
+
+impl Default for NoExitPortsFilter {
+    fn default() -> Self {
+        Self {
+            reject_all: Arc::new(PortPolicy::new_reject_all()),
+        }
+    }
+}
+
+impl DirFilter for NoExitPortsFilter {
+    fn filter_md(&self, mut md: Microdesc) -> tor_dirmgr::Result<Microdesc> {
+        md.ipv4_policy = self.reject_all.clone();
+        md.ipv6_policy = self.reject_all.clone();
+        Ok(md)
+    }
+}
+
+/// A filter to replace the signatures on a consensus with invalid ones.
+///
+/// This change will cause directory validation to fail: we'll get good
+/// certificates and discover that our directory is invalid.
+#[derive(Debug, Default)]
+struct BadSignaturesFilter;
+
+impl DirFilter for BadSignaturesFilter {
+    fn filter_consensus(
+        &self,
+        consensus: UncheckedMdConsensus,
+    ) -> tor_dirmgr::Result<UncheckedMdConsensus> {
+        let (mut consensus, time_bounds) = consensus.dangerously_into_parts();
+
+        // We retain the signatures, but change the declared digest of the
+        // document. This will make all the signatures invalid.
+        consensus.siggroup.sha1 = Some(*b"can you reverse sha1");
+        consensus.siggroup.sha256 = Some(*b"sha256 preimage is harder so far");
+
+        Ok(UncheckedMdConsensus::new(consensus, time_bounds))
+    }
+}
+
+/// A filter that (nastily) claims all the authorities have changed their
+/// signing keys.
+///
+/// This change will make us go looking for a set of certificates that don't
+/// exist so that we can verify the consensus.
+#[derive(Debug, Default)]
+struct NonexistentSigningKeysFilter;
+
+impl DirFilter for NonexistentSigningKeysFilter {
+    fn filter_consensus(
+        &self,
+        consensus: UncheckedMdConsensus,
+    ) -> tor_dirmgr::Result<UncheckedMdConsensus> {
+        let (mut consensus, time_bounds) = consensus.dangerously_into_parts();
+        let mut rng = rand::thread_rng();
+        for signature in consensus.siggroup.signatures.iter_mut() {
+            let sk_fingerprint: [u8; 20] = rng.gen();
+            signature.key_ids.sk_fingerprint = sk_fingerprint.into();
+        }
+
+        Ok(UncheckedMdConsensus::new(consensus, time_bounds))
+    }
+}
+
+/// A filter that replaces all the microdesc digests with ones that don't exist.
+///
+/// This filter will let us validate the consensus, but we'll look forever for
+/// valid the microdescriptors it claims are present.
+#[derive(Debug, Default)]
+struct BadMicrodescDigestsFilter;
+
+impl DirFilter for BadMicrodescDigestsFilter {
+    fn filter_consensus(
+        &self,
+        consensus: UncheckedMdConsensus,
+    ) -> tor_dirmgr::Result<UncheckedMdConsensus> {
+        let (mut consensus, time_bounds) = consensus.dangerously_into_parts();
+        let mut rng = rand::thread_rng();
+        for rs in consensus.consensus.relays.iter_mut() {
+            rs.rs.doc_digest = rng.gen();
+        }
+
+        Ok(UncheckedMdConsensus::new(consensus, time_bounds))
+    }
+}

crates/arti-testing/src/main.rs

@@ -80,6 +80,7 @@
 #![allow(clippy::print_stdout)] // Allowed in this crate only.
 
 mod config;
+mod dirfilter;
 mod rt;
 mod traces;
 
@@ -87,6 +88,7 @@ use arti::ArtiConfig;
 use arti_client::TorClient;
 use futures::task::SpawnExt;
 use rt::badtcp::BrokenTcpProvider;
+use tor_dirmgr::filter::DirFilter;
 use tor_rtcompat::{PreferredRuntime, Runtime, SleepProviderExt};
 
 use anyhow::{anyhow, Result};
@@ -201,6 +203,9 @@ struct Job {
     /// Describes how (if at all) to break the TCP connections.
     tcp_breakage: TcpBreakage,
 
+    /// Describes how (if at all) to mess with directories.
+    dir_filter: Arc<dyn DirFilter + 'static>,
+
     /// The tracing configuration for our console log.
     console_log: String,
 
@@ -220,6 +225,7 @@ impl Job {
         let config: ArtiConfig = self.config.load()?.try_into()?;
         let client = TorClient::with_runtime(runtime)
             .config(config.tor_client_config()?)
+            .dirfilter(self.dir_filter.clone())
             .create_unbootstrapped()?;
         Ok(client)
     }

crates/tor-checkable/Cargo.toml

@@ -6,14 +6,15 @@ edition = "2018"
 license = "MIT OR Apache-2.0"
 homepage = "https://gitlab.torproject.org/tpo/core/arti/-/wikis/home"
 description = "Types to ensure that signed or time-bound data is validated before use"
-keywords = [ "tor", "arti", "typestate" ]
-categories = [ "cryptography", "rust-patterns" ]
-repository="https://gitlab.torproject.org/tpo/core/arti.git/"
+keywords = ["tor", "arti", "typestate"]
+categories = ["cryptography", "rust-patterns"]
+repository = "https://gitlab.torproject.org/tpo/core/arti.git/"
+
+[features]
+experimental-api = []
 
 [dependencies]
-tor-llcrypto = { path="../tor-llcrypto", version = "0.1.0"}
+tor-llcrypto = { path = "../tor-llcrypto", version = "0.1.0" }
 
 signature = "1"
 thiserror = "1"
-
-

crates/tor-checkable/src/timed.rs

@@ -79,6 +79,41 @@ impl<T> TimerangeBound<T> {
         let start = self.start.map(|t| t - d);
         Self { start, ..self }
     }
+
+    /// Consume this TimeRangeBound, and return its underlying time bounds and
+    /// object.
+    ///
+    /// The caller takes responsibility for making sure that the bounds are
+    /// actually checked.
+    ///
+    /// This is an experimental API. Using it voids your stability guarantees.
+    /// It is only available when this crate is compiled with the
+    /// `experimental-api` feature.
+    #[cfg(feature = "experimental-api")]
+    pub fn dangerously_into_parts(self) -> (T, (Bound<time::SystemTime>, Bound<time::SystemTime>)) {
+        (
+            self.obj,
+            (
+                self.start.map(Bound::Included).unwrap_or(Bound::Unbounded),
+                self.end.map(Bound::Included).unwrap_or(Bound::Unbounded),
+            ),
+        )
+    }
+
+    /// Return a reference to the inner object of this TimeRangeBound, without
+    /// checking the time interval.
+    ///
+    /// The caller takes responsibility for making sure that nothing is actually
+    /// done with the inner object that would rely on the bounds being correct, until
+    /// the bounds are (eventually) checked.
+    ///
+    /// This is an experimental API. Using it voids your stability guarantees.
+    /// It is only available when this crate is compiled with the
+    /// `experimental-api` feature.
+    #[cfg(feature = "experimental-api")]
+    pub fn dangerously_peek(&self) -> &T {
+        &self.obj
+    }
 }
 
 impl<T> crate::Timebound<T> for TimerangeBound<T> {

crates/tor-netdoc/Cargo.toml

@@ -29,6 +29,17 @@ ns_consensus = []
 # feature voids your "semver warrantee".
 experimental-api = []
 
+# Expose various struct fields as "pub", for testing.
+#
+# This feature is *super* dangerous for stability and correctness. If you use it
+# for anything besides testing, you are probably putting your users in danger.
+#
+# The struct fields exposed by this feature are not covered by semantic version.
+# In fact, using this feature will give you the opposite of a "semver
+# guarantee": you should be mildly surprised when your code _doesn't_ break from
+# version to version.
+dangerous-expose-struct-fields = ["visible", "visibility"]
+
 [dependencies]
 tor-llcrypto = { path = "../tor-llcrypto", version = "0.1.0" }
 tor-bytes = { path = "../tor-bytes", version = "0.1.0" }
@@ -49,6 +60,8 @@ phf = { version = "0.10.0", features = ["macros"] }
 serde = "1.0.103"
 signature = "1"
 thiserror = "1"
+visible = { version = "0.0.1", optional = true }
+visibility = { version = "0.0.1", optional = true }
 weak-table = "0.3.0"
 
 rand = { version = "0.8", optional = true }

crates/tor-netdoc/src/doc/microdesc.rs

@@ -52,6 +52,11 @@ pub type MdDigest = [u8; 32];
 
 /// A single microdescriptor.
 #[allow(dead_code)]
+#[cfg_attr(
+    feature = "dangerous-expose-struct-fields",
+    visible::StructFields(pub),
+    non_exhaustive
+)]
 #[derive(Clone, Debug)]
 pub struct Microdesc {
     /// The SHA256 digest of the text of this microdescriptor.  This

crates/tor-netdoc/src/doc/netstatus.rs

@@ -235,6 +235,11 @@ impl ConsensusFlavor {
 
 /// The signature of a single directory authority on a networkstatus document.
 #[allow(dead_code)]
+#[cfg_attr(
+    feature = "dangerous-expose-struct-fields",
+    visible::StructFields(pub),
+    non_exhaustive
+)]
 #[derive(Debug, Clone)]
 pub struct Signature {
     /// The name of the digest algorithm used to make the signature.
@@ -251,6 +256,11 @@ pub struct Signature {
 
 /// A collection of signatures that can be checked on a networkstatus document
 #[allow(dead_code)]
+#[cfg_attr(
+    feature = "dangerous-expose-struct-fields",
+    visible::StructFields(pub),
+    non_exhaustive
+)]
 #[derive(Debug, Clone)]
 pub struct SignatureGroup {
     /// The sha256 of the document itself
@@ -263,6 +273,12 @@ pub struct SignatureGroup {
 
 /// A shared-random value produced by the directory authorities.
 #[allow(dead_code)]
+#[cfg_attr(
+    feature = "dangerous-expose-struct-fields",
+    visible::StructFields(pub),
+    visibility::make(pub),
+    non_exhaustive
+)]
 #[derive(Debug, Clone)]
 struct SharedRandVal {
     /// How many authorities revealed shares that contributed to this value.
@@ -281,6 +297,12 @@ struct SharedRandVal {
 /// NOTE: this type is separate from the header parts that are only in
 /// votes or only in consensuses, even though we don't implement votes yet.
 #[allow(dead_code)]
+#[cfg_attr(
+    feature = "dangerous-expose-struct-fields",
+    visible::StructFields(pub),
+    visibility::make(pub),
+    non_exhaustive
+)]
 #[derive(Debug, Clone)]
 struct CommonHeader {
     /// What kind of consensus document is this?  Absent in votes and
@@ -308,6 +330,12 @@ struct CommonHeader {
 
 /// The header of a consensus networkstatus.
 #[allow(dead_code)]
+#[cfg_attr(
+    feature = "dangerous-expose-struct-fields",
+    visible::StructFields(pub),
+    visibility::make(pub),
+    non_exhaustive
+)]
 #[derive(Debug, Clone)]
 struct ConsensusHeader {
     /// Header fields common to votes and consensuses
@@ -326,6 +354,12 @@ struct ConsensusHeader {
 ///
 /// (Corresponds to a dir-source line.)
 #[allow(dead_code)]
+#[cfg_attr(
+    feature = "dangerous-expose-struct-fields",
+    visible::StructFields(pub),
+    visibility::make(pub),
+    non_exhaustive
+)]
 #[derive(Debug, Clone)]
 struct DirSource {
     /// human-readable nickname for this authority.
@@ -398,8 +432,8 @@ bitflags! {
 }
 
 /// Recognized weight fields on a single relay in a consensus
-#[derive(Debug, Clone, Copy)]
 #[non_exhaustive]
+#[derive(Debug, Clone, Copy)]
 pub enum RelayWeight {
     /// An unmeasured weight for a relay.
     Unmeasured(u32),
@@ -420,6 +454,12 @@ impl RelayWeight {
 
 /// All information about a single authority, as represented in a consensus
 #[allow(dead_code)]
+#[cfg_attr(
+    feature = "dangerous-expose-struct-fields",
+    visible::StructFields(pub),
+    visibility::make(pub),
+    non_exhaustive
+)]
 #[derive(Debug, Clone)]
 struct ConsensusVoterInfo {
     /// Contents of the dirsource line about an authority
@@ -433,6 +473,12 @@ struct ConsensusVoterInfo {
 
 /// The signed footer of a consensus netstatus.
 #[allow(dead_code)]
+#[cfg_attr(
+    feature = "dangerous-expose-struct-fields",
+    visible::StructFields(pub),
+    visibility::make(pub),
+    non_exhaustive
+)]
 #[derive(Debug, Clone)]
 struct Footer {
     /// Weights to be applied to certain classes of relays when choosing
@@ -477,6 +523,11 @@ pub trait RouterStatus: Sealed {
 /// TODO: This should possibly turn into a parameterized type, to represent
 /// votes and ns consensuses.
 #[allow(dead_code)]
+#[cfg_attr(
+    feature = "dangerous-expose-struct-fields",
+    visible::StructFields(pub),
+    non_exhaustive
+)]
 #[derive(Debug, Clone)]
 pub struct Consensus<RS> {
     /// Part of the header shared by all consensus types.
@@ -1401,6 +1452,11 @@ impl<RS: RouterStatus + ParseRouterStatus> Consensus<RS> {
 /// check_signature() on that result with the set of certs that you
 /// have.  Make sure only to provide authority certificates representing
 /// real authorities!
+#[cfg_attr(
+    feature = "dangerous-expose-struct-fields",
+    visible::StructFields(pub),
+    non_exhaustive
+)]
 #[derive(Debug, Clone)]
 pub struct UnvalidatedConsensus<RS> {
     /// The consensus object. We don't want to expose this until it's

crates/tor-netdoc/src/doc/netstatus/rs.rs

@@ -27,6 +27,12 @@ pub use md::MdConsensusRouterStatus;
 pub use ns::NsConsensusRouterStatus;
 
 /// Shared implementation of MdConsensusRouterStatus and NsConsensusRouterStatus.
+#[cfg_attr(
+    feature = "dangerous-expose-struct-fields",
+    visible::StructFields(pub),
+    visibility::make(pub),
+    non_exhaustive
+)]
 #[derive(Debug, Clone)]
 struct GenericRouterStatus<D> {
     /// The nickname for this relay.
@@ -150,6 +156,7 @@ pub(crate) use implement_accessors;
 
 /// Helper to decode a document digest in the format in which it
 /// appears in a given kind of routerstatus.
+#[cfg_attr(feature = "dangerous-expose-struct-fields", visibility::make(pub))]
 trait FromRsString: Sized {
     /// Try to decode the given object.
     fn decode(s: &str) -> Result<Self>;

crates/tor-netdoc/src/doc/netstatus/rs/md.rs

@@ -17,6 +17,11 @@ use tor_llcrypto::pk::rsa::RsaIdentity;
 use tor_protover::Protocols;
 
 /// A single relay's status, as represented in a microdesc consensus.
+#[cfg_attr(
+    feature = "dangerous-expose-struct-fields",
+    visible::StructFields(pub),
+    non_exhaustive
+)]
 #[derive(Debug, Clone)]
 pub struct MdConsensusRouterStatus {
     /// Underlying generic routerstatus object.

crates/tor-netdoc/src/doc/netstatus/rs/ns.rs

@@ -20,6 +20,11 @@ use std::convert::TryInto;
 /// A single relay's status, as represented in a "ns" consensus.
 ///
 /// Only available if `tor-netdoc` is built with the `ns_consensus` feature.
+#[cfg_attr(
+    feature = "dangerous-expose-struct-fields",
+    visible::StructFields(pub),
+    non_exhaustive
+)]
 #[derive(Debug, Clone)]
 pub struct NsConsensusRouterStatus {
     /// Underlying generic routerstatus object.

crates/tor-netdoc/src/doc/routerdesc.rs

@@ -91,6 +91,11 @@ pub struct RouterAnnotation {
 /// Before using this type to connect to a relay, you MUST check that
 /// it is valid, using is_expired_at().
 #[allow(dead_code)] // don't warn about fields not getting read.
+#[cfg_attr(
+    feature = "dangerous-expose-struct-fields",
+    visible::StructFields(pub),
+    non_exhaustive
+)]
 pub struct RouterDesc {
     /// Human-readable nickname for this relay.
     ///