i2p: make a time gap between creating transient sessions and using them

vasild commented at 8:29 am on March 14, 2025: contributor

Connecting to an I2P peer consists of creating a session (the SESSION CREATE command) and then connecting to the peer using that session (STREAM CONNECT ID=session_id ...).

This change is only relevant for transient sessions because when a persistent session is used it is created once and used for all connections.

Before this change Bitcoin Core would create the session and use it in quick succession. That is, the SESSION CREATE command would be immediately followed by STREAM CONNECT. This could ease network activity monitoring by an adversary.

To mitigate that, this change creates sessions upfront without an immediate demand for new sessions and later uses them. This creates a time gap between SESSION CREATE and STREAM CONNECT. Note that there is always some demand for new I2P connections due to disconnects.

Summary of the changes in the code:

Create the session from the Session constructor (send SESSION CREATE to the I2P SAM proxy). This constructor was only called when transient sessions were needed and was immediately followed by Connect() which would have created the session. So this is a noop change if viewed in isolation.
Every time we try to connect to any peer (not just I2P) create a new I2P session, up to a limit (currently 10). This way a bunch of sessions are created (SESSION CREATE) at times that are decoupled from the time when sessions will be used (STREAM CONNECT).
Tweak the code in CConnman::ConnectNode() to avoid creating session objects while holding m_unused_i2p_sessions_mutex because now creating i2p::sam::Session involves network activity and could take a few seconds.

Before (note the timestamps):

2025-03-13T09:23:40Z [opencon] [i2p:info] Transient SAM session cd7ea49a7e created 2025-03-13T09:23:43Z [opencon] [i2p] -> STREAM CONNECT ID=cd7ea49a7e DESTINATION=…

2025-03-13T09:23:51Z [opencon] [i2p:info] Transient SAM session cb0956207e created 2025-03-13T09:23:52Z [opencon] [i2p] -> STREAM CONNECT ID=cb0956207e DESTINATION=…

2025-03-13T09:24:04Z [opencon] [i2p:info] Transient SAM session 35ad67f343 created 2025-03-13T09:24:04Z [opencon] [i2p] -> STREAM CONNECT ID=35ad67f343 DESTINATION=…

After:

2025-03-13T17:08:58Z [opencon] [i2p:info] Transient SAM session 16098cd4ea created 2025-03-13T17:09:34Z [opencon] [i2p] -> STREAM CONNECT ID=16098cd4ea DESTINATION=…

2025-03-13T17:09:38Z [opencon] [i2p:info] Transient SAM session 5d209bf1bc created 2025-03-13T17:09:59Z [opencon] [i2p] -> STREAM CONNECT ID=5d209bf1bc DESTINATION=…

2025-03-13T17:09:50Z [opencon] [i2p:info] Transient SAM session e78aa6d2e7 created 2025-03-13T17:12:09Z [opencon] [i2p] -> STREAM CONNECT ID=e78aa6d2e7 DESTINATION=…

This was suggested by @eyedeekay some time ago (in 2023 actually) and has been lingering in my TODO since then :eyes:. A further suggestion is to also add a time gap between the activity cease and the closing of the session. For this, when CNode is destroyed CNode::m_i2p_sam_session would have to be moved elsewhere and destroyed at a later time. cc @zzzi2p

DrahtBot commented at 8:30 am on March 14, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32065.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	zzzi2p, 1440000bytes, eyedeekay

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#30951 (net: option to disallow v1 connection on ipv4 and ipv6 peers by stratospher)
#29415 (Broadcast own transactions only via short-lived Tor or I2P connections by vasild)
#28584 (Fuzz: extend CConnman tests by vasild)
#28463 (p2p: Increase inbound capacity for block-relay only connections by mzumsande)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

laanwj added the label P2P on Mar 14, 2025

zzzi2p commented at 5:45 pm on March 14, 2025: none

Thanks for CC’ing me. I also asked eyedeekay and orignal from i2pd to take a look.

10 standby sessions is a lot, except maybe at startup. If your I2P connections limit is 10, then you’re doubling your network resource usage. 2 or 3 is probably enough, depending on how fast your connections churn and how long is long enough for a unused session to be up to give you the anonymity you’re looking for.

A couple of alternatives:

0int max = MAX_UNUSED_I2P_SESSIONS_SIZE;
1if (uptime() > 15 minutes) max /= 3;

or

0int max = min(MAX_UNUSED_I2P_SESSIONS_SIZE,  3 + MAX_I2P_SESSIONS - current_i2p_sessions.size());

However, I’m also concerned about starting a whole bunch of sessions at once, especially at startup, and if the user started his router when he started bitcoin. If this change will double or triple or more the session-creation rate at startup, that’s probably too fast. The Java router has some tunnel build rate-limiting that may kick in. I won’t attempt any pseudocode for it, but some rate limiting in AddI2PSessionsIfNeeded() or some other special case for the first few minutes after startup is probably desirable.

Concept ACK.

1440000bytes commented at 6:44 am on March 15, 2025: none

Concept ACK

eyedeekay commented at 6:16 pm on March 15, 2025: none

I agree with zzz, the concept ACK is fine but I’m very concerned about having the session bank be 10 sessions long. I think the best method of managing the session bank sort of depends on the rate at which you’ll need them.

If you’re creating sessions fast enough to need a bank of 10 sessions, then I think at some point you have no choice but to keep the sessions alive longer and re-use them more. On the other hand, if you aren’t creating them fast enough to need a bank of 10 sessions, then keeping a bank of 10 of them is pretty wasteful. How you solve it is going to depend on the actual rates you expect people to need sessions. If the need for sessions is too unpredictable, then maybe the bank size limit needs to change size based on need from a very small number to a larger number and back.

I’m going to take a closer look at the code here and try to make some more concrete suggestions.

bitcoin deleted a comment on Mar 16, 2025

vasild force-pushed on Mar 18, 2025

vasild commented at 11:27 am on March 18, 2025: contributor

A few notes:

Bitcoin Core would try to maintain up to 12 outbound connections to peers from all networks that are reachable (IPv4, IPv6, Tor, I2P, CJDNS). From these usually just a few are I2P. So, it is reasonable to assume that from 2,3,4 to 12 I2P connections will be needed. But the case with 12 is when I2P is the only reachable network which is not advisable because of the increased Sybil likelihood.
This PR is not going to create a storm of I2P sessions creation at startup because new session creation is not being done in a tight loop, deliberately. See the timings in the logs below.
The lifetime of an I2P connection varies a lot. I would like to avoid adding assumptions about that. How often I2P connections are needed depends on the lifetime + whether a randomly selected address from the address database for the new connection will be I2P.
Session reuse would degrade privacy because Bitcoin Core would connect to >1 peer from the same I2P source address. This is when transient sessions are used, affected by this PR. Note that there is another less-private mode if Bitcoin Core is listening and accepting new I2P connections. Then a single persistent I2P session is used - it accepts new connections to its permanent address and makes all outgoing connections from that address (= a single I2P session), this PR is not changing that.
The code before this PR would keep a session for later if it cannot use it for connecting to the given I2P peer (NAMING LOOKUP returns an error). I chose 10 for MAX_UNUSED_I2P_SESSIONS_SIZE arbitrarily to avoid a broken runaway code from filling all of the Bitcoin Core’s memory with unused sessions. I think in practice the number of stored sessions is always 0 or 1 in master, before this PR.

Below is some data for when sessions are created, used and how long do they live. From startup to about 10 minutes after that, with this PR. It is just an example from my node. I guess timings will vary a lot between runs and in different environments. t0 is the startup time.

MAX_UNUSED_I2P_SESSIONS_SIZE=10:

 0t0 +  18 seconds: made new session upfront, used 93 seconds after being created, still active after 622 seconds
 1t0 +  19 seconds: made new session upfront, used 177 seconds after being created, still active after 621 seconds
 2t0 +  22 seconds: made new session upfront, used 297 seconds after being created, still active after 618 seconds
 3t0 +  26 seconds: made new session upfront, used 397 seconds after being created, still active after 614 seconds
 4t0 +  36 seconds: made new session upfront, not used in 604 seconds
 5t0 +  44 seconds: made new session upfront, not used in 596 seconds
 6t0 +  52 seconds: made new session upfront, not used in 588 seconds
 7t0 +  60 seconds: made new session upfront, not used in 580 seconds
 8t0 +  66 seconds: made new session upfront, not used in 574 seconds
 9t0 +  69 seconds: made new session upfront, not used in 571 seconds
10t0 + 244 seconds: made new session upfront, not used in 396 seconds

MAX_UNUSED_I2P_SESSIONS_SIZE=4:

0t0 +  20 seconds: made new session upfront, used 469 seconds after being created, still active after 699 seconds
1t0 +  25 seconds: made new session upfront, used 588 seconds after being created, still active after 694 seconds
2t0 +  30 seconds: made new session upfront, used 82 seconds after being created, lifetime: 125 seconds
3t0 +  43 seconds: made new session upfront, used 201 seconds after being created, lifetime: 121 seconds
4t0 + 244 seconds: made new session upfront, not used in 475 seconds
5t0 + 366 seconds: made new session upfront, not used in 353 seconds

Given the above, I changed MAX_UNUSED_I2P_SESSIONS_SIZE from 10 to 4 in 42f98c9dfa...a22a3b3c4c.

mzumsande commented at 4:16 pm on March 18, 2025: contributor

Two thoughts:

As far as I understand it (https://geti2p.net/en/about/performance/future), i2p tunnels last for 10 minutes by default. If we don’t use these extra sessions before they expire, their creation would be a waste of ressources. I would image this to be particularly relevant in a scenario where i2p is used together with other networks. When we have no need for new regular outbounds because we are full and none of our peers disconnect, we make feeler connections every 2 minutes and extra block relay conns every 5 minutes - that’s like 7 new connections every 10 minutes that would be divided over all reachable networks. Would be interesting to run with i2p and clearnet, and see how many of these tunnels expire before they are used.
When using -onlynet=i2p, this may not have much of an effect right after startup - m_unused_i2p_sessions will only grow if a connection attempt fails, for successful connections the newly formed connections will be used right away. Maybe it could make a sense to fill the m_unused_i2p_sessions buffer with 4 connections right after startup?

vasild commented at 5:14 pm on March 19, 2025: contributor

I wonder what happens after the 10 minutes? For example, if a client does SESSION CREATE to acquire a session id and then waits for 12 minutes and does STREAM CONNECT with that session id. Is a new tunnel established by the I2P router immediately and transparently to the client at the 10th minute, or when the session is actually used at the 12th minute?

Thinking about this more, given that opening of new connections is done serially, one after another (not in parallel), and occasionally (except during startup) maybe it would be best to pre-create just one session?

zzzi2p commented at 6:16 pm on March 19, 2025: none

Tunnels get built when the SAM session is opened, and are automatically rebuilt when they expire, the router does that for you as long as the SAM session stays open. A session is just a pool of inbound and outbound tunnels. I assume that’s what you are trying to do here, have the session open and ready before it’s needed.

vasild force-pushed on Mar 20, 2025

vasild closed this on Mar 20, 2025

vasild force-pushed on Mar 20, 2025

vasild commented at 12:05 pm on March 20, 2025: contributor

a22a3b3c4c...48a636cdde: rebase due to conflicts and reduce the pre-created sessions to 1, given the above. Some logs from running this:

 0974be1f338 startup +129s: made new session
 1974be1f338 startup +290s, 161s after creation: connect failed
 2974be1f338 startup +372s, 243s after creation: connect failed
 3974be1f338 startup +678s, 549s after creation: connect ok, connection duration: 1305s, destroyed after that
 4
 555723ba1fc startup +1385s: made new session
 655723ba1fc startup +1441s, 56s after creation: connect ok, connection duration: 62s, destroyed after that
 7
 81177c0761e startup +1469s: made new session
 91177c0761e startup +1853s, 384s after creation: connect ok, still active after 2786s
10
11f9a6456088 startup +1860s: made new session
12f9a6456088 startup +2282s, 422s after creation: connect ok, connection duration: 90s, destroyed after that
13
14832d073906 startup +2362s: made new session
15832d073906 startup +2367s, 5s after creation: connect failed
16832d073906 startup +2444s, 82s after creation: connect ok, connection duration: 1s, destroyed after that
17
18c7e68ca962 startup +2449s: made new session
19c7e68ca962 startup +2487s, 38s after creation: connect ok, connection duration: 2s, destroyed after that
20
21c936c53d2b startup +2453s: made new session
22c936c53d2b startup +2724s, 271s after creation: connect ok, still active after 1802s
23
247f96d02e16 startup +2727s: made new session
257f96d02e16 startup +2765s, 38s after creation: connect failed
267f96d02e16 startup +2798s, 71s after creation: connect ok, still active after 1528s
27
2838d3bccdd4 startup +2884s: made new session
2938d3bccdd4 startup +3261s, 377s after creation: connect failed
3038d3bccdd4 startup +4253s, 1369s after creation: connect ok, connection duration: 2s, destroyed after that

vasild reopened this on Mar 20, 2025

DrahtBot added the label CI failed on Mar 20, 2025

DrahtBot commented at 1:59 pm on March 20, 2025: contributor

🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/39105567001

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

i2p: make a time gap between creating transient sessions and using them

Connecting to an I2P peer consists of creating a session (the
`SESSION CREATE` command) and then connecting to the peer using that
session (`STREAM CONNECT ID=session_id ...`).

This change is only relevant for transient sessions because when a
persistent session is used it is created once and used for all
connections.

Before this change Bitcoin Core would create the session and use it in
quick succession. That is, the `SESSION CREATE` command would be
immediately followed by `STREAM CONNECT`. This could ease network
activity monitoring by an adversary.

To mitigate that, this change creates a transient session upfront
without an immediate demand for new sessions and later uses it. This
creates a time gap between `SESSION CREATE` and `STREAM CONNECT`.
Note that there is always some demand for new I2P connections due to
disconnects.

---

Summary of the changes in the code:

* Create the session from the `Session` constructor (send `SESSION CREATE`
  to the I2P SAM proxy). This constructor was only called when transient
  sessions were needed and was immediately followed by `Connect()` which
  would have created the session. So this is a noop change if viewed
  in isolation.

* Change `CConnman::m_unused_i2p_sessions` from a queue to a single
  entity. Given that normally `CConnman::ConnectNode()` is not executed
  concurrently by multiple threads, the queue could have had either 0 or
  1 entry. Simplify the code by replacing the queue with a single
  session.

* Every time we try to connect to any peer (not just I2P) pre-create a
  new spare I2P session. This way session creation is decoupled from the
  time when the session will be used (`STREAM CONNECT`).

859c259092

vasild force-pushed on Mar 20, 2025

vasild commented at 2:03 pm on March 20, 2025: contributor

48a636cdde...859c259092: fix CI

DrahtBot removed the label CI failed on Mar 20, 2025

i2p: make a time gap between creating transient sessions and using them #32065

Code Coverage & Benchmarks

Reviews

Conflicts