Synss / python-mbedtls

Cryptographic library with an mbed TLS back end
MIT License
79 stars 28 forks source link

Issue with loading private keys and trailing null bytes #75

Closed J-Bodenhausen closed 1 year ago

J-Bodenhausen commented 1 year ago

Hello, I believe I have encountered a bug that I want to bring to your attention.

I am submitting a …

Description

Current behavior

When creating and saving a private key with python-mbedtls, the resulting file contains a large number of trailing null bytes:

For example: '-----BEGIN EC PRIVATE KEY-----\n ... Regular EC Key content ... \n-----END EC PRIVATE KEY-----\n\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00 ...... '

When removing those null bytes, the key can no longer be read. Furthermore, externally generated keys (i.e. with openssl) cannot be loaded. Reading such a file results in the error: mbedtls.exceptions.TLSError: TLSError([0x3D62] 'PK - Invalid key tag or value : ASN1 - ASN1 tag was of an unexpected value')

Expected behavior

I expected the key.export_key("PEM") function to generate a string containing just the PEM encoded key without any trailing bytes. Furthermore, I expected to be able to load openssl generated keys.

Minimal demo of the problem and steps to reproduce

The code I used to generate and save a key:

import mbedtls
key = mbedtls.pk.ECC()
_ = key.generate()
open("./key.pem", "w").write(key.export_key("PEM"))

The code I used to read a key:

import mbedtls
key = mbedtls.pk.ECC.from_file("./key.pem")

The above code works perfectly fine as long as the key was saved with the first code snippet and the null bytes are left in place. It fails however for openssl generated certificates and certificates with the null bytes removed.

Other information

Debian 11 Python 3.9.2 pyhton-mbedtls 2.5.1 mbed TLS 2.28.1

The file size seems to be always 7672 bytes. So perhaps a buffer of fixed size is written out and expected.

I truncated the string obtained from the export method with: rstrip("\x00")

The following code makes the truncated certificate as well as openssl certificates parsable again:

import mbedtls
buf = open("./key.pem").read()
key = mbedtls.pk.ECC.from_PEM(buf.ljust(7672, "\x00"))
Synss commented 1 year ago

Thank you for your report. That, indeed, looks like a bug. I'll have a look.

Synss commented 1 year ago

The very long strip of "\0" bytes is definitely unnecessary. I am not yet sure where it comes from but I see I use mbedtls_pk_write_pubkey_der() + homemade DER-to-PEM conversion to create the PEM certificates, where mbedtls_pk_write_pubkey_pem() might be a better choice.

mbedtls_pk_write_pubkey_pem() is documented as

The output includes a terminating null bytes

and

# buf as above in your repro
key = mbedtls.pk.ECC.from_PEM(buf.rstrip("\0") + "\0"))

does not raise. That is, a single, terminating \0 byte is currently necessary but the following ones are most likely ignored.

I still need to compare with openssl.

Synss commented 1 year ago

I'll have to fix both x509 and pk modules.

The upstream documentation mentions a terminating null byte for mbedtls_pk_write_pubkey_pem as well, which I should probably skip. As you already mentioned, this byte is not in the openssl output.

J-Bodenhausen commented 1 year ago

Appending only one null byte is something I did not try. But I can see why this works and might be required, considering null terminated strings in C.

In any case, I am glad my report helped to draw attention to this.