From 0b74434e2af40743f8ac051d7df0ed80e68310e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 26 Jun 2023 11:28:00 +0200 Subject: [PATCH 1/3] SSL programs: group options processing in 1 place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- programs/ssl/ssl_client2.c | 50 +++++++++++++++++++------------------- programs/ssl/ssl_server2.c | 50 +++++++++++++++++++------------------- 2 files changed, 50 insertions(+), 50 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 12a1068f9725..f4914ec406a4 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -867,31 +867,6 @@ int main(int argc, char *argv[]) mbedtls_test_enable_insecure_external_rng(); #endif /* MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG */ - if (argc < 2) { -usage: - if (ret == 0) { - ret = 1; - } - - mbedtls_printf(USAGE1); - mbedtls_printf(USAGE2); - mbedtls_printf(USAGE3); - mbedtls_printf(USAGE4); - - list = mbedtls_ssl_list_ciphersuites(); - while (*list) { - mbedtls_printf(" %-42s", mbedtls_ssl_get_ciphersuite_name(*list)); - list++; - if (!*list) { - break; - } - mbedtls_printf(" %s\n", mbedtls_ssl_get_ciphersuite_name(*list)); - list++; - } - mbedtls_printf("\n"); - goto exit; - } - opt.server_name = DFL_SERVER_NAME; opt.server_addr = DFL_SERVER_ADDR; opt.server_port = DFL_SERVER_PORT; @@ -976,6 +951,31 @@ int main(int argc, char *argv[]) opt.key_opaque_alg1 = DFL_KEY_OPAQUE_ALG; opt.key_opaque_alg2 = DFL_KEY_OPAQUE_ALG; + if (argc < 2) { +usage: + if (ret == 0) { + ret = 1; + } + + mbedtls_printf(USAGE1); + mbedtls_printf(USAGE2); + mbedtls_printf(USAGE3); + mbedtls_printf(USAGE4); + + list = mbedtls_ssl_list_ciphersuites(); + while (*list) { + mbedtls_printf(" %-42s", mbedtls_ssl_get_ciphersuite_name(*list)); + list++; + if (!*list) { + break; + } + mbedtls_printf(" %s\n", mbedtls_ssl_get_ciphersuite_name(*list)); + list++; + } + mbedtls_printf("\n"); + goto exit; + } + for (i = 1; i < argc; i++) { p = argv[i]; if ((q = strchr(p, '=')) == NULL) { diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 5f8bea93ca49..a3e95ccd5c46 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1643,31 +1643,6 @@ int main(int argc, char *argv[]) signal(SIGINT, term_handler); #endif - if (argc < 2) { -usage: - if (ret == 0) { - ret = 1; - } - - mbedtls_printf(USAGE1); - mbedtls_printf(USAGE2); - mbedtls_printf(USAGE3); - mbedtls_printf(USAGE4); - - list = mbedtls_ssl_list_ciphersuites(); - while (*list) { - mbedtls_printf(" %-42s", mbedtls_ssl_get_ciphersuite_name(*list)); - list++; - if (!*list) { - break; - } - mbedtls_printf(" %s\n", mbedtls_ssl_get_ciphersuite_name(*list)); - list++; - } - mbedtls_printf("\n"); - goto exit; - } - opt.buffer_size = DFL_IO_BUF_LEN; opt.server_addr = DFL_SERVER_ADDR; opt.server_port = DFL_SERVER_PORT; @@ -1766,6 +1741,31 @@ int main(int argc, char *argv[]) opt.key2_opaque_alg1 = DFL_KEY_OPAQUE_ALG; opt.key2_opaque_alg2 = DFL_KEY_OPAQUE_ALG; + if (argc < 2) { +usage: + if (ret == 0) { + ret = 1; + } + + mbedtls_printf(USAGE1); + mbedtls_printf(USAGE2); + mbedtls_printf(USAGE3); + mbedtls_printf(USAGE4); + + list = mbedtls_ssl_list_ciphersuites(); + while (*list) { + mbedtls_printf(" %-42s", mbedtls_ssl_get_ciphersuite_name(*list)); + list++; + if (!*list) { + break; + } + mbedtls_printf(" %s\n", mbedtls_ssl_get_ciphersuite_name(*list)); + list++; + } + mbedtls_printf("\n"); + goto exit; + } + for (i = 1; i < argc; i++) { p = argv[i]; if ((q = strchr(p, '=')) == NULL) { From 3eea9a461cfbffb27f3ca5d95b4ea988b8799b4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 26 Jun 2023 11:29:35 +0200 Subject: [PATCH 2/3] SSL programs: allow invoking without arguments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All options have reasonable default so the programs don't need arguments to do something useful. It is widely accepted for programs that can work without arguments need not insist on the user passing arguments, see 'ls', 'wc', 'sort', 'more' and any number of POSIX utilities that all work without arguments. It is also the historical behaviour of those programs, and something relied one by at least a few team members. Signed-off-by: Manuel Pégourié-Gonnard --- programs/ssl/ssl_client2.c | 2 +- programs/ssl/ssl_server2.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index f4914ec406a4..d84333436c93 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -951,7 +951,7 @@ int main(int argc, char *argv[]) opt.key_opaque_alg1 = DFL_KEY_OPAQUE_ALG; opt.key_opaque_alg2 = DFL_KEY_OPAQUE_ALG; - if (argc < 2) { + if (argc < 1) { usage: if (ret == 0) { ret = 1; diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index a3e95ccd5c46..b305bc534518 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1741,7 +1741,7 @@ int main(int argc, char *argv[]) opt.key2_opaque_alg1 = DFL_KEY_OPAQUE_ALG; opt.key2_opaque_alg2 = DFL_KEY_OPAQUE_ALG; - if (argc < 2) { + if (argc < 1) { usage: if (ret == 0) { ret = 1; From 39a0a76fcc3f8aa0ebae8ee02d8d7966d0cfe062 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 27 Jun 2023 09:28:24 +0200 Subject: [PATCH 3/3] SSL programs: improve command-line error reporting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every now and then, I see of these programs failing with a super-long usage message that gives no clue as to what went wrong. (Recently it happened with a test case in ssl-opt.sh with a fairly long command line that was entirely correct, except some options were not valid in this config - the test should have been skipped but wasn't due to some other bug. It took me longer to figure out than it should have, and could have if the program had simply reported which param was not recognized.) Also, have an explicit "help" command, separate "help_ciphersuites", and have default usage message that's not multiple screens long. Signed-off-by: Manuel Pégourié-Gonnard --- programs/ssl/ssl_client2.c | 58 +++++++++++++++++++++++++++----------- programs/ssl/ssl_server2.c | 58 +++++++++++++++++++++++++++----------- 2 files changed, 82 insertions(+), 34 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index d84333436c93..0e0e9cee7ee4 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -464,7 +464,7 @@ int main(void) " otherwise. The expansion of the macro\n" \ " is printed if it is defined\n" \ USAGE_SERIALIZATION \ - " acceptable ciphersuite names:\n" + "\n" #define ALPN_LIST_SIZE 10 #define CURVE_LIST_SIZE 20 @@ -951,34 +951,54 @@ int main(int argc, char *argv[]) opt.key_opaque_alg1 = DFL_KEY_OPAQUE_ALG; opt.key_opaque_alg2 = DFL_KEY_OPAQUE_ALG; + p = q = NULL; if (argc < 1) { usage: - if (ret == 0) { - ret = 1; + if (p != NULL && q != NULL) { + printf("unrecognized value for '%s': '%s'\n", p, q); + } else if (p != NULL && q == NULL) { + printf("unrecognized param: '%s'\n", p); } - mbedtls_printf(USAGE1); - mbedtls_printf(USAGE2); - mbedtls_printf(USAGE3); - mbedtls_printf(USAGE4); + mbedtls_printf("usage: ssl_client2 [param=value] [...]\n"); + mbedtls_printf(" ssl_client2 help[_theme]\n"); + mbedtls_printf("'help' lists acceptable 'param' and 'value'\n"); + mbedtls_printf("'help_ciphersuites' lists available ciphersuites\n"); + mbedtls_printf("\n"); - list = mbedtls_ssl_list_ciphersuites(); - while (*list) { - mbedtls_printf(" %-42s", mbedtls_ssl_get_ciphersuite_name(*list)); - list++; - if (!*list) { - break; - } - mbedtls_printf(" %s\n", mbedtls_ssl_get_ciphersuite_name(*list)); - list++; + if (ret == 0) { + ret = 1; } - mbedtls_printf("\n"); goto exit; } for (i = 1; i < argc; i++) { p = argv[i]; + + if (strcmp(p, "help") == 0) { + mbedtls_printf(USAGE1); + mbedtls_printf(USAGE2); + mbedtls_printf(USAGE3); + mbedtls_printf(USAGE4); + + ret = 0; + goto exit; + } + if (strcmp(p, "help_ciphersuites") == 0) { + mbedtls_printf(" acceptable ciphersuite names:\n"); + for (list = mbedtls_ssl_list_ciphersuites(); + *list != 0; + list++) { + mbedtls_printf(" %s\n", mbedtls_ssl_get_ciphersuite_name(*list)); + } + + ret = 0; + goto exit; + } + if ((q = strchr(p, '=')) == NULL) { + mbedtls_printf("param requires a value: '%s'\n", p); + p = NULL; // avoid "unrecnognized param" message goto usage; } *q++ = '\0'; @@ -1375,9 +1395,13 @@ int main(int argc, char *argv[]) goto usage; } } else { + /* This signals that the problem is with p not q */ + q = NULL; goto usage; } } + /* This signals that any further errors are not with a single option */ + p = q = NULL; if (opt.nss_keylog != 0 && opt.eap_tls != 0) { mbedtls_printf("Error: eap_tls and nss_keylog options cannot be used together.\n"); diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index b305bc534518..769a56a50ce9 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -584,7 +584,7 @@ int main(void) " otherwise. The expansion of the macro\n" \ " is printed if it is defined\n" \ USAGE_SERIALIZATION \ - " acceptable ciphersuite names:\n" + "\n" #define ALPN_LIST_SIZE 10 #define CURVE_LIST_SIZE 20 @@ -1741,34 +1741,54 @@ int main(int argc, char *argv[]) opt.key2_opaque_alg1 = DFL_KEY_OPAQUE_ALG; opt.key2_opaque_alg2 = DFL_KEY_OPAQUE_ALG; + p = q = NULL; if (argc < 1) { usage: - if (ret == 0) { - ret = 1; + if (p != NULL && q != NULL) { + printf("unrecognized value for '%s': '%s'\n", p, q); + } else if (p != NULL && q == NULL) { + printf("unrecognized param: '%s'\n", p); } - mbedtls_printf(USAGE1); - mbedtls_printf(USAGE2); - mbedtls_printf(USAGE3); - mbedtls_printf(USAGE4); + mbedtls_printf("usage: ssl_client2 [param=value] [...]\n"); + mbedtls_printf(" ssl_client2 help[_theme]\n"); + mbedtls_printf("'help' lists acceptable 'param' and 'value'\n"); + mbedtls_printf("'help_ciphersuites' lists available ciphersuites\n"); + mbedtls_printf("\n"); - list = mbedtls_ssl_list_ciphersuites(); - while (*list) { - mbedtls_printf(" %-42s", mbedtls_ssl_get_ciphersuite_name(*list)); - list++; - if (!*list) { - break; - } - mbedtls_printf(" %s\n", mbedtls_ssl_get_ciphersuite_name(*list)); - list++; + if (ret == 0) { + ret = 1; } - mbedtls_printf("\n"); goto exit; } for (i = 1; i < argc; i++) { p = argv[i]; + + if (strcmp(p, "help") == 0) { + mbedtls_printf(USAGE1); + mbedtls_printf(USAGE2); + mbedtls_printf(USAGE3); + mbedtls_printf(USAGE4); + + ret = 0; + goto exit; + } + if (strcmp(p, "help_ciphersuites") == 0) { + mbedtls_printf(" acceptable ciphersuite names:\n"); + for (list = mbedtls_ssl_list_ciphersuites(); + *list != 0; + list++) { + mbedtls_printf(" %s\n", mbedtls_ssl_get_ciphersuite_name(*list)); + } + + ret = 0; + goto exit; + } + if ((q = strchr(p, '=')) == NULL) { + mbedtls_printf("param requires a value: '%s'\n", p); + p = NULL; // avoid "unrecnognized param" message goto usage; } *q++ = '\0'; @@ -2233,9 +2253,13 @@ int main(int argc, char *argv[]) goto usage; } } else { + /* This signals that the problem is with p not q */ + q = NULL; goto usage; } } + /* This signals that any further erorrs are not with a single option */ + p = q = NULL; if (opt.nss_keylog != 0 && opt.eap_tls != 0) { mbedtls_printf("Error: eap_tls and nss_keylog options cannot be used together.\n");