From 3103549cba603173a5dc0aefa8f9c201d3d1a6e5 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 2 Apr 2022 12:01:20 -0400 Subject: [PATCH 1/6] socksproto: remove some unused accessors. --- crates/tor-socksproto/src/msg.rs | 15 --------------- doc/semver_status.md | 3 +++ 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/crates/tor-socksproto/src/msg.rs b/crates/tor-socksproto/src/msg.rs index addd03d50..9f85d5f8f 100644 --- a/crates/tor-socksproto/src/msg.rs +++ b/crates/tor-socksproto/src/msg.rs @@ -19,15 +19,6 @@ pub enum SocksVersion { V5, } -impl From for u8 { - fn from(v: SocksVersion) -> u8 { - match v { - SocksVersion::V4 => 4, - SocksVersion::V5 => 5, - } - } -} - impl TryFrom for SocksVersion { type Error = Error; fn try_from(v: u8) -> Result { @@ -184,12 +175,6 @@ impl AsRef for SocksHostname { } } -impl From for String { - fn from(s: SocksHostname) -> String { - s.0 - } -} - impl SocksRequest { /// Create a SocksRequest with a given set of fields. /// diff --git a/doc/semver_status.md b/doc/semver_status.md index 948b59f94..51f600ac6 100644 --- a/doc/semver_status.md +++ b/doc/semver_status.md @@ -18,3 +18,6 @@ We can delete older sections here after we bump the releases. ## Since Arti 0.2.0 +### tor-socksproto + +BREAKING: Removed some unused accessors. From 95c8b518a7c25251b7331f402bb4e62ae2b4a24f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 2 Apr 2022 12:19:13 -0400 Subject: [PATCH 2/6] checkable: additional unit tests for more APIs. --- crates/tor-checkable/src/timed.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/crates/tor-checkable/src/timed.rs b/crates/tor-checkable/src/timed.rs index e83da93c4..a88e23119 100644 --- a/crates/tor-checkable/src/timed.rs +++ b/crates/tor-checkable/src/timed.rs @@ -203,16 +203,44 @@ mod test { let eu = SystemTime::UNIX_EPOCH + one_day * 8705; let za = SystemTime::UNIX_EPOCH + one_day * 8882; + // check_valid_at let tr = TimerangeBound::new("Hello world", cz_sk..eu); assert!(tr.check_valid_at(&za).is_err()); let tr = TimerangeBound::new("Hello world", cz_sk..za); assert_eq!(tr.check_valid_at(&eu), Ok("Hello world")); + // check_valid_now let tr = TimerangeBound::new("hello world", de..); assert_eq!(tr.check_valid_now(), Ok("hello world")); let tr = TimerangeBound::new("hello world", ..za); assert!(tr.check_valid_now().is_err()); + + // Now try check_valid_at_opt() api + let tr = TimerangeBound::new("hello world", de..); + assert_eq!(tr.check_valid_at_opt(None), Ok("hello world")); + let tr = TimerangeBound::new("hello world", de..); + assert_eq!( + tr.check_valid_at_opt(Some(SystemTime::now())), + Ok("hello world") + ); + let tr = TimerangeBound::new("hello world", ..za); + assert!(tr.check_valid_at_opt(None).is_err()); + } + + #[cfg(feature = "experimental-api")] + #[test] + fn test_dangerous() { + let t1 = SystemTime::now(); + let t2 = t1 + Duration::from_secs(60 * 525600); + let tr = TimerangeBound::new("cups of coffee", t1..=t2); + + assert_eq!(tr.dangerously_peek(), &"cups of coffee"); + + let (a, b) = tr.dangerously_into_parts(); + assert_eq!(a, "cups of coffee"); + assert_eq!(b.0, Bound::Included(t1)); + assert_eq!(b.1, Bound::Included(t2)); } } From 58a1e89c8dbbe8233337d0220854fe268a754d6a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 2 Apr 2022 15:48:41 -0400 Subject: [PATCH 3/6] rtmock: add the ability to make a connection time out. --- crates/tor-rtmock/src/net.rs | 35 +++++++++++++++++++++++++++++------ doc/semver_status.md | 5 +++++ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/crates/tor-rtmock/src/net.rs b/crates/tor-rtmock/src/net.rs index 983948d46..5575e2475 100644 --- a/crates/tor-rtmock/src/net.rs +++ b/crates/tor-rtmock/src/net.rs @@ -40,7 +40,7 @@ type ConnReceiver = mpsc::Receiver<(LocalStream, SocketAddr)>; /// its own view of its address(es) on the network. pub struct MockNetwork { /// A map from address to the entries about listeners there. - listening: Mutex>, + listening: Mutex>, } /// The `MockNetwork`'s view of a listener. @@ -55,6 +55,15 @@ struct ListenerEntry { tls_cert: Option>, } +/// A possible non-error behavior from an address +#[derive(Clone)] +enum AddrBehavior { + /// There's a listener at this address, which would like to reply. + Listener(ListenerEntry), + /// All connections sent to this address will time out. + Timeout, +} + /// A view of a single host's access to a MockNetwork. /// /// Each simulated host has its own addresses that it's allowed to listen on, @@ -159,6 +168,16 @@ impl MockNetwork { } } + /// Add a "black hole" at the given address, where all traffic will time out. + pub fn add_blackhole(&self, address: SocketAddr) -> IoResult<()> { + let mut listener_map = self.listening.lock().expect("Poisoned lock for listener"); + if listener_map.contains_key(&address) { + return Err(err(ErrorKind::AddrInUse)); + } + listener_map.insert(address, AddrBehavior::Timeout); + Ok(()) + } + /// Tell the listener at `target_addr` (if any) about an incoming /// connection from `source_addr` at `peer_stream`. /// @@ -177,12 +196,16 @@ impl MockNetwork { let listener_map = self.listening.lock().expect("Poisoned lock for listener"); listener_map.get(&target_addr).map(Clone::clone) }; - if let Some(mut entry) = entry { - if entry.send.send((peer_stream, source_addr)).await.is_ok() { - return Ok(entry.tls_cert); + match entry { + Some(AddrBehavior::Listener(mut entry)) => { + if entry.send.send((peer_stream, source_addr)).await.is_ok() { + return Ok(entry.tls_cert); + } + Err(err(ErrorKind::ConnectionRefused)) } + Some(AddrBehavior::Timeout) => futures::future::pending().await, + None => Err(err(ErrorKind::ConnectionRefused)), } - Err(err(ErrorKind::ConnectionRefused)) } /// Register a listener at `addr` and return the ConnReceiver @@ -203,7 +226,7 @@ impl MockNetwork { let entry = ListenerEntry { send, tls_cert }; - listener_map.insert(addr, entry); + listener_map.insert(addr, AddrBehavior::Listener(entry)); Ok(recv) } diff --git a/doc/semver_status.md b/doc/semver_status.md index 51f600ac6..529d3c102 100644 --- a/doc/semver_status.md +++ b/doc/semver_status.md @@ -21,3 +21,8 @@ We can delete older sections here after we bump the releases. ### tor-socksproto BREAKING: Removed some unused accessors. + +### tor-rtmock + +MODIFIED: Added add_blackhole to MockNetwork. + From 8d823dd2e5f2f33db7b1d10ce4028898b1dbcfc0 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 2 Apr 2022 15:50:56 -0400 Subject: [PATCH 4/6] chanmgr: add tests for connect_one. --- crates/tor-chanmgr/src/builder.rs | 86 ++++++++++++++++++++++++++++++- 1 file changed, 84 insertions(+), 2 deletions(-) diff --git a/crates/tor-chanmgr/src/builder.rs b/crates/tor-chanmgr/src/builder.rs index adca41dfb..cfde3b00c 100644 --- a/crates/tor-chanmgr/src/builder.rs +++ b/crates/tor-chanmgr/src/builder.rs @@ -229,10 +229,10 @@ mod test { }; use pk::ed25519::Ed25519Identity; use pk::rsa::RsaIdentity; - use std::net::SocketAddr; use std::time::{Duration, SystemTime}; + use std::{net::SocketAddr, str::FromStr}; use tor_proto::channel::Channel; - use tor_rtcompat::{test_with_one_runtime, TcpListener}; + use tor_rtcompat::{test_with_one_runtime, SleepProviderExt, TcpListener}; use tor_rtmock::{io::LocalStream, net::MockNetwork, MockSleepRuntime}; // Make sure that the builder can build a real channel. To test @@ -303,5 +303,87 @@ mod test { }) } + #[test] + fn test_connect_one() { + let client_addr = "192.0.1.16".parse().unwrap(); + // We'll put a "relay" at this address + let addr1 = SocketAddr::from_str("192.0.2.17:443").unwrap(); + // We'll put nothing at this address, to generate errors. + let addr2 = SocketAddr::from_str("192.0.3.18:443").unwrap(); + // Well put a black hole at this address, to generate timeouts. + let addr3 = SocketAddr::from_str("192.0.4.19:443").unwrap(); + // We'll put a "relay" at this address too + let addr4 = SocketAddr::from_str("192.0.9.9:443").unwrap(); + + test_with_one_runtime!(|rt| async move { + // Stub out the internet so that this connection can work. + let network = MockNetwork::new(); + + // Set up a client and server runtime with a given IP + let client_rt = network + .builder() + .add_address(client_addr) + .runtime(rt.clone()); + let server_rt = network + .builder() + .add_address(addr1.ip()) + .add_address(addr4.ip()) + .runtime(rt.clone()); + let _listener = server_rt.mock_net().listen(&addr1).await.unwrap(); + let _listener2 = server_rt.mock_net().listen(&addr4).await.unwrap(); + // TODO: Because this test doesn't mock time, there will actually be + // delays as we wait for connections to this address to time out. It + // would be good to use MockSleepProvider instead, once we figure + // out how to make it both reliable and convenient. + network.add_blackhole(addr3).unwrap(); + + // No addresses? Can't succeed. + let failure = connect_to_one(&client_rt, &[]).await; + assert!(failure.is_err()); + + // Connect to a set of addresses including addr1? That's a success. + for addresses in [ + &[addr1][..], + &[addr1, addr2][..], + &[addr2, addr1][..], + &[addr1, addr3][..], + &[addr3, addr1][..], + &[addr1, addr2, addr3][..], + &[addr3, addr2, addr1][..], + ] { + let (_conn, addr) = connect_to_one(&client_rt, addresses).await.unwrap(); + assert_eq!(addr, addr1); + } + + // Connect to a set of addresses including addr2 but not addr1? + // That's an error of one kind or another. + for addresses in [ + &[addr2][..], + &[addr2, addr3][..], + &[addr3, addr2][..], + &[addr3][..], + ] { + let expect_timeout = addresses.contains(&addr3); + let failure = rt + .timeout( + Duration::from_millis(300), + connect_to_one(&client_rt, addresses), + ) + .await; + if expect_timeout { + assert!(failure.is_err()); + } else { + assert!(failure.unwrap().is_err()); + } + } + + // Connect to addr1 and addr4? The first one should win. + let (_conn, addr) = connect_to_one(&client_rt, &[addr1, addr4]).await.unwrap(); + assert_eq!(addr, addr1); + let (_conn, addr) = connect_to_one(&client_rt, &[addr4, addr1]).await.unwrap(); + assert_eq!(addr, addr4); + }); + } + // TODO: Write tests for timeout logic, once there is smarter logic. } From f59f68d32de287af40201c9936675b104fb16797 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 2 Apr 2022 16:01:30 -0400 Subject: [PATCH 5/6] chanmgr: add a test for AbstractChannel::duration_unused --- crates/tor-chanmgr/src/builder.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/tor-chanmgr/src/builder.rs b/crates/tor-chanmgr/src/builder.rs index cfde3b00c..be82fe626 100644 --- a/crates/tor-chanmgr/src/builder.rs +++ b/crates/tor-chanmgr/src/builder.rs @@ -274,7 +274,7 @@ mod test { // Tell the client to believe in a different timestamp. client_rt.jump_to(now); - // Create the channelbuilder that we want to test. + // Create the channel builder that we want to test. let (snd, _rcv) = crate::event::channel(); let builder = ChanBuilder::new(client_rt, snd); @@ -298,6 +298,14 @@ mod test { let chan = r1.unwrap(); assert_eq!(chan.ident(), &ed); assert!(chan.is_usable()); + // In theory, time could pass here, so we can't just use + // "assert_eq!(dur_unused, dur_unused2)". + let dur_unused = Channel::duration_unused(&chan); + let dur_unused_2 = AbstractChannel::duration_unused(&chan); + let dur_unused_3 = Channel::duration_unused(&chan); + assert!(dur_unused.unwrap() <= dur_unused_2.unwrap()); + assert!(dur_unused_2.unwrap() <= dur_unused_3.unwrap()); + r2.unwrap(); Ok(()) }) From 24b30c56dbe8784a03f39a9e082d1c271717d707 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 2 Apr 2022 16:06:59 -0400 Subject: [PATCH 6/6] chanmgr: tests for ConnStatus::usable --- crates/tor-chanmgr/src/event.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/tor-chanmgr/src/event.rs b/crates/tor-chanmgr/src/event.rs index 9a2a21e25..3bfb08c6d 100644 --- a/crates/tor-chanmgr/src/event.rs +++ b/crates/tor-chanmgr/src/event.rs @@ -351,6 +351,7 @@ mod test { assert_float_eq!(s1.frac(), 0.0, abs <= TOL); assert!(s1.eq(&s1)); assert!(s1.blockage().is_none()); + assert!(!s1.usable()); let s2 = ConnStatus { online: Some(false), @@ -365,6 +366,7 @@ mod test { s2.blockage().unwrap().to_string(), "unable to connect to the internet" ); + assert!(!s2.usable()); let s3 = ConnStatus { online: Some(true), @@ -374,6 +376,7 @@ mod test { assert_float_eq!(s3.frac(), 0.5, abs <= TOL); assert_eq!(s3.blockage(), None); assert!(!s3.eq(&s1)); + assert!(!s3.usable()); let s4 = ConnStatus { online: Some(true), @@ -390,6 +393,7 @@ mod test { assert!(!s4.eq(&s2)); assert!(!s4.eq(&s3)); assert!(s4.eq(&s4)); + assert!(!s4.usable()); let s5 = ConnStatus { online: Some(true), @@ -400,6 +404,7 @@ mod test { assert!(s5.blockage().is_none()); assert!(s5.eq(&s5)); assert!(!s5.eq(&s4)); + assert!(s5.usable()); } #[test]