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

mbedtls: no longer use MBEDTLS_SSL_VERIFY_OPTIONAL #14591

Closed
wants to merge 2 commits into from

Conversation

jan2000
Copy link
Contributor

@jan2000 jan2000 commented Aug 19, 2024

With mbedTLS if the minimum version of TLS is set to 1.3, MBEDTLS_SSL_VERIFY_OPTIONAL is not available in client mode. See: https://github.com/Mbed-TLS/mbedtls/blob/2ca6c285/library/ssl_tls.c#L1357
Also, there might be plans to remove it completely in future mbedTLS versions.

So, switch to always using MBEDTLS_SSL_VERIFY_REQUIRED. If verifypeer or verifyhost are disabled the corresponding error flags are cleared in the verify callback function. That is also where verification errors are logged.

Depends on #14588

@github-actions github-actions bot added the TLS label Aug 19, 2024
@bagder
Copy link
Member

bagder commented Aug 19, 2024

Note: "This branch has conflicts that must be resolved". Can you take a look?

In prepation of removal of MBEDTLS_SSL_VERIFY_OPTIONAL.
With mbedTLS if the minimum version of TLS is set to 1.3,
MBEDTLS_SSL_VERIFY_OPTIONAL is not available in client mode. See:
https://github.com/Mbed-TLS/mbedtls/blob/2ca6c285/library/ssl_tls.c#L1357
Also, there might be plans to remove it completely in future mbedTLS
versions.

So, switch to always using MBEDTLS_SSL_VERIFY_REQUIRED. If verifypeer
or verifyhost are disabled the corresponding error flags are cleared
in the verify callback function. That is also where verification
errors are logged.
@jan2000
Copy link
Contributor Author

jan2000 commented Aug 19, 2024

Hmm.. I am not quite sure what the best way is to do continuation PRs (this PR depends on that PR) with github. When I rebased locally there were no conflicts. Its a shame that github does not (at least that I known) provide such functionality.

@vszakats
Copy link
Member

There was a merge queue feature announced earlier: https://github.blog/changelog/2023-07-12-pull-request-merge-queue-is-now-generally-available/
(that's all I know about it)

@bagder bagder closed this in 925aea1 Aug 20, 2024
@jan2000 jan2000 deleted the mbedtls-verify-required branch August 20, 2024 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants