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

Change early data flag to input file #8596

Merged
merged 17 commits into from
Dec 11, 2023
Merged

Change early data flag to input file #8596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
70fbdcf
Change early data flag to input file
xkqian Dec 4, 2023
6c678d7
Improve the comments of early data input
xkqian Dec 6, 2023
b1db729
Rename the generic read functions to ssl_read_file_text
xkqian Dec 6, 2023
eaebedb
Refine the detect code to enable early data or not
xkqian Dec 6, 2023
f8fe11d
Remove the generic file read functions and simply the early data read
xkqian Dec 6, 2023
611c717
Sync the early_data option with internal parameters in ssl_client2
xkqian Dec 6, 2023
ae95217
Enable early data depend on whether the early data file exist
xkqian Dec 6, 2023
57db590
Rework to revert the early_data enabled flag
xkqian Dec 7, 2023
2a8035b
Add read early data code
xkqian Dec 7, 2023
dd8a7f8
Revert the early data test case
xkqian Dec 7, 2023
35c026c
Read early data file
xkqian Dec 7, 2023
864c62a
Add one test case with early_data_file
xkqian Dec 7, 2023
daddfb5
Open the file once read in the file path
xkqian Dec 7, 2023
9634680
Add the test framework of early data
xkqian Dec 7, 2023
dce183f
Remove the duplicate cases and add early_data_file option
xkqian Dec 7, 2023
aedfc09
Revert to ae952174a7 and addressing some comments
xkqian Dec 8, 2023
a9581d2
Fix CI failure of uninitialized fp
xkqian Dec 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 26 additions & 14 deletions programs/ssl/ssl_client2.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ int main(void)
#define DFL_KEY_OPAQUE 0
#define DFL_KEY_PWD ""
#define DFL_PSK ""
#define DFL_EARLY_DATA MBEDTLS_SSL_EARLY_DATA_DISABLED
#define DFL_EARLY_DATA ""
#define DFL_PSK_OPAQUE 0
#define DFL_PSK_IDENTITY "Client_identity"
#define DFL_ECJPAKE_PW NULL
Expand Down Expand Up @@ -347,8 +347,9 @@ int main(void)

#if defined(MBEDTLS_SSL_EARLY_DATA)
#define USAGE_EARLY_DATA \
" early_data=%%d default: 0 (disabled)\n" \
" options: 0 (disabled), 1 (enabled)\n"
" early_data=%%s The file path to read early data from\n" \
" default: \"\" (do nothing)\n" \
" option: a file path\n"
#else
#define USAGE_EARLY_DATA ""
#endif /* MBEDTLS_SSL_EARLY_DATA && MBEDTLS_SSL_PROTO_TLS1_3 */
Expand Down Expand Up @@ -543,7 +544,7 @@ struct options {
int reproducible; /* make communication reproducible */
int skip_close_notify; /* skip sending the close_notify alert */
#if defined(MBEDTLS_SSL_EARLY_DATA)
int early_data; /* support for early data */
const char *early_data; /* the path of the file to read early data from */
#endif
int query_config_mode; /* whether to read config */
int use_srtp; /* Support SRTP */
Expand Down Expand Up @@ -741,6 +742,10 @@ int main(int argc, char *argv[])
size_t cid_renego_len = 0;
#endif

#if defined(MBEDTLS_SSL_EARLY_DATA)
FILE *early_data_fp = NULL;
#endif /* MBEDTLS_SSL_EARLY_DATA */

#if defined(MBEDTLS_SSL_ALPN)
const char *alpn_list[ALPN_LIST_SIZE];
#endif
Expand Down Expand Up @@ -1196,15 +1201,7 @@ int main(int argc, char *argv[])
#if defined(MBEDTLS_SSL_PROTO_TLS1_3)
#if defined(MBEDTLS_SSL_EARLY_DATA)
else if (strcmp(p, "early_data") == 0) {
switch (atoi(q)) {
case 0:
opt.early_data = MBEDTLS_SSL_EARLY_DATA_DISABLED;
break;
case 1:
opt.early_data = MBEDTLS_SSL_EARLY_DATA_ENABLED;
break;
default: goto usage;
}
opt.early_data = q;
}
#endif /* MBEDTLS_SSL_EARLY_DATA */

Expand Down Expand Up @@ -1971,7 +1968,16 @@ int main(int argc, char *argv[])
}

#if defined(MBEDTLS_SSL_EARLY_DATA)
mbedtls_ssl_conf_early_data(&conf, opt.early_data);
int early_data_enabled = MBEDTLS_SSL_EARLY_DATA_DISABLED;
if (strlen(opt.early_data) > 0) {
if ((early_data_fp = fopen(opt.early_data, "rb")) == NULL) {
mbedtls_printf("failed\n ! Cannot open '%s' for reading.\n",
opt.early_data);
goto exit;
}
early_data_enabled = MBEDTLS_SSL_EARLY_DATA_ENABLED;
}
Comment on lines +1972 to +1979
Copy link
Contributor

@yuhaoth yuhaoth Dec 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

To fix the CI failure

Suggested change
if (strlen(opt.early_data) > 0) {
if ((early_data_fp = fopen(opt.early_data, "rb")) == NULL) {
mbedtls_printf("failed\n ! Cannot open '%s' for reading.\n",
opt.early_data);
goto exit;
}
early_data_enabled = MBEDTLS_SSL_EARLY_DATA_ENABLED;
}
if (opt.early_data) {
if ((early_data_fp = fopen(opt.early_data, "rb")) == NULL) {
mbedtls_printf("failed\n ! Cannot open '%s' for reading.\n",
opt.early_data);
goto exit;
}
early_data_enabled = MBEDTLS_SSL_EARLY_DATA_ENABLED;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The CI failure is caused by the uninitialized file pointer early_data_fp.
opt.early_data will be default to "", this check maybe useless...

Comment on lines +1972 to +1979
Copy link
Contributor

@yuhaoth yuhaoth Dec 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Nitpick comment : move it to option check like previous version

Suggested change
if (strlen(opt.early_data) > 0) {
if ((early_data_fp = fopen(opt.early_data, "rb")) == NULL) {
mbedtls_printf("failed\n ! Cannot open '%s' for reading.\n",
opt.early_data);
goto exit;
}
early_data_enabled = MBEDTLS_SSL_EARLY_DATA_ENABLED;
}
mbedtls_ssl_conf_early_data(&conf, early_data_fp ?MBEDTLS_SSL_EARLY_DATA_ENABLE: MBEDTLS_SSL_EARLY_DATA_DISABLE);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just see there are file processings like context_file, will treat it like what I have done, so I prefer that way.

mbedtls_ssl_conf_early_data(&conf, early_data_enabled);
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit) presumably early data defaults to disabled, so we could move this into the if() and remove the variable?

#endif /* MBEDTLS_SSL_EARLY_DATA */

if ((ret = mbedtls_ssl_setup(&ssl, &conf)) != 0) {
Expand Down Expand Up @@ -3029,6 +3035,12 @@ int main(int argc, char *argv[])
mbedtls_ssl_config_free(&conf);
mbedtls_ssl_session_free(&saved_session);

#if defined(MBEDTLS_SSL_EARLY_DATA)
if (early_data_fp != NULL) {
fclose(early_data_fp);
}
#endif

if (session_data != NULL) {
mbedtls_platform_zeroize(session_data, session_data_len);
}
Expand Down
4 changes: 2 additions & 2 deletions tests/opt-testcases/tls13-misc.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ requires_any_configs_enabled MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL_
run_test "TLS 1.3 m->G: EarlyData: basic check, good" \
"$G_NEXT_SRV -d 10 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:+CIPHER-ALL:+ECDHE-PSK:+PSK \
--earlydata --maxearlydata 16384 --disable-client-cert" \
"$P_CLI debug_level=4 early_data=1 reco_mode=1 reconnect=1 reco_delay=900" \
"$P_CLI debug_level=4 early_data=$EARLY_DATA_INPUT reco_mode=1 reconnect=1 reco_delay=900" \
0 \
-c "received max_early_data_size: 16384" \
-c "Reconnecting with saved session" \
Expand All @@ -287,7 +287,7 @@ requires_any_configs_enabled MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_EPHEMERAL_
MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_ENABLED
run_test "TLS 1.3 m->G: EarlyData: no early_data in NewSessionTicket, good" \
"$G_NEXT_SRV -d 10 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:+CIPHER-ALL:+ECDHE-PSK:+PSK --disable-client-cert" \
"$P_CLI debug_level=4 early_data=1 reco_mode=1 reconnect=1" \
"$P_CLI debug_level=4 early_data=$EARLY_DATA_INPUT reco_mode=1 reconnect=1" \
0 \
-c "Reconnecting with saved session" \
-C "NewSessionTicket: early_data(42) extension received." \
Expand Down