lws_tls_client_create_vhost_context: (mbedtls) problem interpreting private key

[Davidovory03]

I tried connecting with a client certificate, but upon calling lws_create_context, I got the error lws_tls_client_create_vhost_context: (mbedtls) problem interpreting private key. I've narrowed it down to the following example:

std::string client_ca = "-----BEGIN CERTIFICATE-----\n...." // some PEM certificate
std::string client_pk = "-----BEGIN RSA PRIVATE KEY-----\n" // some PEM RSA key
struct lws_context_creation_info info = {};
info.port = CONTEXT_PORT_NO_LISTEN;
info.options = LWS_SERVER_OPTION_DO_SSL_GLOBAL_INIT;

info.client_ssl_ca_mem = client_ca.c_str();
info.client_ssl_ca_mem_len = client_ca.size();

info.client_ssl_key_mem = client_pk.c_str();
info.client_ssl_key_mem_len = client_pk.size();

I went through the code and tried to understand why that happens. It seems that error originates from the function lws_tls_client_create_vhost_context, upon calling to SSL_CTX_use_PrivateKey_ASN1. This were I stumbled upon first interesting behavior:

		n = SSL_CTX_use_PrivateKey_ASN1(0, vh->tls.ssl_client_ctx,
				key_mem, (long)key_mem_len - 1);

where key_mem=info.client_ssl_key_mem and key_mem_len=info.client_ssl_key_mem_len. So it seems that it's expected that client_ssl_key_mem_len will be including the '\0', meaning I should be passing client_pk.size() + 1. However, it still doesn't work. Upon looking into the deeper function calls, I got to the function pkey_pm_load, where I found this piece of code:

load_buf = ssl_mem_malloc((unsigned int)len + 1);
    if (!load_buf) {
        SSL_DEBUG(SSL_PLATFORM_ERROR_LEVEL, "no enough memory > (load_buf)");
        goto failed;
    }

    ssl_memcpy(load_buf, buffer, (unsigned int)len);
    load_buf[len] = '\0';

    mbedtls_pk_init(pkey_pm->pkey);

#if defined(MBEDTLS_VERSION_NUMBER) && MBEDTLS_VERSION_NUMBER >= 0x03000000
#if defined(MBEDTLS_VERSION_NUMBER) && MBEDTLS_VERSION_NUMBER >= 0x03050000
    ret = mbedtls_pk_parse_key(pkey_pm->pkey, load_buf, (unsigned int)len, NULL, 0,
		    mbedtls_ctr_drbg_random, pkey_pm->rngctx);
#else
    ret = mbedtls_pk_parse_key(pkey_pm->pkey, load_buf, (unsigned int)len + 1, NULL, 0,
		    mbedtls_ctr_drbg_random, pkey_pm->rngctx);
#endif
#else
    ret = mbedtls_pk_parse_key(pkey_pm->pkey, load_buf, (unsigned int)len + 1, NULL, 0);
#endif
    ssl_mem_free(load_buf);

It seems that client_ssl_key_mem is being copied to load_buf, which is then null terminated. However, in mbedtls >= 3.5, the keylen passed to mbedtls_pk_parse_key is len, and not len+1. mbedtls_pk_parse_key then checks if key[keylen-1] != '\0', and if the statement is true, the function fails with the error MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT.

Conclusion: It's a bit unclear if to the client_ssl_key_mem and client_ssl_key_mem_len argument need to match a null terminated string, or not. Even when they match a null-terminated string, when passed from lws_tls_client_create_vhost_context to SSL_CTX_use_PrivateKey_ASN1, the key_mem_len argument gets decreased, therefore later meaning that the key passed to mbedtls_pk_parse_key is not null terminated. I think the way to fix this is either:

• Sending len+1 in pkey_pm_load, thus allowing the both null-terminated strings and non-null-terminated string to be passed to client_ssl_key_mem (this can be easily done by reverting this commit).
• Changing the call to SSL_CTX_use_PrivateKey_ASN1 to pass key_mem_len instead of key_mem_len-1, therefore forcing users to pass a null-terminated string.

[lws-team]

It's less than ideal because the lws api needs to work with OpenSSL if that's what you're using, as well as mbedtls. The two tls libs seem to have fundamentally different behaviours for this natively (which probably differs by tls lib version as well).

You don't mention the lws version, you should try main branch if it isn't already.

[Davidovory03]

I've tried main branch. I'm working with mbedtls version 3.6. If that is the case, why not revert this commit I linked to? It would allow users of the lws library to not care about null byte at all, and eventually the null byte will be added in pkey_pm_load, in lib/tls/mbedtls/wrapper/platform/ssl_pm.c, which I assume is specific for mbedtls, therefore the api should work for both OpenSSL and mbedtls