From 8e0ece920b1be74e5e63e932657de64acc1f73ea Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 19 Jan 2022 09:20:45 -0800 Subject: [PATCH] Test cleanups. Fix possible leak in `TLSX_UseSRTP`. --- examples/client/client.c | 30 ++++++++++++++++-------------- examples/server/server.c | 30 ++++++++++++++++-------------- src/ssl.c | 9 +++------ src/tls.c | 11 +++++------ tests/suites.c | 6 +++--- wolfssl/test.h | 20 ++++++++++---------- 6 files changed, 53 insertions(+), 53 deletions(-) diff --git a/examples/client/client.c b/examples/client/client.c index 0a71b6960..8927c6c4d 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -1770,32 +1770,34 @@ static void Usage(void) * calls srtp_helper_get_ekm() to wait and then get the ekm computed by the * server, then check if it matches the one computed by itself. */ -static int client_srtp_test(WOLFSSL *ssl, struct srtp_test_helper *srtp_helper) +static int client_srtp_test(WOLFSSL *ssl, srtp_test_helper *srtp_helper) { - uint8_t *srtp_secret, *other_secret, *p; - size_t srtp_secret_length, other_size; + byte *srtp_secret, *other_secret = NULL, *p; + size_t srtp_secret_length, other_size = 0; int ret; ret = wolfSSL_export_dtls_srtp_keying_material(ssl, NULL, &srtp_secret_length); if (ret != LENGTH_ONLY_E) { - printf("SRTP: can't get dtsl_srtp keying material"); + printf("DTLS SRTP: Error getting keying material length\n"); return ret; } - srtp_secret = (uint8_t*)XMALLOC(srtp_secret_length, + srtp_secret = (byte*)XMALLOC(srtp_secret_length, NULL, DYNAMIC_TYPE_TMP_BUFFER); - if (srtp_secret == NULL) - err_sys("SRTP: low memory"); + if (srtp_secret == NULL) { + err_sys("DTLS SRTP: Low memory"); + } ret = wolfSSL_export_dtls_srtp_keying_material(ssl, srtp_secret, &srtp_secret_length); if (ret != WOLFSSL_SUCCESS) { - printf("SRTP: can't get dtsl_srtp keying material"); + XFREE(srtp_secret, NULL, DYNAMIC_TYPE_TMP_BUFFER); + printf("DTLS SRTP: Error getting keying material\n"); return ret; } - printf("DTLS-SRTP exported key material:\n"); + printf("DTLS SRTP: Exported key material:\n"); for (p = srtp_secret; p < srtp_secret + srtp_secret_length; p++) printf("%02X", *p); printf("\n"); @@ -1808,7 +1810,7 @@ static int client_srtp_test(WOLFSSL *ssl, struct srtp_test_helper *srtp_helper) /* we are delegated from server to free this buffer */ XFREE(other_secret, NULL, DYNAMIC_TYPE_TMP_BUFFER); - printf("SRTP: Exported Keying Material mismatch"); + printf("DTLS SRTP: Exported Keying Material mismatch\n"); return WOLFSSL_UNKNOWN; } @@ -1820,7 +1822,7 @@ static int client_srtp_test(WOLFSSL *ssl, struct srtp_test_helper *srtp_helper) return 0; } -#endif +#endif /* WOLFSSL_SRTP */ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) { @@ -3974,7 +3976,7 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) #ifdef WOLFSSL_SRTP if (dtlsSrtpProfiles != NULL) { - err = client_srtp_test(ssl, ((func_args*)args)->srtp_test_helper); + err = client_srtp_test(ssl, ((func_args*)args)->srtp_helper); if (err != 0) { if (exitWithRet) { ((func_args*)args)->return_code = err; @@ -3986,7 +3988,7 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) err_sys("SRTP check failed"); } } -#endif +#endif /* WOLFSSL_SRTP */ #ifdef WOLFSSL_TLS13 if (updateKeysIVs) @@ -4340,7 +4342,7 @@ exit: StartTCP(); #ifdef WOLFSSL_SRTP - args.srtp_test_helper = NULL; + args.srtp_helper = NULL; #endif args.argc = argc; args.argv = argv; diff --git a/examples/server/server.c b/examples/server/server.c index 8e6434005..8bec45332 100644 --- a/examples/server/server.c +++ b/examples/server/server.c @@ -1290,41 +1290,44 @@ static void Usage(void) * calls srtp_helper_set_ekm() to wake the client and share the ekm with * him. The client will check that the ekm matches the one computed by itself. */ -static int server_srtp_test(WOLFSSL *ssl, struct srtp_test_helper *srtp_helper) +static int server_srtp_test(WOLFSSL *ssl, srtp_test_helper *srtp_helper) { size_t srtp_secret_length; - uint8_t *srtp_secret, *p; + byte *srtp_secret, *p; int ret; ret = wolfSSL_export_dtls_srtp_keying_material(ssl, NULL, &srtp_secret_length); if (ret != LENGTH_ONLY_E) { - printf("SRTP: can't get dtsl_srtp keying material"); + printf("DTLS SRTP: Error getting key material length\n"); return ret; } - srtp_secret = (uint8_t*)XMALLOC(srtp_secret_length, + srtp_secret = (byte*)XMALLOC(srtp_secret_length, NULL, DYNAMIC_TYPE_TMP_BUFFER); - if (srtp_secret == NULL) - err_sys("SRTP: low memory"); + if (srtp_secret == NULL) { + err_sys("DTLS SRTP: Memory error"); + } ret = wolfSSL_export_dtls_srtp_keying_material(ssl, srtp_secret, &srtp_secret_length); if (ret != WOLFSSL_SUCCESS) { - printf("SRTP: can't get dtsl_srtp keying material"); + XFREE(srtp_secret, NULL, DYNAMIC_TYPE_TMP_BUFFER); + printf("DTLS SRTP: Error getting key material\n"); return ret; } - printf("DTLS-SRTP exported key material:\n"); + printf("DTLS SRTP: Exported key material:\n"); for (p = srtp_secret; p < srtp_secret + srtp_secret_length; p++) printf("%02X", *p); printf("\n"); if (srtp_helper != NULL) { srtp_helper_set_ekm(srtp_helper, srtp_secret, srtp_secret_length); - /* client code will free srtp_sercret buffer after checking for + /* client code will free srtp_secret buffer after checking for correctness */ - } else { + } + else { XFREE(srtp_secret, NULL, DYNAMIC_TYPE_TMP_BUFFER); } @@ -3143,7 +3146,7 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args) #ifdef WOLFSSL_SRTP if (dtlsSrtpProfiles != NULL) { - err = server_srtp_test(ssl, ((func_args*)args)->srtp_test_helper); + err = server_srtp_test(ssl, ((func_args*)args)->srtp_helper); if (err != 0) { if (exitWithRet) { ((func_args*)args)->return_code = err; @@ -3155,8 +3158,7 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args) err_sys("SRTP check failed"); } } - -#endif +#endif /* WOLFSSL_SRTP */ #ifdef HAVE_ALPN if (alpnList != NULL) { @@ -3422,7 +3424,7 @@ exit: args.signal = &ready; args.return_code = 0; #ifdef WOLFSSL_SRTP - args.srtp_test_helper = NULL; + args.srtp_helper = NULL; #endif InitTcpReady(&ready); diff --git a/src/ssl.c b/src/ssl.c index a1deb34fc..7a7540f94 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -1301,18 +1301,15 @@ static const WOLFSSL_SRTP_PROTECTION_PROFILE gSrtpProfiles[] = { static const WOLFSSL_SRTP_PROTECTION_PROFILE* DtlsSrtpFindProfile( const char* profile_str, word32 profile_str_len, unsigned long id) { - size_t srtp_profile_len; int i; const WOLFSSL_SRTP_PROTECTION_PROFILE* profile = NULL; for (i=0; i<(int)(sizeof(gSrtpProfiles)/sizeof(WOLFSSL_SRTP_PROTECTION_PROFILE)); i++) { if (profile_str != NULL) { - srtp_profile_len = strlen(gSrtpProfiles[i].name); - if (srtp_profile_len != profile_str_len) - continue; - - if (XMEMCMP(gSrtpProfiles[i].name, profile_str, profile_str_len) + word32 srtp_profile_len = (word32)XSTRLEN(gSrtpProfiles[i].name); + if (srtp_profile_len == profile_str_len && + XMEMCMP(gSrtpProfiles[i].name, profile_str, profile_str_len) == 0) { profile = &gSrtpProfiles[i]; break; diff --git a/src/tls.c b/src/tls.c index c1fe13b21..a9139a47c 100644 --- a/src/tls.c +++ b/src/tls.c @@ -5507,20 +5507,19 @@ static word16 TLSX_UseSRTP_Write(TlsxSrtp* srtp, byte* output) static int TLSX_UseSRTP(TLSX** extensions, word16 profiles, void* heap) { int ret = 0; - TlsxSrtp *srtp = NULL; TLSX* extension; if (extensions == NULL) { return BAD_FUNC_ARG; } - srtp = TLSX_UseSRTP_New(profiles, heap); - if (srtp == NULL) { - return MEMORY_E; - } - extension = TLSX_Find(*extensions, TLSX_USE_SRTP); if (extension == NULL) { + TlsxSrtp* srtp = TLSX_UseSRTP_New(profiles, heap); + if (srtp == NULL) { + return MEMORY_E; + } + ret = TLSX_Push(extensions, TLSX_USE_SRTP, (void*)srtp, heap); if (ret != 0) { TLSX_UseSRTP_Free(srtp, heap); diff --git a/tests/suites.c b/tests/suites.c index 422bc5b36..39764ff97 100644 --- a/tests/suites.c +++ b/tests/suites.c @@ -322,7 +322,7 @@ static int execute_test_case(int svr_argc, char** svr_argv, #endif #ifdef WOLFSSL_SRTP - struct srtp_test_helper srtp_helper; + srtp_test_helper srtp_helper; #endif /* Is Valid Cipher and Version Checks */ /* build command list for the Is checks below */ @@ -454,8 +454,8 @@ static int execute_test_case(int svr_argc, char** svr_argv, #ifdef WOLFSSL_SRTP srtp_helper_init(&srtp_helper); - cliArgs.srtp_test_helper = &srtp_helper; - svrArgs.srtp_test_helper = &srtp_helper; + cliArgs.srtp_helper = &srtp_helper; + svrArgs.srtp_helper = &srtp_helper; #endif #ifdef WOLFSSL_TIRTOS fdOpenSession(Task_self()); diff --git a/wolfssl/test.h b/wolfssl/test.h index 65ad2f042..885dd2b01 100644 --- a/wolfssl/test.h +++ b/wolfssl/test.h @@ -521,14 +521,14 @@ typedef struct callback_functions { } callback_functions; #ifdef WOLFSSL_SRTP -struct srtp_test_helper { +typedef struct srtp_test_helper { #if defined(_POSIX_THREADS) && !defined(__MINGW32__) pthread_mutex_t mutex; pthread_cond_t cond; #endif - uint8_t *server_srtp_ekm; - size_t server_srtp_ekm_size; -}; + uint8_t* server_srtp_ekm; + size_t server_srtp_ekm_size; +} srtp_test_helper; #endif typedef struct func_args { @@ -538,7 +538,7 @@ typedef struct func_args { tcp_ready* signal; callback_functions *callbacks; #ifdef WOLFSSL_SRTP - struct srtp_test_helper *srtp_test_helper; + srtp_test_helper* srtp_helper; #endif } func_args; @@ -645,7 +645,7 @@ extern char* myoptarg; #ifdef WOLFSSL_SRTP -static WC_INLINE void srtp_helper_init(struct srtp_test_helper *srtp) +static WC_INLINE void srtp_helper_init(srtp_test_helper *srtp) { srtp->server_srtp_ekm_size = 0; srtp->server_srtp_ekm = NULL; @@ -664,7 +664,7 @@ static WC_INLINE void srtp_helper_init(struct srtp_test_helper *srtp) * This function wait that the other peer calls strp_helper_set_ekm() and then * store the buffer pointer/size in @ekm and @size. */ -static WC_INLINE void srtp_helper_get_ekm(struct srtp_test_helper *srtp, +static WC_INLINE void srtp_helper_get_ekm(srtp_test_helper *srtp, uint8_t **ekm, size_t *size) { #if defined(_POSIX_THREADS) && !defined(__MINGW32__) @@ -695,7 +695,7 @@ static WC_INLINE void srtp_helper_get_ekm(struct srtp_test_helper *srtp, * * used in client_srtp_test()/server_srtp_test() */ -static WC_INLINE void srtp_helper_set_ekm(struct srtp_test_helper *srtp, +static WC_INLINE void srtp_helper_set_ekm(srtp_test_helper *srtp, uint8_t *ekm, size_t size) { #if defined(_POSIX_THREADS) && !defined(__MINGW32__) @@ -709,7 +709,7 @@ static WC_INLINE void srtp_helper_set_ekm(struct srtp_test_helper *srtp, #endif } -static WC_INLINE void srtp_helper_free(struct srtp_test_helper *srtp) +static WC_INLINE void srtp_helper_free(srtp_test_helper *srtp) { #if defined(_POSIX_THREADS) && !defined(__MINGW32__) pthread_mutex_destroy(&srtp->mutex); @@ -717,7 +717,7 @@ static WC_INLINE void srtp_helper_free(struct srtp_test_helper *srtp) #endif } -#endif +#endif /* WOLFSSL_SRTP */ /** *