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

net: Use alternative port for incoming Tor connections #19991

Merged
merged 8 commits into from
Oct 2, 2020

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Sep 22, 2020

This PR adds ability to label incoming Tor connections as different from normal localhost connections.

Closes #8973.
Closes #16693.

Default onion service target ports are:

  • 8334 on mainnnet
  • 18334 on testnet
  • 38334 on signet
  • 18445 on regtest

To set the onion service target socket manually the extended -bind config option could be used:

$ src/bitcoind -help | grep -A 6 -e '-bind'
  -bind=<addr>[:<port>][=onion]
       Bind to given address and always listen on it (default: 0.0.0.0). Use
       [host]:port notation for IPv6. Append =onion to tag any incoming
       connections to that address and port as incoming Tor connections
       (default: 127.0.0.1:8334=onion, testnet: 127.0.0.1:18334=onion,
       signet: 127.0.0.1:38334=onion, regtest: 127.0.0.1:18445=onion)

Since pr19991.02 update this PR is an alternative to #19043.

@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 22, 2020

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

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.

src/net.cpp Show resolved Hide resolved
@practicalswift
Copy link
Contributor

Concept ACK

Being able to differentiate between incoming Tor connections from normal localhost connections would be very useful

Copy link
Member

@glozow glozow left a comment

Choose a reason for hiding this comment

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

Concept ACK

@laanwj
Copy link
Member

laanwj commented Sep 22, 2020

Concept ACK! Thank you for working on this.

Copy link
Contributor

@vasild vasild left a comment

Choose a reason for hiding this comment

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

  • This will always open 127.0.0.1:8334, even if the new option -onionservicetargetport= is not specified and the system does not use Tor in any way. I think this deserves a release notes mention.

  • Binding to 127.0.0.1 would make it impossible (or hard) to run the Tor daemon on another host. What about changing to -onionservicetarget=bind_address:port without a default value - i.e. don't do the new bind unless -onionservicetarget= is specified? This would also allow to bind to an external address or to 0.0.0.0 (any).

  • Notice that if another peer manages to reach and connect to the new bind without tor, then we would mistakenly think it is an incoming tor connection.

  • I am just curious - what will happen if -onionservicetargetport=8333 is specified?

src/init.cpp Outdated Show resolved Hide resolved
@hebasto
Copy link
Member Author

hebasto commented Sep 23, 2020

@vasild

  • This will always open 127.0.0.1:8334, even if the new option -onionservicetargetport= is not specified and the system does not use Tor in any way. I think this deserves a release notes mention.

Target port opening could be dependent on -listenonion option value. What do you think?

  • Binding to 127.0.0.1 would make it impossible (or hard) to run the Tor daemon on another host. What about changing to -onionservicetarget=bind_address:port without a default value - i.e. don't do the new bind unless -onionservicetarget= is specified? This would also allow to bind to an external address or to 0.0.0.0 (any).

This issue is addressed in #19043, IIUC.

  • Notice that if another peer manages to reach and connect to the new bind without tor, then we would mistakenly think it is an incoming tor connection.

Hmmm... Need to think about this.

  • I am just curious - what will happen if -onionservicetargetport=8333 is specified?

It will still work but without strong distinction between Tor and clearnet, no?

@naumenkogs
Copy link
Member

Concept ACK

@hebasto
Copy link
Member Author

hebasto commented Sep 24, 2020

From IRC discussion:

<vasild> what about extending the -bind=addr:port option to something like -bind=addr:port[=tor], for example: -bind=1.2.3.4:8333 -bind=1.2.3.4:8334=tor
<vasild> instead of adding a new option -onionservicetarget= ?
<wumpus> making it an attribute makes sense
<wumpus> (of the bind)
<vasild> I don't know yet how i2p works, but if it works in a similar way (the i2p daemon connects to us, like in tor) then for i2p that would mean just recognizing =i2p in addition to recognizing =tor, instead of adding new option -i2pservicetarget=addr:port
<wumpus> right, it's elegant way to think about it and allows for extensibility

@jonatack
Copy link
Contributor

Concept ACK. Could use test coverage. I'm running #20002 that is built on these changes and those in #19998.

@hebasto
Copy link
Member Author

hebasto commented Sep 24, 2020

@vasild @jonatack

what about extending the -bind=addr:port option to something like -bind=addr:port[=tor]

I like that ^ idea very much. Implemented in https://github.com/hebasto/bitcoin/commits/200924-WIP

As many other PRs are built on top of this one, I'm begging you to make some preliminary review of the suggested branch to avoid unneeded rebasing.

New version features:

Also a small Q: is it better to drop doc commit cfa17e3 in favor of #19961 ?

UPDATE: already upstreamed.

@hebasto
Copy link
Member Author

hebasto commented Sep 24, 2020

@vasild

  • Notice that if another peer manages to reach and connect to the new bind without tor, then we would mistakenly think it is an incoming tor connection.

This behavior could be mitigated in #19998 by additional checking of peer.addr and peer.GetAddrLocal().

@jonatack
Copy link
Contributor

jonatack commented Sep 24, 2020

I like that approach, too. Will look at your branch. Edit: done.

@hebasto
Copy link
Member Author

hebasto commented Sep 25, 2020

Updated 6e05a44 -> cfa17e3 (pr19991.01 -> pr19991.02, diff):

Copy link
Contributor

@vasild vasild left a comment

Choose a reason for hiding this comment

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

Just to summarize the default behavior:

Before this PR

  • If no -bind is given then we will bind on 0.0.0.0:8333 and [::]:8333.
  • If -bind=1.2.3.4:8333 is given then we will bind only on 1.2.3.4:8333.

With this PR

  • If no -bind is given then we will bind on 0.0.0.0:8333, [::]:8333 and 127.0.0.1:8334.
  • If -bind=1.2.3.4:8333 is given then we will bind on 1.2.3.4:8333 and on 127.0.0.1:8334 (is this ok?).
  • If -bind=1.2.3.4:8333 -bind=1.2.3.4:8334=onion is given then we will bind on 1.2.3.4:8333 and 1.2.3.4:8334.

src/init.cpp Outdated Show resolved Hide resolved
src/init.cpp Outdated Show resolved Hide resolved
src/init.cpp Outdated Show resolved Hide resolved
src/net.h Outdated Show resolved Hide resolved
src/net.cpp Outdated Show resolved Hide resolved
src/net.cpp Outdated Show resolved Hide resolved
src/net.cpp Show resolved Hide resolved
src/torcontrol.cpp Outdated Show resolved Hide resolved
src/torcontrol.cpp Show resolved Hide resolved
@hebasto
Copy link
Member Author

hebasto commented Sep 25, 2020

@vasild

Just to summarize the default behavior:

Before this PR

  • If no -bind is given then we will bind on 0.0.0.0:8333 and [::]:8333.

  • If -bind=1.2.3.4:8333 is given then we will bind only on 1.2.3.4:8333.

With this PR

  • If no -bind is given then we will bind on 0.0.0.0:8333, [::]:8333 and 127.0.0.1:8334.

  • If -bind=1.2.3.4:8333 is given then we will bind on 1.2.3.4:8333 and on 127.0.0.1:8334 (is this ok?).

  • If -bind=1.2.3.4:8333 -bind=1.2.3.4:8334=onion is given then we will bind on 1.2.3.4:8333 and 1.2.3.4:8334.

The node will not bind on 127.0.0.1:8334 if non-default listenonion=0 is provided.

@vasild
Copy link
Contributor

vasild commented Sep 25, 2020

This could be an alternative way to check if an incoming connection is from tor - to ask the tor daemon if it made it. But it seems this is not possible now and even if made possible in the future then it will only be available if bitcoind has access to tor-control.

vasild added a commit to vasild/bitcoin that referenced this pull request Oct 24, 2020
The semantic of `-bind` is to restrict the binding only to some address.
If not specified, then the user does not care and we bind to `0.0.0.0`.
If specified then we should honor the restriction and bind only to the
specified address.

Before this change, if no `-bind` is given then we would bind to
`0.0.0.0` and to `127.0.0.1:8334` (incoming Tor) which is ok - the user
does not care to restrict the binding. However:

* If only `-bind=addr` is given (without `-bind=...=onion`) then we
  would bind to `addr` _and_ to `127.0.0.1:8334` in addition which may
  be unexpected assuming the perception of `-bind=addr` is
  "bind _only_ to `addr`".

* If only `-bind=addr=onion` is given (without ordinary `-bind=`)
  then we would bind to `addr` _and_ to `0.0.0.0` in addition.

Change the above two cases to not do the additional bind. If `-bind`
is given then only bind to the specified address:

* If only `-bind=addr` is given (without `-bind=...=onion`) then bind to
  `addr` (only). If we are creating tor hidden service then use `addr`
  as target. Same behavior as before
  bitcoin#19991.

* If only `-bind=addr=onion` is given (without ordinary `-bind=`)
  then bind to `addr` (only) and consider incoming connections as Tor
  and do not advertise `addr`. I.e. a Tor-only node.
vasild added a commit to vasild/bitcoin that referenced this pull request Oct 29, 2020
The semantic of `-bind` is to restrict the binding only to some address.
If not specified, then the user does not care and we bind to `0.0.0.0`.
If specified then we should honor the restriction and bind only to the
specified address.

Before this change, if no `-bind` is given then we would bind to
`0.0.0.0` and to `127.0.0.1:8334` (incoming Tor) which is ok - the user
does not care to restrict the binding. However:

* If only `-bind=addr` is given (without `-bind=...=onion`) then we
  would bind to `addr` _and_ to `127.0.0.1:8334` in addition which may
  be unexpected assuming the perception of `-bind=addr` is
  "bind _only_ to `addr`".

* If only `-bind=addr=onion` is given (without ordinary `-bind=`)
  then we would bind to `addr` _and_ to `0.0.0.0` in addition.

Change the above two cases to not do the additional bind. If `-bind`
is given then only bind to the specified address:

* If only `-bind=addr` is given (without `-bind=...=onion`) then bind to
  `addr` (only). If we are creating tor hidden service then use `addr`
  as target. Same behavior as before
  bitcoin#19991.

* If only `-bind=addr=onion` is given (without ordinary `-bind=`)
  then bind to `addr` (only) and consider incoming connections as Tor
  and do not advertise `addr`. I.e. a Tor-only node.
vasild added a commit to vasild/bitcoin that referenced this pull request Oct 29, 2020
The semantic of `-bind` is to restrict the binding only to some address.
If not specified, then the user does not care and we bind to `0.0.0.0`.
If specified then we should honor the restriction and bind only to the
specified address.

Before this change, if no `-bind` is given then we would bind to
`0.0.0.0` and to `127.0.0.1:8334` (incoming Tor) which is ok - the user
does not care to restrict the binding. However:

* If only `-bind=addr` is given (without `-bind=...=onion`) then we
  would bind to `addr` _and_ to `127.0.0.1:8334` in addition which may
  be unexpected assuming the perception of `-bind=addr` is
  "bind _only_ to `addr`".

* If only `-bind=addr=onion` is given (without ordinary `-bind=`)
  then we would bind to `addr` _and_ to `0.0.0.0` in addition.

Change the above two cases to not do the additional bind. If `-bind`
is given then only bind to the specified address:

* If only `-bind=addr` is given (without `-bind=...=onion`) then bind to
  `addr` (only). If we are creating tor hidden service then use `addr`
  as target. Same behavior as before
  bitcoin#19991.

* If only `-bind=addr=onion` is given (without ordinary `-bind=`)
  then bind to `addr` (only) and consider incoming connections as Tor
  and do not advertise `addr`. I.e. a Tor-only node.
vasild added a commit to vasild/bitcoin that referenced this pull request Oct 29, 2020
The semantic of `-bind` is to restrict the binding only to some address.
If not specified, then the user does not care and we bind to `0.0.0.0`.
If specified then we should honor the restriction and bind only to the
specified address.

Before this change, if no `-bind` is given then we would bind to
`0.0.0.0` and to `127.0.0.1:8334` (incoming Tor) which is ok - the user
does not care to restrict the binding. However:

* If only `-bind=addr` is given (without `-bind=...=onion`) then we
  would bind to `addr` _and_ to `127.0.0.1:8334` in addition which may
  be unexpected assuming the perception of `-bind=addr` is
  "bind _only_ to `addr`".

* If only `-bind=addr=onion` is given (without ordinary `-bind=`)
  then we would bind to `addr` _and_ to `0.0.0.0` in addition.

Change the above two cases to not do the additional bind. If `-bind`
is given then only bind to the specified address:

* If only `-bind=addr` is given (without `-bind=...=onion`) then bind to
  `addr` (only). If we are creating tor hidden service then use `addr`
  as target. Same behavior as before
  bitcoin#19991.

* If only `-bind=addr=onion` is given (without ordinary `-bind=`)
  then bind to `addr` (only) and consider incoming connections as Tor
  and do not advertise `addr`. I.e. a Tor-only node.
vasild added a commit to vasild/bitcoin that referenced this pull request Dec 18, 2020
The semantic of `-bind` is to restrict the binding only to some address.
If not specified, then the user does not care and we bind to `0.0.0.0`.
If specified then we should honor the restriction and bind only to the
specified address.

Before this change, if no `-bind` is given then we would bind to
`0.0.0.0` and to `127.0.0.1:8334` (incoming Tor) which is ok - the user
does not care to restrict the binding. However:

* If only `-bind=addr` is given (without `-bind=...=onion`) then we
  would bind to `addr` _and_ to `127.0.0.1:8334` in addition which may
  be unexpected assuming the perception of `-bind=addr` is
  "bind _only_ to `addr`".

* If only `-bind=addr=onion` is given (without ordinary `-bind=`)
  then we would bind to `addr` _and_ to `0.0.0.0` in addition.

Change the above two cases to not do the additional bind. If `-bind`
is given then only bind to the specified address:

* If only `-bind=addr` is given (without `-bind=...=onion`) then bind to
  `addr` (only). If we are creating tor hidden service then use `addr`
  as target. Same behavior as before
  bitcoin#19991.

* If only `-bind=addr=onion` is given (without ordinary `-bind=`)
  then bind to `addr` (only) and consider incoming connections as Tor
  and do not advertise `addr`. I.e. a Tor-only node.
vasild added a commit to vasild/bitcoin that referenced this pull request Jan 14, 2021
The semantic of `-bind` is to restrict the binding only to some address.
If not specified, then the user does not care and we bind to `0.0.0.0`.
If specified then we should honor the restriction and bind only to the
specified address.

Before this change, if no `-bind` is given then we would bind to
`0.0.0.0` and to `127.0.0.1:8334` (incoming Tor) which is ok - the user
does not care to restrict the binding. However:

* If only `-bind=addr` is given (without `-bind=...=onion`) then we
  would bind to `addr` _and_ to `127.0.0.1:8334` in addition which may
  be unexpected assuming the perception of `-bind=addr` is
  "bind _only_ to `addr`".

* If only `-bind=addr=onion` is given (without ordinary `-bind=`)
  then we would bind to `addr` _and_ to `0.0.0.0` in addition.

Change the above two cases to not do the additional bind. If `-bind`
is given then only bind to the specified address:

* If only `-bind=addr` is given (without `-bind=...=onion`) then bind to
  `addr` (only). If we are creating tor hidden service then use `addr`
  as target. Same behavior as before
  bitcoin#19991.

* If only `-bind=addr=onion` is given (without ordinary `-bind=`)
  then bind to `addr` (only) and consider incoming connections as Tor
  and do not advertise `addr`. I.e. a Tor-only node.
laanwj added a commit that referenced this pull request Mar 30, 2021
…eviction protection test coverage

0cca08a Add unit test coverage for our onion peer eviction protection (Jon Atack)
caa21f5 Protect onion+localhost peers in ProtectEvictionCandidatesByRatio() (Jon Atack)
8f1a53e Use EraseLastKElements() throughout SelectNodeToEvict() (Jon Atack)
8b1e156 Add m_inbound_onion to AttemptToEvictConnection() (Jon Atack)
72e30e8 Add unit tests for ProtectEvictionCandidatesByRatio() (Jon Atack)
ca63b53 Use std::unordered_set instead of std::vector in IsEvicted() (Jon Atack)
41f84d5 Move peer eviction tests to a separate test file (Jon Atack)
f126cbd Extract ProtectEvictionCandidatesByRatio from SelectNodeToEvict (Jon Atack)

Pull request description:

  Now that #19991 and #20210 have been merged, we can determine inbound onion peers using `CNode::m_inbound_onion` and add it to the localhost peers protection in `AttemptToEvictConnection`, which was added in #19670 to address issue #19500.

  Update 28 February 2021: I've updated this to follow gmaxwell's suggestion in #20197 (comment).

  This branch now protects up to 1/4 onion peers (connected via our tor control service), if any, sorted by longest uptime. If any (or all) onion slots remain after that operation, they are then allocated to protect localhost peers, or a minimum of 2 localhost peers in the case that no onion slots remain and 2 or more onion peers were protected, sorted as before by longest uptime.

  This patch also adds test coverage for the longest uptime, localhost, and onion peer eviction protection logic to build on the welcome initial unit testing of #20477.

  Suggest reviewing the commits that move code with `colorMoved = dimmed-zebra` and `colorMovedWs = allow-indentation-change`.

  Closes #11537.

ACKs for top commit:
  laanwj:
    Code review ACK 0cca08a
  vasild:
    ACK 0cca08a

Tree-SHA512: 2f5a63f942acaae7882920fc61f0185dcd51da85e5b736df9d1fc72343726dd17da740e02f30fa5dc5eb3b2d8345707aed96031bec143d48a2497a610aa19abd
vasild added a commit to vasild/bitcoin that referenced this pull request May 19, 2021
The semantic of `-bind` is to restrict the binding only to some address.
If not specified, then the user does not care and we bind to `0.0.0.0`.
If specified then we should honor the restriction and bind only to the
specified address.

Before this change, if no `-bind` is given then we would bind to
`0.0.0.0` and to `127.0.0.1:8334` (incoming Tor) which is ok - the user
does not care to restrict the binding. However:

* If only `-bind=addr` is given (without `-bind=...=onion`) then we
  would bind to `addr` _and_ to `127.0.0.1:8334` in addition which may
  be unexpected assuming the perception of `-bind=addr` is
  "bind _only_ to `addr`".

* If only `-bind=addr=onion` is given (without ordinary `-bind=`)
  then we would bind to `addr` _and_ to `0.0.0.0` in addition.

Change the above two cases to not do the additional bind. If `-bind`
is given then only bind to the specified address:

* If only `-bind=addr` is given (without `-bind=...=onion`) then bind to
  `addr` (only). If we are creating tor hidden service then use `addr`
  as target. Same behavior as before
  bitcoin#19991.

* If only `-bind=addr=onion` is given (without ordinary `-bind=`)
  then bind to `addr` (only) and consider incoming connections as Tor
  and do not advertise `addr`. I.e. a Tor-only node.
@SimonVrouwe
Copy link

SimonVrouwe commented Jul 25, 2021

@hebasto Hello, I ended up here to find reasons why my node (bitcoind v0.21.1) is listening on port 8334 even when listenonion=0 is set. Reading the discussion in this PR, it is not clear to me if this was intended or not.

@vasild

...

With this PR

  • If no -bind is given then we will bind on 0.0.0.0:8333, [::]:8333 and 127.0.0.1:8334.
  • If -bind=1.2.3.4:8333 is given then we will bind on 1.2.3.4:8333 and on 127.0.0.1:8334 (is this ok?).
  • If -bind=1.2.3.4:8333 -bind=1.2.3.4:8334=onion is given then we will bind on 1.2.3.4:8333 and 1.2.3.4:8334.

The node will not bind on 127.0.0.1:8334 if non-default listenonion=0 is provided.

Why is the onion_binds.push_back not placed inside the if (args.GetBoolArg("-listenonion", DEFAULT_LISTEN_ONION))?

bitcoin/src/init.cpp

Lines 1725 to 1740 in 2b5563b

CService onion_service_target;
if (!connOptions.onion_binds.empty()) {
onion_service_target = connOptions.onion_binds.front();
} else {
onion_service_target = DefaultOnionServiceTarget();
connOptions.onion_binds.push_back(onion_service_target);
}
if (args.GetBoolArg("-listenonion", DEFAULT_LISTEN_ONION)) {
if (connOptions.onion_binds.size() > 1) {
InitWarning(strprintf(_("More than one onion bind address is provided. Using %s "
"for the automatically created Tor onion service."),
onion_service_target.ToStringIPPort()));
}
StartTorControl(onion_service_target);
}

vasild added a commit to vasild/bitcoin that referenced this pull request Aug 2, 2021
If only `-bind=addr:port` is given (without `-bind=...=onion`) then we
would bind to `addr:port` _and_ to `127.0.0.1:8334` in addition which
may be unexpected, assuming the semantic of `-bind=addr:port` is
"bind _only_ to `addr:port`".

Change the above to not do the additional bind: if only
`-bind=addr:port` is given (without `-bind=...=onion`) then bind to
`addr:port` (only). If we are creating a Tor hidden service then use
`addr:port` as target (same behavior as before
bitcoin#19991).
vasild added a commit to vasild/bitcoin that referenced this pull request Aug 3, 2021
If only `-bind=addr:port` is given (without `-bind=...=onion`) then we
would bind to `addr:port` _and_ to `127.0.0.1:8334` in addition which
may be unexpected, assuming the semantic of `-bind=addr:port` is
"bind _only_ to `addr:port`".

Change the above to not do the additional bind: if only
`-bind=addr:port` is given (without `-bind=...=onion`) then bind to
`addr:port` (only). If we are creating a Tor hidden service then use
`addr:port` as target (same behavior as before
bitcoin#19991).
vasild added a commit to vasild/bitcoin that referenced this pull request Aug 3, 2021
If only `-bind=addr:port` is given (without `-bind=...=onion`) then we
would bind to `addr:port` _and_ to `127.0.0.1:8334` in addition which
may be unexpected, assuming the semantic of `-bind=addr:port` is
"bind _only_ to `addr:port`".

Change the above to not do the additional bind: if only
`-bind=addr:port` is given (without `-bind=...=onion`) then bind to
`addr:port` (only). If we are creating a Tor hidden service then use
`addr:port` as target (same behavior as before
bitcoin#19991).
random-zebra added a commit to PIVX-Project/PIVX that referenced this pull request Aug 11, 2021
ecde04a [Consensus] Bump Active Protocol version to 70923 for v5.3 (random-zebra)
b63e4f5 Consensus: Add v5.3 enforcement height for testnet. (furszy)
f44be94 Only relay IPv4, IPv6, Tor addresses (Pieter Wuille)
015298c fix: tor: Call event_base_loopbreak from the event's callback (furszy)
34ff7a8 Consensus: Add mnb ADDRv2 guard. (furszy)
b4515dc GUI: Present v3 onion addresses properly in MNs list. (furszy)
337d43d tests: don't export in6addr_loopback (Vasil Dimov)
2cde8e0 GUI: Do not show the tor v3 onion address in the topbar. (furszy)
0b5f406 Doc: update tor.md with latest upstream information. (furszy)
89df7f2 addrman: ensure old versions don't parse peers.dat (Vasil Dimov)
bb90c5c test: add getnetworkinfo network name regression tests (Jon Atack)
d8e01b5 rpc: update GetNetworksInfo() to not return unsupported networks (Jon Atack)
57fc7b0 net: update GetNetworkName() with all enum Network cases (Jon Atack)
647d60b tests: Modify rpc_bind to conform to bitcoin#14532 behaviour. (Carl Dong)
d4d6729 Allow running rpc_bind.py --nonloopback test without IPv6 (Kristaps Kaupe)
4a034d8 test: Add rpc_bind test to default-run tests (Wladimir J. van der Laan)
61a08af [tests] bind functional test nodes to 127.0.0.1  Prevents OSX firewall (Sjors Provoost)
6a4f1e0 test: Add basic addr relay test (furszy)
78aa61c net: Make addr relay mockable (furszy)
ba954ca Send and require SENDADDRV2 before VERACK (Pieter Wuille)
61c2ed4 Bump net protocol version + don't send 'sendaddrv2' to pre-70923 software (furszy)
ccd508a tor: make a TORv3 hidden service instead of TORv2 (Vasil Dimov)
6da9a14 net: advertise support for ADDRv2 via new message (furszy)
e58d5d0 Migrate to test_large_inv() to Misbehaving logging. (furszy)
d496b64 [QA] fix mininode CAddress ser/deser (Jonas Schnelli)
cec9567 net: CAddress & CAddrMan: (un)serialize as ADDRv2 Change the serialization of `CAddrMan` to serialize its addresses in ADDRv2/BIP155 format by default. Introduce a new `CAddrMan` format version (3). (furszy)
b8c1dda streams update: get rid of nType and nVersion. (furszy)
3eaa273 Support bypassing range check in ReadCompactSize (Pieter Wuille)
a237ba4 net: recognize TORv3/I2P/CJDNS networks (Vasil Dimov)
8e50853 util: make EncodeBase32 consume Spans (Sebastian Falbesoner)
1f67e30 net: CNetAddr: add support to (un)serialize as ADDRv2 (Vasil Dimov)
2455420 test: move HasReason so it can be reused (furszy)
d41adb4 util: move HasPrefix() so it can be reused (Vasil Dimov)
f6f86af Unroll Keccak-f implementation (Pieter Wuille)
45222e6 Implement keccak-f[1600] and SHA3-256 (Pieter Wuille)
08ad06d net: change CNetAddr::ip to have flexible size (furszy)
3337219 net: improve encapsulation of CNetAddr. (furszy)
910d5c4 test: Do not instantiate CAddrDB for static call (Hennadii Stepanov)
6b607ef Drop IsLimited in favor of IsReachable (Ben Woosley)
a40711b IsReachable is the inverse of IsLimited (DRY). Includes unit tests (marcaiaf)
8839828 net: don't accept non-left-contiguous netmasks (Vasil Dimov)
5d7f864 rpcbind: Warn about exposing RPC to untrusted networks (Luke Dashjr)
2a6abd8 CNetAddr: Add IsBindAny method to check for INADDR_ANY (Luke Dashjr)
4fdfa45 net: Always default rpcbind to localhost, never "all interfaces" (Luke Dashjr)
31064a8 net: Minor accumulated cleanups (furszy)
9f9c871 tests: Avoid using C-style NUL-terminated strings as arguments (practicalswift)
f6c52a3 tests: Add tests to make sure lookup methods fail on std::string parameters with embedded NUL characters (practicalswift)
a751b9b net: Avoid using C-style NUL-terminated strings as arguments in the netbase interface (furszy)
f30869d test: add IsRFC2544 tests (Mark Tyneway)
ed5abe1 Net: Proper CService deserialization + GetIn6Addr return false if addr isn't an IPv6 addr (furszy)
86d73fb net: save the network type explicitly in CNetAddr (Vasil Dimov)
ad57dfc net: document `enum Network` (Vasil Dimov)
cb160de netaddress: Update CNetAddr for ORCHIDv2 (Carl Dong)
c3c04e4 net: Better misbehaving logging (furszy)
3660487 net: Use C++11 member initialization in protocol (Marco)
082baa3 refactor: Drop unused CBufferedFile::Seek() (Hennadii Stepanov)
e2d776a util: CBufferedFile fixes (Larry Ruane)
6921f42 streams: backport OverrideStream class (furszy)

Pull request description:

  Conjunction of a large number of back ports, updates and refactorings that made with the final goal of implementing v3 Onion addresses support (BIP155 https://github.com/bitcoin/bips/blob/master/bip-0155.mediawiki) before the tor v2 addresses EOL, scheduled, by the Tor project, for (1) July 15th: v2 addr support removal from the code base, and (2) October 15th: v2 addr network disable, where **every peer in our network running under Tor will loose the connection and drop the network**.

  As BIP155 describes, this is introducing a new P2P message to gossip longer node addresses over the P2P network. This is required to support new-generation Onion addresses, I2P, and potentially other networks that have longer endpoint addresses than fit in the 128 bits of the current addr message.

  In order to achieve the end goal, had to:
  1.  Create Span class and push it up to latest Bitcoin implementation.
  2.  Update the whole serialization framework and every object using it up to latest Bitcoin implementation (3-4 years ahead of what we currently are in master).
  3.  Update the address manager implementing ASN-based bucketing of the network nodes.
  4.  Update and refactor the netAddress and address manager tests to latest Bitcoin implementation (4 years ahead of what we currently are in master).
  5.  Several util string, vector, encodings, parsing, hashing backports and more..

  Important note:
  This PR it is not meant to be merged as a standalone PR, will decouple smaller ones moving on. Adding on each sub-PR its own description isolated from this big monster.

  Second note:
  This is still a **work-in-progress**, not ready for testing yet. I'm probably missing to mention few PRs that have already adapted to our sources. Just making it public so can decouple the changes, we can start merging them and i can continue working a bit more confortable (rebase a +170 commits separate branch is not fun..).

  ### List of back ported and adapted PRs:

  Span and Serialization:
  ----------------
  *  bitcoin#12886.
  *  bitcoin#12916.
  *  bitcoin#13558.
  *  bitcoin#13697. (Only Span's commit 29943a9)
  *  bitcoin#17850.
  *  bitcoin#17896.
  *  bitcoin#12752.
  *  bitcoin#16577.
  *  bitcoin#16670. (without faebf62)
  *  bitcoin#17957.
  *  bitcoin#18021.
  *  bitcoin#18087.
  *  bitcoin#18112 (only from 353f376 that we don't support).
  *  bitcoin#18167.
  *  bitcoin#18317.
  *  bitcoin#18591 (only Span's commit 0fbde48)
  *  bitcoin#18468.
  *  bitcoin#19020.
  *  bitcoin#19032.
  *  bitcoin#19367.
  *  bitcoin#19387.

  Net, NetAddress and AddrMan:
  ----------------

  *  bitcoin#7932.
  *  bitcoin#10756.
  *  bitcoin#10765.
  *  bitcoin#12218.
  *  bitcoin#12855.
  *  bitcoin#13532.
  *  bitcoin#13575.
  *  bitcoin#13815.
  *  bitcoin#14532.
  *  bitcoin#15051.
  *  bitcoin#15138.
  *  bitcoin#15689.
  *  bitcoin#16702.
  *  bitcoin#17243.
  *  bitcoin#17345.
  *  bitcoin#17754.
  *  bitcoin#17758.
  *  bitcoin#17812.
  *  bitcoin#18023.
  *  bitcoin#18454.
  *  bitcoin#18512.
  *  bitcoin#19314.
  *  bitcoin#19687

  Keys and Addresses encoding:
  ----------------
  * bitcoin#11372.
  * bitcoin#17511.
  * bitcoin#17721.

  Util:
  ----------------
  * bitcoin#9140.
  * bitcoin#16577.
  * bitcoin#16889.
  * bitcoin#19593.

  Bench:
  ----------------
  * bitcoin#16299.

  BIP155:
  ----------------
  *  bitcoin#19351.
  *  bitcoin#19360.
  *  bitcoin#19534.
  *  bitcoin#19628.
  *  bitcoin#19841.
  *  bitcoin#19845.
  *  bitcoin#19954.
  *  bitcoin#19991 (pending).
  *  bitcoin#19845.
  *  bitcoin#20000 (pending).
  *  bitcoin#20120.
  *  bitcoin#20284.
  *  bitcoin#20564.
  *  bitcoin#21157 (pending).
  *  bitcoin#21564 (pending).
  *  Fully removed v2 onion addr support.
  *  Add hardcoded seeds.
  *  Add release-notes, changes to files.md and every needed documentation.

  I'm currently working on the PRs marked as "pending", this isn't over, but I'm pretty pretty close :). What a long road..

ACKs for top commit:
  random-zebra:
    utACK ecde04a
  Fuzzbawls:
    ACK ecde04a

Tree-SHA512: 82c95fbda76fce63f96d8a9af7fa9a89cb1e1b302b7891e27118a6103af0be23606bf202c7332fa61908205e6b6351764e2ec23d753f1e2484028f57c2e8b51a
vasild added a commit to vasild/bitcoin that referenced this pull request Aug 17, 2021
If only `-bind=addr:port` is given (without `-bind=...=onion`) then we
would bind to `addr:port` _and_ to `127.0.0.1:8334` in addition which
may be unexpected, assuming the semantic of `-bind=addr:port` is
"bind _only_ to `addr:port`".

Change the above to not do the additional bind: if only
`-bind=addr:port` is given (without `-bind=...=onion`) then bind to
`addr:port` (only). If we are creating a Tor hidden service then use
`addr:port` as target (same behavior as before
bitcoin#19991).

This allows disabling binding on the onion port.

Fixes bitcoin#22726
vasild added a commit to vasild/bitcoin that referenced this pull request Sep 16, 2021
If only `-bind=addr:port` is given (without `-bind=...=onion`) then we
would bind to `addr:port` _and_ to `127.0.0.1:8334` in addition which
may be unexpected, assuming the semantic of `-bind=addr:port` is
"bind _only_ to `addr:port`".

Change the above to not do the additional bind: if only
`-bind=addr:port` is given (without `-bind=...=onion`) then bind to
`addr:port` (only). If we are creating a Tor hidden service then use
`addr:port` as target (same behavior as before
bitcoin#19991).

This allows disabling binding on the onion port.

Fixes bitcoin#22726
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Nov 10, 2021
Summary:
It is documented in the ip(7) manual page.

This is a backport of [[bitcoin/bitcoin#19991 | core#19991]] [1/8]
bitcoin/bitcoin@b3273cf

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D10444
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Nov 10, 2021
Summary:
This change allows to distinguish incoming Tor connections from others.
This is a backport of [[bitcoin/bitcoin#19991 | core#19991]] [2/8]
bitcoin/bitcoin@a5266d4

Depends on D10444

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D10445
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Nov 10, 2021
Summary:
This is a backport of [[bitcoin/bitcoin#19991 | core#19991]] [3/8]
bitcoin/bitcoin@fdd3ae4

Depends on D10445

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D10446
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Nov 10, 2021
Summary:
`target` is a proper name for the onion service target address and port.
This change is required for the following commit.

This is a backport of [[bitcoin/bitcoin#19991 | core#19991]] [4/8]
bitcoin/bitcoin@e3f0785

Depends on D10446

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D10447
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Nov 10, 2021
Summary:
This is a backport of [[bitcoin/bitcoin#19991 | core#19991]] [5/8]
bitcoin/bitcoin@57f17e5

Depends on D10447

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D10448
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Nov 10, 2021
Summary:
This change simplifies the following commit.

This is a backport of [[bitcoin/bitcoin#19991 | core#19991]] [6/8]
bitcoin/bitcoin@92bd3c1

Depends on D10448

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Subscribers: majcosta

Differential Revision: https://reviews.bitcoinabc.org/D10449
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Nov 10, 2021
Summary:
This is a backport of [[bitcoin/bitcoin#19991 | core#19991]] [7/8]
 bitcoin/bitcoin@bb145c9

Depends on D10449

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D10450
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Nov 10, 2021
Summary:
This is a backport of [[bitcoin/bitcoin#19991 | core#19991]] [8/8]
bitcoin/bitcoin@96571b3

Depends on D10450

Test Plan: proofreading

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D10451
vasild added a commit to vasild/bitcoin that referenced this pull request Jun 13, 2022
If only `-bind=addr:port` is given (without `-bind=...=onion`) then we
would bind to `addr:port` _and_ to `127.0.0.1:8334` in addition which
may be unexpected, assuming the semantic of `-bind=addr:port` is
"bind _only_ to `addr:port`".

Change the above to not do the additional bind: if only
`-bind=addr:port` is given (without `-bind=...=onion`) then bind to
`addr:port` (only). If we are creating a Tor hidden service then use
`addr:port` as target (same behavior as before
bitcoin#19991).

This allows disabling binding on the onion port.

Fixes bitcoin#22726
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet