search

libp2p / specs

Technical specifications for the libp2p networking stack
https://libp2p.io
1.58k stars 275 forks source link

addressing: Specify security protocols in multiaddr #353

Open marten-seemann opened 3 years ago

marten-seemann commented 3 years ago

Proposal to advertise security protocols in multiaddr, e.g. /ip4/6.6.6.6/tcp/1234/tls.

This is split out of the Protocol Select specification proposal (https://github.com/libp2p/specs/pull/349) and instead specified as a independent change.

mxinden commented 3 years ago

This proposal is ready for review and we welcome feedback by everyone across the whole libp2p community. Note that this proposal touches two fundamental building block of libp2p, namely addressing and protocol negotiation.

For additional context: Protocol Select builds on top of this proposal (https://github.com/libp2p/specs/pull/349).

marten-seemann commented 3 years ago

Do we need to plan for a "phased roll out" for this, where nodes advertise both styles of multiaddr for a while?

We'll need a phased rollout, as the current code fails to properly parse / handle a security-enabled multiaddr (at least on the Go side). This is good. There are two ways to do a phased rollout here:

  1. As you suggest, advertise both the old and the new, security-enabled multiaddrs at the same time.
  2. Update the parsing / dialing code first, and switch to security-enabled multiaddrs once a large enough fraction of the network is able to handle these addresses.

Option 1 allows for a faster rollout, at the expense of more addresses being advertised. I think we can live with that overhead, so this would be my preference.

vyzo commented 3 years ago

It seems to me that we are doing this backwards with the relay addrs. What's wrong with /addr.../p2p-circuit/sec-proto? This allows us to easily compose and create a valid AddrInfo with /addr.../p2p-circuit/tls/p2p/QmX and has no ambiguities.

vyzo commented 3 years ago

If we must introduce p2p-circuit-inner (I really want to call this p2p-circuit-sec), then it should come before the /p2p/QmX part in the multiaddr.

So the circuit multiaddr for QmX would be /relay-addr/p2p-circuit/p2p-circuit-sec/tls/p2p/QmX and not /relay-add/p2p-circuit/p2p/QmX/p2p-circuit-sec/tls.

mxinden commented 3 years ago

@vyzo thank you for taking a look. I am having difficulties following your five comments above. Would you mind summarizing them in a single one?

Do I understand correctly that you would like to:

vyzo commented 3 years ago

yes. the second part is important for composability and parsing of multiaddrs; it allows us to omit the p2p part for our own addresses and parse complete multiaddrs as AddrInfos.

On Wed, Sep 29, 2021, 14:55 Max Inden @.***> wrote:

@vyzo https://github.com/vyzo thank you for taking a look. I am having difficulties following your five comments above. Would you mind summarizing them in a single one?

Do I understand correctly that you would like to:

  • Rename /p2p-circuit-inner to /p2p-circuit-sec?
  • Move p2p-circuit-sec before the /p2p/QmX of the destination node?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/libp2p/specs/pull/353#issuecomment-930106422, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAI4STOFS6UYQXL7I4GFUDUEL5DPANCNFSM5A6TGU5A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

mxinden commented 3 years ago

https://github.com/libp2p/specs/pull/353/commits/3c1487a106943ade936c5185def680c67c9496c8 and https://github.com/libp2p/specs/pull/353/commits/90bef6fa4752add96bff7ebf016d247cc120c34d change the two points discussed above. Note that instead of p2p-circuit-sec it uses p2p-circuit-security to disambiguate SECurity with SECond.

@vyzo would you mind taking another look?

mxinden commented 3 years ago

Friendly ping @vyzo. Do the latest commits address your "request for change"?

Menduist commented 2 years ago

If I understand correctly, we won't be able to support multiple security protocol on a single port anymore? ie, need to do /ip6/::1/tcp/1234/tls/ /ip6/::1/tcp/4567/noise/

That seems bad to me, it would become very difficult to, for instance, update noise to a new non-compatible version. (if that seem unlikely to you, https://github.com/libp2p/specs/issues/355 requires a new non-compatible noise version to be fixed, for instance) The only solution would be to listen on multiple ports for the duration of the transition, which forces end-users to setup multiple ports, multiple redirection in their NAT, etc

I'm also not 100% convinced by the "downgrade" argument. Sure, it applies to mobile phones because you can't update the 3310s to safer protocols, but in our case, if a protocol is less secure, just disable it (like it was done for secio), push an update, and problem solved. EDIT: If we want to be super safe, we could actually send the supported security protocols as part of "identify", and if we detect that the preferred one wasn't selected, kill the connection.

Finally, I'm having a hard time to imagine how active relays work in this scenario. A: only supports tcp/noise R: supports quic + tcp/noise B: supports quic + noise + tls (not TCP)

How does B advertise that it handles noise? Is it supposed to advertise something like

Weird, but ok. Except that now we're back to my first remark, when it will receive a connection coming from a relay, it will not know if it's a noise or a tls one. So again, need to listen on 2 different ports.

Now, let's be crazy and say that B also handles Webrtc, it will need to advertise:

To sum up, it seems to me that we are putting "capabilities" into the MA, and that doesn't seem right. Each MA is already a capability. My understanding is that this grew out of the goal of 0/low-RTT setups, but low RTTs can also be achieved with a negotiation.

A: supports Noise, TLS, Secio B: supports TLS

A -> B: open connection using Noise, TLS or Secio If you are compatible with noise, here is some early data: [the noise handshake] B -> A: only compatible with TLS, please retry A -> B: [TLS handshake]

You can improve on this in multiple ways (include handshake for both noise & tls in the first message, or in case of incompatibility send a TLS handshake from B to A to save half a RTT)

marten-seemann commented 2 years ago

@Menduist Thank for you for your review.

There are multiple reasons for including the security in the multiaddr:

  1. It saves one roundtrip (for a reply regarding your example see below).
  2. It prevents downgrade attacks.
  3. It makes it possible to disguise libp2p traffic as regular web traffic (by running /tls on port 443).

Note that is not required to run different handshake algorithms on different ports. You could also try to demux on the first (couple of) bytes of the first handshake message. Using different ports is the implementation strategy we will use in go-libp2p though. The censorship-resistance argument is actually the convincing one for me.

I'm also not 100% convinced by the "downgrade" argument. Sure, it applies to mobile phones because you can't update the 3310s to safer protocols, but in our case, if a protocol is less secure, just disable it (like it was done for secio), push an update, and problem solved.

Sure, this is clear-cut example. If a protocol is broken, you disable it. But the more realistic option is that you (strongly) prefer one protocol over the other, even if it's not considered broken yet. This was actually the case with TLS / Noise vs. secio for the longest time.

To sum up, it seems to me that we are putting "capabilities" into the MA, and that doesn't seem right.

Another angle to look is that we're using port number to run multiple incompatible protocols on the same host, which is exactly what port numbers were invented for.

A -> B: open connection using Noise, TLS or Secio If you are compatible with noise, here is some early data: [the noise handshake] B -> A: only compatible with TLS, please retry A -> B: [TLS handshake]

  1. Saving a connection after an invalid message has been sent has proved quite difficult. Realistically, we'd need to introduce special framing for that.
  2. You're assuming that Noise is supported by all / most implementations. That's the case today, but may not be the case in the future.
Menduist commented 2 years ago

Thanks for the details, let me address the benefits you talked about:

It prevents downgrade attacks.

I would argue that since different security protocols runs on different ports, it becomes even easier to pull up a downgrade attack. Let me introduce the Generalized MITM Libp2p Connection Downgrader™:

proc newConnectionDetected(connection):
  if connectionIsUnsafe:
    # hurray!
  else:
    blockThePort()

Since libp2p will try every address, it will eventually try a port/security protocol combo that is unsafe, or the connection won't go through. It's a bit slower than the previous attack, but can be sped up by knowing the "safe" ports before hand, and blocking them

Note that even if you manage to put every security protocol on the same port, This attack could still work: instead of blocking the port, just block the specific message.

I presented in my first review a solution to downgrade attacks, I'll go in more detail here: We could, once the connection is safe, restart the security negotiation. It doesn't have to block the upgrade process, and doesn't have to do anything in the end, but if we come out with a different security protocol, we kill the connection because we've been downgraded.

It's slow and stupid, but the fact that this security exist should deter anyone from trying to downgrade the connections. It also has the added benefit that we could reuse the same system to check security protocols we don't control (eg, in quic, check that we use TLS 1.3 or whatever the case may be) We could piggy-back by adding it to the identify / SPR, which wouldn't cause any more round trips, and would have other benefits

It saves one roundtrip

Saving a connection after an invalid message has been sent has proved quite difficult

See my proposal here: https://github.com/libp2p/specs/pull/349#discussion_r822768801 It's very straightforward, once protocol-select is implemented, I suspect this would represent ~15 LOC in nim-libp2p to add this extension

You're assuming that Noise is supported by all / most implementations

I used noise as an example, but it could be any protocol. Here are a few possibilities to improve that:

Also, I'm may be biased because of how the nim-libp2p is used, but sometimes, the security protocols are defined by an upper spec, eg eth2

It makes it possible to disguise libp2p traffic as regular web traffic (by running /tls on port 443).

I agree. However, if we take a step back, that is, as far as I know, just a "theoretical" problem for now, ie, no one blocked libp2p using traffic pattern detection so far. If it happens one day*, we already have some transports that can't be detected (WSS, Quic, Webrtc), which would leave us enough time to apply proper countermeasures.

* I personally hope that libp2p will spread enough, and "blocking libp2p" won't just imply "I can't access eth2, ipfs and some dApps", but "I can't talk to my friends nor play this cool game". But as they say, hope for the best, plan for the worst

Other comments

Another angle to look is that we're using port number to run multiple incompatible protocols on the same host, which is exactly what port numbers were invented for.

Yes, though you don't see a new port being used everytime TLS is updated. I mean, in the 30 years HTTP existed, they only used 2 ports (80, 443), but in my last relay example I already need 4 ports. imo, more port is just super user-unfriendly, and that applies at every level:

Let's just take as an example "noise migration from /noise to /noise/2" from the perspective of a eth2 CL user:

You could argue that the port should be transparent to the user, and in some cases that apply. But if you run a node worth >70k$, you probably want drastic security on it, with every unknown port blocked. That whole upgrade process is kinda painful, and quite error-prone.

Closing thoughts

With this proposal, the downgrade is not addressed completely, the round trip could be saved in other ways.

The complexity added to relays is imo, unnecessary (if a relay is blocking libp2p.. just use another one)

imo, the only valid improvement is the ability to do /ip4/6.6.6.6/tcp/1234/tls in the case where your ISP/whatever actively blocks libp2p traffic. That's a very specific use case, that, AFAIK, no one hit yet. However, this proposal have multiple downsides: you need more ports, security protocols upgrades becomes extra painful

So it should not become a default, but just something we could do if at some point we start to see traffic-based censorship