ptrd / kwik

A QUIC client, client library and server implementation in Java. Supports HTTP3 with "Flupke" add-on.
GNU General Public License v3.0
383 stars 55 forks source link

Retiring the connection ID used during the handshake #14

Closed WesleyRosenblum closed 2 years ago

WesleyRosenblum commented 2 years ago

When testing kwik server, after sending NEW_CONNECTION_ID frames requesting the retirement of the connection ID used during the handshake (sequence number 0), we are not receiving a RETIRE_CONNECTION_ID frame.

For example, when a client sends this frame to kwik:

NEW_CONNECTION_ID
    Frame Type: NEW_CONNECTION_ID (0x0000000000000018)
    Sequence: 1
    Retire Prior To: 1
    Connection ID Length: 16
    Connection ID: 0a0d2ab253240f0f348d03cf6bfb4bf2
    Stateless Reset Token: 6d7c068b2f8e7d0a553efae3c5147eaa

kwik should transmit a RETIRE_CONNECTION_ID frame with Sequence: 0, but it does not always do this. In some cases it does not send the RETIRE_CONNECTION_ID frame at all, in other cases it sends an invalid RETIRE_CONNECTION_ID frame, see https://github.com/ptrd/kwik/issues/13.

Thanks for considering this issue and let me know if you need any further information!

ptrd commented 2 years ago

Hi Wesley, Thanks for you feedback. Connection-id functionality is not yet implemented in the server, and not at all tested by me, so i'm sure it will be flaky. Also, i think the client implementation needs an overhaul, i planned to do these things at the same time, as there will be a lot of shared functionality. That is also the reason why i haven't picked op #13 yet. Currently, i'm working on server session resumption and zero-rtt. When that's done, i think i should pick-up the connection id stuff, to ensure both implementations are at least conforming to specification.

ptrd commented 2 years ago

For server role, this should now be fixed by https://bitbucket.org/pjtr/kwik/commits/b8fb5b14a6a9a07e229dc98ee62a443de6fff70b?at=connection-ids. Will merge to master after client role is fixed too.

ptrd commented 2 years ago

Now also merged to master.