-
-
Notifications
You must be signed in to change notification settings - Fork 369
Firo I2P support #1771
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Firo I2P support #1771
Conversation
This commit adds I2P (Invisible Internet Project) support to Firo, following Bitcoin Core's implementation as a guide. Changes: - Add i2p.h and i2p.cpp: I2P SAM (Simple Anonymous Messaging) protocol session management for connecting to I2P peers - Update netbase.cpp: Add NET_I2P to ParseNetwork() and GetNetworkName() - Update init.cpp: Add -i2psam and -i2pacceptincoming configuration options, handle I2P network initialization and -onlynet=i2p validation - Add i2p_tests.cpp: Unit tests for I2P address parsing, serialization, network type detection, and reachability Features: - I2P SAM proxy connection via -i2psam=<ip:port> option - Support for both persistent and transient I2P sessions - Automatic I2P private key generation and storage - BIP155 (ADDRv2) compatible I2P address serialization - I2P address parsing for .b32.i2p addresses The implementation uses the SAM 3.1 protocol to communicate with the I2P router. I2P addresses are 52 character base32-encoded hashes with .b32.i2p suffix. Co-authored-by: reuben <[email protected]>
- Add I2P header include to net.cpp and net.h - Add m_i2p_sam_session member to CConnman for I2P session management - Update ConnectNode() to use I2P session for connecting to I2P peers - Create I2P SAM session in init.cpp when -i2psam is configured - Store I2P private key in data directory as i2p_private_key Co-authored-by: reuben <[email protected]>
Security fixes: - Add secure file permissions (0600) for I2P private key file on Unix - Add secure memory clearing for private key in destructor - Prevent private key leakage in error messages by adding is_sensitive flag - Properly use transient sessions when -i2pacceptincoming=0 Bug fixes: - Remove redundant and confusing memcpy in MyDestination() - Remove dead code in i2p_reachability test Co-authored-by: reuben <[email protected]>
- Fix socket leak in Hello() if HELLO handshake fails - Add SAM protocol injection protection (validate destinations) - Add validation for empty private key file (regenerate if corrupted) - Add IsSafeSAMValue() to reject control characters in SAM responses Co-authored-by: reuben <[email protected]>
|
Cursor Agent can help with this pull request. Just |
WalkthroughAdds native I2P (SAM v3.1) support: new i2p header/implementation, SAM session and key management, wiring into init and CConnman for outgoing/incoming I2P connections, new network type handling, and tests + build entries for I2P. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application (init)
participant Conn as CConnman
participant SAM as i2p::sam::Session
participant Proxy as SAM Proxy
participant Peer as I2P Peer
App->>Conn: Start with -i2psam / InitI2P(...)
Conn->>SAM: Construct Session(control_host[, keyfile])
Conn->>SAM: CreateIfNotCreatedAlready()
SAM->>Proxy: HELLO (open control connection)
Proxy-->>SAM: Reply (OK / session id)
SAM->>Proxy: DEST GENERATE / IMPORT (key handling)
Proxy-->>SAM: DESTINATION (public/private)
Conn->>SAM: Connect(i2p_peer_address)
SAM->>Proxy: STREAM CONNECT dest=...
Proxy-->>SAM: RESULT=OK + stream/socket
SAM-->>Conn: Return connected socket
Conn->>Peer: Exchange data via I2P tunnel
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/i2p.cpp`:
- Around line 508-527: The code only regenerates when m_private_key is empty but
not when it is corrupted; update the load/validation flow to detect invalid keys
by attempting to construct/validate the destination and regenerate on failure:
after reading m_private_key from m_private_key_file, try to create/validate the
MyDestination (or call whatever validate method you have) inside a try/catch (or
check return/error) and if it throws or fails log via LogPrintf including the
file name and error, call GenerateAndSavePrivateKey(sock) to regenerate, and
continue; reference m_private_key, m_private_key_file, MyDestination(),
LogPrintf, and GenerateAndSavePrivateKey(sock) when implementing the change so
corrupted/truncated files are regenerated in place.
In `@src/net.cpp`:
- Around line 399-422: The I2P SAM path is being skipped when callers pass
pszDest because addrConnect isn't populated before the IsI2P() check; update the
logic so pszDest is parsed/resolved into addrConnect prior to checking
addrConnect.IsI2P() (or alternatively ensure ConnectSocketByName/ConnectSocket
consult NET_I2P and m_i2p_sam_session for proxy routing), so that .b32.i2p
destinations hit the m_i2p_sam_session branch; modify the code paths around
pszDest, addrConnect, IsI2P(), ConnectSocketByName and ConnectSocket to populate
addrConnect (or route NET_I2P to SAM) before deciding to call the SAM session.
In `@src/test/i2p_tests.cpp`:
- Around line 109-124: The test i2p_service has a mismatch between its comment
and assertions: decide whether I2P ports should be normalized to 0 or preserved;
if ports are intentionally preserved, update the comment to state that CService
preserves a supplied port for I2P addresses (leave the assertions as-is using
service.GetPort()==0 for the zero-case and service2.GetPort()==8333 for the
preserved-case); otherwise, change the second assertion to expect 0
(service2.GetPort()==0) and adjust the comment to state I2P ports are normalized
to 0. Locate the test in i2p_service, and modify the comment or the second
BOOST_CHECK_EQUAL accordingly (references: CService constructor,
CNetAddr::SetSpecial, service, service2, IsI2P, GetPort).
🧹 Nitpick comments (3)
src/net.h (1)
24-24: Optional: forward declarei2p::sam::Sessionto slimnet.hdependencies.
net.his widely included; a forward declaration with the include moved tonet.cppcan reduce rebuilds.src/test/i2p_tests.cpp (1)
126-140: Consider asserting the exact reachability level.
If a constant likeREACH_PRIVATEis available, asserting equality will make this test more regression‑resistant.💡 Example
- int reachability = addr.GetReachabilityFrom(&addr2); - BOOST_CHECK(reachability > 0); + int reachability = addr.GetReachabilityFrom(&addr2); + BOOST_CHECK_EQUAL(reachability, REACH_PRIVATE);src/init.cpp (1)
1615-1643: Consider defaulting I2P inbound behavior to-listen.
Right now-i2psam+ default-i2pacceptincomingenables inbound I2P even when users set-listen=0. For consistency with other networks, consider defaulting-i2pacceptincomingto the-listenvalue unless explicitly set, and update help text if the default changes.♻️ Suggested change
- const bool i2p_accept_incoming = GetBoolArg("-i2pacceptincoming", true); + const bool i2p_accept_incoming = + GetBoolArg("-i2pacceptincoming", GetBoolArg("-listen", DEFAULT_LISTEN));
Bug fixes: 1. i2p_tests.cpp: Fix comment/assertion mismatch in i2p_service test - Updated comments to clarify that CService preserves ports but SAM rejects non-zero ports 2. net.cpp: Fix I2P connections when pszDest is provided - Parse .b32.i2p addresses from pszDest before checking IsI2P() - Update addrConnect with parsed I2P address for correct CNode creation - This ensures -addnode=xxx.b32.i2p works correctly 3. i2p.cpp: Validate corrupted private keys, not just empty ones - Call MyDestination() after loading private key to detect corruption - Regenerate key if validation fails (truncated/corrupted files) Co-authored-by: reuben <[email protected]>
Add ThreadI2PAcceptIncoming() to handle incoming I2P connections: - Creates a dedicated thread that listens for incoming I2P connections - Adds local I2P address to address book for network advertising - Accepts connections and creates CNode objects for each - Respects -i2pacceptincoming option (default: true) - Properly handles connection limits and eviction - Integrates with Dandelion routing - Thread is started/stopped with other network threads This matches Bitcoin Core's approach to I2P incoming connections. Co-authored-by: reuben <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/i2p.cpp`:
- Around line 207-211: The loop that calls Wait(conn.sock, MAX_WAIT_FOR_IO,
true) treats any false as a harmless timeout and can busy-loop if select()
errors; update the loop in the accept/IO routine that checks *m_interrupt to
distinguish timeout vs error by calling IsConnected() (or equivalent) on the
socket when Wait(...) returns false and, if the socket is not connected or
reports an error, break/return to avoid spinning; keep the existing continue
only for genuine timeouts and ensure you reference conn.sock, MAX_WAIT_FOR_IO,
m_interrupt, Wait(), and IsConnected() when making the change.
🧹 Nitpick comments (1)
src/i2p.cpp (1)
755-827: Consider reusing the existingConnectSocketDirectlyto avoid drift.
This helper largely duplicatesnetbase.cpplogic (and differs on socket options), which can lead to subtle behavior divergence. Exposing a shared utility would reduce maintenance overhead.
The Wait() function returns false for both timeout and socket errors. Previously, the Accept loop treated all false returns as timeouts and continued, which could cause a busy-loop if the socket had an error. Now we call IsConnected() when Wait() returns false to distinguish: - If socket has error: break out of loop with error message - If genuine timeout: continue waiting for connections This prevents CPU spinning when the socket is in an error state. Co-authored-by: reuben <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/net.cpp`:
- Around line 2483-2501: Inbound I2P connection handling in the block around
fNetworkActive/IsSelectableSocket/AttemptToEvictConnection skips ban checks, so
manual bans are ignored; update the path to consult IsBanned just like
AcceptConnection does: before accepting the peer (using conn.peer / conn.addr
and conn.sock), call IsBanned(peer) and if banned log the ban and
CloseSocket(conn.sock) and continue, mirroring AcceptConnection’s ban handling
(including any banlist reason/logging) so I2P connections cannot bypass manual
bans.
- Around line 409-425: The suffix check for .b32.i2p fails when pszDest contains
a port (e.g. .b32.i2p:0); update the block that examines pszDest (the code that
currently uses dest, suffix, CNetAddr::SetSpecial, CService and sets
i2pDest/isI2P) to first strip an optional ":port" tail before doing the
case‑insensitive ".b32.i2p" detection and before calling CNetAddr::SetSpecial so
SAM path is used instead of falling back to ConnectSocketByName; ensure the
stripped value is what you pass into SetSpecial and constructing i2pDest (but
preserve original pszDest elsewhere if needed).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/i2p.cpp`:
- Around line 763-835: The ConnectSocketDirectly function is missing the same
socket options used in netbase.cpp; after creating hSocket in
ConnectSocketDirectly, set SO_NOSIGPIPE when defined and enable TCP_NODELAY
(disable Nagle) before calling SetSocketNonBlocking; use conditional compilation
like `#ifdef` SO_NOSIGPIPE to call setsockopt(hSocket, SOL_SOCKET, SO_NOSIGPIPE,
...) and use a WIN32 vs non-WIN32 variant for setsockopt(hSocket, IPPROTO_TCP,
TCP_NODELAY, ...) so the socket behavior matches the original implementation.
The m_i2p_sam_session and interruptNet members are private in CConnman, so init.cpp cannot access them directly. Add a public InitI2P() method that encapsulates the I2P session creation. This fixes the build errors: - error: 'm_i2p_sam_session' is a private member of 'CConnman' - error: 'interruptNet' is a private member of 'CConnman' Co-authored-by: reuben <[email protected]>
1. i2p.cpp ConnectSocketDirectly: Add SO_NOSIGPIPE (BSD) and TCP_NODELAY socket options before SetSocketNonBlocking to match netbase.cpp 2. net.cpp ThreadI2PAcceptIncoming: Add IsBanned() check for incoming I2P connections to prevent banned peers from bypassing manual bans 3. net.cpp I2P pszDest parsing: Strip optional :port suffix before checking for .b32.i2p suffix to handle cases like 'xxx.b32.i2p:0' Co-authored-by: reuben <[email protected]>
1. EncodeBase32: Use vector instead of pointer range - the function expects Span<const unsigned char>, not (ptr, ptr, bool) 2. ConnectSocketDirectly: Add forward declaration inside namespace sam since the function is defined at end of file but called earlier Co-authored-by: reuben <[email protected]>
I2P uses base32 without padding, but Firo's DecodeBase32 function expects proper RFC 4648 padding. For 52-character I2P addresses (8n+4), 4 '=' padding characters are required. This fixes the i2p_tests test suite failures where SetSpecial() was returning false for valid I2P addresses. Co-authored-by: reuben <[email protected]>
1. IsRoutable() now returns false for privacy networks (Tor, I2P, CJDNS) as they are not routable on the regular internet 2. Fix i2p_reachability test: the address was 53 characters instead of the required 52 characters for I2P base32 addresses Co-authored-by: reuben <[email protected]>
PR intention
This PR adds I2P (Invisible Internet Project) support to Firo, allowing nodes to connect to and accept connections from I2P peers via a SAM (Simple Anonymous Messaging) proxy. This enhances network privacy and censorship resistance for Firo users.
Code changes brief
src/i2p.h,src/i2p.cpp): Implement I2P SAM 3.1 protocol for session management, including private key handling, destination generation, and socket communication with the SAM proxy.src/netbase.cpp,src/net.h,src/net.cpp): IntroduceNET_I2Pfor address recognition, integratei2p::sam::SessionintoCConnman, and modifyConnectNodeto handle I2P connections.src/init.cpp): Add-i2psam=<ip:port>and-i2pacceptincomingoptions, along with initialization logic for the I2P SAM session.src/test/i2p_tests.cpp): Provide tests for I2P address parsing, network type identification, and ADDRv2 serialization.CMakeLists.txt): Update build configurations to include new I2P source and test files.