Commit 4d6d8498 authored by Peter Züger's avatar Peter Züger Committed by Damien George

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: default avatarPeter Züger <zueger.peter@icloud.com>
parent 288a0362
...@@ -100,6 +100,12 @@ static void mbedtls_debug(void *ctx, int level, const char *file, int line, cons ...@@ -100,6 +100,12 @@ static void mbedtls_debug(void *ctx, int level, const char *file, int line, cons
} }
#endif #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) { static NORETURN void mbedtls_raise_error(int err) {
// Handle special cases. // Handle special cases.
if (err == MBEDTLS_ERR_SSL_ALLOC_FAILED) { 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 ...@@ -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) { 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; size_t key_len;
const byte *key = (const byte *)mp_obj_str_get_data(key_obj, &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; int ret;
#if MBEDTLS_VERSION_NUMBER >= 0x03000000 #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 #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 #endif
if (ret != 0) { if (ret != 0) {
mbedtls_raise_error(MBEDTLS_ERR_PK_BAD_INPUT_DATA); // use general error for all key errors 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 ...@@ -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; size_t cert_len;
const byte *cert = (const byte *)mp_obj_str_get_data(cert_obj, &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) { if (ret != 0) {
mbedtls_raise_error(MBEDTLS_ERR_X509_BAD_INPUT_DATA); // use general error for all cert errors 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 ...@@ -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) { static void ssl_context_load_cadata(mp_obj_ssl_context_t *self, mp_obj_t cadata_obj) {
size_t cacert_len; size_t cacert_len;
const byte *cacert = (const byte *)mp_obj_str_get_data(cadata_obj, &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) { if (ret != 0) {
mbedtls_raise_error(MBEDTLS_ERR_X509_BAD_INPUT_DATA); // use general error for all cert errors mbedtls_raise_error(MBEDTLS_ERR_X509_BAD_INPUT_DATA); // use general error for all cert errors
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment