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.

Sign up for GitHub

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

Jump to bottom

TLS 1.3: EarlyData SRV: Write early_data extension of NewSessionTicket #6721

Merged
merged 25 commits into from
Dec 7, 2023
Merged

TLS 1.3: EarlyData SRV: Write early_data extension of NewSessionTicket #6721

merged 25 commits into from
Dec 7, 2023

Conversation

yuhaoth
Copy link
Contributor

@yuhaoth yuhaoth commented Dec 5, 2022 •
edited
Loading

Description

fix #6347

This PR write early_data extension of NST message. max_early_data_field is come from configured value. Next PRs will fix that.

Gatekeeper checklist

  • changelog provided, or not required
  • backport done, or not required
  • tests provided, or not required

Notes for the submitter

Please refer to the contributing guidelines, especially the
checklist for PR contributors.

@yuhaoth yuhaoth changed the title Pr/tls13 early data extension of nst TLS 1.3: EarlyData SRV: Add max_early_data_size support. Dec 5, 2022
@yuhaoth yuhaoth added needs-review Every commit must be reviewed by at least two team members, needs-preceding-pr Requires another PR to be merged first needs-reviewer This PR needs someone to pick it up for review component-tls13 labels Dec 5, 2022
@yuhaoth yuhaoth added the priority-high High priority - will be reviewed soon label Dec 5, 2022
@yuhaoth yuhaoth mentioned this pull request Dec 5, 2022
Closed
5 tasks
@yuhaoth yuhaoth force-pushed the pr/tls13-early-data-extension-of-nst branch 6 times, most recently from 99890f9 to 7b8e5aa Compare December 5, 2022 10:08
@yuhaoth yuhaoth changed the title TLS 1.3: EarlyData SRV: Add max_early_data_size support. TLS 1.3: EarlyData SRV: Add max_early_data_size of NewSessionTicket support. Dec 6, 2022
@yuhaoth yuhaoth force-pushed the pr/tls13-early-data-extension-of-nst branch 5 times, most recently from 9fd6b8f to baf774d Compare December 12, 2022 08:45
@yuhaoth yuhaoth force-pushed the pr/tls13-early-data-extension-of-nst branch 3 times, most recently from 21a2d11 to 447bb1c Compare December 17, 2022 03:08
@yuhaoth yuhaoth force-pushed the pr/tls13-early-data-extension-of-nst branch from 447bb1c to 9b8f3bd Compare December 30, 2022 03:36
@yuhaoth yuhaoth mentioned this pull request Jan 4, 2023
Merged
5 tasks
@yuhaoth yuhaoth force-pushed the pr/tls13-early-data-extension-of-nst branch 2 times, most recently from 603dfbd to 0f17947 Compare January 5, 2023 02:51
@yuhaoth yuhaoth force-pushed the pr/tls13-early-data-extension-of-nst branch from 0f17947 to ea490d3 Compare January 12, 2023 09:46
@yuhaoth yuhaoth force-pushed the pr/tls13-early-data-extension-of-nst branch from ea490d3 to da3b712 Compare February 7, 2023 06:19
yuhaoth added 12 commits December 6, 2023 18:18
@yuhaoth
Revert "change max_early_data_size source"
930ce4c 
This reverts commit 3d8d6a7.

Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
@yuhaoth
Add max_early_data_size check
f135bac 
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
@yuhaoth
set max_early_data_size of ticket to keep consistent
1a16070 
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
@yuhaoth
improve comments of new session ticket
0069abc 
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
@yuhaoth
share write_early_data_ext function
5233539 
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
@yuhaoth
add ticket max_early_data_size check
db97163 
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
@yuhaoth
Add missing debug message
9e7f9bc 
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
@yuhaoth
fix various issue
ebe1de6 
- rename connection time variable
- remove unnecessary comments

Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
@yuhaoth
remove assignment for session->max_early_data_size
163e12f 
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
@yuhaoth
change prototype of write_early_data_ext
c59c586 
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
@yuhaoth
Some minor improvement
95648b0 
- move early data check to `prepare`
- avoid `((void) output_len)
- replace check with `session_ticket_allow`  in 2nd place

Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
@yuhaoth
remove misbehavior tests and code
750e067 
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
@yuhaoth yuhaoth force-pushed the pr/tls13-early-data-extension-of-nst branch from 0a74e67 to 750e067 Compare December 6, 2023 10:22
@yuhaoth
Copy link
Contributor Author

yuhaoth commented Dec 6, 2023

Rebased to resolve conflicts

@yuhaoth yuhaoth mentioned this pull request Dec 7, 2023
Closed
3 tasks
@yuhaoth
Copy link
Contributor Author

yuhaoth commented Dec 7, 2023

OpenCI passed.

ronald-cron-arm
ronald-cron-arm approved these changes Dec 7, 2023
View reviewed changes
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

yanrayw
yanrayw approved these changes Dec 7, 2023
View reviewed changes
Copy link

@yanrayw yanrayw left a comment

Choose a reason for hiding this comment

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

LGTM, only a niptick comment.

tests/opt-testcases/tls13-misc.sh
@@ -518,7 +502,8 @@ run_test "TLS 1.3 G->m: EarlyData: feature is enabled, good." \
"$G_NEXT_CLI localhost --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:+GROUP-ALL:+KX-ALL \
-d 10 -r --earlydata $EARLY_DATA_INPUT " \
0 \
-s "NewSessionTicket: early_data(42) extension exists." \
-s "Sent max_early_data_size=$EARLY_DATA_INPUT_LEN" \
Copy link

Choose a reason for hiding this comment

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

Niptick comment: it seems Sent max_early_data_size=$EARLY_DATA_INPUT_LEN appears first in the logs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will change it in next PR

@yanrayw yanrayw 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, labels Dec 7, 2023
@ronald-cron-arm ronald-cron-arm added this pull request to the merge queue Dec 7, 2023
Merged via the queue into Mbed-TLS:development with commit 90d0711 Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ronald-cron-arm ronald-cron-arm ronald-cron-arm approved these changes

@yanrayw yanrayw yanrayw approved these changes

@lpy4105 lpy4105 Awaiting requested review from lpy4105

@tom-cosgrove-arm tom-cosgrove-arm Awaiting requested review from tom-cosgrove-arm

Assignees
No one assigned
Labels
approved Design and code approved - may be waiting for CI or backports component-tls13 priority-high High priority - will be reviewed soon size-s Estimated task size: small (~2d)
Projects
No open projects
Roadmap Board for Mbed TLS
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

TLS 1.3 server: Maximize the number of early data that can be received
6 participants
@yuhaoth @lpy4105 @ronald-cron-arm @yanrayw @tom-cosgrove-arm @yanesca