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

Please adjust MBEDTLS_TLS_EXT_CID to 53 #3892

Closed
boaks opened this issue Nov 18, 2020 · 14 comments
Closed

Please adjust MBEDTLS_TLS_EXT_CID to 53 #3892

boaks opened this issue Nov 18, 2020 · 14 comments
Assignees
Labels
component-tls enhancement size-s Estimated task size: small (~2d)

Comments

@boaks
Copy link

boaks commented Nov 18, 2020

According IETF - mailinglist the value for the DTLS Connection ID extension is assigned to 53.

Please adjust ssl.h - MBEDTLS_TLS_EXT_CID to that assigned value for interoperability with Eclipse Californium.

@mpg
Copy link
Contributor

mpg commented Nov 19, 2020

Hi @boaks and thanks for letting us know about this temporary assignment. I double-checked, and it's also visible directly in IANA's public registry: https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml

We'll update the value.

@mpg
Copy link
Contributor

mpg commented Jan 4, 2021

Note: while just updating the value is a fairly trivial change, this was held so far as it might cause upgrade issues if people already have large-scale deployments relying on this. I've just sent an email to the list about that, and based on the replies (or lack thereof) we'll see if there is an actual issue or not and when we can plan this update.

@hanno-becker
Copy link

Note also that there are functional changes underway in the CID Draft, e.g. tlswg/dtls-conn-id#77, which will require further work in Mbed TLS' implementation.

@boaks
Copy link
Author

boaks commented Jan 4, 2021

@hanno-arm

Do you plan to adapt the MAC before the RFC gets released?
Are you aware of the proposal from Ben, to adapt the code-point for the MAC change?

@mpg
Copy link
Contributor

mpg commented Jan 6, 2021

Our current plan is to wait until the spec has become more stable, then implement both the new code-point (be it 53 or a new one as per the proposal above) and the functional changes at the same time. When we do that, we'll also make an option for servers to accept both the current extension with its current number (254) and behaviour (draft-05), and the final extension with its final number (53 or TBD) and behaviour (draft-09+), in order to provide a smooth upgrade path to people who already have complex production deployments (as it turns out there are such deployments already).

We'd rather avoid updating the code point before the spec is stable from a functional point of view, as we'd like to avoid having to provide more than one compatibility option. (Also, if Ben's proposal to change the code-point is rejected, postponing the code point update is our only chance at providing a smooth upgrade path.)

@boaks
Copy link
Author

boaks commented Jan 6, 2021

Our current plan is to wait until the spec has become more stable

That's also my plan with Eclipse/Californium.

then implement both the new code-point (be it 53 or a new one as per the proposal above) and the functional changes at the same time.

Just to mention, the 53 is not that new (more than 1.5 years). I'm wondering, that there are complex production deployments with the "254 (TBD)". Seems, people don't read or patch without reporting it back.

(Also, if Ben's proposal to change the code-point is rejected, postponing the code point update is our only chance at providing a smooth upgrade path.)

We will see ...

@mpg
Copy link
Contributor

mpg commented Jan 6, 2021

We released initial support for C-ID (draft-05) in June 2019 and the 53 was assigned in July... Indeed we should have tracked that; do you know of a convenient way to get notified of such assignments? Tracking new versions of the draft should be easy, but the assignment does not seem to be reflected in the latest version yet, so that doesn't really work here. The TLS WG list is pretty high-volume so not great either.

Also, it looks like we could have used stronger wording in our warning in config.h, perhaps users didn't fully realise what it meant for the specification to still be a draft.

@boaks
Copy link
Author

boaks commented Jan 10, 2021

do you know of a convenient way to get notified of such assignments?

I don't know, I tracked the mailing list and sometime I proactive ask the list about the state.

FMPOV, I'm not sure, how long t will take until that RFC get release. would it be possible to replace the

#define MBEDTLS_TLS_EXT_CID                        254 /* TBD */

with

#ifndef MBEDTLS_TLS_EXT_CID
#define MBEDTLS_TLS_EXT_CID                        254 /* TBD */
#endif

in the meantime?
Eclipse/Californum uses the IANA value and with that it would be possible to make builds without patching.

@mpg
Copy link
Contributor

mpg commented Feb 1, 2021

That sounds like a good compromise for a work-around until the spec gets stable. Would you like to submit a PR for that?

@boaks
Copy link
Author

boaks commented Feb 1, 2021

For me it's much easier, if you apply the simple changes.

@boaks
Copy link
Author

boaks commented Apr 24, 2021

Ping ... I still think, that adding the

#ifndef MBEDTLS_TLS_EXT_CID
#define MBEDTLS_TLS_EXT_CID                        254 /* TBD */
#endif

will "pay-off" :-).

@gilles-peskine-arm
Copy link
Contributor

Well, it's only going to happen if someone makes the patch. So here it is.

@boaks
Copy link
Author

boaks commented Jun 25, 2021

Just to mention:

IANA, TLS ExtensionType Values has been updated.

The new hello extension id for the adapted MAC is 54.

Hope, the RFC release will now be soon.

@gilles-peskine-arm gilles-peskine-arm added the size-s Estimated task size: small (~2d) label Jun 25, 2021
@bensze01 bensze01 modified the milestone: 2.x pre-LTS work Jul 28, 2021
@bensze01 bensze01 removed this from the 2.x pre-LTS work milestone Aug 11, 2021
@mpg
Copy link
Contributor

mpg commented Aug 13, 2021

Since #4427 and #4413 have been merged, and further work to (1) align with the final draft and (2) provide a migration path for existing users is tracked in #4860, I'm closing this issue.

Note that currently the default is not changed to 53: it's still 254, but now adjustable at compile time. I prefer not to change anything for now, and wait for #4860 to think about the migration path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component-tls enhancement size-s Estimated task size: small (~2d)
Projects
None yet
Development

No branches or pull requests

5 participants