Skip to content
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

Full CJDNS support #23077

Merged
merged 13 commits into from Nov 8, 2021
Merged

Full CJDNS support #23077

merged 13 commits into from Nov 8, 2021

Conversation

vasild
Copy link
Contributor

@vasild vasild commented Sep 23, 2021

CJDNS overview

CJDNS is like a distributed, shared VPN with multiple entry points where every participant can reach any other participant. All participants use addresses from the fc00::/8 network (reserved IPv6 range). Installation and configuration is done outside of applications, similarly to VPN (either in the host/OS or on the network router).

Motivation

Even without this PR it is possible to connect two Bitcoin Core nodes through CJDNS manually by using e.g. -addnode in environments where CJDNS is set up. However, this PR is necessary for address relay to work properly and automatic connections to be made to CJDNS peers. I.e. to make CJDNS a first class citizen network like IPv4, IPv6, Tor and I2P.

Considerations

An address from the fc00::/8 network, could mean two things:

  1. Part of a local network, as defined in RFC 4193. Like 10.0.0.0/8. Bitcoin Core could be running on a machine with such address and have peers with those (e.g. in a local network), but those addresses are not relayed to other peers because they are not globally routable on the internet.
  2. Part of the CJDNS network. This is like Tor or I2P - if we have connectivity to that network then we could reach such peers and we do relay them to other peers.

So, Bitcoin Core needs to be able to tell which one is it when it encounters a bare fc00::/8 address, e.g. from -externalip= or by looking up the machine's own addresses. Thus a new config option is introduced -cjdnsreachable:

  • -cjdnsreachable=0: it is assumed a fc00::/8 address is a private IPv6 (1.)
  • -cjdnsreachable=1: it is assumed a fc00::/8 address is a CJDNS one (2.)

After setting up CJDNS outside of Bitcoin Core, a node operator only needs to enable this option.
Addresses from P2P relay/gossip don't need that because they are properly tagged as IPv6 or as CJDNS.

For testing

[fc32:17ea:e415:c3bf:9808:149d:b5a2:c9aa]:8333
[fc68:7026:cb27:b014:5910:e609:dcdb:22a2]:8333
[fcb3:dc50:e1ae:7998:7dc0:7fa6:4582:8e46]:8333
[fcc7:be49:ccd1:dc91:3125:f0da:457d:8ce]:8333
[fcf2:d9e:3a25:4eef:8f84:251b:1b4d:c596]:8333

@laanwj laanwj added the P2P label Sep 23, 2021
@jonatack
Copy link
Contributor

jonatack commented Sep 23, 2021

Concept ACK and building to try it 😃

Edit: debug build is clean and a first read through the commits looks straightforward.

@kristapsk
Copy link
Contributor

Concept ACK

@laanwj laanwj added the Feature label Sep 23, 2021
@Zero-1729
Copy link
Contributor

Concept ACK 🌱

@ghost
Copy link

ghost commented Sep 23, 2021

CJDNS is like a distributed, shared VPN with multiple entry points where every participant can reach any other participant. All participants use addresses from the fc00::/8 network (reserved IPv6 range). Installation and configuration is done outside of applications, similarly to VPN (either in the host/OS or on the network router).

Sorry but this doesn't look good enough to add one more network to Bitcoin Core. We already have 4 options: ipv4, ipv6, Tor, i2p and 14 combinations which can be used in onlynet for outgoing connections.

Will do more research about CJDNS and compare with other networks before ACKing the concept. Let me know if you know any links.

@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 23, 2021

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #21879 (Wrap accept() and extend usage of Sock by vasild)
  • #21878 (Make all networking code mockable by vasild)
  • #19358 (torcontrol : avoid to set wrong outbound proxy and network settings when creating an inbound onion service. by Saibato)

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.

@amitiuttarwar
Copy link
Contributor

@vasild

However, this PR is necessary for address relay to work properly and automatic connections to be made to CJDNS peers. I.e. to make CJDNS a first class citizen network like IPv4, IPv6, Tor and I2P.

Thanks for providing this motivation, it's helpful to understand the aim of this PR. Could you go a bit further and share your perspective on why you think CJDNS should be a first class citizen network in bitcoin core? Does it provide different features / privacy / etc. from other networks? Or is this a case where you think optionality is good, so supporting more networks is better? Thanks in advance!

@sipa
Copy link
Member

sipa commented Sep 23, 2021

FWIW, BIP155 does already reserve a network ID for CJDNS (that's not an argument either way of course, but perhaps the BIP's author have opinions on this too). @laanwj?

@ghost
Copy link

ghost commented Sep 24, 2021

I did some research about CJDNS, GNUnet and Yggdrasil.

Will read more about these networks and also compare with few others in next few days. Shared initial thoughts and things I did here: https://blockchaincommons.github.io/Bitcoin-Camouflage//blog/cjdns/

@vasild
Copy link
Contributor Author

vasild commented Sep 24, 2021

@prayank23, @amitiuttarwar,

Compared to IPv4/6, CJDNS provides e2e encryption and protects nodes from the traditional traffic analysis and filtering. For example, an ISP blocks port 8333 (dumb) or analyses the traffic to/from a given node, regardless of the port (it is easy to detect that it is Bitcoin P2P protocol being talked on a TCP connection, even if the port is not 8333).

Compared to Tor and I2P, CJDNS provides redundancy. Each of those have their own shades, which may suit different users. For example Tor is popular/widespread but is somewhat centralized, which could lead to problems. I2P connections have source address, but I2P is somewhat slow. CJDNS does not hide the sender and the recipient from intermediate routers, but it is fast. Different things may work for different people. It is good to have options.

So, why not have support for CJDNS? It must be noted that support for X does not come for free. We have to pay the following prices:

  • Implementation cost
  • Review cost
  • Maintenance cost

For the implementation cost, notice that CJDNS is already included in BIP155 and that there is already rudimentary support for it in master:

$ git grep -i cjdns origin/master |wc -l
      49

As a result this PR is relatively small and straight-forward. Review cost should be low. From the application's point of view, connecting to CJDNS is as easy as connecting to a normal IPv6 address. Bitcoin Core need not implement support for a specific proxy that talks a specific protocol. Tor and I2P require that. We have this maintenance cost for them:

$ wc -l src/torcontrol.cpp src/i2p.cpp 
     636 src/torcontrol.cpp
     418 src/i2p.cpp

Support for CJDNS goes without that, so its maintenance cost is low compared to Tor and I2P.

Notice also that OpenWRT routers support CJDNS, so from user's perspective it would be a matter of enabling it on their routers and running Bitcoin Core with -cjdnsreachable.

In the case of CJDNS, my personal opinion is that the benefits outweigh the cost.

@jonatack
Copy link
Contributor

Yes, BIP155 and Tor v3 implementation were both fairly deep changes. I2P was also extensive. This change set is relatively tiny, I've done a first review pass, and in terms of time it was well under 10% that of any of the BIP155 impl, Tor v3, and I2P for me.

@benthecarman
Copy link
Contributor

Concept ACK, awesome to see this!

@dunxen
Copy link
Contributor

dunxen commented Sep 25, 2021

Concept ACK seeing as there is already basic existing support for this in master and noting that this may match some users' needs of speed while still protecting against traffic analysis.

Going to build and try this out

@vasild
Copy link
Contributor Author

vasild commented Sep 27, 2021

A testing/experimental Bitcoin Core node is running (most of the time) at [fcc7:be49:ccd1:dc91:3125:f0da:457d:8ce]:8333.

@DrahtBot DrahtBot mentioned this pull request Sep 29, 2021
12 tasks
// learn a new local address
bool AddLocal(const CService& addr, int nScore)
bool AddLocal(const CService& addr_, int nScore)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In commit ec2dc39,

Not sure I like this addr_ approach.
Don't you think a caller should be aware of the transition to CJDNS?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the callers of AddLocal() do not care that a passed fc... address may be interpreted as CJDNS instead of IPv6.

Actually, I started by doing the flip just before AddLocal() is called, but ended up with duplicated code all over the place in all callers of AddLocal() which prompted me to move that snippet inside AddLocal().

Ideally a CJDNS object should be returned by LookupIntern() which is the function which creates the CNetAddr objects, so they will not be created as IPv6 and later flipped to CJDNS. However LookupIntern() and WrappedGetAddrInfo() (which it uses) are defined in netbase.cpp and IsReachable() (needed to tell whether IPv6 or CJDNS should be created) is in net.cpp which would create a circular dependency. Hmm... maybe I should move IsReachable() to netbase.cpp?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I started by doing the flip just before AddLocal() is called, but ended up with duplicated code all over the place in all callers of AddLocal() which prompted me to move that snippet inside AddLocal().

I think this is a good move, i was just wondering whether it's ok to do it quietly.
Imagine logging something like "finished processing IPv6 addr" after calling AddLocal, while it's not actually IPv6.

I think since this is not really the case now, making a comment along AddrLocal might be sufficient to prevent the confusion.

Hmm... maybe I should move IsReachable() to netbase.cpp?

Perhaps attempt that in a separate PR then?

@naumenkogs
Copy link
Member

ACK baae5be modulo questions above.
I expect it to be implemented very similarly to Tor/I2P which are already manually tested, so it seems we aren't missing anything.

I haven't run the code.

@luke-jr
Copy link
Member

luke-jr commented Oct 3, 2021

How about simply requiring a "%cjdns" suffix on CJDNS addresses?

@vasild
Copy link
Contributor Author

vasild commented Oct 4, 2021

How about simply requiring a "%cjdns" suffix on CJDNS addresses?

Internally the addresses are already distinguished in the CNetAddr class by having m_net set to either NET_IPV6 or NET_CJDNS.

For representation purposes:

  • On output: yes, we can print CJDNS ones with %cjdns suffix
  • On input: we can recognize e.g. fc00::123 as reserved-IPv6 and fc00::123%cjdns as CJDNS, however that would not remove the need to have the config flag -cjdnsreachable because if we stumble on fc00::123%cjdns we need to know whether we can connect to it or not (e.g. whether the OS is configured to connect to the CJDNS network). I think the existence of a config flag -cjdnsreachable makes it unnecessary to require %cjdns suffix. In this sense, having a %cjdns suffix would not make it possible to remove any of the bits of this PR. It would be possible, but would add some complexity on top of this PR.

Does that answer the question or did I misunderstand it?

@laanwj
Copy link
Member

laanwj commented Oct 4, 2021

FWIW, BIP155 does already reserve a network ID for CJDNS (that's not an argument either way of course, but perhaps the BIP's author have opinions on this too). @laanwj?

From what I understood back then is that CJDNS is fairly pervasive in mesh networking. You can peer it over arbitrary networks and physical layers but you'll still have a global address from the viewpoint of software. With no central authority that needs to assign that.

What makes it different to I2P and Tor is that it is not an anonymity network. It encrypts the payload but does not hide where it comes from. This means it can be very lightweight and low-latency. This is good for initial sync and gossip, maybe not so much for transaction broadcasting. But it's not worse than clearnet.

Also unlike those, from the viewpoint of software it's simply another IPv6 interface. This means it requires only little extra code to support (mostly related to gossip and reachability). No need for proxies or special negotiation (as @vasild already mentions above).

@jonatack
Copy link
Contributor

jonatack commented Oct 4, 2021

Connected to @vasild with his help (thanks!) and the connection has been reliable and fast so far.

Running a bitcoind cjdns service at [fc32:17ea:e415:c3bf:9808:149d:b5a2:c9aa]:8333 if anyone wants to connect.

@dunxen
Copy link
Contributor

dunxen commented Oct 4, 2021

Connected to @vasild with his help (thanks!) and the connection has been reliable and fast so far.

Running a bitcoind cjdns service at [fc32:17ea:e415:c3bf:9808:149d:b5a2:c9aa]:8333 if anyone wants to connect.

Ah, I did try to connect to Vasil a few days ago and had trouble. Was there something specific you had to resolve before you could? I'll try again tomorrow when I can.

This will help with propagation, so that multi-homed nodes can learn
CJDNS addresses outside of the CJDNS network.
CJDNS addresses start with constant 8 bits, so in order to account for
the first 4 random ones, we must take the first 12. Otherwise the entire
CJDNS network will belong to one group.
An IPv6 address from fc00::/8 could be either from the CJDNS network or
from a private-unroutable-reserved segment of IPv6. A seed node with
such an address must be from the CJDNS network, otherwise other peers
will not be able to connect to it.
@vasild
Copy link
Contributor Author

vasild commented Nov 3, 2021

4fbff39e8f...420695c193: rebase due to a silent merge conflict with #22766

Thanks, @jonatack!

Invalidates ACKs from @naumenkogs, @dunxen, @jonatack

Previously invalidated ACK from @jamesob

@dunxen
Copy link
Contributor

dunxen commented Nov 3, 2021

ACK 420695c

@jonatack
Copy link
Contributor

jonatack commented Nov 3, 2021

re-ACK 420695c per git range-diff 23ae793 4fbff39 420695c

Have been running this patch with that change since yesterday evening.

@laanwj
Copy link
Member

laanwj commented Nov 8, 2021

Code review ACK 420695c

@laanwj laanwj merged commit 8346004 into bitcoin:master Nov 8, 2021
@vasild vasild deleted the cjdns branch November 8, 2021 13:46
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 8, 2021
laanwj added a commit that referenced this pull request Nov 15, 2021
7b65287 cli: hoist networks class data members to a constant (Jon Atack)
5bd40a3 cli: add cjdns network to -addrinfo and -netinfo (Jon Atack)

Pull request description:

  Follow-up to #23077 and #23324.
  ```
  $ ./src/bitcoin-cli -addrinfo
  {
    "addresses_known": {
      "ipv4": 47782,
      "ipv6": 10307,
      "onion": 8030,
      "i2p": 41,
      "cjdns": 1,
      "total": 66161
    }
  }
  $ ./src/bitcoin-cli -netinfo
  Bitcoin Core client v22.99.0-deb6223d4c55 - server 70016/Satoshi:22.99.0(jon)/

          ipv4    ipv6   onion     i2p   cjdns   total   block  manual
  in         0       5      12       5       1      23
  out        2       2       9       5       2      20       2      10
  total      2       7      21      10       3      43
  ```
  ```
  $ ./src/bitcoin-cli -netinfo 1
  ```

  ![Screenshot from 2021-10-10 12-01-58](https://user-images.githubusercontent.com/2415484/136691258-8b3fa7aa-3edb-4428-854a-adadfef302e3.png)

ACKs for top commit:
  laanwj:
    Code review ACK 7b65287

Tree-SHA512: 9c740d394d9842d38a1c01a824271b25277baac11ed090f0430daa15b580c2bf3d114ac6b8254b19d6aaee57cbe1ca6a414996e6994e0bf4a577bed771382eca
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 16, 2021
7b65287 cli: hoist networks class data members to a constant (Jon Atack)
5bd40a3 cli: add cjdns network to -addrinfo and -netinfo (Jon Atack)

Pull request description:

  Follow-up to bitcoin#23077 and bitcoin#23324.
  ```
  $ ./src/bitcoin-cli -addrinfo
  {
    "addresses_known": {
      "ipv4": 47782,
      "ipv6": 10307,
      "onion": 8030,
      "i2p": 41,
      "cjdns": 1,
      "total": 66161
    }
  }
  $ ./src/bitcoin-cli -netinfo
  Bitcoin Core client v22.99.0-deb6223d4c55 - server 70016/Satoshi:22.99.0(jon)/

          ipv4    ipv6   onion     i2p   cjdns   total   block  manual
  in         0       5      12       5       1      23
  out        2       2       9       5       2      20       2      10
  total      2       7      21      10       3      43
  ```
  ```
  $ ./src/bitcoin-cli -netinfo 1
  ```

  ![Screenshot from 2021-10-10 12-01-58](https://user-images.githubusercontent.com/2415484/136691258-8b3fa7aa-3edb-4428-854a-adadfef302e3.png)

ACKs for top commit:
  laanwj:
    Code review ACK 7b65287

Tree-SHA512: 9c740d394d9842d38a1c01a824271b25277baac11ed090f0430daa15b580c2bf3d114ac6b8254b19d6aaee57cbe1ca6a414996e6994e0bf4a577bed771382eca
@prusnak prusnak mentioned this pull request Nov 16, 2021
laanwj added a commit that referenced this pull request Mar 2, 2022
…JDNS peers

b7be28c test: add combined CJDNS/I2P/localhost/onion eviction protection tests (Jon Atack)
0a1bb84 test: add tests for inbound eviction protection of CJDNS peers (Jon Atack)
0c00c0c test: fix off-by-one logic in an eviction protection test (Jon Atack)
f7b8094 p2p: extend inbound eviction protection by network to CJDNS peers (Jon Atack)

Pull request description:

  Extend inbound eviction protection for peers connected over CJDNS, as is the case for peers connected via onion, localhost, and I2P since #21261 and #20197.  CJDNS peers seem to have better min ping latency than onion and I2P peers but still higher than that of unencrypted IPv4/6 peers and can be disadvantaged under our eviction criteria. They are also very few in number, which is a further reason to protect them, as the goal of this logic is to favorise the diversity of our peer connections.  CJDNS support was added in #23077 for the upcoming v23 release.

ACKs for top commit:
  laanwj:
    Concept and code review ACK b7be28c
  w0xlt:
    tACK b7be28c

Tree-SHA512: 89ebdd217602e16ae14b9bd0d5a25fc09f9b2384c951f820bc0f5a6d8452bbc9042065db817d5d5296c0ad22988491a83fc5b9a611e660c40ebd4f03448c4061
jonatack added a commit to jonatack/bitcoin that referenced this pull request Mar 14, 2022
Adapted in part from the CJDNS description in bitcoin#23077 by Vasil Dimov.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
jonatack added a commit to jonatack/bitcoin that referenced this pull request Mar 14, 2022
Adapted in part from the CJDNS description in bitcoin#23077 by Vasil Dimov.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
jonatack added a commit to jonatack/bitcoin that referenced this pull request Mar 14, 2022
Adapted in part from the CJDNS description in bitcoin#23077 by Vasil Dimov.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
jonatack added a commit to jonatack/bitcoin that referenced this pull request Mar 17, 2022
Adapted in part from the CJDNS description in bitcoin#23077 by Vasil Dimov.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
jonatack added a commit to jonatack/bitcoin that referenced this pull request Mar 21, 2022
Adapted in part from the CJDNS description in bitcoin#23077 by Vasil Dimov
and from CJDNS documentation and feedback by Caleb James DeLisle.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
jonatack added a commit to jonatack/bitcoin that referenced this pull request Mar 22, 2022
Adapted in part from the CJDNS description in bitcoin#23077 by Vasil Dimov
and from CJDNS documentation and feedback by Caleb James DeLisle.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
laanwj added a commit to bitcoin-core/gui that referenced this pull request Mar 24, 2022
…NS how-to documentation

f44efc3 doc: update i2p.md with cjdns, improve local addresses section (Jon Atack)
3bf6f0c doc: update tor.md with cjdns and getnodeaddresses, fix tor grep, (Jon Atack)
ed15848 doc: create initial doc/cjdns.md for cjdns how-to documentation (Jon Atack)

Pull request description:

  and update and improve doc/tor.md and doc/i2p.md.

  Adapted in part from the CJDNS description in bitcoin/bitcoin#23077 and feedback by Vasil Dimov and from the CJDNS documentation and feedback by Caleb James DeLisle.

  Targets backport to v23.x.

  Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

ACKs for top commit:
  vasild:
    ACK f44efc3
  lsilva01:
    ACK f44efc3

Tree-SHA512: 7b7c69f76bc8a5705d324892f32bfe0eb21bcf048054748053eca167c65a2121f6332f40ac6ff98c955e6e8b53233c74c365d887c364ef1d5944f1c49675a6b4
hebasto pushed a commit to hebasto/bitcoin that referenced this pull request Mar 31, 2022
Adapted in part from the CJDNS description in bitcoin#23077 by Vasil Dimov
and from CJDNS documentation and feedback by Caleb James DeLisle.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

Github-Pull: bitcoin#24555
Rebased-From: ed15848
jonatack added a commit to jonatack/bitcoin that referenced this pull request Mar 31, 2022
Adapted in part from the CJDNS description in bitcoin#23077 by Vasil Dimov
and from CJDNS documentation and feedback by Caleb James DeLisle.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

Github-Pull: bitcoin#24555
Rebased-From: ed15848
@dunxen dunxen mentioned this pull request Apr 3, 2022
5 tasks
@ghost ghost mentioned this pull request Sep 29, 2022
@bitcoin bitcoin locked and limited conversation to collaborators Mar 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet