extmod/modtls_mbedtls: Fix DER parsing and calculation of key/cert len.

`mbedtls_pk_parse_key()` expects `key_len` to include the NULL terminator
for PEM data but not for DER encoded data.  This also applies to
`mbedtls_x509_crt_parse()` and `cert_len`.

Since all PEM data contains "-----BEGIN" this is used to check if the data
is PEM (as per mbedtls code).

This can be done for both v2 and v3 of mbedtls since the fundamental
behaviour/expectation did not change.  What changed is that in v3 the
PKCS#8 DER parser now checks that the passed key buffer is fully utilized
and no bytes are remaining (all other DER formats still do not check this).

Signed-off-by: Peter Züger <zueger.peter@icloud.com>

1 files changed, 34 insertions, 7 deletions

diff --git a/extmod/modtls_mbedtls.c b/extmod/modtls_mbedtls.c
index 5a9487ab89..d3fc26fad2 100644
--- a/extmod/modtls_mbedtls.c
+++ b/extmod/modtls_mbedtls.c
@@ -100,6 +100,12 @@ static void mbedtls_debug(void *ctx, int level, const char *file, int line, cons
 }
 #endif
 
+#if defined(MBEDTLS_PEM_PARSE_C)
+static int mbedtls_is_pem(const byte *data, size_t len) {
+    return (len >= 10) && (strstr((const char *)data, "-----BEGIN") != NULL);
+}
+#endif
+
 static NORETURN void mbedtls_raise_error(int err) {
     // Handle special cases.
     if (err == MBEDTLS_ERR_SSL_ALLOC_FAILED) {
@@ -347,12 +353,19 @@ static MP_DEFINE_CONST_FUN_OBJ_2(ssl_context_set_ciphers_obj, ssl_context_set_ci
 static void ssl_context_load_key(mp_obj_ssl_context_t *self, mp_obj_t key_obj, mp_obj_t cert_obj) {
     size_t key_len;
     const byte *key = (const byte *)mp_obj_str_get_data(key_obj, &key_len);
-    // len should include terminating null
+
+    #if defined(MBEDTLS_PEM_PARSE_C)
+    // len should include terminating null if the data is PEM encoded
+    if (mbedtls_is_pem(key, key_len)) {
+        key_len += 1;
+    }
+    #endif
+
     int ret;
     #if MBEDTLS_VERSION_NUMBER >= 0x03000000
-    ret = mbedtls_pk_parse_key(&self->pkey, key, key_len + 1, NULL, 0, mbedtls_ctr_drbg_random, &self->ctr_drbg);
+    ret = mbedtls_pk_parse_key(&self->pkey, key, key_len, NULL, 0, mbedtls_ctr_drbg_random, &self->ctr_drbg);
     #else
-    ret = mbedtls_pk_parse_key(&self->pkey, key, key_len + 1, NULL, 0);
+    ret = mbedtls_pk_parse_key(&self->pkey, key, key_len, NULL, 0);
     #endif
     if (ret != 0) {
         mbedtls_raise_error(MBEDTLS_ERR_PK_BAD_INPUT_DATA); // use general error for all key errors
@@ -360,8 +373,15 @@ static void ssl_context_load_key(mp_obj_ssl_context_t *self, mp_obj_t key_obj, m
 
     size_t cert_len;
     const byte *cert = (const byte *)mp_obj_str_get_data(cert_obj, &cert_len);
-    // len should include terminating null
-    ret = mbedtls_x509_crt_parse(&self->cert, cert, cert_len + 1);
+
+    #if defined(MBEDTLS_PEM_PARSE_C)
+    // len should include terminating null if the data is PEM encoded
+    if (mbedtls_is_pem(cert, cert_len)) {
+        cert_len += 1;
+    }
+    #endif
+
+    ret = mbedtls_x509_crt_parse(&self->cert, cert, cert_len);
     if (ret != 0) {
         mbedtls_raise_error(MBEDTLS_ERR_X509_BAD_INPUT_DATA); // use general error for all cert errors
     }
@@ -383,8 +403,15 @@ static MP_DEFINE_CONST_FUN_OBJ_3(ssl_context_load_cert_chain_obj, ssl_context_lo
 static void ssl_context_load_cadata(mp_obj_ssl_context_t *self, mp_obj_t cadata_obj) {
     size_t cacert_len;
     const byte *cacert = (const byte *)mp_obj_str_get_data(cadata_obj, &cacert_len);
-    // len should include terminating null
-    int ret = mbedtls_x509_crt_parse(&self->cacert, cacert, cacert_len + 1);
+
+    #if defined(MBEDTLS_PEM_PARSE_C)
+    // len should include terminating null if the data is PEM encoded
+    if (mbedtls_is_pem(cacert, cacert_len)) {
+        cacert_len += 1;
+    }
+    #endif
+
+    int ret = mbedtls_x509_crt_parse(&self->cacert, cacert, cacert_len);
     if (ret != 0) {
         mbedtls_raise_error(MBEDTLS_ERR_X509_BAD_INPUT_DATA); // use general error for all cert errors
     }