Remove Extraneous bytes from buffer post pem write
In order to remove large buffers from the stack, the der data is written
into the same buffer that the pem is eventually written into, however
although the pem data is zero terminated, there is now data left in the
buffer after the zero termination, which can cause
mbedtls_x509_crt_parse to fail to parse the same buffer if passed back
in. Patches also applied to mbedtls_pk_write_pubkey_pem, and
mbedtls_pk_write_key_pem, which use similar methods of writing der data
to the same buffer, and tests modified to hopefully catch any future
regression on this.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
diff --git a/ChangeLog.d/clean_pem_buffers.txt b/ChangeLog.d/clean_pem_buffers.txt
new file mode 100644
index 0000000..818fad9
--- /dev/null
+++ b/ChangeLog.d/clean_pem_buffers.txt
@@ -0,0 +1,6 @@
+Bugfix
+ * In PEM writing functions, fill the trailing part of the buffer with null
+ bytes. This guarantees that the corresponding parsing function can read
+ the buffer back, which was the case for mbedtls_x509write_{crt,csr}_pem
+ until this property was inadvertently broken in Mbed TLS 2.19.0.
+ Fixes #3682.
diff --git a/library/pem.c b/library/pem.c
index 534d071..969d492 100644
--- a/library/pem.c
+++ b/library/pem.c
@@ -478,8 +478,12 @@
*p++ = '\0';
*olen = p - buf;
+ /* Clean any remaining data previously written to the buffer */
+ memset( buf + *olen, 0, buf_len - *olen );
+
mbedtls_free( encode_buf );
return( 0 );
}
#endif /* MBEDTLS_PEM_WRITE_C */
#endif /* MBEDTLS_PEM_PARSE_C || MBEDTLS_PEM_WRITE_C */
+
diff --git a/tests/suites/test_suite_pkwrite.function b/tests/suites/test_suite_pkwrite.function
index 43c275e..2bad4ed 100644
--- a/tests/suites/test_suite_pkwrite.function
+++ b/tests/suites/test_suite_pkwrite.function
@@ -17,7 +17,7 @@
unsigned char check_buf[5000];
int ret;
FILE *f;
- size_t ilen;
+ size_t ilen, pem_len, buf_index;
memset( buf, 0, sizeof( buf ) );
memset( check_buf, 0, sizeof( check_buf ) );
@@ -28,12 +28,20 @@
ret = mbedtls_pk_write_pubkey_pem( &key, buf, sizeof( buf ));
TEST_ASSERT( ret == 0 );
+ pem_len = strlen( (char *) buf );
+
+ // check that the rest of the buffer remains clear
+ for( buf_index = pem_len; buf_index < sizeof( buf ); ++buf_index )
+ {
+ TEST_ASSERT( buf[buf_index] == 0 );
+ }
+
f = fopen( key_file, "r" );
TEST_ASSERT( f != NULL );
ilen = fread( check_buf, 1, sizeof( check_buf ), f );
fclose( f );
- TEST_ASSERT( ilen == strlen( (char *) buf ) );
+ TEST_ASSERT( ilen == pem_len );
TEST_ASSERT( memcmp( (char *) buf, (char *) check_buf, ilen ) == 0 );
exit:
@@ -49,7 +57,7 @@
unsigned char check_buf[5000];
int ret;
FILE *f;
- size_t ilen;
+ size_t ilen, pem_len, buf_index;
memset( buf, 0, sizeof( buf ) );
memset( check_buf, 0, sizeof( check_buf ) );
@@ -60,6 +68,14 @@
ret = mbedtls_pk_write_key_pem( &key, buf, sizeof( buf ));
TEST_ASSERT( ret == 0 );
+ pem_len = strlen( (char *) buf );
+
+ // check that the rest of the buffer remains clear
+ for( buf_index = pem_len; buf_index < sizeof( buf ); ++buf_index )
+ {
+ TEST_ASSERT( buf[buf_index] == 0 );
+ }
+
f = fopen( key_file, "r" );
TEST_ASSERT( f != NULL );
ilen = fread( check_buf, 1, sizeof( check_buf ), f );
diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function
index 9f2007d..3803377 100644
--- a/tests/suites/test_suite_x509write.function
+++ b/tests/suites/test_suite_x509write.function
@@ -104,7 +104,7 @@
unsigned char buf[4096];
unsigned char check_buf[4000];
int ret;
- size_t olen = 0, pem_len = 0;
+ size_t olen = 0, pem_len = 0, buf_index;
int der_len = -1;
FILE *f;
const char *subject_name = "C=NL,O=PolarSSL,CN=PolarSSL Server 1";
@@ -130,6 +130,11 @@
pem_len = strlen( (char *) buf );
+ for( buf_index = pem_len; buf_index < sizeof( buf ); ++buf_index )
+ {
+ TEST_ASSERT( buf[buf_index] == 0 );
+ }
+
f = fopen( cert_req_check_file, "r" );
TEST_ASSERT( f != NULL );
olen = fread( check_buf, 1, sizeof( check_buf ), f );
@@ -224,7 +229,7 @@
unsigned char check_buf[5000];
mbedtls_mpi serial;
int ret;
- size_t olen = 0, pem_len = 0;
+ size_t olen = 0, pem_len = 0, buf_index = 0;
int der_len = -1;
FILE *f;
mbedtls_test_rnd_pseudo_info rnd_info;
@@ -293,6 +298,12 @@
pem_len = strlen( (char *) buf );
+ // check that the rest of the buffer remains clear
+ for( buf_index = pem_len; buf_index < sizeof( buf ); ++buf_index )
+ {
+ TEST_ASSERT( buf[buf_index] == 0 );
+ }
+
f = fopen( cert_check_file, "r" );
TEST_ASSERT( f != NULL );
olen = fread( check_buf, 1, sizeof( check_buf ), f );