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

Move client_auth to handshake #5472

Merged
merged 5 commits into from
Feb 9, 2022
Merged

Conversation

yuhaoth
Copy link
Contributor

@yuhaoth yuhaoth commented Jan 28, 2022

ssl->client_auth is used to indicate CertificateRequest received from server. If received, CertificateVerfiy and Certificate MUST be sent by client. The variable is used only in handshake stage.

  • Guard it with MBEDTLS_SSL_PROTO_TLS1_2
  • Move it to ssl->handshake

Status

READY

The variable is used for client side only

Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
@yuhaoth yuhaoth added needs-ci Needs to pass CI tests needs-review Every commit must be reviewed by at least two team members, enhancement needs-reviewer This PR needs someone to pick it up for review labels Jan 28, 2022
@yuhaoth
Copy link
Contributor Author

yuhaoth commented Jan 28, 2022

ABI-API-checking fail only

mpg
mpg previously approved these changes Jan 28, 2022
Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for making those changes!

@mpg mpg removed the needs-ci Needs to pass CI tests label Jan 28, 2022
@ronald-cron-arm ronald-cron-arm self-requested a review January 31, 2022 07:53
include/mbedtls/ssl.h Show resolved Hide resolved
library/ssl_misc.h Outdated Show resolved Hide resolved
library/ssl_misc.h Outdated Show resolved Hide resolved
@ronald-cron-arm ronald-cron-arm added needs-work and removed needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Feb 2, 2022
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
@yuhaoth yuhaoth added the needs-review Every commit must be reviewed by at least two team members, label Feb 8, 2022
library/ssl_misc.h Outdated Show resolved Hide resolved
library/ssl_misc.h Outdated Show resolved Hide resolved
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
Copy link
Contributor

@ronald-cron-arm ronald-cron-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks. @mpg please have another look.

Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for cleaning this up!

@mpg mpg added approved Design and code approved - may be waiting for CI or backports and removed needs-work needs-review Every commit must be reviewed by at least two team members, labels Feb 9, 2022
@mpg mpg merged commit 62b49cd into Mbed-TLS:development Feb 9, 2022
ronald-cron-arm added a commit to ronald-cron-arm/mbedtls that referenced this pull request Feb 9, 2022
The TLS `client_auth` has been moved from the SSL
to the handshake context by Mbed-TLS#5472.

Fix the accesses to this field in the SSL context
that were introduced by Mbed-TLS#5080 that has been merged
a bit before Mbed-TLS#5472.

The CI did not re-run on the merge of Mbed-TLS#5472 into
developement after the merge of Mbed-TLS#5080 thus did not
catch the issue.

Signed-off-by: Ronald Cron <ronald.cron@arm.com>
@yuhaoth yuhaoth deleted the pr/move-client-auth branch December 6, 2023 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants