Handshake controller for QUIC

[kazu-yamamoto]

As discussed in #392, I have implemented QUIC APIs based on threads. This does not change Client.hs and Server.hs a lot. QUIC APIs are provided in Network.TLS.QUIC. So, Network.TLS remains the same.

I joined 16th QUIC interop and am now confident the quality of this code: https://docs.google.com/spreadsheets/d/1D0tW89vOoaScs3IY9RGC0UesWGAwE6xyLk0l4JtvTVg

@ocheron I don't expect that this PR will be merged quickly. But I would like to get feedback if any.

[kazu-yamamoto]

An example usage: https://github.com/kazu-yamamoto/quic/blob/master/Network/QUIC/Handshake.hs

[kazu-yamamoto]

@ocheron FYI: https://kazu-yamamoto.hatenablog.jp/entry/2020/02/18/145038

[ocheron]

The design with handshake-controller limits the impact on client/server code quite well.

However the exposed API:

type ServerController = ServerControl -> IO ServerStatus
type ClientController = ClientControl -> IO ClientStatus

replicates a state machine in tls and quic, in addition to the one already existing with client/server code. Implementing this requires some positional logic based on handshake fragments like done in handshakeCheck, while we already have something similar available when encoding/decoding packets.

I'd rather try removing the stateful computations going through the API, i.e.

• QUIC added as record layer: it provides transport in Tx/Rx directions, without attempting to correlate what happens in both directions. After all QUIC offers to applications a bidirectional stream interface, so it must be possible to read and write independently. When refactoring like this, the handshake logic remains in tls only. And QUIC transport can be invoked directly from forked TLS threads, without intermediate Chan/MVar.

• Callbacks added to notify of cipher selection, change of Tx/Rx secrets. QUIC needs to know the current step in the key schedule to determine the packet type.

The tricky part is the server flight concatenating both Initial and Handshake content together. It must be possible to send the composite flight with an API similar to:

send :: [(Level, ByteString)] -> IO ()
recv :: Level -> IO ByteString

[kazu-yamamoto]

Removing the state in QUIC is a good idea.

The tricky part is the server flight concatenating both Initial and Handshake content together. It must be possible to send the composite flight with an API similar to:

Does this mean that a new type of backend should be introduced?

[kazu-yamamoto]

Ah, you don't intend to introduce a new type of backend. Just making use of RecordLayer.

To depend on the quic package, we should create the tls-quic package (or quic-tls) which uses both on tls and quic.

[kazu-yamamoto]

Never mind. In this approach, quic cannot use tls-quic.

So, quic should pass callbacks to tls.

[ocheron]

For now I will try some experiments in a consolidated repository:
https://github.com/ocheron/quic-tls-work/tree/work

[kazu-yamamoto]

Thanks. I should concentrate on QPACK and HTTP/3 now. Please remind me when ready.

[ocheron]

I implemented a large part of the callback approach but found a blocking point.
Details are in the implementation notes: https://github.com/ocheron/quic-tls-work/tree/f6c3f122e6c4921221c10286ff80e52ebc3bf516#doubts

Do you think the more general reordering based on packet encryption level could be added to the Receiver?

(or it's already there but I didn't see it)

[ocheron]

Hmm... maybe the reordering is there (with checkEncryptionLevel) but not doing anything because there is a single encryption level (instead of separate Tx/Rx).

[kazu-yamamoto]

I understand why the blocking happens. But I don't understand what you are asking.

Connection has only one encryptionLevel because a client secret and a server secret for a encryption level are supplied at the same time. encryptionLevel ensures that encrypted packets can be decrypted if their levels are lower than or equal to encryptionLevel. To encrypt a packet, we specify a level. Again, if the level is lower than or equal to encryptionLevel, the packet can be encrypted.

Do you think the more general reordering based on packet encryption level could be added to the Receiver?

The big problem for me was that RTT1 packets are received before handshake messages to generate RTT1 keys are received. There is no way to decrypt the RTT1 packets. As workaround, I put timeout in processCryptPacketHandshake. If they cannot be decrypted and timeout occurs, they are thrown away hoping they are resent by the peer later. This is a rare case. So, I don't take the inefficiency serious.

processFrame is called after decryption. So, we can do anything we want. To avoid blocking, forkIO seems reasonable to me. But I don't understand what a "managed" thread means in your comment at this moment.

[kazu-yamamoto]

What about this?

• checkEncryptionLevel returns True if encryptionLevel is higher than or equal to a given level. Otherwise, it stores the target packet to a kind of stroge of the level and returns False.
• setEncryptionLevel sets the encryptionLevel. Also it calls unGetTQueue to prepend the packets in the storage to RecvQ.

For instance, if checkEncryptionLevel stores packets like [C,B,A], they will be prepended to the RecvQ as [A,B,C].

[kazu-yamamoto]

Also, I'm OK with separating encryption level into RX and TX. This would enable that 1RTT packets can be sent while preventing received 1RTT packets from processing before receiving Finished.

[kazu-yamamoto]

Please give a look at:

• https://github.com/kazu-yamamoto/hs-tls/tree/olivier
• https://github.com/kazu-yamamoto/quic/tree/olivier

I modified checkEncryptionLevel and setEncryptionLevel as I said above. The first timeout is gone.

[kazu-yamamoto]

If you remove setEncryptionLevel conn RTT1Level from handshakeServer and use it when Client Finished is received, we probably can reorder packets. Would you try this approach?

[ocheron]

Thank you, I try this.

For me the issue was not necessarily the use of timeout, but the fact that Stream frames are delayed also with waitEstablished.

If setEncryptionLevel conn RTT1Level is run after receiving Client Finished the waitEstablished part can probably be removed.

Actually I see this is all well explained in section 5.7.

[kazu-yamamoto]

If setEncryptionLevel conn RTT1Level is run after receiving Client Finished the waitEstablished part can probably be removed.

Yup. I hope so.

[ocheron]

I pushed a version for review.
This confirms the approach is possible and decreases functional coupling between the projects.
I left some todos but all minor compared to the changes added.
Along the way I think it improves error/alert handling quite a bit but I don't know the protocol enough to be sure it's all correct.

All changes on the combined project:
ocheron/quic-tls-work@master...work

Same commits but for tls only (git subtree split):
master...ocheron:tls-changes-for-quic

Same commits but for quic only (git subtree split):
kazu-yamamoto/quic@master...ocheron:quic-changes-for-tls

[kazu-yamamoto]

I have rebased this branch onto master.

Most part looks good. However, since your change converts QUIC NextVersion error into TLS Error_Misc, version negotiation does not work anymore:

% runghc -threaded -iutil util/client.hs -d www.mew.org 4433 -V
------------------------
Original CID: a819b70c9aec5e75
client.hs: HandshakeFailed "Error_Misc \"NextVersion Draft27\""

[kazu-yamamoto]

This is not relating to this PR but QUIC now does not allow compatible mode. This means that Client Hello must includ 0-byte session id.

To make the work to fix two issues above easier, I would like to merge this PR anyway.
@ocheron What do you think?

[kazu-yamamoto]

I pushed a workaround for version negotiation to master of quic.

[ocheron]

OK good the workaround is possible.
So I merge the rebased PR and we can continue some changes with new PRs (with less time for me).

For the 0-byte session id I think it would be good to introduce a QUIC mode.
This would also replace the quicEarlyData checks.
Similarly, another TLS mode would be to disable 1.3 middlebox compatibility.