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

(Pie in the sky) Supporting Noise as the cryptographic protocol #719

Closed
djc opened this issue Apr 18, 2020 · 24 comments · Fixed by #1106
Closed

(Pie in the sky) Supporting Noise as the cryptographic protocol #719

djc opened this issue Apr 18, 2020 · 24 comments · Fixed by #1106
Labels
help wanted Extra attention is needed

Comments

@djc
Copy link
Member

djc commented Apr 18, 2020

We have an internal abstraction over the cryptographic protocol used called the crypto::Session. (Note that this trait is currently private, so you'd have to make this public to start work. We'd be happy to merge that, but would likely prefer doing so when something materializes that actually uses that.)

Currently we have a single implementation for rustls. In QUIC v1, only TLS 1.3 is officially supported, so supporting something else is technically outside of the spec and can thus probably only be made to work between two consenting endpoints.

If we wanted to support Noise, that would likely start by writing an implementation of this thread that is based on Noise. I think there are some crates that implement (parts of) Noise -- snow probably makes for a good start.

There was a 2018 attempt at integrating Noise support into a much earlier version of Quinn. Quinn looks basically nothing like it did at the time, but maybe there are still useful bits to look at there.

If you want to work on this, we're usually available for questions on our Gitter channel -- just commenting in this issue should also work fine.

@djc djc added the help wanted Extra attention is needed label Apr 18, 2020
@Ralith
Copy link
Collaborator

Ralith commented Apr 18, 2020

Is the main upside to this a (perhaps drastic) reduction in the amount of data exchanged during the handshake?

@djc
Copy link
Member Author

djc commented Apr 19, 2020

I think it likely also saves on handshake CPU cycles, has different privacy considerations in some cases, and is purpose-built not to rely on the web PKI's CA system.

(Someone asked via email specifically if they could help out with Noise -- I felt it would be more useful to write up any instructions publicly.)

@dvc94ch
Copy link
Contributor

dvc94ch commented Apr 6, 2021

so for using noise, much of the code stays the same. the HmacKey, HandshakeTokenKey and retry_tag, is_valid_retry, don't need to be changed at all, and I'm not sure what the advantage would be to do something else.

most of the work is getting the read_handshake, write_handshake to work which is a little weird, because quinn expects Initial/Handshake/Transport keys at the right time. For noise without 0rtt, you can use no encryption during the initial phase, and since there is no 0rtt the handshake phase wouldn't be needed at all.

-> initial
<- initial
-> initial
-> data

instead of

-> initial
<- initial
<- handshake (I'm currently sending a payload of &[0] unencrypted here to sync the quinn/noise state machines)
-> handshake
-> data

Some open questions are how to implement next_1rtt_keys. With noise you'd perform a new handshake after 2^64 messages have been sent, so this doesn't seems to map well either.

@Ralith
Copy link
Collaborator

Ralith commented Apr 6, 2021

Do noise's ciphers not have documented confidentiality limits after which keys must be rotated? That's surprising. You can set up PacketKey::confidentiality_limit to return u64::MAX if you like, in which case next_1rtt_keys will in practice never get called, but I'm skeptical that it's correct to do so.

@dvc94ch
Copy link
Contributor

dvc94ch commented Apr 6, 2021

There are serveral mentions:

If EncryptWithAd() or DecryptWithAd() signal an error due to nonce exhaustion, then the application must delete the CipherState and terminate the session.

Note that rekey only updates the cipherstate's k value, it doesn't reset the cipherstate's n value, so applications performing rekey must still perform a new handshake if sending 2^64^ or more transport messages.

Incrementing nonces: Reusing a nonce value for n with the same key k for encryption would be catastrophic. Implementations must carefully follow the rules for nonces. Nonces are not allowed to wrap back to zero due to integer overflow, and the maximum nonce value is reserved. This means parties are not allowed to send more than 2^64^-1 transport messages.

Data volumes: The AESGCM cipher functions suffer a gradual reduction in security as the volume of data encrypted under a single key increases. Due to this, parties should not send more than 2^56^ bytes (roughly 72 petabytes) encrypted by a single key. If sending such large volumes of data is a possibility then different cipher functions should be chosen.

To summarize it in my own words, reusing nonces is bad, nonces are 64bit and aesgcm needs special consideration, but chachapoly is fine.

@dvc94ch
Copy link
Contributor

dvc94ch commented Apr 6, 2021

So will quinn terminate the session in the unlikely case occurs that 2^64-1 packets are sent? At the moment it will panic if something is encrypted/decrypted with the next_rtt1_key. chacha8poly1305 should be faster than aes with hardware acceleration. I'm currently using chacha20, but according to the blake3 author chacha8 provides enough practical security. [0]

@djc
Copy link
Member Author

djc commented Apr 6, 2021

Hmm, I feel like Initial corresponds to Noise pre-messages and Handshake corresponds to the remaining handshake messages?

@dvc94ch
Copy link
Contributor

dvc94ch commented Apr 6, 2021

the handshake encrypts the parts that need encrypting and takes an arbitrary payload, so there is no need to split it into two crypto frames with different packet encryption keys. I'm not sure if there is any leakage, like an ack not being encrypted? the alternative would be to write a custom noise implementation specific for quic/quinn so that this can be handled.

to better illustrate the point, a noise packet looks like this: [optional handshake || optional payload] while a quic packet has nesting that looks more like this [[optional handshake in crypto frame] [optional ack frame]]

@Ralith
Copy link
Collaborator

Ralith commented Apr 6, 2021

Sounds like noise w/ AESGCM might want to use key rotation, at least.

Another issue is that the peer can unilaterally force a key update at any time, so you should probably implement key rotation even if noise claims it's not needed.

@kim
Copy link
Contributor

kim commented Apr 9, 2021

What happened to “nQUIC”, do you know? I believe the mentioned implementation was a PoC of that paper.

What seemed a little unclear from that is how negotiation of the crypto protocol would happen — say a network using TLS would want to upgrade (roll-forward) to Noise. The authors suggest tying to the QUIC version, but that’s transparent at the session layer afaiu.

@dvc94ch
Copy link
Contributor

dvc94ch commented Apr 9, 2021

What happened to “nQUIC”, do you know? I believe the mentioned implementation was a PoC of that paper.

I think it died for multiple reasons. One is the choice of handshake IK instead of a more conservative generally applicable one like XX and the other is that the incentive of researchers is to publish papers, not building something that a practitioning engineer can deploy. But obviously that is just my opinion.

What seemed a little unclear from that is how negotiation of the crypto protocol would happen — say a network using TLS would want to upgrade (roll-forward) to Noise. The authors suggest tying to the QUIC version, but that’s transparent at the session layer afaiu.

Good question. I guess it depends on if you need PKI or not, so my understanding is that it would serve a different audience and different applications. quic tls seems to be targeted towards the web/http3 use case which is probably somewhat different from the p2p use case that doesn't rely on PKI. If you use both in the same application, you'd probably bind quic-tls and the http api to port 443 and the p2p part using quic-noise to some other port.

@Ralith
Copy link
Collaborator

Ralith commented Apr 9, 2021

Note that TLS has extensions to operate in a raw-key mode when PKI is not desired; see https://tools.ietf.org/html/rfc7250.

@djc
Copy link
Member Author

djc commented Apr 10, 2021

In my understanding, nQUIC was indeed more of a research project. Even when the paper was published, it was based on an old version of Quinn that was way out of date (I think even predating the quicr/Quinn merge). I haven't heard from anyone attempting to do TLS -> Noise upgrades or negotiation.

@kim
Copy link
Contributor

kim commented Apr 11, 2021

Thanks for the responses.

I believe it was they same people who claimed on the noise mailing list that they’re working with the IETF to standardise it, that’s why I was asking. But yeah, guess they went on researching something else instead :)

RFC7250 would certainly simplify things, but I don’t think rustls supports it, and it wouldn’t seem trivial to teach it to.

I suppose I’ll just keep watching this thread, and figure out some migration strategy for us once the time comes :)

One last question for @dvc94ch though: I think I would want XX in the general case, but IK in some cases. I was looking at NoiseSocket. It does not seem to be widely adopted, but perhaps would yield a handshake more similar to TLS?

@dvc94ch
Copy link
Contributor

dvc94ch commented Apr 12, 2021

I guess the IK handshake could work. The reason it doesn't work for libp2p is because we need to support rsa/secp256k1 identities, which means we have to generate an ephemeral s and then sign it with the identity key. But if those were to be removed, ed25519 can be converted into a x25519 key pair (libsodium thinks it's acceptable to do so) and newer schemes like sr25519 can be used for signing and dh.

@djc
Copy link
Member Author

djc commented Apr 12, 2021

I believe it was they same people who claimed on the noise mailing list that they’re working with the IETF to standardise it, that’s why I was asking. But yeah, guess they went on researching something else instead :)

I think Noise as a security protocol might have been discussed as desirable in the early days of the QUIC WG, but I think at some point they scoped down the charter of the group to focus on shipping QUIC v1 with TLS 1.3 and HTTP/3. Now that the QUIC v1 and HTTP/3 work is nearing completion, there might well be some interest in specifying QUIC + Noise.

@dvc94ch
Copy link
Contributor

dvc94ch commented Apr 17, 2021

So I have a wip implementation of what was discussed here. I still need to finish debugging it tomorrow, but there should be enough there so that you can see if it fits your requirements (@kim) and the README should provide (some) documentation/design rationale for how it works.

One problem I'm running into is still the strict sequence of messages that quinn expects.

-> Initial
-> Handshake
-> 0rtt Data
<- Handshake
<- Data

doesn't match quinn's expectations which are noted in a previous comment.

@dvc94ch
Copy link
Contributor

dvc94ch commented Apr 18, 2021

Turns out that there where a few problems on my end, it works now. There is one thing I was wondering about, that is if read_handshake should be able to return new keys. Thoughts? Actually this turns out to be non-optional, won't work without.

@kim
Copy link
Contributor

kim commented Apr 18, 2021

Oh amazing. I have a ton of questions about this, which are not at all related to quinn. What would be a good place to direct them to, @dvc94ch ?

@dvc94ch
Copy link
Contributor

dvc94ch commented Apr 18, 2021

Best would probably be to open issues in quinn-noise. But you can send me an email if you prefer.

@Ralith
Copy link
Collaborator

Ralith commented Apr 18, 2021

There is one thing I was wondering about, that is if read_handshake should be able to return new keys. Thoughts?

You can always have write_handshake write nothing and return keys immediately. That makes things easier for quinn, since we only have a single path to worry about.

@LPardue
Copy link

LPardue commented Apr 23, 2021

I'm curious how this looks on the Wire. For instance, in the Initial packet what is the value of the version field?

The reason I ask is that If you're advertising one of the QUIC v1 versions but not using QUIC-TLS, that could cause some problems for other deployments.

We maintian a list of versions at https://github.com/quicwg/base-drafts/wiki/QUIC-Versions and it is a low barrier to pick some that you could use for experimentation.

Apologies if I'm misunderstanding or overlooking something.

@dvc94ch
Copy link
Contributor

dvc94ch commented Apr 29, 2021

I registered a version for quinn-noise. Once #1106 lands I can release the first version of quinn-noise and the first version of ipfs-embed with quic support.

@LPardue
Copy link

LPardue commented Apr 29, 2021

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants