Introduce WOLFSSL_DEBUG_CERTS Certificate Debug Messages #8902
Conversation
gojimmypi
force-pushed
the
pr-cert-logging
branch
from
381cb1e to
3bdf4c4
Compare
gojimmypi
force-pushed
the
pr-cert-logging
branch
3 times, most recently
from
fdfcd2b to
8eec175
Compare
|
Jenkins retest this please: |
There was a problem hiding this comment.
Please consider add a test to os-check.yml and also adding a small comment about the build option at top of logging.c.
Failure: ./configure CFLAGS="-DWOLFSSL_DEBUG_CERTS" && make
wolfcrypt/src/logging.c:307:6: error: no previous prototype for 'WOLFSSL_MSG_EX' [-Werror=missing-prototypes]
307 | void WOLFSSL_MSG_EX(const char* fmt, ...)
| ^~~~~~~~~~~~~~
wolfcrypt/src/logging.c:356:6: error: no previous prototype for 'WOLFSSL_MSG' [-Werror=missing-prototypes]
356 | void WOLFSSL_MSG(const char* msg)
| ^~~~~~~~~~~
wolfcrypt/src/logging.c:448:6: error: no previous prototype for 'WOLFSSL_ENTER' [-Werror=missing-prototypes]
448 | void WOLFSSL_ENTER(const char* msg)
| ^~~~~~~~~~~~~
wolfcrypt/src/logging.c:477:6: error: no previous prototype for 'WOLFSSL_LEAVE' [-Werror=missing-prototypes]
477 | void WOLFSSL_LEAVE(const char* msg, int ret)
| ^~~~~~~~~~~~~
wolfcrypt/src/logging.c:518:17: error: no previous prototype for 'WOLFSSL_IS_DEBUG_ON' [-Werror=missing-prototypes]
518 | WOLFSSL_API int WOLFSSL_IS_DEBUG_ON(void)
| ^~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[2]: *** [Makefile:8233: wolfcrypt/src/src_libwolfssl_la-logging.lo] Error 1
make[2]: *** Waiting for unfinished jobs....
make[2]: Leaving directory '/home/davidgarske/GitHub/wolfssl'
make[1]: *** [Makefile:10156: check-recursive] Error 1
make[1]: Leaving directory '/home/davidgarske/GitHub/wolfssl'
make: *** [Makefile:10650: check] Error 2```
gojimmypi
force-pushed
the
pr-cert-logging
branch
from
8eec175 to
65c681b
Compare
|
After addressing items in the most recent code review from @dgarske, I added additional |
gojimmypi
force-pushed
the
pr-cert-logging
branch
from
65c681b to
f22b741
Compare
Added
done
|
gojimmypi
force-pushed
the
pr-cert-logging
branch
from
f22b741 to
f6aca39
Compare
gojimmypi
force-pushed
the
pr-cert-logging
branch
from
f6aca39 to
55801ba
Compare
gojimmypi
force-pushed
the
pr-cert-logging
branch
from
55801ba to
4aeadb8
Compare
|
Thank you @dgarske ! Nice improvement in your suggested patch. Applied in 4aeadb8 Confirmed working on my ESP32 wolfssl_client WIP as well as both of these: |
gojimmypi
force-pushed
the
pr-cert-logging
branch
from
4b4b228 to
ade5618
Compare
|
Jenkins retest this please for wolf-linux-cloud-node-[n] is offline |
There was a problem hiding this comment.
have you looked at what WOLFSSL_DEBUG_CERTIFICATE_LOADS does?
$ git grep WOLFSSL_DEBUG_CERTIFICATE_LOADS
README.md:* Added WOLFSSL_DEBUG_PRINTF and WOLFSSL_DEBUG_CERTIFICATE_LOADS for improved debugging output. (PR #8769, PR #8770)
src/ssl_load.c:#ifdef WOLFSSL_DEBUG_CERTIFICATE_LOADS
src/ssl_load.c:#ifdef WOLFSSL_DEBUG_CERTIFICATE_LOADS
src/ssl_load.c:#endif /* WOLFSSL_DEBUG_CERTIFICATE_LOADS */
src/ssl_load.c:#ifdef WOLFSSL_DEBUG_CERTIFICATE_LOADS
src/ssl_load.c:#endif /* WOLFSSL_DEBUG_CERTIFICATE_LOADS */
src/ssl_load.c:#ifdef WOLFSSL_DEBUG_CERTIFICATE_LOADS
it looks like you're getting us a lot more coverage with this PR, but I don't think it makes sense to have both WOLFSSL_DEBUG_CERTIFICATE_LOADS and WOLFSSL_DEBUG_CERTS. at a minimum we should settle on a consistent naming scheme for the feature macros that either merges them into one, or gives them names that make clear what's different between them.
|
Hi @douzzer - yes, I recently noticed the (relatively new?) I've modelled the I'm curious why There are relatively few instances of Is there any pressing reason to have Any objection to have the I already have more certificate debugging in my dev branch, this PR was mainly to introduce the concept. In any case, I'm glad you also had the good idea to have some addition, certificate-specific debugging. Please let me know you preference on how to proceed. |
|
Please resolve merge conflicts. Please see if changing to a simplier WOLFSSL_DEBUG_CERTIFICATE_LOADS makes more sense. If not then please cleanup WOLFSSL_DEBUG_CERTIFICATE_LOADS. |
|
There were a small number of I applied this change in 7020b3a, and left a note in |
|
Jenkins retest this please: "AgentOfflineException" |
There was a problem hiding this comment.
Please do not substitute WOLFSSL_MSG_CERT_EX() for WOLFSSL_DEBUG_PRINTF().
WOLFSSL_DEBUG_PRINTF() is a bufferless, non-truncating debug message renderer.
The refactor in ssl_load.c to use WOLFSSL_MSG_CERT_EX() causes destructive message truncation.
Also, there are merge conflicts on logging.c.
Description
Adds a new troubleshooting capability to view only interesting certificate-related messages:
Improves debugging messages: modifies
WOLFSSL_MSG_EXandWOLFSSL_MSG_CERT_EXon no-variadic macro compiler such as Watcom.Also adds
WOLFSSL_DEBUG_LINE_ENDINGto suppress LF characters on message printed for systems that supply their own line feeds during messaging, such as the EspressifESP_LOG.Turning on
WOLFSSL_DEBUGalways enablesWOLFSSL_DEBUG_CERTS.However
WOLFSSL_DEBUG_CERTScan be used withoutWOLFSSL_DEBUG.Macros of interest related to this PR:
Why?
Turning on full debugging is often overly verbose. On embedded devices the delay in printing debug messages can have an adverse effect on timing-critical code, such as certificate validation during TLS connections.
Inspiration
See wolfSSL forum questions related to certificates. For me, recently:
Usage
To use, add to
user_settings.h:or from command-line:
Launch a server:
Launch a client:
Sample Linux Output:
Server:
Client:
Sample Espressif output:
FP_MAX_BITSinsight.and this suggestion to turn on
WOLFSSL_ALT_CERT_CHAINS, in addition to our old frienderror: -188:Fixes zd# n/a
Testing
How did you test?
Tested manually on embedded ESP32 / ESP-IDF.
Also tested with:
Checklist