20240608-WOLFSSL_DEBUG_TRACE_ERROR_CODES#7634
Merged
SparkiDev merged 2 commits intowolfSSL:masterfrom Jun 11, 2024
Merged
Conversation
…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.
323f7fa to
b3e8f0a
Compare
Contributor
Author
|
retest this please |
bandi13
reviewed
Jun 10, 2024
| int wolfSSL_UseSecureRenegotiation(WOLFSSL* ssl) | ||
| { | ||
| int ret = BAD_FUNC_ARG; | ||
| int ret; |
Contributor
There was a problem hiding this comment.
You should have an initializer defined anyway.
bandi13
reviewed
Jun 10, 2024
| static int Hmac_OuterHash(Hmac* hmac, unsigned char* mac) | ||
| { | ||
| int ret = BAD_FUNC_ARG; | ||
| int ret; |
Contributor
Author
There was a problem hiding this comment.
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.
SparkiDev
requested changes
Jun 11, 2024
SparkiDev
approved these changes
Jun 11, 2024
Comment on lines
+304
to
+306
| ( fprintf(stderr, \ | ||
| "ERR TRACE: %s L %d " #label " (%d)\n", \ | ||
| __FILE__, __LINE__, label), label) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
New global debugging aid --
--enable-debug-trace-errcodesaka-DWOLFSSL_DEBUG_TRACE_ERROR_CODEScauses the library to render tostderra 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-debugalso enabled (they are independent of each other):and another example, from
testwolfcryptoutput showing results from the SRTP-KDF expected-failure tests:WC_ERR_TRACE(label)can be overridden (e.g. fromuser_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 theWC_NO_ERR_TRACE()macro (which is always a constant numeric value) is for internal use only. Everything outside the library -- applications, of course, but alsotestwolfcrypt,benchmark.c, etc. -- always see the same numeric constantenumerror codes as ever.On non-autotools builds, manually running
support/gen-debug-trace-error-codes.shwill 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-sourcewithall-gcc-debug-c99tweaked to have--enable-debug-trace-errcodes.additional notes:
missing
WC_NO_ERR_TRACE()annotations are benign even in-DWOLFSSL_DEBUG_TRACE_ERROR_CODESbuilds -- 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 ofWC_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 withWC_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:
find initializations to error codes (require manual mitigation):
count and rank occurrences in
testwolfcryptoutput, to orient+prioritize auditing and manual mitigation of frivolous errcode traces: