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

Update DTLS CID implementation to comply with the "final" draft. #4860

Closed
boaks opened this issue Aug 10, 2021 · 9 comments
Closed

Update DTLS CID implementation to comply with the "final" draft. #4860

boaks opened this issue Aug 10, 2021 · 9 comments

Comments

@boaks
Copy link

boaks commented Aug 10, 2021

The draft-ietf-tls-dtls-connection-id has reached version 13 is is short before being released.

The current implementation in mbedTLS (2.28) reflects version 05 and is using a proprietary code point for the extension (see my issue #3892).

In order to comply with the upcoming definitions in that draft, it is required to

Implementations will only be interoperable with other implementations, if the comply to the definitions of RFCs. For Eclipse/Californium I updated the implementation to support the new definitions along with the possibility to use the deprecated definitions (before 09). It will be contained in the upcoming Californium release 3.0.

@mpg
Copy link
Contributor

mpg commented Aug 12, 2021

Hi @boaks and thanks for letting us know about the final draft. Unfortunately we don't have the capacity to implement this at the moment. We've added this to our backlog and hopefully schedule this in the next couple of months.

@boaks
Copy link
Author

boaks commented Aug 12, 2021

Thanks!
I don't expect that this will be done in the next time. PR #4427 is also still open.
It's more for other users, which are may wonder, why mbedtls is not interoperable.

@mpg
Copy link
Contributor

mpg commented Aug 12, 2021

Yes, it's good to have public record about that, and it's good for us to have an issue in our backlog as a reminder.

Thanks for the reminder about 4427, I thought we had merged it already. We're currently in an effort to finish off pending things, to hopefully we should be able to close it in the coming days!

@boaks
Copy link
Author

boaks commented Sep 15, 2021

Just as some hints, if someone want to spend time in:

If only the new IANA code-point (54) and the new MAC must be supported (maybe by a compile-time switch), then for v2.27 the following changes are required:

In "include/mbedtls/ssl.h":
Adapt #define MBEDTLS_TLS_EXT_CID to the new IANA code-point 54.

In "library/ssl_msg.c":
Adapt the length of add_data in mbedtls_ssl_encrypt_buf and mbedtls_ssl_decrypt_buf. The new MAC definition contains 8 additional bytes for the seq_num_placeholder and 1 byte for the extra
record_type tls12_cid (see the MAC definition of version 13). Therefore add + 8 + 1to the add_data array size.
Adapt the ssl_extract_add_data_from_record for the new MAC. In my PoC I move the CID specific stuff to the begin of the function and do all there, leaving the rest of that function then for the MAC without CID. I'm not sure, if that DTLS 1.2 CID MAC has any relation to the one 1.3. Others will know.

@boaks
Copy link
Author

boaks commented Oct 13, 2021

See PR #5061

@boaks boaks mentioned this issue Oct 13, 2021
2 tasks
@plskeggs
Copy link

plskeggs commented Feb 14, 2022

Just as some hints, if someone want to spend time in:

If only the new IANA code-point (54) and the new MAC must be supported (maybe by a compile-time switch), then for v2.27 the following changes are required:

In "include/mbedtls/ssl.h": Adapt #define MBEDTLS_TLS_EXT_CID to the new IANA code-point 54.

In "library/ssl_msg.c": Adapt the length of add_data in mbedtls_ssl_encrypt_buf and mbedtls_ssl_decrypt_buf. The new MAC definition contains 8 additional bytes for the seq_num_placeholder and 1 byte for the extra record_type tls12_cid (see the MAC definition of version 13). Therefore add + 8 + 1to the add_data array size. Adapt the ssl_extract_add_data_from_record for the new MAC. In my PoC I move the CID specific stuff to the begin of the function and do all there, leaving the rest of that function then for the MAC without CID. I'm not sure, if that DTLS 1.2 CID MAC has any relation to the one 1.3. Others will know.

Hello -- I've been following this CID saga, and noticed that above you mentioned the need to adapt the length of add_data in mbedtls_ssl_decrypt_buf. But in the PR #5061, the author did not change this. Do you know the reason?

@boaks
Copy link
Author

boaks commented Feb 15, 2022

My saga was about adapting mbedtls version 2.27.
PR #5061 is about mbedtls version 3.x.y
Hope that helps. If not, don't hesitate to ask again.

@boaks
Copy link
Author

boaks commented Mar 22, 2022

Just to mention:
RFC9146 is published.

@daverodgman
Copy link
Contributor

Done in #6264 and #6681

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants