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

Allow configuring MBEDTLS_TLS_EXT_CID at compile time #4413

Merged
merged 1 commit into from
Aug 13, 2021

Conversation

gilles-peskine-arm
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm commented Apr 24, 2021

The numerical identifier of the CID extension hasn't been settled yet and different implementations use values from different drafts. Allow configuring the value at compile time as discussed in #3892 (comment).

This PR is for 3.0. 2.x PR

@boaks
Copy link

boaks commented Apr 26, 2021

First, thanks for addressing my issue with this PR.

I made this PR against 3.0. Should we do it in 2.2x as well, since the CID extension ID is likely to fluctuate then settle during its lifetime?

FMPOV, it's more important for 2.2x.

The draft-ietf-tls-dtls-connection-id is currently reviewed and the requested changes so far are about terms on expressions, but not longer the used data and security functions. So, hope it gets released within the next months and then with a stable defined MAC and code points.

mpg
mpg previously approved these changes Apr 26, 2021
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

@mpg mpg added the needs-backports Backports are missing or are pending review and approval. label Apr 26, 2021
@gilles-peskine-arm gilles-peskine-arm changed the base branch from development_3.0 to development April 27, 2021 09:36
@gilles-peskine-arm gilles-peskine-arm dismissed mpg’s stale review April 27, 2021 09:36

The base branch was changed.

OBSOLETE - PLEASE SEE https://github.com/orgs/Mbed-TLS/projects/2 automation moved this from Has Approval to In Progress Apr 27, 2021
mpg
mpg previously approved these changes Apr 27, 2021
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.

Re-approving after the name of the target branch was changed.

@ronald-cron-arm
Copy link
Contributor

Two reviewers thus removing the "needs: reviewer" label. @hanno-arm please shout if you don't have time to review it.

@ronald-cron-arm ronald-cron-arm removed the needs-reviewer This PR needs someone to pick it up for review label May 10, 2021
hanno-becker
hanno-becker previously approved these changes Jun 4, 2021
Copy link

@hanno-becker hanno-becker left a comment

Choose a reason for hiding this comment

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

LGTM. Just needs resolving of the merge conflict.

@mpg mpg added needs-work and removed needs-review Every commit must be reviewed by at least two team members, labels Aug 12, 2021
@mpg
Copy link
Contributor

mpg commented Aug 12, 2021

Needs rebasing to resolve conflicts.

The numerical identifier of the CID extension hasn't been settled yet
and different implementations use values from different drafts. Allow
configuring the value at compile time.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@mpg mpg dismissed stale reviews from hanno-becker and themself via 7dd2f50 August 12, 2021 08:35
OBSOLETE - PLEASE SEE https://github.com/orgs/Mbed-TLS/projects/2 automation moved this from Has Approval to In Development Aug 12, 2021
@mpg mpg added 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 and removed needs-work labels Aug 12, 2021
Copy link
Contributor

@yanesca yanesca left a comment

Choose a reason for hiding this comment

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

LGTM.

(Reviewed the PR from the start, didn't follow the review history.)

@mpg mpg added approved Design and code approved - may be waiting for CI or backports 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 needs-backports Backports are missing or are pending review and approval. labels Aug 13, 2021
@mpg mpg merged commit 93a3ca6 into Mbed-TLS:development Aug 13, 2021
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 component-tls
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants