20240608-WOLFSSL_DEBUG_TRACE_ERROR_CODES #7634

douzzer · 2024-06-08T20:13:17Z

New global debugging aid -- --enable-debug-trace-errcodes aka -DWOLFSSL_DEBUG_TRACE_ERROR_CODES causes the library to render to stderr a message with the filename, line number, error code name, and error number, for each and every error code throw.

Example log fragment from an application, with --enable-debug also enabled (they are independent of each other):

[...]
Processing CA PEM file
wolfSSL Entering ProcessBuffer
wolfSSL Entering PemToDer
Adding a CA
ERR TRACE: wolfcrypt/src/asn.c L 1598 ASN_OBJECT_ID_E (-144)
Date AFTER check failed
ERR TRACE: wolfcrypt/src/asn.c L 21754 ASN_AFTER_DATE_E (-151)
Getting Cert Name
wolfSSL Entering wolfSSL_X509_NAME_new_ex
wolfSSL Entering wolfSSL_X509_NAME_add_entry_by_NID
Found place for name entry
[...]

and another example, from testwolfcrypt output showing results from the SRTP-KDF expected-failure tests:

[...]
wolfSSL Entering srtpkdf_test
ERR TRACE: wolfcrypt/src/kdf.c L 1048 BAD_FUNC_ARG (-173)
ERR TRACE: wolfcrypt/src/kdf.c L 1152 BAD_FUNC_ARG (-173)
ERR TRACE: wolfcrypt/src/aes.c L 4358 BAD_FUNC_ARG (-173)
ERR TRACE: wolfcrypt/src/aes.c L 4358 BAD_FUNC_ARG (-173)
ERR TRACE: wolfcrypt/src/kdf.c L 1048 BAD_FUNC_ARG (-173)
ERR TRACE: wolfcrypt/src/kdf.c L 1152 BAD_FUNC_ARG (-173)
ERR TRACE: wolfcrypt/src/kdf.c L 1048 BAD_FUNC_ARG (-173)
ERR TRACE: wolfcrypt/src/kdf.c L 1152 BAD_FUNC_ARG (-173)
ERR TRACE: wolfcrypt/src/kdf.c L 1048 BAD_FUNC_ARG (-173)
ERR TRACE: wolfcrypt/src/kdf.c L 1152 BAD_FUNC_ARG (-173)
ERR TRACE: wolfcrypt/src/kdf.c L 1048 BAD_FUNC_ARG (-173)
ERR TRACE: wolfcrypt/src/kdf.c L 1152 BAD_FUNC_ARG (-173)
ERR TRACE: wolfcrypt/src/kdf.c L 1048 BAD_FUNC_ARG (-173)
ERR TRACE: wolfcrypt/src/kdf.c L 1152 BAD_FUNC_ARG (-173)
SRTP KDF test passed!
[...]

WC_ERR_TRACE(label) can be overridden (e.g. from user_settings.h) with an embedded-friendly or otherwise specialized definition.

Note that error codes are instrumented only inside the library -- the shimming requires defined(BUILDING_LIBWOLFSSL). Thus the WC_NO_ERR_TRACE() macro (which is always a constant numeric value) is for internal use only. Everything outside the library -- applications, of course, but also testwolfcrypt, benchmark.c, etc. -- always see the same numeric constant enum error codes as ever.

On non-autotools builds, manually running support/gen-debug-trace-error-codes.sh will be necessary by some mechanism. It's fine to run manually and directly, and takes no args.

The nitty gritty:

add --enable-debug-trace-errcodes, WOLFSSL_DEBUG_TRACE_ERROR_CODES, WC_ERR_TRACE(), WC_NO_ERR_TRACE(), support/gen-debug-trace-error-codes.sh.

also add numerous deployments of WC_NO_ERR_TRACE() to inhibit frivolous/misleading errcode traces when -DWOLFSSL_DEBUG_TRACE_ERROR_CODES.

tested with wolfssl-multi-test.sh ... quick-check all-gcc-debug-c99 cppcheck-force-source with all-gcc-debug-c99 tweaked to have --enable-debug-trace-errcodes.

additional notes:

missing WC_NO_ERR_TRACE() annotations are benign even in -DWOLFSSL_DEBUG_TRACE_ERROR_CODES builds -- they just cause frivolous errcode traces to render, obviously self-explanatory to the developer because the file and line number are rendered.
building -UWOLFSSL_DEBUG_TRACE_ERROR_CODES (the default) is identical to status quo, because the default definition of WC_NO_ERR_TRACE() is identity. Several frivolous or otherwise unnecessary error code initializations have been removed, but even then most such initializations have been left in place with WC_NO_ERR_TRACE() wrappers.
I used several scripts to autoconvert code for WC_NO_ERR_TRACE() and identify code needing manual tweaks:

autoconvert comparisons to error codes:

for file in $(find src wolfcrypt/src -type f -name \*.c|xargs egrep -l '[!=]=[[:space:]]*([A-Z][A-Z0-9_]*_E|BAD_FUNC_ARG|NOT_COMPILED_IN|NO_PASSWORD|BAD_OCSP_RESPONDER|CRL_CERT_DATE_ERR|ASN_NO_PEM_HEADER|ASN_NO_SKID|ASN_NO_AKID|ASN_NO_KEYUSAGE|BAD_PATH_ERROR|ZLIB_INIT_ERROR|ZLIB_COMPRESS_ERROR|ZLIB_DECOMPRESS_ERROR|CRYPTOCB_UNAVAILABLE|PKCS7_SIGNEEDS_CHECK|CHACHA_POLY_OVERFLOW|MISSING_IV|MISSING_KEY|PROTOCOLCB_UNAVAILABLE|NO_VALID_DEVID|USE_HW_PSK|BUFFER_ERROR|DECRYPT_ERROR|DTLS_CID_ERROR|DTLS_SIZE_ERROR|HRR_COOKIE_ERROR|MATCH_SUITE_ERROR|PSK_KEY_ERROR|SEQUENCE_ERROR|VERIFY_FINISHED_ERROR|VERIFY_MAC_ERROR|VERSION_ERROR|CRL_MISSING|CRL_CERT_REVOKED|OCSP_WANT_READ|APP_DATA_READY|INVALID_PARAMETER|NO_PEER_CERT|OCSP_CERT_REVOKED|OCSP_CERT_UNKNOWN|OCSP_INVALID_STATUS|OCSP_LOOKUP_FAIL|OCSP_WANT_READ|UNSUPPORTED_CERTIFICATE)[ )]'); do sed --in-place=.bak-20240607 -E -e 's/([=!]=[[:space:]]*)([A-Z][A-Z0-9_]*_E|BAD_FUNC_ARG|NOT_COMPILED_IN|NO_PASSWORD|BAD_OCSP_RESPONDER|CRL_CERT_DATE_ERR|ASN_NO_PEM_HEADER|ASN_NO_SKID|ASN_NO_AKID|ASN_NO_KEYUSAGE|BAD_PATH_ERROR|ZLIB_INIT_ERROR|ZLIB_COMPRESS_ERROR|ZLIB_DECOMPRESS_ERROR|CRYPTOCB_UNAVAILABLE|PKCS7_SIGNEEDS_CHECK|CHACHA_POLY_OVERFLOW|MISSING_IV|MISSING_KEY|PROTOCOLCB_UNAVAILABLE|NO_VALID_DEVID|USE_HW_PSK|BUFFER_ERROR|DECRYPT_ERROR|DTLS_CID_ERROR|DTLS_SIZE_ERROR|HRR_COOKIE_ERROR|MATCH_SUITE_ERROR|PSK_KEY_ERROR|SEQUENCE_ERROR|VERIFY_FINISHED_ERROR|VERIFY_MAC_ERROR|VERSION_ERROR|CRL_MISSING|CRL_CERT_REVOKED|OCSP_WANT_READ|APP_DATA_READY|INVALID_PARAMETER|NO_PEER_CERT|OCSP_CERT_REVOKED|OCSP_CERT_UNKNOWN|OCSP_INVALID_STATUS|OCSP_LOOKUP_FAIL|OCSP_WANT_READ|UNSUPPORTED_CERTIFICATE)([ )])/\1WC_NO_ERR_TRACE(\2)\3/g' "$file" || break; done

find initializations to error codes (require manual mitigation):

find src wolfcrypt/src -name \*.c -type f -print | xargs egrep -n 'int +(ret|res|err) *= *[A-Z][A-Z0-9_]* *;' | egrep -v 'WOLFSSL_SUCCESS|WOLFSSL_FAILURE|WOLFSSL_FATAL_ERROR|FP_WOULDBLOCK|MP_|FP_|WOLFSSL_TICKET_RET_FATAL|WOLFSSL_BIO_|WC_READDIR_NOFILE|DRBG_FAILURE|ED448_.*_SIZE|ESP_OK|FALSE|ESP_FAIL|R_PROCESS_COMPLETE|FSP_SUCCESS|TSIP_SUCCESS|EOK|EBADMSG' | less

count and rank occurrences in testwolfcrypt output, to orient+prioritize auditing and manual mitigation of frivolous errcode traces:

wolfcrypt/test/testwolfcrypt 2>&1 | grep -F 'ERR TRACE' | sort | uniq -c | sort -nr | less

…C_ERR_TRACE(), WC_NO_ERR_TRACE(), support/gen-debug-trace-error-codes.sh. also add numerous deployments of WC_NO_ERR_TRACE() to inhibit frivolous/misleading errcode traces when -DWOLFSSL_DEBUG_TRACE_ERROR_CODES.

douzzer · 2024-06-08T23:55:31Z

retest this please

bandi13 · 2024-06-10T18:01:34Z

src/ssl.c

@@ -3532,12 +3532,14 @@ int wolfSSL_ALPN_FreePeerProtocol(WOLFSSL* ssl, char **list)
 /* user is forcing ability to use secure renegotiation, we discourage it */
 int wolfSSL_UseSecureRenegotiation(WOLFSSL* ssl)
 {
-    int ret = BAD_FUNC_ARG;
+    int ret;


You should have an initializer defined anyway.

bandi13 · 2024-06-10T18:02:11Z

src/tls.c

@@ -878,7 +883,7 @@ static int Hmac_HashFinalRaw(Hmac* hmac, unsigned char* hash)
 */
 static int Hmac_OuterHash(Hmac* hmac, unsigned char* mac)
 {
-    int ret = BAD_FUNC_ARG;
+    int ret;


it's fine not to initialize that one but also harmless to initialize it (to = WC_NO_ERR_TRACE(BAD_FUNC_ARG)). going with the latter, at your suggestion. that's already what I went with in nearly every other case like this.

… because needed (in wolfSSL_UseSecureRenegotiation()), the rest in an abundance of caution, and rearrange wolfSSL_CryptHwMutexInit() and wolfSSL_CryptHwMutexUnLock() in a similar abundance of caution.

src/internal.c

src/dtls.c

julek-wolfssl · 2024-06-18T15:41:30Z

wolfssl/wolfcrypt/error-crypt.h

+            ( fprintf(stderr,                                 \
+                      "ERR TRACE: %s L %d " #label " (%d)\n", \
+                      __FILE__, __LINE__, label), label)


@douzzer Should use WOLFSSL_MSG_EX.

douzzer requested review from dgarske, SparkiDev and embhorn June 8, 2024 20:13

douzzer self-assigned this Jun 8, 2024

douzzer force-pushed the 20240608-WOLFSSL_DEBUG_TRACE_ERROR_CODES branch from 323f7fa to b3e8f0a Compare June 8, 2024 21:40

douzzer assigned wolfSSL-Bot and unassigned douzzer Jun 9, 2024

bandi13 reviewed Jun 10, 2024

View reviewed changes

WOLFSSL_DEBUG_TRACE_ERROR_CODES: restore several initializations, one…

1b907d0

… because needed (in wolfSSL_UseSecureRenegotiation()), the rest in an abundance of caution, and rearrange wolfSSL_CryptHwMutexInit() and wolfSSL_CryptHwMutexUnLock() in a similar abundance of caution.

douzzer requested a review from bandi13 June 10, 2024 18:45

SparkiDev requested changes Jun 11, 2024

View reviewed changes

src/internal.c Show resolved Hide resolved

src/dtls.c Show resolved Hide resolved

douzzer requested a review from SparkiDev June 11, 2024 02:00

SparkiDev approved these changes Jun 11, 2024

View reviewed changes

SparkiDev removed request for dgarske, bandi13 and embhorn June 11, 2024 11:24

SparkiDev merged commit d49308e into wolfSSL:master Jun 11, 2024
111 checks passed

SparkiDev self-assigned this Jun 11, 2024

julek-wolfssl reviewed Jun 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

20240608-WOLFSSL_DEBUG_TRACE_ERROR_CODES #7634

20240608-WOLFSSL_DEBUG_TRACE_ERROR_CODES #7634

douzzer commented Jun 8, 2024 •

edited

Loading

douzzer commented Jun 8, 2024

bandi13 Jun 10, 2024

bandi13 Jun 10, 2024

douzzer Jun 10, 2024

julek-wolfssl Jun 18, 2024

20240608-WOLFSSL_DEBUG_TRACE_ERROR_CODES #7634

20240608-WOLFSSL_DEBUG_TRACE_ERROR_CODES #7634

Conversation

douzzer commented Jun 8, 2024 • edited Loading

douzzer commented Jun 8, 2024

bandi13 Jun 10, 2024

Choose a reason for hiding this comment

bandi13 Jun 10, 2024

Choose a reason for hiding this comment

douzzer Jun 10, 2024

Choose a reason for hiding this comment

julek-wolfssl Jun 18, 2024

Choose a reason for hiding this comment

douzzer commented Jun 8, 2024 •

edited

Loading