net: Prevent routing of deprecated Site Local IPv6

n-thumann commented at 5:02 pm on September 20, 2020: contributor

This PR fixes #19978. Currently Site Local IPv6 addresses as treated as routable, even though as of RFC 3879 they should not be routed. I therefore implemented a check to detect and flag them as not routable.

n-thumann force-pushed on Sep 20, 2020

DrahtBot added the label P2P on Sep 20, 2020

naumenkogs commented at 6:52 am on September 21, 2020: member

I’d say our code doesn’t have to implement the full spec, unless it’s beneficial for Bitcoin. Not sure we get any practical benefit from this change:

if someone wants to spam us with fake ipv6 addresses, there’s a plenty other ways to do so :)
the probability of an old/buggy node relaying these specific unroutable addresses is very low, and the harm is very little

On the other hand:

it adds more lines
we might break some existing setting (yeah, based on the outdated addresses, who knows)

I’m -0 on this right now. Perhaps you have another reasoning wrt this change on mind?

laanwj commented at 2:57 pm on September 21, 2020: member

I think it makes sense to consistently reject all local networks here, even the deprecated one. It’s a small straightforward code change too. Code review ACK fbab5ed98fc936fe0904b05c1ebefcf8beb30ef6

Saibato commented at 6:23 pm on September 21, 2020: contributor

Tyi. Not waving in here for now, if that makes sense or not.

But since rfc5000 is not longer the STD1 RFC and deprecated in rfc7100

I can only recommend to grep and use for standard advice in https://www.rfc-editor.org/standards

There are only few binding Standad’s defined in regard to ipv6,

AFAICS rfc3879 is no Standard.

practicalswift commented at 6:52 pm on September 21, 2020: contributor

Concept ACK

Rationale:

We want to have the same definition of what constitutes a local network as Tor: connecting to fec0:: is disallowed via Tor due to being a local address
We don’t want to run the risk of leaking users’ local addresses to peers: could theoretically be used for fingerprinting or network reconnaissance?
We don’t want our peers to be able to feed us addresses that are local and thus potentially make us connect to internal servers

Unfortunately fec0:: addresses still exist in the wild, so this is not a purely a theoretical issue :)

in src/test/netbase_tests.cpp:68 in fbab5ed98f outdated

64@@ -65,6 +65,7 @@ BOOST_AUTO_TEST_CASE(netbase_properties)
65     BOOST_CHECK(ResolveIP("2001:10::").IsRFC4843());
66     BOOST_CHECK(ResolveIP("2001:20::").IsRFC7343());
67     BOOST_CHECK(ResolveIP("FE80::").IsRFC4862());
68+    BOOST_CHECK(ResolveIP("FEC0::").IsRFC3897());

practicalswift commented at 6:57 pm on September 21, 2020:

Could add BOOST_CHECK(!ResolveIP("FEC0::").IsRoutable()); too? :)

n-thumann commented at 3:38 pm on September 25, 2020:

Sure, added ✌️

Saibato commented at 9:02 pm on September 21, 2020: contributor

We definitely should not use Link-Local addresses

0   |   10     |
1   |  bits    |         54 bits         |          64 bits           |
2   +----------+-------------------------+----------------------------+
3   |1111111010|           0             |       interface ID         |
4   +----------+-------------------------+----------------------------+

But digging further. Not sure something was mixed and Tor is outdated since the newer Draft Standard rfc4291 clearly states about address of the Site-Local addresses format we discuss here

0   |   10     |
1   |  bits    |         54 bits         |         64 bits            |
2   +----------+-------------------------+----------------------------+
3   |1111111011|        subnet ID        |       interface ID         |
4   +----------+-------------------------+----------------------------+

0   The special behavior of this prefix defined in [RFC3513] must no
1   longer be supported in new implementations (i.e., new implementations
2   must treat this prefix as Global Unicast).
3
4   Existing implementations and deployments may continue to use this
5   prefix.

And thereby those address are to be treated as any global unicast like any routeable ipv6 address.

So rfc3879 is outdated as it refer to rfc3513 where those address are defined non routeable but now are routeable and then Tor needs some update.

Or I am simply wrong here? and we should anyway better follow Tor and risk no leaks and not follow the internet Draft Standard but use an outdated recommendation and an obsoleted proposed standard?

Its clearly true if we follow Tor we must change but since we had this practice all along and Tor follows outdated rfc, i am not sure if it might be better to file a Tor trac and earn for the bitcoin dev there some bug points?

practicalswift commented at 5:14 am on September 22, 2020: contributor

@Saibato

Oh, TIL! Thanks a lot!

I was wrong about the current routability status of fce0::/10 per the current standard.

Turns out this prefix has had a bumpy road:

It was non-routable as of RFC 3513 (2003): “Routers must not forward any packets with site-local source or destination addresses outside of the site.”
It was non-routable as of RFC 3879 (2004): “However, router implementations SHOULD be configured to prevent routing of this prefix by default.”
It became routable again as of RFC 4291 (2006, current standard): “The special behavior of this prefix defined in [RFC3513] must no longer be supported in new implementations (i.e., new implementations must treat this prefix as Global Unicast).”

I guess Tor is erring on the side of caution by disallowing connections to a prefix that has previously been for site local use :)

practicalswift commented at 2:46 pm on September 22, 2020: contributor

It should be noted that fec0::/10 has always been reserved by the IETF and has thus never been in active Internet use. Any use of fec0:: in a non-site local context (such as in ADDR messages) is thus likely accidental. I think it makes sense to have the same definition as Tor on what constitutes a local address to rule out potential issues such as IP address leaks and node fingerprinting.

From https://www.iana.org/assignments/ipv6-address-space/ipv6-address-space.xhtml:

0fec0::/10 | Reserved by IETF | [RFC3879] | Deprecated by [RFC3879] in September 2004. Formerly a Site-Local scoped address prefix.

Saibato commented at 4:07 pm on September 22, 2020: contributor

@practicalswift Time to make some internet history, I find the IETF in contrast to IANA approach always a bit more in favor of an open internet, those address blocking’s where more like proposal rfc spam than ever be a standard or close to that In my view the … attempt to get blocking list into routers and to fragment the internet.

In roundabout 2007 those source code lines to follow blocking rules by rfc3879 came into Tor core. from utils.c to address.c they where since then never updated to follow the newest standard.

I tend to file a bug trac issue on Tor to decide whom they want to follow, but since u noticed first that we did never blocked those ip ranges. please outpace me in that.

as in ADDR messages) is thus likely accidental.

Though i hope u see the potential here to dig some privacy tunnels, IETF had granted us to get the internet back again.

Although if u can outline one real world scenario where those addresses could even remotely leak or fingerprint in any way like what the client does since eons with seeding and -onlynet=onion, i consent instantly.

practicalswift commented at 6:38 pm on September 22, 2020: contributor

I tend to file a bug trac issue on Tor to decide whom they want to follow, but since u noticed first that we did never blocked those ip ranges. please outpace me in that.

Feel free to open such an issue if you want their position clarified.

Personally I think Tor is intentionally erring on the side of caution here, and I think we should do that too :)

FWIW you’ll occasionally see fec0::/10 addresses in ADDR messages on mainnet. In other words this is not purely theoretical :)

net: Prevent routing of deprecated Site Local IPv6 c4ba79b7f7

n-thumann force-pushed on Sep 25, 2020

practicalswift commented at 7:49 pm on October 11, 2020: contributor

An example from mainnet where Bitcoin Core tries to connect to a fec0::/10 address learned from a mainnet peer.

The connections is blocked by Tor:

0Oct 10 12:34:56.000 [warn] Rejecting SOCKS request for anonymous connection to private address fec0::5b4f:fa94:a34b:1731.

fanquake commented at 3:48 am on October 15, 2020: member

@gmaxwell do you have any thoughts/insight here?

laanwj commented at 4:58 am on November 20, 2020: member

We should either merge this or close #19978 as “not an issue”. I’d personally err on the side of merging this, though I also get the point that it makes very little difference and we can’t and shouldn’t maintain a global IPv6 blacklist as part of bitcoin core and make a dayjob out of rule-lawyering IETF specs.

practicalswift commented at 8:04 am on November 20, 2020: contributor

Agree with @laanwj. FWIW I think this is the only divergence between what we consider to be the public Internet and what Tor considers to be the public Internet :)

Saibato commented at 9:40 am on November 20, 2020: contributor

shouldn’t maintain a global IPv6 blacklist as part of bitcoin core and make a dayjob out of rule-lawyering IETF specs.

Agree, I also think we can now close #19978, since Tor will move from 0.4.6.x there codebase in favor of also not exempt those addresses any more and follow the advance of the Internet standardization process.

Tyi: https://gitlab.torproject.org/tpo/core/tor/-/milestones/65

So thx, to all who participated in this,

Once again, Bitcoin has proved that we stand for no censorship at all and specially thx to @practicalswift to notice that issue and we together prevented shit happened before ipv6 is wildly used,

Also special thx to @n-thumann for done this now void work but be sure u where part of something really important.

jonatack commented at 4:53 pm on November 20, 2020: member

I’ve reviewed and tested this but it’s not clear to me if it should be done.

ACK c4ba79b7f7bdf32bbfb6bd9ba57be5e8045fabbd if we want to do this

Saibato commented at 5:02 pm on November 20, 2020: contributor

@jonatack tyi.

if it should be done. ???

Concept NACK Bcs. Tor made to *not block those any longer* a milestone for next releases and probably even backport’s, since to block them even in the first place is technical debt on Tor side to not keep it since it was never a standard but just on the wish list of early drafts of censors that was reverted by the standard process.

sipa commented at 11:08 pm on November 20, 2020: member

Should we just implement https://www.iana.org/assignments/ipv6-address-space/ipv6-address-space.xhtml? Only 2000::/3 is global unicast. Other ranges may be allocated in the future of course, but given that only 1.39% of 2000::/3 is even assigned to RIRs, I don’t expect that to change any time soon. And the current blacklisted ranges can change too… so perhaps it’s easier to just have 1 whitelisted range rather than dozens of individually blacklisted ranges.

Saibato commented at 9:04 am on November 21, 2020: contributor

Only 2000::/3 is global unicast @sipa That is what IANA defines, suppose they assign port 8333 solely for router internal use after 2020? Will we then follow that Whitelist too, when RFC’s track say;s otherwise? What IANA assigned or assigns is imo irrelevant to the Internet Standard process and Bitcoin the Internet Money is good advised to only follow the RFC’s.

There is and will probably ever be only one exemption in ipv6 from the unicast range to be not global unicast assignable (what in effect mean they will be not routeable like ::0 and ::1) and that is the whole ff00::/8 range defined by Standard Track RFC4291

If we would do a Whitelist, where do you start and when do u stop and what is the update interval to that Whitelist?, u become a censor and slave of IANA, when in fact there is only one Internet Standard and that very app that clams to be Internet compatible has to follow strict, its the RFC standard track process, When Tor followed RFC 3879 what was not even in the standards track but just a proposed standard wish list document like any other opinion one can have or not, that was in hindsight a mistake in Tor . We should not do the same.

The internet as such is the RFC’s track there is no second Internet, same as we would never allow miners to censor tx that we and the user define as valid.,

laanwj commented at 11:01 am on December 21, 2020: member

so perhaps it’s easier to just have 1 whitelisted range rather than dozens of individually blacklisted ranges.

I really like this idea from a simplicity point of view. I sometimes wonder what other P2P software does with regard to whitelisting/blacklisting subnet ranges for relayed addresses. By the definition it is only relevant to relay globally routable unicast addresses, not local ones, not special-purpose ones, not multi-cast ones.

DrahtBot commented at 6:11 pm on June 7, 2021: member

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#22179 by vasild

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.

DrahtBot commented at 3:40 pm on July 8, 2021: member

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.

DrahtBot added the label Needs rebase on Jul 8, 2021

DrahtBot commented at 12:29 pm on December 22, 2021: member

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it ‘Up for grabs’ with the label, so that it can be picked up in the future.

MarcoFalke added the label Up for grabs on Apr 29, 2022

MarcoFalke commented at 2:16 pm on April 29, 2022: member

Marked “up for grabs”

MarcoFalke commented at 2:16 pm on April 29, 2022: member

Closing for now. Let us know when it should be reopened.

MarcoFalke closed this on Apr 29, 2022

DrahtBot locked this on Apr 29, 2023

net: Prevent routing of deprecated Site Local IPv6 #19985

Conflicts