libp2p / specs

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

add spec for quic + noise #315

Closed dvc94ch closed 3 years ago

dvc94ch commented 3 years ago

The implementation used for ipfs-embed is going to be (or based on) [0]. In particular ipfs-embed only supports ed25519 keys and dialing addresses that contain a peer_id. This means that when using libp2p-quic [1] we can use 0-rtt encryption to negotiate the substream protocol. It also supports private nets without requiring an additional protocol, a pre shared key is mixed into the handshake state if one is provided.

Stebalien commented 3 years ago

I'm confused. Why QUIC + Noise? QUIC has built-in encryption. If you want private network support, the correct solution is https://tools.ietf.org/html/rfc8773.

dvc94ch commented 3 years ago

Well the tls spec is quite a bit more complicated and the security properties are less obvious. A lot of the complexity comes from pki infrastructure which is not used by libp2p. If you looked at the implementation in quinn-noise, it's pretty small and straight forward. Also the 0-rtt is much more complex in tls and requires persisting state across connections. In quinn-noise we can have 0-rtt with any addr/peer_id. We also save an entire trip using a 2-way handshake leading to lower latency.

I'm not sure why one solution would be correcter than the other solution or why a libp2p transport requires full blown tls. In either case I'd probably want to customize the cipher since chacha8 is 2.4x faster than chacha20, provides enough security and has an impact on maximum throughput, as roughly 50% of the time is spent encrypting/decrypting packets leading to a bottleneck.

Stebalien commented 3 years ago

Also the 0-rtt is much more complex in tls and requires persisting state across connections. In quinn-noise we can have 0-rtt with any addr/peer_id.

0-rtt requires persisting state to prevent replay attacks. If your variant doesn't persist state, it's not secure. This isn't some strange requirement of TLS, it's impossible for any protocol to do 0-rtt handshakes without replay without this.

I'm not sure why one solution would be correcter than the other solution

Because QUIC depends on TLS and TLS has a "blessed" way to do this.

Obviously, if we're using QUIC+Noise, we'd need to do this some other way. But I wouldn't consider this a reason to add support for QUIC+Noise.


Basically, I'm very concerned about implementing custom crypto protocols and/or altering existing crypto protocols without fully understanding the repercussions. We can do this if necessary and done carefully (e.g., libp2p's noise handshake) but it's not as simple as "someone implements, we write a spec, done".

dvc94ch commented 3 years ago

tls 0rtt isn't secure against replay attacks either afaik. The reason why you need to maintain state is because you don't know your peers public key. in libp2p we have the peerid.

You need to consider if you can tolerate having some messages replayed or not. As a payment service provider or blockchain the answer is no. For many applications it likely doesn't matter much, especially if all you're doing is negotiating the substream protocol. (Also just because you can do something doesn't mean you have to)

I don't think there is any rocket science involved, but I do understand your point.

Stebalien commented 3 years ago

tls 0rtt isn't secure against replay attacks either afaik.

It can be if you only allow session resumption keys to be used exactly once.

But you're absolutely right, this isn't why TLS needs state. I've been thinking in "TLS for libp2p" so long I had completely forgotten...

I don't think there is any rocket science involved, but I do understand your point.

I agree. I just want to make sure we have a very good reason and a very good spec.

Now that I've gotten over my initial terror/confusion, I can see why this might be useful.

dvc94ch commented 3 years ago

I agree. I just want to make sure we have a very good reason and a very good spec.

I welcome constructive feedback/criticism. I haven't been able to absorb all of the quic spec yet and they're surely much more knowledgeable people than me. I thought the implementation could serve as a starting ground, and since the handshake string is included in the initial message ipfs-embed should be able to transition if a better spec does materialize.

marten-seemann commented 3 years ago

For future reference, I just composed a list of topics that would need to be addressed if one wanted to specify Noise+QUIC: https://github.com/libp2p/specs/pull/351#issuecomment-882871930. As one can see, just specifying this would be a massive undertaking, and then it's questionable if we can find enough people for a thorough security review, as has happened for TLS 1.3 and QUIC.

dvc94ch commented 3 years ago

Not sure why you need to review noises security. However noise is more conservative and therefore complicated than what I presented. It is pretty similar to the disco extension that uses strobe but more performant. The construction is copied verbatim from the xoodyak paper submitted to the nist light weight crypto competition. And it's from the keckak team. So it's likely to become an official nist standard in the next year.

dvc94ch commented 3 years ago

FYI in regards to performance (since you apparently wrote some of quic-go). You should know that the way google achieves performance is by using DPDK so they can get away with slow ciphers. I can link at least three papers that say encryption is the largest bottleneck in quic implementations as you have surely experienced yourself. Who will use quic instead of tcp/noise if it's 3x slower?

marten-seemann commented 3 years ago

Not sure why you need to review noises security.

What needs to be reviewed is how QUIC would use Noise. There are a bunch of non-trivial bits here, including packet encryption, header protection, key updates and AEAD limits. See the appendix of RFC 9001 for a very short summary of the type of analysis that went into the cipher suites used by QUIC.

You should know that the way google achieves performance is by using DPDK so they can get away with slow ciphers.

Not sure if that's true in 2021 any more, but it doesn't really matter. I'd also dispute that ChaCha20 and AES GCM are "slow ciphers". A lot of other companies use TLS 1.3 (which uses the same cipher suites) and / or QUIC, even if all that their machines are doing is serving content (like the edge server of a CDN).

Who will use quic instead of tcp/noise if it's 3x slower?

Given that libp2p is a p2p library, squeezing out the last bit of CPU performance is far less critical for our use case. p2p nodes do a lot of other stuff that costs CPU cycles (as compared to for example a CDN edge node, which basically only encrypts and ships bytes). To answer your question, in many cases, I'd be very happy to trade a faster handshaker, better network performance due to superior loss recovery, higher hole punching success rates, lower file descriptor usage, etc etc for a 3x increase in CPU load of a code path didn't account for much load in the first place.

dvc94ch commented 3 years ago

So is this discussion about my progressive choice of crypto? We can easily make it more conservative. The IK handshake allows 0rtt encryption, which was requested by someone on GitHub. My initial prototype was closer to the libp2p noise spec using an xx handshake. The cipher was chosen after realizing it had a large impact on performance. Xoodyak was chosen after I realized that the snow (rust noise implementation) didn't match the quic requirements and I decided I had to reimplement it from scratch. Xoodyak seemed much simpler than correctly combining three cryptographic primitives like the noise spec does. Xoodyak/strobe were designed for non cryptographers implementing protocols.

vyzo commented 3 years ago

and now you have a second terrified libp2p engineer...

On Tue, Jul 20, 2021, 02:10 David Craven @.***> wrote:

So is this discussion about my progressive choice of crypto? We can easily make it more conservative. The IK handshake allows 0rtt encryption, which was requested by someone on GitHub. My initial prototype was closer to the libp2p noise spec using an xx handshake. The cipher was chosen after realizing it had a large impact on performance. Xoodyak was chosen after I realized that the snow (rust noise implementation) didn't match the quic requirements and I decided I had to reimplement it from scratch. Xoodyak seemed much simpler than correctly combining three cryptographic primitives like the noise spec does. Xoodyak/strobe were designed for non cryptographers implementing protocols.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/libp2p/specs/issues/315#issuecomment-882920422, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAI4SXJ4X4FVC6HNUJWBSLTYSWIBANCNFSM43EAUE5Q .

dvc94ch commented 3 years ago

and now you have a second terrified libp2p engineer...

very helpful comment, thanks a lot

dvc94ch commented 3 years ago

@vyzo not sure which of these two statements you're mocking however:

I decided I had to reimplement it from scratch

there are good reasons for this as explained elsewhere.

than correctly combining three cryptographic primitives

since there are good reasons for this it is not a 1:1 clone.

also I'm a bit confused on what makes you a cryptography or quic expert. however even if you are but have spent less than 5min on the topic of quic-noise, I'd suggest a little less condecention.

vyzo commented 3 years ago

sorry, I really didn't mean to mock anything. I am just terrified of the "I decided I had to reimplement it from scratch" statement when it comes to crypto.

On Tue, Jul 20, 2021 at 9:08 AM David Craven @.***> wrote:

@vyzo https://github.com/vyzo not sure which of these two statements you're mocking however:

I decided I had to reimplement it from scratch

there are good reasons for this as explained elsewhere.

than correctly combining three cryptographic primitives

since there are good reasons for this it is not a 1:1 clone.

also I'm a bit confused on what makes you a cryptography or quic expert. however even if you are but have spent less than 5min on the topic of quic-noise, I'd suggest a little less condecention.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/libp2p/specs/issues/315#issuecomment-883099207, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAI4SU5NOA6QWPHDKQYVILTYUHEZANCNFSM43EAUE5Q .

dvc94ch commented 3 years ago

I am just terrified of the "decided I had to reimplement it from scratch" statement when it comes to crypto.

Let's hope you're a bit more confident when it comes to the file coin crypto ;) I think a lot of these fears come from past times with past algorithms that were notoriously difficult to implement and use correctly. The critical parts like diffie hellman are luckily implemented by a cryptographer (ed25519-dalek). And the handshake itself is also analyzed, so the properties of an IKpsk1 handshake are understood.

whyrusleeping commented 3 years ago

At the end of the day, cryptography is not like playing with legos. Even if the individual shapes are simple and well understood, the specific way you piece them together always needs to be thought about more deeply.

This discussion (and discussion in the associated PR) is not the libp2p team telling you (@dvc94ch ) that this idea cant work, but rather that it is complicated and will need additional effort and review to get it to a point where we can collectively feel good about it. Your responses boil down to you insisting that "no it doesnt need anyone to review it", which is entirely false, I would want someone knowledgeable to review something as straightforward as a adding tls on top of a tcp pipe.

Let's hope you're a bit more confident when it comes to the file coin crypto

Yes, we're moderately confident in the cryptography we have put together there, after three years of work, many papers, academic reviews, and audits. We didnt just throw together an implementation, toss a sketch in a markdown document and call it good because zkSNARKs were well understood and we had an implementation of depth robust graphs from a cryptographer.

To move this forward, please address the concerns that @marten-seemann put forward in his comments.

dvc94ch commented 3 years ago

Your responses boil down to you insisting that "no it doesnt need anyone to review it", which is entirely false

I don't think that is an adequate representation of the situation.

I would want someone knowledgeable to review something

I do agree with the sentiment that peer review is useful.

To move this forward, please address the concerns that @marten-seemann put forward in his comments.

I can somewhat address them. From the comments I can somewhat discern what he is unhappy with, but not necessarily what would make him happy. So I guess you expect me to come up with my next best guess of what something "better" is and open a new PR.

Yes, we're moderately confident in the cryptography we have put together there, after three years of work, many papers, academic reviews, and audits. We didnt just throw together an implementation, toss a sketch in a markdown document and call it good because zkSNARKs were well understood and we had an implementation of depth robust graphs from a cryptographer.

Give me a billion dollars and I'll hire a team to write papers and perform security audits ;) The entire cost of developing quinn-noise was 0$ but I'm happy to send you a bill if you think that's too little.

whyrusleeping commented 3 years ago

So you touch on a great point here. The cost of something like this.

You implementing it so far only costs your own time, but this conversation is you asking the libp2p team for a 'rubber stamp' on this spec document, to mark it as an official and accepted part of the stack. The team does not merge just anything into the spec, because that status implies a certain level of rigor around the protocols being documented. In order to get to that level of rigor, there is a cost. Martens time, Stevens time, mine and vyzos time, and anyone else who gets involved in the process. And it doesnt just stop there, audits, reviews, other implementations, maintenance overhead of any implementations that end up popping up in the ecosystem, and so on. The team is generally happy to pay these costs, as the tradeoffs are usually clear and worth it. In this case, the value add for spending our time reviewing and merging this document and making it a formal part of the ecosystem is not clear.

So coming back to quic/noise. First, It isnt clear why this a useful thing to have (I'm picking up that it might let you have 0-rtt handshakes, and the encryption is a bit faster than what quic ships with?), so a well phrased pitch as to why this is a good idea is in order.

Second, it has been made clear by both Steven and Marten that in order for a spec of this sort to be merged in, it needs to be very detailed. Marten has pointed you at another spec for securing quic with a given cryptographic protocol, which should be a good template since thats exactly the sort of thing you are writing. Your claims that you don't understand what would make him happy seem a bit disingenuous given he has stated that very clearly:

Basically, what one would have to do is produce a document along the lines of RFC 9001.

DemiMarie commented 3 years ago

@dvc94ch I strongly recommend going with QUIC + TLS, at least to start with. You can use either my PR or @tomaka’s as the basis.

dvc94ch commented 3 years ago

So how important do you think a libp2p rubber stamp is to me? You're demanding I do XYZ to get it while obviously not understanding why I even bothered opening this PR or what value I get out of it. Obviously you having unrealistic expectations just means it's not worth the cost of my time.

whyrusleeping commented 3 years ago

Our job here is to maintain and improve the libp2p ecosystem, not to make everyone happy. Spec writing is hard, I get it, not everyone can do it. You don't want to, or can't put in the work required to get a spec merged? Thats okay, have a nice day. If you change your mind, the requirements are clear, and the door is open.

whyrusleeping commented 3 years ago

In the future, please try to avoid engaging so aggressively and in bad faith. You seem to be taking everything as personally as possible, when nobody here intends it that way, This also appears to be a pattern, I distinctly remember you being involved in a large amount of drama around the rust ipfs implementation, Please take the time to give everyone you interact with the benefit of the doubt, Open source maintenance is hard enough as it is.

dvc94ch commented 3 years ago

distinctly remember you being involved in a large amount of drama around the rust ipfs implementation

rust-ipfs had severe deficiencies, and I know because at least at that point I had written most of the code. I pointed out those issues repeatedly, with the intention of fixing them. Instead they were covered up. And as you know now, it was grant driven development. As far as I know that project isn't maintained and had it's last commit in february. Not sure how you would feel if your project was abused to extract money from protocol labs.

However you can easily make a case that I'm not easy to get along with. I don't think I'd argue that.