Codecov Report

Merging #12370 (0789e15) into master (a18d62e) will decrease coverage by 0.01%.
Report is 70 commits behind head on master.
The diff coverage is 100.00%.

❗ Current head 0789e15 differs from pull request most recent head 3637252. Consider uploading reports for the commit 3637252 to get more accurate results

@@            Coverage Diff             @@
##           master   #12370      +/-   ##
==========================================
- Coverage   98.38%   98.38%   -0.01%     
==========================================
  Files         158      158              
  Lines       20900    20933      +33     
==========================================
+ Hits        20563    20595      +32     
- Misses        337      338       +1     

... and 6 files with indirect coverage changes

@iabdalkader

Also ssl_cert_rsa.py fails with MBEDTLS_ERR_X509_CERT_VERIFY_FAILED

It may be a certificate time validation issue? Does any test sync/check device local time?

To figure out verification failure reason could you test #11897? with #define MICROPY_SSL_MBEDTLS_EXTRAS (1) enabled in mpconfigport.h it will print more info about that error 👍🏼

It may be a certificate time validation issue? Does any test sync/check device local time?

Yes I set the time from ntp on both nodes, still no luck.

Weird, well here are the current x509 verify codes:

in lib/mbedtls/include/mbedtls/x509.h 101-127

/**
 * \name X509 Verify codes
 * \{
 */
/* Reminder: update x509_crt_verify_strings[] in library/x509_crt.c */
#define MBEDTLS_X509_BADCERT_EXPIRED             0x01  /**< The certificate validity has expired. */
#define MBEDTLS_X509_BADCERT_REVOKED             0x02  /**< The certificate has been revoked (is on a CRL). */
#define MBEDTLS_X509_BADCERT_CN_MISMATCH         0x04  /**< The certificate Common Name (CN) does not match with the expected CN. */
#define MBEDTLS_X509_BADCERT_NOT_TRUSTED         0x08  /**< The certificate is not correctly signed by the trusted CA. */
#define MBEDTLS_X509_BADCRL_NOT_TRUSTED          0x10  /**< The CRL is not correctly signed by the trusted CA. */
#define MBEDTLS_X509_BADCRL_EXPIRED              0x20  /**< The CRL is expired. */
#define MBEDTLS_X509_BADCERT_MISSING             0x40  /**< Certificate was missing. */
#define MBEDTLS_X509_BADCERT_SKIP_VERIFY         0x80  /**< Certificate verification was skipped. */
#define MBEDTLS_X509_BADCERT_OTHER             0x0100  /**< Other reason (can be used by verify callback) */
#define MBEDTLS_X509_BADCERT_FUTURE            0x0200  /**< The certificate validity starts in the future. */
#define MBEDTLS_X509_BADCRL_FUTURE             0x0400  /**< The CRL is from the future */
#define MBEDTLS_X509_BADCERT_KEY_USAGE         0x0800  /**< Usage does not match the keyUsage extension. */
#define MBEDTLS_X509_BADCERT_EXT_KEY_USAGE     0x1000  /**< Usage does not match the extendedKeyUsage extension. */
#define MBEDTLS_X509_BADCERT_NS_CERT_TYPE      0x2000  /**< Usage does not match the nsCertType extension. */
#define MBEDTLS_X509_BADCERT_BAD_MD            0x4000  /**< The certificate is signed with an unacceptable hash. */
#define MBEDTLS_X509_BADCERT_BAD_PK            0x8000  /**< The certificate is signed with an unacceptable PK alg (eg RSA vs ECDSA). */
#define MBEDTLS_X509_BADCERT_BAD_KEY         0x010000  /**< The certificate is signed with an unacceptable key (eg bad curve, RSA too short). */
#define MBEDTLS_X509_BADCRL_BAD_MD           0x020000  /**< The CRL is signed with an unacceptable hash. */
#define MBEDTLS_X509_BADCRL_BAD_PK           0x040000  /**< The CRL is signed with an unacceptable PK alg (eg RSA vs ECDSA). */
#define MBEDTLS_X509_BADCRL_BAD_KEY          0x080000  /**< The CRL is signed with an unacceptable key (eg bad curve, RSA too short). */

/** \} name X509 Verify codes */

It will be interesting to see which ones of these are

I can enable the verbose output and double check, will get around to it later.

I tested this on PYBD's for 30 minutes and all there were no failures (disregarding SSL and UDP tests).

with #define MICROPY_SSL_MBEDTLS_EXTRAS (1) enabled in mpconfigport.h it will print more info about that error 👍🏼

@Carglglz the mimxrt port was missing time validation, I added it in 0bc2968 and I also tried the above, couldn't see any additional error info, and it's still failing. If you know which function gets called in mbedtls and returns that error I can just break there and see the specific error.

@iabdalkader did you try #11897 ? The macro is only useful there. It should enable additional error info when the error is MBEDTLS_ERR_X509_CERT_VERIFY_FAILED in modssl_mbedtls.c at @@ STATIC mp_obj_t ssl_socket_make_new

@@ -394,8 +533,29 @@ STATIC mp_obj_t ssl_socket_make_new(mp_obj_ssl_context_t *ssl_context, mp_obj_t
     return MP_OBJ_FROM_PTR(o);

 cleanup:
+    #ifdef MICROPY_SSL_MBEDTLS_EXTRAS
+    if (ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED) {
+        flags = mbedtls_ssl_get_verify_result(&o->ssl);
+    }
+    #endif
+
     o->sock = MP_OBJ_NULL;
     mbedtls_ssl_free(&o->ssl);
+
+    #ifdef MICROPY_SSL_MBEDTLS_EXTRAS
+    if (ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED) {
+        char xcbuf[512];
+        ret = mbedtls_x509_crt_verify_info(xcbuf, sizeof(xcbuf), "\n", flags);
+        // The length of the string written (not including the terminated nul byte),
+        // or a negative err code.
+        if (ret > 0) {
+            mp_raise_ValueError(MP_ERROR_TEXT(xcbuf));
+        } else {
+            mbedtls_raise_error(ret);
+        }
+    }
+    #endif
+
     mbedtls_raise_error(ret);
 }

If you know which function gets called in mbedtls and returns that error I can just break there and see the specific error.

The functions are mbedtls_ssl_get_verify_result which returns bit flags and mbedtls_x509_crt_verify_info which maps these bit flags to the mentioned X509 Verify codes above.
Here is the snippet if you want to try:

cleanup:
     
     if (ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED) {
         uint32_t flags = mbedtls_ssl_get_verify_result(&o->ssl);
     }

     o->sock = MP_OBJ_NULL;
     mbedtls_ssl_free(&o->ssl);

  
     if (ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED) {
         char xcbuf[512];
         ret = mbedtls_x509_crt_verify_info(xcbuf, sizeof(xcbuf), "\n", flags);
         // The length of the string written (not including the terminated nul byte),
         // or a negative err code.
         if (ret > 0) {
             mp_raise_ValueError(MP_ERROR_TEXT(xcbuf));
         }
     }

     mbedtls_raise_error(ret);

@Carglglz the mimxrt port was missing time validation, I added it in 0bc2968 and I also tried the above, couldn't see any additional error info, and it's still failing

This reminds me of this at #11896

In esp32 port MBEDTLS_PLATFORM_TIME_ALT macro is needed due to esp32 using EPOCH 1/1/2000 to get current time in
seconds which is not what mbedtls expects. MBEDTLS_PLATFORM_TIME_ALT gives the option to define
an alternative function to get current time.

mbedtls expects UNIX epoch time, but I guess that's already fixed in 0bc2968

Also the error only happens with multi_net tests? or e.g. net_inet/ssl_cert.py fails too?

Also the error only happens with multi_net tests? or e.g. net_inet/ssl_cert.py fails too?

No only ssl_cert_rsa.py fails.

With your patches I see that it's a certificate time validation issue:

multi_net/ssl_cert_rsa.py on ttyACM0|ttyACM1: FAIL
### TEST ###
--- instance0 ---

--- instance1 ---

Traceback (most recent call last):
  File "<stdin>", line 213, in <module>
  File "<stdin>", line 150, in instance1
ValueError: 
The certificate validity has expired

Could it simply be that the certificate in the test did actually expire ? It's supposed to be valid for 1 year.

Yep, if I set the time to ntp (year -1) it passes the test 🤦🏼

Could it simply be that the certificate in the test did actually expire ? It's supposed to be valid for 1 year.
Yep, if I set the time to ntp (year -1) it passes the test 🤦🏼

Ah that would be it, actually @dpgeorge bumped into the same issue , I did update the certs in #11897 to be at least 5 years I think 😓 here is the new version 👍🏼 https://github.com/micropython/micropython/blob/f1bee31dc8d2ecdbf125af5905c7f1c676c0f162/tests/multi_net/ssl_cert_rsa.py

@Carglglz Thank you! Can you please also fix those typos:

diff --git a/tests/multi_net/ssl_cert_rsa.py b/tests/multi_net/ssl_cert_rsa.py
index 872855edb..7e4b2ec05 100644
--- a/tests/multi_net/ssl_cert_rsa.py
+++ b/tests/multi_net/ssl_cert_rsa.py
@@ -14,11 +14,11 @@ PORT = 8000
 # testing/demonstration only.  You should always generate your own key/cert.
 
 # To generate a new self-signed key/cert pair with openssl do:
-# $ openssl req -x509 -newkey rsa:4096 -keyout rsa_key.pem -out rsa_cert.pem -days 365 -node
+# $ openssl req -x509 -newkey rsa:4096 -keyout rsa_key.pem -out rsa_cert.pem -days 365 -nodes
 #
 # Convert them to DER format:
 # $ openssl rsa -in rsa_key.pem -out rsa_key.der -outform DER
-# $ openssl x509 -in rsa_cert.pem -out rsa_key.der -outform DER
+# $ openssl x509 -in rsa_cert.pem -out rsa_cert.der -outform DER
 #
 # Then convert to hex format, eg using binascii.hexlify(data).```

@iabdalkader Done in #12387 , Thanks!