tls-client: Check certificate verification flags to exclude time failures #109
Conversation
andresag01
force-pushed
the
iotssl-1594-rtc-integration
branch
from
f117d53
to
38a5bc7
Compare
|
@k-stachowiak - Can you please test this PR, review it and approve it if possible? |
|
|
||
| int ret = -1; | ||
| #if HELLO_HTTPS_CLIENT_DEBUG_LEVEL > 0 |
There was a problem hiding this comment.
@mazimkhan / @andresag01 - Will changing what's displayed as debug impact the CI, which uses text output to validate the example?
There was a problem hiding this comment.
This does not change what is displayed on the terminal unless the HELLO_HTTPS_CLIENT_DEBUG_LEVEL macro is > 0 (which I think isnt by default). In fact, the changes in the placing of the macro is to ensure this happens. Note that in the past the sslVerify function cas not configured unless HELLO_HTTPS_CLIENT_DEBUG_LEVEL> 0. However, now it is because we need it to clear the timing flags. Hence, I had to put extra macro checks around the printing code in sslVerify() to prevent it from printing by default.
cc @k-stachowiak: I think this answers your question as well.
There was a problem hiding this comment.
If this change does not modify the prints that appear in mbed-os-example-tls/tests/tls-client.log then this will not impact the CI. Any additional prints are ignored.
simonbutcher
requested review from
AndrzejKurek
and removed request for
yanesca
There was a problem hiding this comment.
I have tested this PR on a K64F device using armcc and the ARM's gcc compilers (can't test against IAR in my setup), ane it works. I am not sure about the purpose of adding the second #if guard, but I can see that @sbutcher-arm has already asked about it so I'll approve the rest of the PR assuming the #if issue will be addressed in Simon's review.
|
retest |
|
CI fails due to a known issue #138. Logs: https://jenkins-internal.mbed.com/view/mbed-tls/job/mbedtls-mbed-os-examples-pr/138/execution/node/632/log/ |
|
@RonEld @sbutcher-arm: I think we have the necessary approvals and the relevant CI tests seem to pass. Can we add the 'ready for merge' tag? |
This patch is necessary when the
MBEDTLS_HAVE_TIME_DATE functionality is enabled because the Real-Time
Clock needs to be manually set to the correct time, which is not a
trivial task in this example code. The idea here is to configure the certificate verification function to filter out date/time failures during the TLS handshake.
NOTES: