Size limit is too small for x509 extension size(2KB) and x509 certificate size(4KB) in x509write_crt.c #2631
Comments
The CRT writing routine mbedtls_x509write_crt_der() prepares the TBS (to-be-signed) part of the CRT in a temporary stack-allocated buffer, copying it to the actual output buffer at the end of the routine. This comes at the cost of a very large stack buffer. Moreover, its size must be hardcoded to an upper bound for the lengths of all CRTs to be written through the routine. So far, this upper bound was set to 2Kb, which isn't sufficient some larger certificates, as was reported e.g. in Mbed-TLS#2631. This commit fixes this by changing mbedtls_x509write_crt_der() to write the certificate in-place in the output buffer, thereby avoiding the use of a statically sized stack buffer for the TBS. Fixes Mbed-TLS#2631.
|
Hi @soccerGB, thank you for your detailed report, I agree that we have an issue here. We should avoid the use of such large stack buffers altogether and instead work in-place, as for CSRs is done in #2118. I have drafted a PR #2632 to fix the issue - could you check if it works for you? Also, if yes, could you please let us know how you would like to be credited? (GitHub name, full name, name + affiliation, ...) Kind regards, |
|
ARM Internal Ref: IOTSSL-2844 |
|
@hanno-arm Your fix works well for my scenarios. Looking forward to seeing this great fix in the upstream! |
|
By the way, I forgot to mention the changes look good to me. |
|
Hi @soccerGB, you're very welcome, and thanks again for your report. Could you please leave a note in the PR stating how you'd like to be credited? Kind regards, |
|
Hi @soccerGB, I saw that you already made use of the fix in PR #2632 in your PR on the OpenEnclave repository - while I'm very happy to see that it solves your issue, please be aware that the PR hasn't received any review from the Mbed TLS team yet, so I'd suggest that you either wait with using it or make sure a few critical eyes from your side check it first. But that's of course entirely at your discretion. Kind regards, |
|
@hanno-arm, thanks for the note. |
|
It looks like mbedtls_x509write_csr_der has similar constraints that need similar fix. |
|
Hi @soccerGB, yes that's right (see my initial comment), there's #2118 open to fix it.
It needs to be approved by two team members, plus pass CI and gatekeeper check, and @Patater might be able to give an estimate when that might happen. Kind regards, |
The CRT writing routine mbedtls_x509write_crt_der() prepares the TBS (to-be-signed) part of the CRT in a temporary stack-allocated buffer, copying it to the actual output buffer at the end of the routine. This comes at the cost of a very large stack buffer. Moreover, its size must be hardcoded to an upper bound for the lengths of all CRTs to be written through the routine. So far, this upper bound was set to 2Kb, which isn't sufficient some larger certificates, as was reported e.g. in Mbed-TLS#2631. This commit fixes this by changing mbedtls_x509write_crt_der() to write the certificate in-place in the output buffer, thereby avoiding the use of a statically sized stack buffer for the TBS. Fixes Mbed-TLS#2631.
Description
The current low limit x509 extension size(2KB) and x509 certificate size(4KB) in x509write_crt.c is too small for our application, which requires embedding some attestation data (around 6 kb) for helping authenticating tls' connecting parties. I found those two limitations were set by some local buffers in mbedtls/mbedtls/library/x509write_crt.c
int mbedtls_x509write_crt_der(...)
{
unsigned char tmp_buf[2048]; <--- extension limitation
...
}
int mbedtls_x509write_crt_pem(...)
{
..
unsigned char output_buf[4096]; <--- certificate limitation
...
}
We found OpenSSL have no such limitation. I hate to move to Openssl because mbedtls is more well structured in many ways.
Currently, I have to adjust to 8k in both buffers in our local snapshot.
Is it possible to remove this limitation or include the limit in the master branch?
Bug
OS
Mbed OS|linux|
mbed TLS build:
Version: 2.7.9
OS version: Ubuntu 18.04
Configuration: please attach config.h file where possible
Compiler and options (if you used a pre-built binary, please indicate how you obtained it):
Additional environment information:
Peer device TLS stack and version
OpenSSL|GnuTls|Chrome|NSS(Firefox)|SecureChannel (IIS/Internet Explorer/Edge)|Other
Version:
Expected behavior
No limit or higher limit (at least 8KB) x509 extension size and x509 certificate size
Actual behavior
low size limit x509 extension size(2KB) and x509 certificate size(4KB)
Steps to reproduce
Involved source code was mentioned in the issue description
Enhancement\Feature Request
Justification - why does the library need this feature?
mentioned in the issue description.
Suggested enhancement
I can provide changes or PR.
Question
Please first check for answers in the Mbed TLS knowledge Base, and preferably file an issue in the Mbed TLS support forum
The text was updated successfully, but these errors were encountered: