Allow to specify dcid when creating a client side connection #2234

normanmaurer · 2025-11-06T07:23:00Z

Motivation:

Sometimes the user want to have control over generating the dcid when creating a new client side connection. This was not possible before.

Modification:

Only use dcid for local transport params when the connection is for server side
Use provided odcid as dcid on the client side when given and if not just randomly generate one

Result:

More flexible usage possible

normanmaurer · 2025-11-06T07:25:25Z

I really only care about be able to do this using the c headers but never less thought it might also be useful to be able to do this via the rust api so I added two new functions there as well.

Motivation: Sometimes the user want to have control over generating the dcid when creating a new client side connection. This was not possible before. Modification: - Only use dcid for local transport params when the connection is for server side - Use provided odcid as dcid on the client side when given and if not just randomly generate one Result: More flexible usage possible

LPardue · 2025-11-06T07:35:10Z

The RFC places requirements for unpredictability and length on the client DCID field; see https://datatracker.ietf.org/doc/html/rfc9000#section-7.2-3

I'm concerned this change can make QUIC connections vulnerable, in ways described in the RFC security considerations, or in ways that are yet to be determined.

That said, there could be use cases where a deterministic CID (yet unpredictable to some actors) is useful.

Providing safety rails for the majority of default users is important. Perhaps this could be placed behind a very explicit opt-in feature flag with some clear warning are references to the relevant RFC text

normanmaurer · 2025-11-06T07:41:02Z

The RFC places requirements for unpredictability and length on the client DCID field; see https://datatracker.ietf.org/doc/html/rfc9000#section-7.2-3

I'm concerned this change can make QUIC connections vulnerable, in ways described in the RFC security considerations, or in ways that are yet to be determined.

Fair enough...

That said, there could be use cases where a deterministic CID (yet unpredictable to some actors) is useful.

Yeah that's why I need it :)

Providing safety rails for the majority of default users is important. Perhaps this could be placed behind a very explicit opt-in feature flag with some clear warning are references to the relevant RFC text

That would be fine with me ... That said I think we could also add some validations for the length at least (must be at least 8 bytes). Let me do this in any case.

normanmaurer · 2025-11-06T07:57:08Z

@LPardue added the length check and also added some more docs. That said I would also be ok to just remove the two new rust functions completely as long as I can do it via FFI / c :)

normanmaurer · 2025-11-06T07:57:42Z

quiche/src/lib.rs

+                // The Minimum length is 8.
+                // See https://datatracker.ietf.org/doc/html/rfc9000#section-7.2-3
+                if dcid.to_vec().len() < 8 {
+                    return Err(Error::InvalidTransportParam);


Not sure this is the best Error value to use...

I think it would be better to put this validation in the new connect_with_dcid* functions since that is where the new logic is being added. It will also make this common function less busy.

@toidiu I did put it here to have the same safe-guards when using quiche via the C api (I use quiche via the C API) as this function is directly used from there. Otherwise I would have had to duplicate this which I think is more error-prone. WDYT ?

regarding error value, I would define a new one like InvalidDcidInitialization. This would be especially useful for implementing the suggestion to add a new client_dcid: Option<&ConnectionId>, param to the function. We can check that only one of odcid or client_dcd are Some and return that error if some expectation failed.

I'd prefer that level of robustness vs. just relying on debug_assert

normanmaurer · 2025-11-06T18:07:13Z

Just to make it clear.... I basically (as a library user) would like to be able to control how this random value is generated for the DCID.

normanmaurer · 2025-11-10T08:23:32Z

@LPardue @ghedo would be possible to add ?

normanmaurer · 2025-11-17T15:29:38Z

ping again...

LPardue · 2025-12-02T14:54:57Z

As noted in #2234 (comment), I think this need to be put behind a feature flag. I'm not willing to make this the default behaviour and risk clients accidentally using the API incorrectly

normanmaurer · 2025-12-02T15:04:20Z

@LPardue that's fair enough let me make the change later today

normanmaurer · 2025-12-09T01:07:44Z

@LPardue I added the feature flag as requested... PTAL

quiche/src/lib.rs

toidiu · 2025-12-09T07:55:23Z

quiche/src/lib.rs

+                // The Minimum length is 8.
+                // See https://datatracker.ietf.org/doc/html/rfc9000#section-7.2-3
+                if dcid.to_vec().len() < 8 {
+                    return Err(Error::InvalidTransportParam);


I think it would be better to put this validation in the new connect_with_dcid* functions since that is where the new logic is being added. It will also make this common function less busy.

quiche/src/lib.rs

toidiu · 2025-12-09T08:12:59Z

quiche/src/lib.rs

    fn with_tls(
-        scid: &ConnectionId, odcid: Option<&ConnectionId>, local: SocketAddr,
+        scid: &ConnectionId, dcid: Option<&ConnectionId>, local: SocketAddr,
        peer: SocketAddr, config: &Config, tls: tls::Handshake, is_server: bool,
    ) -> Result<Connection<F>> {


The Option odcid is currently being used to detect if the connection is a Server or Client. i.e. if odcid is Some then implement some server specific logic.

I dont love that we are using Option for this and now that the client can also provide a dcid, we now need a bunch of if server logic, which makes things brittle and harder to maintain.

Given that this is an internal function, I think to make this less error prone we should introduce a new parameter, client_dcid, here instead of reusing odcid.

fn with_tls( scid: &ConnectionId, odcid: Option<&ConnectionId>, client_dcid: Option<&ConnectionId>, local: SocketAddr, peer: SocketAddr, config: &Config, tls: tls::Handshake, is_server: bool, ) -> Result<Connection<F>>

@toidiu I am fine with this but then I will also need to add another FFI / c api to use it. I did aim to keep the changes minimal and that's why I did it this way.

Basically I only found out that I need changes when I tried to pass in the dcid via the C api and it just didn't work. So I added the changes here to make it work without any changes in the FFI / c layer.

Just let me know what you guys think should be done and I am happy to do so

Ahh that makes sense. Guessing that would be this function. I am less familiar with where ffi is used and if a breaking change is acceptable so I'll let @LPardue speak more on that.

From the Rust and lib.rs logic maintenance perspective, I would love to not mix dcid and odcid.

Another option could be to add a new function to the ffi layer.. something like quiche_conn_new_with_client_dcid.

I'm a bit confused, are you not using quiche_connect()?

@LPardue no I don't do... I use quiche_conn_new_with_tls as I need to use my own existing SSL pointer:
https://github.com/cloudflare/quiche/blob/master/quiche/src/ffi.rs#L608C19-L608C43

But yeah I think I agree with @toidu that we can probably tweak the internal with_tls and then wrap it by add something in the FFI layer like below where returning null means something failed

pub extern "C" fn quiche_conn_new_with_tls_and_client_dcid( scid: *const u8, scid_len: size_t, dcid: *const u8, dcid_len: size_t, local: &sockaddr, local_len: socklen_t, peer: &sockaddr, peer_len: socklen_t, config: &Config, ssl: *mut c_void, ) -> *mut Connection { #[cfg(not(feature = "connect-with-dcid"))] { return ptr::null_mut(); } let scid = unsafe { slice::from_raw_parts(scid, scid_len) }; let scid = ConnectionId::from_ref(scid); let dcid = if !dcid.is_null() && dcid_len >= 8 { Some(ConnectionId::from_ref(unsafe { slice::from_raw_parts(dcid, dcid_len) })) } else { return ptr::null_mut(); }; let local = std_addr_from_c(local, local_len); let peer = std_addr_from_c(peer, peer_len); let tls = unsafe { tls::Handshake::from_ptr(ssl) }; match Connection::with_tls( &scid, None, // odcid dcid.as_ref(), local, peer, config, tls, false, // only ever used for client connections ) { Ok(c) => Box::into_raw(Box::new(c)), Err(_) => ptr::null_mut(), } }

toidiu · 2025-12-09T08:17:41Z

quiche/Cargo.toml

+# Allow to provide generated dcid and connect with it.
+connect-with-dcid = []


nit: what do you think about

Suggested change

# Allow to provide generated dcid and connect with it.

connect-with-dcid = []

# Allow client connections to provide a custom dcid.

custom-client-dcid = []

I kinda prefer this name

@LPardue you prefer the suggested one correct ? I also like it more... Just wanted to check before starting to write the code

Yes. lets go with this suggested one. Also while I'm here nitpicking, any comment that refers to DCID should be capitalized (there's a few of those through the PR. The feautre/functions themself obviously needs to be lowercase

toidiu · 2025-12-09T08:21:24Z

quiche/src/lib.rs

+/// certificate.
+#[cfg(feature = "connect-with-dcid")]
+#[cfg_attr(docsrs, doc(cfg(feature = "connect-with-dcid")))]
+pub fn connect_with_dcid(


It would be nice to also include a test for this. Although not sure if we currently have a great way of enable features in tests.

I didn't see how :( I did validate it via our integration in netty tho.

I think this is possible. There's a few examples of tests only being run behind certain features in quiche/src/tests.rs, e.g., https://github.com/cloudflare/quiche/blob/master/quiche/src/tests.rs#L7545

Then, we can add another CI test step. For example something like

add a new env in https://github.com/cloudflare/quiche/blob/master/.github/workflows/stable.yml#L9

env: DEFAULT_OPTIONS: "--features=async,ffi,qlog --workspace" NO_BORING_OPTIONS: "--features=ffi,qlog --workspace --exclude h3i --exclude tokio-quiche" CONNECT_WITH_DCID_OPTIONS: "--features=ffi,connect-with-dcid --workspace --exclude h3i --exclude tokio-quiche"

then add a new step around https://github.com/cloudflare/quiche/blob/master/.github/workflows/stable.yml#L85

- name: Run cargo test for connect-with-dcd if: ${{ matrix.tls-feature == 'boringssl-boring-crate' }} run: cargo test --verbose --all-targets --features=${{ matrix.tls-feature }} ${{ env.CONNECT_WITH_DCID_OPTIONS }}

normanmaurer requested a review from a team as a code owner November 6, 2025 07:23

normanmaurer force-pushed the odcid branch from 6f864bb to a7878b9 Compare November 6, 2025 07:24

normanmaurer force-pushed the odcid branch from a7878b9 to c5622d5 Compare November 6, 2025 07:26

normanmaurer changed the title ~~Allow to specific dcid when creating a client side connection~~ Allow to specify dcid when creating a client side connection Nov 6, 2025

normanmaurer mentioned this pull request Nov 6, 2025

Allow to set destination connection id when creating a client side Qu… netty/netty-incubator-codec-quic#833

Open

normanmaurer added 2 commits November 6, 2025 08:51

Validate length

c3c6d0b

Add some more docs

e38fb9d

normanmaurer commented Nov 6, 2025

View reviewed changes

normanmaurer added 3 commits December 8, 2025 17:45

Use feature flag to enable new methods and disable by default

d5c7650

formatting

93e19ef

fix formatting again

3105438

normanmaurer commented Dec 9, 2025

View reviewed changes

quiche/src/lib.rs Outdated Show resolved Hide resolved

toidiu reviewed Dec 9, 2025

View reviewed changes

Use debug_assert

51f7ca1

normanmaurer requested a review from toidiu December 9, 2025 14:40

		# Allow to provide generated dcid and connect with it.
		connect-with-dcid = []

Allow to specify dcid when creating a client side connection #2234

Are you sure you want to change the base?

Allow to specify dcid when creating a client side connection #2234

Conversation

normanmaurer commented Nov 6, 2025

Uh oh!

normanmaurer commented Nov 6, 2025

Uh oh!

LPardue commented Nov 6, 2025

Uh oh!

normanmaurer commented Nov 6, 2025

Uh oh!

normanmaurer commented Nov 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

normanmaurer commented Nov 6, 2025

Uh oh!

normanmaurer commented Nov 10, 2025

Uh oh!

normanmaurer commented Nov 17, 2025

Uh oh!

LPardue commented Dec 2, 2025

Uh oh!

normanmaurer commented Dec 2, 2025

Uh oh!

normanmaurer commented Dec 9, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

normanmaurer Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

toidiu Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

normanmaurer Dec 11, 2025 •

edited

Loading

toidiu Dec 9, 2025 •

edited

Loading