-
Notifications
You must be signed in to change notification settings - Fork 7.3k
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
mbedtls: define MBEDTLS_SSL_CID_TLS1_3_PAD_GRANULARITY for CID padding #12177
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm seeing the following:
MBEDTLS_SSL_CID_TLS1_3_PADDING_GRANULARITY is 42 characters long and it should be 40 at most
Anyway to override so that we can match the underlying mbedtls config?
Hello, @hasheddan! Thanks for the contribution!
|
@laukik-hase is there any possibility of overriding the pre-commit hook? I think it would be unfortunate to not mirror the |
a9825ee
to
df2b475
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@laukik-hase any updates here? Is shortening the config option an absolute requirement? Would love to get this merged as Connection ID support is currently broken without it.
I'm afraid no, this limit is currently enforced globally and I would prefer not to change it just for this case. Please see if you can shorten the name. Additionally, since you are renaming a Kconfig option, to ensure compatibility please create components/mbedtls/sdkconfig.rename and add the old and the new option name there (docs, example). |
@igrr in that case, would it make sense to just keep the previous option and have it select the new one? |
df2b475
to
c531a18
Compare
Updates config to define the new MBEDTLS_SSL_CID_TLS1_3_PAD_GRANULARITY option, which replaced the previously used MBEDTLS_SSL_CID_PADDING_GRANULARITY. The old option is continuing to be used as the new one exceeds the maximum length for an option name in esp-idf. See Mbed-TLS/mbedtls#4490 for more information. Signed-off-by: Daniel Mangum <[email protected]>
c531a18
to
7c63769
Compare
@igrr I have pushed an update here reflecting the use of the existing option to define the new mbedTLS option |
sha=7c63769edefda724ef5c7f7d5e20b33f9e3036ee |
@igrr @laukik-hase anything else needed here to merge? |
@hasheddan, the PR has been merged internally - it will close automatically when it is synced to GitHub. |
Merged with a57c8dc, closing this PR! |
Updates config to define the new MBEDTLS_SSL_CID_TLS1_3_PAD_GRANULARITY
option, which replaced the previously used
MBEDTLS_SSL_CID_PADDING_GRANULARITY. The old option is continuing to be
used as the new one exceeds the maximum length for an option name in
esp-idf.
See Mbed-TLS/mbedtls#4490 for more information.
Signed-off-by: Daniel Mangum [email protected]