T2. Add isolated Tor connection API, but don't enable it by default #3303

teor2345 · 2021-12-28T00:52:18Z

Motivation

When Zebra sends user-generated transactions over the internet, the transaction IDs can be linked to the sending and receiving IP addresses. This can link people with specific Zcash transfers. (But the amount of information disclosed is much less for fully shielded transactions.)

But if we send user-generated transactions over Tor, it's much harder to link them to the sender's IP address.

It's ok to merge this PR because:

the Tor code changes are in a new API, in a newly created separate Rust modules
the Tor feature is off by default at compile-time, and it uses a function that isn't called by other code at runtime
the arti dependency is a specific public crate patch release, so it won't change unless we change it
it will take us less effort to update to each new arti release

Solution

Tor API:

Add a method for isolated anonymised Tor connections to a specific hostname, closes Merge minimal Tor integration for future zebra-client work #1643
Use a shared tor client instance for all isolated connections

Dependencies:

Add arti as a zebra-network dependency
- This upgrades a bunch of dependencies in the lockfile, but we'll have to do those upgrades eventually anyway
Make tor support optional, activate it via a new "tor" feature

Tests:

Add tests for isolated tor connections
Silence a spurious tor warning in tests

Close #1643

Review

This PR is ready for review, but it's based on PR #3302, so we should keep it in draft until that PR merges.

Let's assign reviewers when people are back from leave.

Reviewer Checklist

Code compiles and tests pass with cargo test --features tor

jvff

Looks good overall, I only added some minor things. It would be nice if the test vectors were refactored a bit to reduce repeated code though.

zebra-network/src/peer/handshake.rs

zebra-network/src/isolated.rs

zebra-network/src/isolated/tests/vectors.rs

zebra-network/src/isolated/tor.rs

zebra-network/src/isolated.rs

…stname

codecov · 2022-01-24T05:14:43Z

Codecov Report

Merging #3303 (1495f66) into main (a1f4cec) will decrease coverage by 0.01%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##             main    #3303      +/-   ##
==========================================
- Coverage   78.43%   78.42%   -0.02%     
==========================================
  Files         267      267              
  Lines       31530    31526       -4     
==========================================
- Hits        24732    24724       -8     
- Misses       6798     6802       +4

teor2345 · 2022-01-25T00:45:06Z

@Mergifyio update

mergify · 2022-01-25T00:45:37Z

update

✅ Branch has been successfully updated

teor2345 · 2022-01-25T00:47:18Z

This PR is blocking ticket #3234 due to merge conflicts. @jvff would you mind reviewing it tomorrow, so we can get it merged?

jvff

Approved! Yay, progress towards getting Tor support in Zebra :D

zebra-network/src/isolated/tor.rs

dconnolly · 2022-01-25T20:04:24Z

zebra-network/src/isolated/tor.rs

+/// Returns a new tor client instance, and updates [`SHARED_TOR_CLIENT`].
+///
+/// If there is a bootstrap error, [`SHARED_TOR_CLIENT`] is not modified.
+async fn new_tor_client() -> Result<TorClient<TokioRuntimeHandle>, BoxError> {
+    let runtime = tokio::runtime::Handle::current();
+    let runtime = TokioRuntimeHandle::new(runtime);
+    let tor_client = TorClient::bootstrap(runtime, TorClientConfig::default()).await?;
+
+    // # Correctness
+    //
+    // It is ok for multiple tasks to race, because all tor clients have identical configs.
+    // And all connections are isolated, regardless of whether they use a new or cloned client.
+    // (Any replaced clients will be dropped.)
+    let mut shared_tor_client = SHARED_TOR_CLIENT
+        .lock()
+        .expect("panic in shared tor client mutex guard");
+    *shared_tor_client = Some(tor_client.isolated_client());
+
+    Ok(tor_client)
+}


@teor2345 Looking at the rustdoc for TorClient and its config structs, it doesn't seem obvious how to config/ensure a new circuit is used every time we create a new TorClient here, or perhaps every time we use it. Am I missing context or this work arti needs to do first? @teor2345

I gave similar feedback to the Tor team, and they have opened a ticket to improve the API:
https://forum.zcashcommunity.com/t/arti-a-pure-rust-tor-implementation-for-zcash-and-beyond/38776/58?u=teor

I expect we will rewrite this code when we upgrade to the next arti version.

But here's what's going on in detail:

connect_isolated_tor either boostraps a new client, or creates an isolated clone of the shared client instance.

new_tor_client is a once-off method for bootstrapping a new client.

The isolated_client method ensures that the shared client's streams are on different circuits from the returned client's streams. (And if multiple initial connections race, and bootstrap multiple clients, all their streams are independent, because they make independent connections.)

cloned_tor_client is the method for cloning a new isolated client. It calls isolated_client for each clone.

zebra-network/src/peer/handshake.rs

rex4539 · 2022-02-02T09:53:59Z

Is there any upside (from a privacy-only standpoint, I don't care about other aspects) to use arti instead of just running zebra through proxychains-ng?

The latter proxies everything through tor, and its the way I've been using zebra so far.

teor2345 · 2022-02-02T19:46:18Z

Is there any upside (from a privacy-only standpoint, I don't care about other aspects) to use arti instead of just running zebra through proxychains-ng?

Currently, Zebra does not send any data through arti, even when the arti feature is active. This PR is only for a draft isolated Tor connection API.

In the future, Zebra might send user-generated transactions through arti, after we have completed lightwalletd or other client support.

We want to avoid syncing the entire blockchain over Tor. If hundreds of Zebra clients did that, it would put a lot of load on the Tor network. Particularly after a Zebra database upgrade, when nodes sync the full chain again.

The latter proxies everything through tor, and its the way I've been using zebra so far.

I'm glad to know that this configuration works!

I think this configuration is fine in general. But there are some things you should be aware of.

Zebra's initial sync of 20 GB of Zcash blockchain data might make you stand out on the Tor network. This could reduce your anonymity, even when using Tor. (Zebra does an initial sync every time the database format changes.)

Since all the blockchain data is public, and Zebra doesn't generate transactions yet, there isn't much sensitive information that gets hidden using Tor.

Some possibly sensitive data is the block you're currently syncing, the exact content of your node's mempool, and the fact that your home IP address is using Zcash.

teor2345 added A-dependencies Area: Dependency file updates C-enhancement Category: This is an improvement C-security Category: Security issues I-privacy Zebra discloses private information P-Optional A-network Area: Network protocol updates or fixes labels Dec 28, 2021

teor2345 self-assigned this Dec 28, 2021

teor2345 force-pushed the isolated-with-tor branch from 0f76943 to a9104ff Compare December 28, 2021 00:53

teor2345 mentioned this pull request Dec 28, 2021

Fix a type resolution error in the tests #3304

Merged

2 tasks

teor2345 force-pushed the isolated-generic branch from 8fccc5d to 4593eed Compare January 6, 2022 00:16

teor2345 linked an issue Jan 7, 2022 that may be closed by this pull request

Merge minimal Tor integration for future zebra-client work #1643

Closed

12 tasks

Base automatically changed from isolated-generic to main January 14, 2022 19:35

mpguerra marked this pull request as ready for review January 19, 2022 09:25

mpguerra requested a review from jvff January 19, 2022 09:26

jvff previously approved these changes Jan 20, 2022

View reviewed changes

teor2345 added 6 commits January 24, 2022 14:17

Add arti as a zebra-network dependency

677e9b8

Add a method for isolated anonymised Tor connections to a specific ho…

ef94b0c

…stname

Add tests for isolated tor connections

555cbb4

Use a shared tor client instance for all isolated connections

7926081

Silence a spurious tor warning in tests

a8655f3

Make tor support optional, activate it via a new "tor" feature

3c407e2

teor2345 dismissed jvff’s stale review via 1fe25a9 January 24, 2022 04:37

teor2345 force-pushed the isolated-with-tor branch from 8c2630b to 1fe25a9 Compare January 24, 2022 04:37

teor2345 added 3 commits January 24, 2022 14:40

Extra Cargo.lock changes

3f266cd

fastmod AsyncReadWrite PeerTransport zebra*

adf143f

Remove unnecessary PeerTransport generics

f238100

teor2345 force-pushed the isolated-with-tor branch from 1fe25a9 to f238100 Compare January 24, 2022 04:40

teor2345 added 2 commits January 24, 2022 14:58

Refactor common test code into a function

9a5c79a

Don't drop the stream until the end of the test

ff2e152

teor2345 force-pushed the isolated-with-tor branch from 94122f0 to ff2e152 Compare January 24, 2022 05:00

teor2345 requested a review from jvff January 24, 2022 05:01

Merge branch 'main' into isolated-with-tor

1495f66

jvff approved these changes Jan 25, 2022

View reviewed changes

mergify bot merged commit 499ae89 into main Jan 25, 2022

mergify bot deleted the isolated-with-tor branch January 25, 2022 01:46

dconnolly reviewed Jan 25, 2022

View reviewed changes

zebra-network/src/isolated/tor.rs Show resolved Hide resolved

dconnolly reviewed Jan 25, 2022

View reviewed changes

zebra-network/src/peer/handshake.rs Show resolved Hide resolved

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

T2. Add isolated Tor connection API, but don't enable it by default #3303

T2. Add isolated Tor connection API, but don't enable it by default #3303

teor2345 commented Dec 28, 2021 •

edited

Loading

jvff left a comment

codecov bot commented Jan 24, 2022 •

edited

Loading

teor2345 commented Jan 25, 2022

mergify bot commented Jan 25, 2022

teor2345 commented Jan 25, 2022

jvff left a comment

dconnolly Jan 25, 2022

teor2345 Jan 26, 2022

teor2345 Jan 26, 2022

rex4539 commented Feb 2, 2022

teor2345 commented Feb 2, 2022

T2. Add isolated Tor connection API, but don't enable it by default #3303

T2. Add isolated Tor connection API, but don't enable it by default #3303

Conversation

teor2345 commented Dec 28, 2021 • edited Loading

Motivation

Solution

Review

Reviewer Checklist

jvff left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 24, 2022 • edited Loading

Codecov Report

teor2345 commented Jan 25, 2022

mergify bot commented Jan 25, 2022

✅ Branch has been successfully updated

teor2345 commented Jan 25, 2022

jvff left a comment

Choose a reason for hiding this comment

dconnolly Jan 25, 2022

Choose a reason for hiding this comment

teor2345 Jan 26, 2022

Choose a reason for hiding this comment

teor2345 Jan 26, 2022

Choose a reason for hiding this comment

rex4539 commented Feb 2, 2022

teor2345 commented Feb 2, 2022

teor2345 commented Dec 28, 2021 •

edited

Loading

codecov bot commented Jan 24, 2022 •

edited

Loading