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

Session ID of each connection from one host is not unique #25

Closed
JustSomeDude301 opened this issue Jul 10, 2023 · 6 comments
Closed

Session ID of each connection from one host is not unique #25

JustSomeDude301 opened this issue Jul 10, 2023 · 6 comments
Assignees
Labels
enhancement New feature or request question Further information is requested

Comments

@JustSomeDude301
Copy link
Contributor

JustSomeDude301 commented Jul 10, 2023

Source Code Reference: https://gist.github.com/JustSomeDude301/9803efa298a46b6b55015f69a98b0e01

Filing this moreso for information. Is this behavior intentional?

I am using a concurrent hashmap implementation (DashMap) for storing the Connection objects. Currently the Key I use for each connection is the remote address of the connection. But if this is intentional and each Connection with an identical SessionId should be treated as one pooled "Connection" I may have to rethink my approach.

When making multiple connections from localhost each one has a unique remote address, however the SessionId is identical:

Connection 1:

Incoming request
 Authority: localhost:4433
 Path: /
 Headers: Headers(
    {
        "origin": "https://webtransport.day",
        ":path": "/",
        ":protocol": "webtransport",
        "sec-webtransport-http3-draft02": "1",
        ":scheme": "https",
        ":method": "CONNECT",
        ":authority": "localhost:4433",
    },
)
Added new connection from: [::1]:51920
Received (datagram): HEY from client, remote addr: [::1]:51920, session id: 0

Connection 2:

Incoming request
 Authority: localhost:4433
 Path: /
 Headers: Headers(
    {
        "origin": "https://webtransport.day",
        ":authority": "localhost:4433",
        ":path": "/",
        "sec-webtransport-http3-draft02": "1",
        ":scheme": "https",
        ":method": "CONNECT",
        ":protocol": "webtransport",
    },
)
Added new connection from: [::1]:53210
Received (datagram): HELLOW from client, remote addr: [::1]:53210, session id: 0

The IETF Draft says:

WebTransport sessions are initiated inside a given HTTP/3 connection by the client, who sends an extended CONNECT request [RFC8441]. If the server accepts the request, an WebTransport session is established. The resulting stream will be further referred to as a CONNECT stream, and its stream ID is used to uniquely identify a given WebTransport session within the connection. The ID of the CONNECT stream that established a given WebTransport session will be further referred to as a Session ID.

The way I am interpreting this is that two unique Connections from the same host will have unique SessionId's, but feel free to correct my understanding

BTW I love the library, I am using it to make a Datagram server framework intended for porting UDP Multiplayer Netcode libraries

@BiagioFesta
Copy link
Owner

BTW I love the library, I am using it to make a Datagram server framework intended for porting UDP Multiplayer Netcode libraries

Nice :)
Thank you; please, feel free to continue providing your feedback and/or contribute to this library at your needs.


[...] each Connection with an identical SessionId should be treated as one pooled "Connection" [...]

Not really.

WebTransport Session ID is not intended to be used as a discriminator for client connections. In other words, you cannot use the WebTransport session ID to uniquely identify a connection or a client origin.

[...] The way I am interpreting this is that two unique Connections from the same host will have unique SessionId's, but feel free to correct my understanding.

I'm sorry, but I believe you may have misunderstood. Please let me try to clarify on this.

The draft states that a Session ID uniquely identifies a WebTransport session within the same connection. In other words, with a single connection, you might have multiple sessions, and each of them has a different Session ID. Different connections (even if the two connections come from the same endpoint/client) do not have any correlation with session ID numbers.

Having said that, I might add a few considerations that could be useful in your use case.

As I understand it, you are attempting to create a map of connections. In this regard, I would advise not to use the remote address of the peer as an index key.

In QUIC (RFC 9000, section 9), an endpoint has the ability to change its network address and perform a migration. Since WebTransport is built on top of QUIC, it might also support connection migration.

In your use case, the client might change its network address while the index of the map remains the same, resulting in inconsistency.

Please refer to the documentation: https://docs.rs/wtransport/latest/wtransport/struct.Connection.html#method.remote_address

Note: As QUIC supports migration, the remote address may change during the connection.


In conclusion, I am thinking two possible action items for now:

  1. Provide an interface in the configuration in order to manually disable the connection migration (in case one does not want it).

  2. Provide a real unique identifier of each WebTransport connection. In this regard, I could leverage the underlying QUIC library (https://docs.rs/quinn/latest/quinn/struct.Connection.html#method.stable_id).

Do you think that having something like:

wtransport::Connection::stable_id(&self) might fit your use case?

Thanks

@JustSomeDude301
Copy link
Contributor Author

JustSomeDude301 commented Jul 11, 2023

Not really. WebTransport Session ID is not intended to be used as a discriminator for client connections. In other words, you cannot use the WebTransport session ID to uniquely identify a connection or a client origin.

I'm sorry, but I believe you may have misunderstood. Please let me try to clarify on this. The draft states that a Session ID uniquely identifies a WebTransport session within the same connection. In other words, with a single connection, you might have multiple sessions, and each of them has a different Session ID. Different connections (even if the two connections come from the same endpoint/client) do not have any correlation with session ID numbers.

That makes a lot of sense I appreciate the clear up, it appears my understanding of the SessionId was incorrect.

For my usecase it seems you are correct, I must stop using the remote address of the client as the index key of the connection map due to the QUIC connection migration. I did see the notice in the docs for the method you linked I just needed something for a proof of concept

A library provided unique identifier for each connection like the stable_id and an interface in the configuration to disable connection migration would indeed be immensely helpful. I will try to support QUIC connection migration (since it actually seems pretty useful) instead of outright disabling it but for prototype purposes that would be awesome.

I put up a PR for number 2 here which exposes a couple more helpful methods for my use case alongside the stable_id, feel free to suggest revisions.

I don't know how to do number 1 cleanly, in that the way I would do it, is just a method argument for one of the ServerConfigBuilder methods to avoid decoupling the ClientConfigBuilder and ServerConfigBuilder state tracking Generic structs. If you can provide some guidance I can do number 1 too

EDIT: Updated source code reference using new stable_id as hashmap keys https://gist.github.com/JustSomeDude301/86226921eaa3b0edbf94c45f40724e28

Connection 1

Incoming request
 Authority: localhost:4433
 Path: /
 Headers: Headers(
    {
        ":protocol": "webtransport",
        "origin": "https://webtransport.day",
        ":path": "/",
        ":scheme": "https",
        ":method": "CONNECT",
        ":authority": "localhost:4433",
        "sec-webtransport-http3-draft02": "1",
    },
)
Added new connection, connection stable id: 2980128946992
Received datagram from connection with stable id: 2980128946992

Connection 2

Incoming request
 Authority: localhost:4433
 Path: /
 Headers: Headers(
    {
        ":scheme": "https",
        ":protocol": "webtransport",
        "sec-webtransport-http3-draft02": "1",
        "origin": "https://webtransport.day",
        ":path": "/",
        ":method": "CONNECT",
        ":authority": "localhost:4433",
    },
)
Added new connection, connection stable id: 2980129047424
Received datagram from connection with stable id: 2980129047424

@BiagioFesta BiagioFesta added the question Further information is requested label Jul 11, 2023
@BiagioFesta BiagioFesta self-assigned this Jul 11, 2023
@BiagioFesta BiagioFesta added the enhancement New feature or request label Jul 11, 2023
@BiagioFesta
Copy link
Owner

Thank you for the contribution.

Regarding the configuration setting, I opened this PR: #28


If I may, I would like to add a further consideration regarding your use case.

Using stable_id as a key for identifying a connection is a better option compared to using the remote address. When examining the transport implementation, it can be observed that the stable_id corresponds to the heap memory address of the connection structure. This implementation is straightforward and generally effective for many use cases.

However, I am curious as to why you cannot simply utilize an internal incremental counter for your map-keys.

For instance, you can refer to the example in this crate: https://github.com/BiagioFesta/wtransport/blob/master/wtransport/examples/server.rs#L28-L31

In the example, an incremental ID is assigned (primarily for logging purposes) to each incoming connection.

This approach is generally simpler and does not rely on an internal ID implementation.

@JustSomeDude301
Copy link
Contributor Author

JustSomeDude301 commented Jul 11, 2023

Thank you for the contribution. Regarding the configuration setting, I opened this PR: #28

Thanks man I really appreciate the fast turnaround on the migration config!

However, I am curious as to why you cannot simply utilize an internal incremental counter for your map-keys.

I could, but if there's a valid identifier exposed by the library I'm generally more inclined to just use that than roll my own. This is pretty much just semantics so whatever you wanna do for this is totally fine by me. Thank you again for clearing up my misunderstandings about how sessions and migration work!

EDIT: I've elected to go with a UUID for the connection key in my hashmap, I can remove the stable_id() method from PR #26 if you'd like. The max_datagram_size() and rtt() methods would still be beneficial to my usecase

@BiagioFesta
Copy link
Owner

I think your PR is fine like that and I already merged.

My concern was strictly for your use case, not for avoiding having stable_id in the library

@BiagioFesta
Copy link
Owner

Closing this issue as solved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants