Skip to content
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

Open
2 tasks done
peterzuger opened this issue Apr 24, 2024 · 8 comments
Open
2 tasks done

Mbedtls: load_cert_chain NULL terminator uncertainty #14371

peterzuger opened this issue Apr 24, 2024 · 8 comments
Labels

Comments

@peterzuger
Copy link
Contributor

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.

openssl genrsa -out key.pem 1024

openssl req -x509 -newkey rsa:1024 -keyout key.pem -out cert.pem -sha256 -days 3650 -nodes -subj "/C=XX/ST=StateName/L=CityName/O=CompanyName/OU=CompanySectionName/CN=CommonNameOrHostname"

openssl rsa -in key.pem -outform DER -out key.der
openssl x509 -in cert.pem -out cert.der -outform DER

Run interactivley or via script on unix or any STM32 port (all mbedtls ports will be affected)

import tls
ctx = tls.SSLContext(tls.PROTOCOL_TLS_CLIENT)

key = open("key.der", "rb").read()
cert = open("cert.der", "rb").read()

ctx.load_cert_chain(cert, key)

Expected behaviour

This script is expected to run without issue.

Observed behaviour

Traceback (most recent call last):
  File "test.py", line 8, in <module>
ValueError: invalid key

Additional Information

As described here (mbedtls_pk_parse_key):

Size of key in bytes. For PEM data, this includes the terminating null byte, so keylen must be equal to strlen(key) + 1

But for DER data it must:

The buffer must contain the input exactly, with no extra trailing material.

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 with key_len + 1 for PEM data and with key_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 and key_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

@peterzuger peterzuger added the bug label Apr 24, 2024
@Carglglz
Copy link
Contributor

openssl pkey -in key.pem -outform der -out key.der

Which version of openssl are you using?

check https://www.openssl.org/docs/man1.1.1/man1/openssl-rsa.html
and https://www.openssl.org/docs/man1.1.1/man1/openssl-pkey.html

If it is openssl 1.x then openssl rsa :

Note this command uses the traditional SSLeay compatible format for private key encryption: newer applications should use the more secure PKCS#8 format using the pkcs8 utility.

openssl pkey --> PKCS#8 (this is true with any openssl version 1.x and 3.x)

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 multi_net tests (here)

Also MicroPython v1.20.0-1189.g49ce7a607 on 2024-04-24; linux [GCC 13.2.1] version I think you need to update git tags?

@peterzuger
Copy link
Contributor Author

peterzuger commented Apr 25, 2024

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 mbedtls is successfull in parsing the key.

When converting the Key with openssl rsa it is parsed by pk_parse_key_pkcs8_unencrypted_der in mbedtls (unsuccesfully if the function is called with keylen + 1 because of the check at the end of the function).

When converting the Key using openssl pkey it is parsed successfully by pk_parse_key_pkcs1_der (with both keylen + 1 and keylen because the function does not check that the buffer was fully utilized).

Yes i forgot to update my tags:
v1.22.0-333-g49ce7a607-dirty

@Carglglz
Copy link
Contributor

@peterzuger I'm afraid you're right which also means the openssl pkey command has a bug or it is not properly documented 🤦🏼‍♂️

https://www.openssl.org/docs/man3.3/man1/openssl-pkey.html

-traditional
Normally a private key is written using standard format: this is PKCS#8 form with the appropriate encryption algorithm (if any). If the -traditional option is specified then the older "traditional" format is used instead.

From mbedtls/library/pkparse.c

PKCS#1

     * This function parses the RSAPrivateKey (PKCS#1)
     *
     *  RSAPrivateKey ::= SEQUENCE {
     *      version           Version,
     *      modulus           INTEGER,  -- n
     *      publicExponent    INTEGER,  -- e
     *      privateExponent   INTEGER,  -- d
     *      prime1            INTEGER,  -- p
     *      prime2            INTEGER,  -- q
     *      exponent1         INTEGER,  -- d mod (p-1)
     *      exponent2         INTEGER,  -- d mod (q-1)
     *      coefficient       INTEGER,  -- (inverse of q) mod p
     *      otherPrimeInfos   OtherPrimeInfos OPTIONAL

PKCS#8

     * This function parses the PrivateKeyInfo object (PKCS#8 v1.2 = RFC 5208)
     *
     *    PrivateKeyInfo ::= SEQUENCE {
     *      version                   Version,
     *      privateKeyAlgorithm       PrivateKeyAlgorithmIdentifier,
     *      privateKey                PrivateKey,
     *      attributes           [0]  IMPLICIT Attributes OPTIONAL }
     *
     *    Version ::= INTEGER
     *    PrivateKeyAlgorithmIdentifier ::= AlgorithmIdentifier
     *    PrivateKey ::= OCTET STRING
     *
     *  The PrivateKey OCTET STRING is a SEC1 ECPrivateKey


$ openssl@3 genpkey -algorithm RSA -pkeyopt rsa_keygen_bits:2048 -out rsa2048.pkcs8.pem 2>/dev/null

$ openssl@3 asn1parse -inform pem -in rsa2048.pkcs8.pem

    0:d=0  hl=4 l=1215 cons: SEQUENCE
    4:d=1  hl=2 l=   1 prim: INTEGER           :00
    7:d=1  hl=2 l=  13 cons: SEQUENCE
    9:d=2  hl=2 l=   9 prim: OBJECT            :rsaEncryption
   20:d=2  hl=2 l=   0 prim: NULL
   22:d=1  hl=4 l=1193 prim: OCTET STRING      [HEX DUMP]:308204A50201000282010100ED626832D69B014E74CFB0471C7E934AA655C1831FAD703F380B5344094236E99CE1208A9D545538A6680A0C722C66E155ECD5A61654DA8A2348E0E185F8D41C7CC437E31F7177E839A880378B382D499727861FD1696B09BE6BC7EB6CC47995CC7132B66768EDA9A94ABE60F8C57BD6DF3A815F811C2F01BB6940C017724681D4783825E4E6918B88FF0F79F8B4AF54DE14D7E3131D77E54F700E362C05B307322A9C22F2E6ED2E648D0E61889DAA0CC0040D19E282A13B27C2021C2348FE516B1AD2BD99BEBD2A00B4879D6A86C26AF5B39E95FEE0C59F7650E6649828AF59E7764B500BAED10D83C27E7E10D46197579F8D77FC42F2E265BBE1752AA316CB02030100010282010014AA880AC0FF2E9E6F71F355458849776AC2B57D9E60E19047674900F7D3616776266D48540917003F1078AB7BB71CDFCA34BBA6E173D2B0CB0099BF588400A7583F56E439850DDCD44434A16147EB3D6860E2FEDB22D3D43DD5314AF025ACB61B5DD17DEAC3CA49067980D87D5FE7F79D688101D902249F807AD9CDBF5CCE016D99B8598A9D122A5D4C545A6E8A4740CD7F64A539F89E6F88E3F1C2A3501A7C7C3BBBD3A936CE91FBFEEE5A814C74F70082285AF05C5B8A5EE0F11E4148DBF6BD0CD4C27518CBB39D994A67C5EA4D58B25F4EE649C331EEB026425005075DD437B95DBFD5BC5B61044A5FB903B36CCBF4195EC8F9007757449CB20ECBE0B38D02818100FA7CB1FEA29D4C5ED6B7A55532E3E4F1D23D09C5B5EBB8E72EB311E018ADB2CC2679D36390F13E67F3D9BD17778D32AF728F7B947235E5CB1B3C39016E84C30CDDEE059EB21B0B4D75AF5796DB4D28FAAA8BE466D6C39A02240E85A2772302629BE97EE9AA645A4DC17DFB583C3998555409C67E67BB1CCA920E5DC8B97DBEB702818100F29BE355A1F6DB15B3E2B30B44FCBFBFD2F67777A29D13AD0EB3AB2A1F83CD52A30BD7D40B014A8099440C99077C8B38889BA20D1721BACC9E04C9910D6C2A3C513FD590C9ECC0224187712C2AF07EFD09AB2E63FB8F4EEE8211CF19E09E2B55F1A0F7DE733AD8249265B6F9F9779BE6991E6E522382251E8971958DE39B548D02818100C4D340E62E8F5D98542E400B1F8EF5B2931A0558C7276FC6B0EA62CC6D77E7BC052BCB503C6F643338E8CA6DEB321736C7D5392D1F457CC8C920DC4664BA8BADCBA1A1E6DE5AE3D9BCF4C7E5B17303155CF7C9C489536FEBE8BB4484B20ADF6FEEC55E456BEFA909DA560D2F7B3B6299020D205CE9539D3F0BDFA00FFEB97127028181008DAB965F250B3FF8366945606952A6AE5F84E9528534F08E782D56E06C32CD2D5A64CA0DDAB750483437D69B6E11BE42AF1322D83E0FC643426B4D4154F33ADEAA1B7A9CE82D462D3AD2DAB58E058E4238E92B57DB485E314CE71566E911B400A5EEF78F80FE75C535700C1F811BE5800D1E0612150624A2E663DB8F55C018E90281810094FBAFBC3955900A185523D77B21B7B9DFD1BD0E72A3FBEEDCBF09EB485C90742CB46BB78AD9B35868ABD0D8855ABD1A2ADC3B1F7A9AC17A2A383DB771114A0136D320E26AC3AD38AE3198A15EF9575C22EB9732205F8A2E61EE380D9BECFCFE997A11316AFFD8D3EACF1B69483C04F17E6FF6A7936EFA06F4F7DFC587D59DCB

$ openssl@3 pkey -in rsa2048.pkcs8.pem -out rsa2048.pkcs8.der -outform DER

For some reason pkey converts to PKCS#1 if using DER format.

$ openssl@3 asn1parse -inform der -in rsa2048.pkcs8.der

 0:d=0  hl=4 l=1189 cons: SEQUENCE
    4:d=1  hl=2 l=   1 prim: INTEGER           :00
    7:d=1  hl=4 l= 257 prim: INTEGER           :ED626832D69B014E74CFB0471C7E934AA655C1831FAD703F380B5344094236E99CE1208A9D545538A6680A0C722C66E155ECD5A61654DA8A2348E0E185F8D41C7CC437E31F7177E839A880378B382D499727861FD1696B09BE6BC7EB6CC47995CC7132B66768EDA9A94ABE60F8C57BD6DF3A815F811C2F01BB6940C017724681D4783825E4E6918B88FF0F79F8B4AF54DE14D7E3131D77E54F700E362C05B307322A9C22F2E6ED2E648D0E61889DAA0CC0040D19E282A13B27C2021C2348FE516B1AD2BD99BEBD2A00B4879D6A86C26AF5B39E95FEE0C59F7650E6649828AF59E7764B500BAED10D83C27E7E10D46197579F8D77FC42F2E265BBE1752AA316CB
  268:d=1  hl=2 l=   3 prim: INTEGER           :010001
  273:d=1  hl=4 l= 256 prim: INTEGER           :14AA880AC0FF2E9E6F71F355458849776AC2B57D9E60E19047674900F7D3616776266D48540917003F1078AB7BB71CDFCA34BBA6E173D2B0CB0099BF588400A7583F56E439850DDCD44434A16147EB3D6860E2FEDB22D3D43DD5314AF025ACB61B5DD17DEAC3CA49067980D87D5FE7F79D688101D902249F807AD9CDBF5CCE016D99B8598A9D122A5D4C545A6E8A4740CD7F64A539F89E6F88E3F1C2A3501A7C7C3BBBD3A936CE91FBFEEE5A814C74F70082285AF05C5B8A5EE0F11E4148DBF6BD0CD4C27518CBB39D994A67C5EA4D58B25F4EE649C331EEB026425005075DD437B95DBFD5BC5B61044A5FB903B36CCBF4195EC8F9007757449CB20ECBE0B38D
  533:d=1  hl=3 l= 129 prim: INTEGER           :FA7CB1FEA29D4C5ED6B7A55532E3E4F1D23D09C5B5EBB8E72EB311E018ADB2CC2679D36390F13E67F3D9BD17778D32AF728F7B947235E5CB1B3C39016E84C30CDDEE059EB21B0B4D75AF5796DB4D28FAAA8BE466D6C39A02240E85A2772302629BE97EE9AA645A4DC17DFB583C3998555409C67E67BB1CCA920E5DC8B97DBEB7
  665:d=1  hl=3 l= 129 prim: INTEGER           :F29BE355A1F6DB15B3E2B30B44FCBFBFD2F67777A29D13AD0EB3AB2A1F83CD52A30BD7D40B014A8099440C99077C8B38889BA20D1721BACC9E04C9910D6C2A3C513FD590C9ECC0224187712C2AF07EFD09AB2E63FB8F4EEE8211CF19E09E2B55F1A0F7DE733AD8249265B6F9F9779BE6991E6E522382251E8971958DE39B548D
  797:d=1  hl=3 l= 129 prim: INTEGER           :C4D340E62E8F5D98542E400B1F8EF5B2931A0558C7276FC6B0EA62CC6D77E7BC052BCB503C6F643338E8CA6DEB321736C7D5392D1F457CC8C920DC4664BA8BADCBA1A1E6DE5AE3D9BCF4C7E5B17303155CF7C9C489536FEBE8BB4484B20ADF6FEEC55E456BEFA909DA560D2F7B3B6299020D205CE9539D3F0BDFA00FFEB97127
  929:d=1  hl=3 l= 129 prim: INTEGER           :8DAB965F250B3FF8366945606952A6AE5F84E9528534F08E782D56E06C32CD2D5A64CA0DDAB750483437D69B6E11BE42AF1322D83E0FC643426B4D4154F33ADEAA1B7A9CE82D462D3AD2DAB58E058E4238E92B57DB485E314CE71566E911B400A5EEF78F80FE75C535700C1F811BE5800D1E0612150624A2E663DB8F55C018E9
 1061:d=1  hl=3 l= 129 prim: INTEGER           :94FBAFBC3955900A185523D77B21B7B9DFD1BD0E72A3FBEEDCBF09EB485C90742CB46BB78AD9B35868ABD0D8855ABD1A2ADC3B1F7A9AC17A2A383DB771114A0136D320E26AC3AD38AE3198A15EF9575C22EB9732205F8A2E61EE380D9BECFCFE997A11316AFFD8D3EACF1B69483C04F17E6FF6A7936EFA06F4F7DFC587D59DCB

$ openssl@3 rsa -in rsa2048.pkcs8.pem -outform DER -out rsa2048.true-pkcs8.der

however rsa command maintains PKCS#8

$ openssl@3 asn1parse -inform der -in rsa2048.true-pkcs8.der

0:d=0  hl=4 l=1215 cons: SEQUENCE
    4:d=1  hl=2 l=   1 prim: INTEGER           :00
    7:d=1  hl=2 l=  13 cons: SEQUENCE
    9:d=2  hl=2 l=   9 prim: OBJECT            :rsaEncryption
   20:d=2  hl=2 l=   0 prim: NULL
   22:d=1  hl=4 l=1193 prim: OCTET STRING      [HEX DUMP]:308204A50201000282010100ED626832D69B014E74CFB0471C7E934AA655C1831FAD703F380B5344094236E99CE1208A9D545538A6680A0C722C66E155ECD5A61654DA8A2348E0E185F8D41C7CC437E31F7177E839A880378B382D499727861FD1696B09BE6BC7EB6CC47995CC7132B66768EDA9A94ABE60F8C57BD6DF3A815F811C2F01BB6940C017724681D4783825E4E6918B88FF0F79F8B4AF54DE14D7E3131D77E54F700E362C05B307322A9C22F2E6ED2E648D0E61889DAA0CC0040D19E282A13B27C2021C2348FE516B1AD2BD99BEBD2A00B4879D6A86C26AF5B39E95FEE0C59F7650E6649828AF59E7764B500BAED10D83C27E7E10D46197579F8D77FC42F2E265BBE1752AA316CB02030100010282010014AA880AC0FF2E9E6F71F355458849776AC2B57D9E60E19047674900F7D3616776266D48540917003F1078AB7BB71CDFCA34BBA6E173D2B0CB0099BF588400A7583F56E439850DDCD44434A16147EB3D6860E2FEDB22D3D43DD5314AF025ACB61B5DD17DEAC3CA49067980D87D5FE7F79D688101D902249F807AD9CDBF5CCE016D99B8598A9D122A5D4C545A6E8A4740CD7F64A539F89E6F88E3F1C2A3501A7C7C3BBBD3A936CE91FBFEEE5A814C74F70082285AF05C5B8A5EE0F11E4148DBF6BD0CD4C27518CBB39D994A67C5EA4D58B25F4EE649C331EEB026425005075DD437B95DBFD5BC5B61044A5FB903B36CCBF4195EC8F9007757449CB20ECBE0B38D02818100FA7CB1FEA29D4C5ED6B7A55532E3E4F1D23D09C5B5EBB8E72EB311E018ADB2CC2679D36390F13E67F3D9BD17778D32AF728F7B947235E5CB1B3C39016E84C30CDDEE059EB21B0B4D75AF5796DB4D28FAAA8BE466D6C39A02240E85A2772302629BE97EE9AA645A4DC17DFB583C3998555409C67E67BB1CCA920E5DC8B97DBEB702818100F29BE355A1F6DB15B3E2B30B44FCBFBFD2F67777A29D13AD0EB3AB2A1F83CD52A30BD7D40B014A8099440C99077C8B38889BA20D1721BACC9E04C9910D6C2A3C513FD590C9ECC0224187712C2AF07EFD09AB2E63FB8F4EEE8211CF19E09E2B55F1A0F7DE733AD8249265B6F9F9779BE6991E6E522382251E8971958DE39B548D02818100C4D340E62E8F5D98542E400B1F8EF5B2931A0558C7276FC6B0EA62CC6D77E7BC052BCB503C6F643338E8CA6DEB321736C7D5392D1F457CC8C920DC4664BA8BADCBA1A1E6DE5AE3D9BCF4C7E5B17303155CF7C9C489536FEBE8BB4484B20ADF6FEEC55E456BEFA909DA560D2F7B3B6299020D205CE9539D3F0BDFA00FFEB97127028181008DAB965F250B3FF8366945606952A6AE5F84E9528534F08E782D56E06C32CD2D5A64CA0DDAB750483437D69B6E11BE42AF1322D83E0FC643426B4D4154F33ADEAA1B7A9CE82D462D3AD2DAB58E058E4238E92B57DB485E314CE71566E911B400A5EEF78F80FE75C535700C1F811BE5800D1E0612150624A2E663DB8F55C018E90281810094FBAFBC3955900A185523D77B21B7B9DFD1BD0E72A3FBEEDCBF09EB485C90742CB46BB78AD9B35868ABD0D8855ABD1A2ADC3B1F7A9AC17A2A383DB771114A0136D320E26AC3AD38AE3198A15EF9575C22EB9732205F8A2E61EE380D9BECFCFE997A11316AFFD8D3EACF1B69483C04F17E6FF6A7936EFA06F4F7DFC587D59DCB

Also ec keys are not affected because openssl uses SEC 1 format

The openssl-ec(1) command processes EC keys. They can be converted between various forms and their components printed out. Note OpenSSL uses the private key format specified in 'SEC 1: Elliptic Curve Cryptography' (http://www.secg.org/). To convert an OpenSSL EC private key into the PKCS#8 private key format use the openssl-pkcs8(1) command.

Re: on how to fix this

If the simultaneous support for DER and PEM data is desired,

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 👍🏼 ?

@peterzuger
Copy link
Contributor Author

Well this would also need to be wrapped in the:
#if MBEDTLS_VERSION_NUMBER >= 0x03000000
because of the differences from mbedtls v2 and v3.

Another idea I had was to differentiate passing in string and bytes, calling mbedtls_pk_parse_key with keylen + 1 for string and keylen for bytes.

But both solutions seem very hacky to me, which is why I opened an issue instead of a PR.

@Carglglz
Copy link
Contributor

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 library/pkparse.c comment 😅

     * At this point we only know it's not a PEM formatted key. Could be any
     * of the known DER encoded private key formats
     *
     * We try the different DER format parsers to see if one passes without
     * error

And looking again this seems another possible fix ?

/* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */
    if (key[keylen - 1] != '\0') 

@peterzuger
Copy link
Contributor Author

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 -----BEGIN we could just check if the buffer starts with this and if it does call with keylen + 1 and keylen otherwise ?

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 ?

@Carglglz
Copy link
Contributor

I think they are the same?
It seems like all PEM encoded data is null-terminated so that's what they check:

    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 b'\x00'?

so feel free to open a PR 👍🏼

@peterzuger
Copy link
Contributor Author

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.
In that case the key_len + 1 would not be necessary, that could also be a potential fix having the user append the NULL terminator for PEM data manually and just pass it to mbedtls_pk_parse_key as is.

The NULL terminator is appended by mp_obj_str_get_data which means if we would check that, both PEM and DER data has the NULL Terminator.

DER can certainly contain b'\x00' anywhere so probably also at the end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants