New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mbedtls: load_cert_chain NULL terminator uncertainty #14371
Comments
Which version of check https://www.openssl.org/docs/man1.1.1/man1/openssl-rsa.html If it is openssl 1.x then
see also https://stackoverflow.com/questions/40822328/openssl-rsa-key-pem-and-der-conversion-does-not-match, so I think is the other way around, I remember bumping into this while updating Also |
I am using OpenSSL 3.2.1, but I did not check the OpenSSL documentation to check wich key Format is generated by wich command, I only went by which function in When converting the Key with When converting the Key using Yes i forgot to update my tags: |
@peterzuger I'm afraid you're right which also means the https://www.openssl.org/docs/man3.3/man1/openssl-pkey.html
From PKCS#1
PKCS#8
For some reason
however
Also ec keys are not affected because openssl uses
Re: on how to fix this
I think this could be the fix then (already tested and it seems to work) git df
diff --git a/extmod/modtls_mbedtls.c b/extmod/modtls_mbedtls.c
index 6db6ac195..7c6aa8ffe 100644
--- a/extmod/modtls_mbedtls.c
+++ b/extmod/modtls_mbedtls.c
@@ -355,8 +355,11 @@ static void ssl_context_load_key(mp_obj_ssl_context_t *self, mp_obj_t key_obj, m
ret = mbedtls_pk_parse_key(&self->pkey, key, key_len + 1, NULL, 0);
#endif
if (ret != 0) {
+
+ ret = mbedtls_pk_parse_key(&self->pkey, key, key_len, NULL, 0, mbedtls_ctr_drbg_random, &self->ctr_drbg);
+ if (ret != 0){
mbedtls_raise_error(MBEDTLS_ERR_PK_BAD_INPUT_DATA); // use general error for all key errors
- }
+ }}
size_t cert_len;
const byte *cert = (const byte *)mp_obj_str_get_data(cert_obj, &cert_len); Would you like to open a PR then 👍🏼 ? |
Well this would also need to be wrapped in the: Another idea I had was to differentiate passing in But both solutions seem very hacky to me, which is why I opened an issue instead of a PR. |
Yes, although I took the idea from
And looking again this seems another possible fix ? /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */
if (key[keylen - 1] != '\0') |
Hmm not quite sure how the second thing could be used to fix the problem, but I just realized another maybe less hacky solution: Since all PEM encoded data starts with I'm open for other ideas but so far this seems like the least hacky fix, should I open a PR or is your idea maybe better ? |
I think they are the same? if (key[keylen - 1] != '\0') {
ret = MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT; Although your idea may be better because I'm not sure that DER keys cannot end with a so feel free to open a PR 👍🏼 |
Ah I see what you meant now, but that would not work since the PEM data as passed from Python does not include the NULL terminator unless you manually add it. The NULL terminator is appended by DER can certainly contain |
Checks
I agree to follow the MicroPython Code of Conduct to ensure a safe and respectful space for everyone.
I've searched for existing issues matching this bug, and didn't find any.
Port, board and/or hardware
unix, stm32, pybd
MicroPython version
MicroPython v1.20.0-1189.g49ce7a607 on 2024-04-24; linux [GCC 13.2.1] version
Reproduction
Generate the specific type of DER encoded KEY/CERT pair.
Run interactivley or via script on unix or any STM32 port (all mbedtls ports will be affected)
Expected behaviour
This script is expected to run without issue.
Observed behaviour
Additional Information
As described here (mbedtls_pk_parse_key):
But for DER data it must:
The behaviour of mbedtls has changed since the switch to v3.5, it now checks that the provided key buffer does not contain any additional data but only of unencrypted PKCS8 data.
With the change added here, mbedtls_pk_parse_key now fails because the null terminator is included for the "DER" data.
If the simultaneous support for DER and PEM data is desired, there must be a distinction before calling
mbedtls_pk_parse_key
which must be called withkey_len + 1
for PEM data and withkey_len
for DER data.When converting the key with the following which results in a PKCS1 DER, the script runs without issue (with both
key_len + 1
andkey_len
) because the PKCS1 parser does not check that the key buffer was fully utilized:openssl pkey -in key.pem -outform der -out key.der
The text was updated successfully, but these errors were encountered: