Fix use of pem_read_buffer() in PK, DHM and X509
diff --git a/ChangeLog b/ChangeLog
index 361eea3..58c925f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -55,6 +55,9 @@
available if POLARSSL_PEM_PARSE_C is defined (it never worked without).
* Test certificates in certs.c are no longer guaranteed to be nul-terminated
strings; use the new *_len variables instead of strlen().
+ * Functions mbedtls_x509_xxx_parse(), mbedtls_pk_parse_key(),
+ mbedtls_pk_parse_public_key() and mbedtls_dhm_parse_dhm() now expect the
+ length parameter to include the terminating null byte for PEM input.
* Signature of mpi_mul_mpi() changed to make the last argument unsigned
* Change SSL_DISABLE_RENEGOTIATION config.h flag to SSL_RENEGOTIATION
(support for renegotiation now needs explicit enabling in config.h).
diff --git a/include/mbedtls/dhm.h b/include/mbedtls/dhm.h
index 4d16619..caa630d 100644
--- a/include/mbedtls/dhm.h
+++ b/include/mbedtls/dhm.h
@@ -269,11 +269,12 @@
#if defined(MBEDTLS_ASN1_PARSE_C)
/** \ingroup x509_module */
/**
- * \brief Parse DHM parameters
+ * \brief Parse DHM parameters in PEM or DER format
*
* \param dhm DHM context to be initialized
* \param dhmin input buffer
* \param dhminlen size of the buffer
+ * (including the terminating null byte for PEM data)
*
* \return 0 if successful, or a specific DHM or PEM error code
*/
diff --git a/include/mbedtls/pem.h b/include/mbedtls/pem.h
index ef94529..f826abd 100644
--- a/include/mbedtls/pem.h
+++ b/include/mbedtls/pem.h
@@ -73,7 +73,7 @@
* \param ctx context to use
* \param header header string to seek and expect
* \param footer footer string to seek and expect
- * \param data source data to look in
+ * \param data source data to look in (must be nul-terminated)
* \param pwd password for decryption (can be NULL)
* \param pwdlen length of password
* \param use_len destination for total length used (set after header is
diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h
index 44efef0..3ed8b5e 100644
--- a/include/mbedtls/pk.h
+++ b/include/mbedtls/pk.h
@@ -427,11 +427,12 @@
#if defined(MBEDTLS_PK_PARSE_C)
/** \ingroup pk_module */
/**
- * \brief Parse a private key
+ * \brief Parse a private key in PEM or DER format
*
* \param ctx key to be initialized
* \param key input buffer
* \param keylen size of the buffer
+ * (including the terminating null byte for PEM data)
* \param pwd password for decryption (optional)
* \param pwdlen size of the password
*
@@ -449,11 +450,12 @@
/** \ingroup pk_module */
/**
- * \brief Parse a public key
+ * \brief Parse a public key in PEM or DER format
*
* \param ctx key to be initialized
* \param key input buffer
* \param keylen size of the buffer
+ * (including the terminating null byte for PEM data)
*
* \note On entry, ctx must be empty, either freshly initialised
* with mbedtls_pk_init() or reset with mbedtls_pk_free(). If you need a
diff --git a/include/mbedtls/x509_crl.h b/include/mbedtls/x509_crl.h
index 395c1b3..7b40739 100644
--- a/include/mbedtls/x509_crl.h
+++ b/include/mbedtls/x509_crl.h
@@ -101,7 +101,7 @@
*
* \param chain points to the start of the chain
* \param buf buffer holding the CRL data in DER format
- * \param buflen size of the buffer
+ * (including the terminating null byte for PEM data)
*
* \return 0 if successful, or a specific X509 or PEM error code
*/
@@ -115,6 +115,7 @@
* \param chain points to the start of the chain
* \param buf buffer holding the CRL data in PEM or DER format
* \param buflen size of the buffer
+ * (including the terminating null byte for PEM data)
*
* \return 0 if successful, or a specific X509 or PEM error code
*/
diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h
index e184dee..aced7eb 100644
--- a/include/mbedtls/x509_crt.h
+++ b/include/mbedtls/x509_crt.h
@@ -141,8 +141,9 @@
* correctly, the first error is returned.
*
* \param chain points to the start of the chain
- * \param buf buffer holding the certificate data
+ * \param buf buffer holding the certificate data in PEM or DER format
* \param buflen size of the buffer
+ * (including the terminating null byte for PEM data)
*
* \return 0 if all certificates parsed successfully, a positive number
* if partly successful or a specific X509 or PEM error code
diff --git a/include/mbedtls/x509_csr.h b/include/mbedtls/x509_csr.h
index c46bc61..0082ada 100644
--- a/include/mbedtls/x509_csr.h
+++ b/include/mbedtls/x509_csr.h
@@ -99,6 +99,7 @@
* \param csr CSR context to fill
* \param buf buffer holding the CRL data
* \param buflen size of the buffer
+ * (including the terminating null byte for PEM data)
*
* \return 0 if successful, or a specific X509 or PEM error code
*/
diff --git a/library/dhm.c b/library/dhm.c
index c5e41a3..92fd611 100644
--- a/library/dhm.c
+++ b/library/dhm.c
@@ -421,10 +421,14 @@
mbedtls_pem_init( &pem );
- ret = mbedtls_pem_read_buffer( &pem,
- "-----BEGIN DH PARAMETERS-----",
- "-----END DH PARAMETERS-----",
- dhmin, NULL, 0, &dhminlen );
+ /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */
+ if( dhmin[dhminlen - 1] != '\0' )
+ ret = MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT;
+ else
+ ret = mbedtls_pem_read_buffer( &pem,
+ "-----BEGIN DH PARAMETERS-----",
+ "-----END DH PARAMETERS-----",
+ dhmin, NULL, 0, &dhminlen );
if( ret == 0 )
{
@@ -503,6 +507,10 @@
#if defined(MBEDTLS_FS_IO)
/*
* Load all data from a file into a given buffer.
+ *
+ * The file is expected to contain either PEM or DER encoded data.
+ * A terminating null byte is always appended. It is included in the announced
+ * length only if the data looks like it is PEM encoded.
*/
static int load_file( const char *path, unsigned char **buf, size_t *n )
{
@@ -540,6 +548,9 @@
(*buf)[*n] = '\0';
+ if( strstr( (const char *) *buf, "-----BEGIN " ) != NULL )
+ ++*n;
+
return( 0 );
}
@@ -557,7 +568,7 @@
ret = mbedtls_dhm_parse_dhm( dhm, buf, n );
- mbedtls_zeroize( buf, n + 1 );
+ mbedtls_zeroize( buf, n );
mbedtls_free( buf );
return( ret );
@@ -584,7 +595,7 @@
mbedtls_printf( " DHM parameter load: " );
if( ( ret = mbedtls_dhm_parse_dhm( &dhm, (const unsigned char *) mbedtls_test_dhm_params,
- strlen( mbedtls_test_dhm_params ) ) ) != 0 )
+ mbedtls_test_dhm_params_len ) ) != 0 )
{
if( verbose != 0 )
mbedtls_printf( "failed\n" );
diff --git a/library/pkparse.c b/library/pkparse.c
index f5a9853..edf6e31 100644
--- a/library/pkparse.c
+++ b/library/pkparse.c
@@ -69,6 +69,10 @@
/*
* Load all data from a file into a given buffer.
+ *
+ * The file is expected to contain either PEM or DER encoded data.
+ * A terminating null byte is always appended. It is included in the announced
+ * length only if the data looks like it is PEM encoded.
*/
int mbedtls_pk_load_file( const char *path, unsigned char **buf, size_t *n )
{
@@ -106,6 +110,9 @@
(*buf)[*n] = '\0';
+ if( strstr( (const char *) *buf, "-----BEGIN " ) != NULL )
+ ++*n;
+
return( 0 );
}
@@ -128,7 +135,7 @@
ret = mbedtls_pk_parse_key( ctx, buf, n,
(const unsigned char *) pwd, strlen( pwd ) );
- mbedtls_zeroize( buf, n + 1 );
+ mbedtls_zeroize( buf, n );
mbedtls_free( buf );
return( ret );
@@ -148,7 +155,7 @@
ret = mbedtls_pk_parse_public_key( ctx, buf, n );
- mbedtls_zeroize( buf, n + 1 );
+ mbedtls_zeroize( buf, n );
mbedtls_free( buf );
return( ret );
@@ -1064,10 +1071,15 @@
mbedtls_pem_init( &pem );
#if defined(MBEDTLS_RSA_C)
- ret = mbedtls_pem_read_buffer( &pem,
- "-----BEGIN RSA PRIVATE KEY-----",
- "-----END RSA PRIVATE KEY-----",
- key, pwd, pwdlen, &len );
+ /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */
+ if( key[keylen - 1] != '\0' )
+ ret = MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT;
+ else
+ ret = mbedtls_pem_read_buffer( &pem,
+ "-----BEGIN RSA PRIVATE KEY-----",
+ "-----END RSA PRIVATE KEY-----",
+ key, pwd, pwdlen, &len );
+
if( ret == 0 )
{
if( ( pk_info = mbedtls_pk_info_from_type( MBEDTLS_PK_RSA ) ) == NULL )
@@ -1092,10 +1104,14 @@
#endif /* MBEDTLS_RSA_C */
#if defined(MBEDTLS_ECP_C)
- ret = mbedtls_pem_read_buffer( &pem,
- "-----BEGIN EC PRIVATE KEY-----",
- "-----END EC PRIVATE KEY-----",
- key, pwd, pwdlen, &len );
+ /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */
+ if( key[keylen - 1] != '\0' )
+ ret = MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT;
+ else
+ ret = mbedtls_pem_read_buffer( &pem,
+ "-----BEGIN EC PRIVATE KEY-----",
+ "-----END EC PRIVATE KEY-----",
+ key, pwd, pwdlen, &len );
if( ret == 0 )
{
if( ( pk_info = mbedtls_pk_info_from_type( MBEDTLS_PK_ECKEY ) ) == NULL )
@@ -1119,10 +1135,14 @@
return( ret );
#endif /* MBEDTLS_ECP_C */
- ret = mbedtls_pem_read_buffer( &pem,
- "-----BEGIN PRIVATE KEY-----",
- "-----END PRIVATE KEY-----",
- key, NULL, 0, &len );
+ /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */
+ if( key[keylen - 1] != '\0' )
+ ret = MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT;
+ else
+ ret = mbedtls_pem_read_buffer( &pem,
+ "-----BEGIN PRIVATE KEY-----",
+ "-----END PRIVATE KEY-----",
+ key, NULL, 0, &len );
if( ret == 0 )
{
if( ( ret = pk_parse_key_pkcs8_unencrypted_der( pk,
@@ -1138,10 +1158,14 @@
return( ret );
#if defined(MBEDTLS_PKCS12_C) || defined(MBEDTLS_PKCS5_C)
- ret = mbedtls_pem_read_buffer( &pem,
- "-----BEGIN ENCRYPTED PRIVATE KEY-----",
- "-----END ENCRYPTED PRIVATE KEY-----",
- key, NULL, 0, &len );
+ /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */
+ if( key[keylen - 1] != '\0' )
+ ret = MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT;
+ else
+ ret = mbedtls_pem_read_buffer( &pem,
+ "-----BEGIN ENCRYPTED PRIVATE KEY-----",
+ "-----END ENCRYPTED PRIVATE KEY-----",
+ key, NULL, 0, &len );
if( ret == 0 )
{
if( ( ret = pk_parse_key_pkcs8_encrypted_der( pk,
@@ -1231,10 +1255,15 @@
mbedtls_pem_context pem;
mbedtls_pem_init( &pem );
- ret = mbedtls_pem_read_buffer( &pem,
- "-----BEGIN PUBLIC KEY-----",
- "-----END PUBLIC KEY-----",
- key, NULL, 0, &len );
+
+ /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */
+ if( key[keylen - 1] != '\0' )
+ ret = MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT;
+ else
+ ret = mbedtls_pem_read_buffer( &pem,
+ "-----BEGIN PUBLIC KEY-----",
+ "-----END PUBLIC KEY-----",
+ key, NULL, 0, &len );
if( ret == 0 )
{
diff --git a/library/x509.c b/library/x509.c
index 55daf74..08a1ec3 100644
--- a/library/x509.c
+++ b/library/x509.c
@@ -1008,7 +1008,7 @@
mbedtls_x509_crt_init( &clicert );
ret = mbedtls_x509_crt_parse( &clicert, (const unsigned char *) mbedtls_test_cli_crt,
- strlen( mbedtls_test_cli_crt ) );
+ mbedtls_test_cli_crt_len );
if( ret != 0 )
{
if( verbose != 0 )
@@ -1020,7 +1020,7 @@
mbedtls_x509_crt_init( &cacert );
ret = mbedtls_x509_crt_parse( &cacert, (const unsigned char *) mbedtls_test_ca_crt,
- strlen( mbedtls_test_ca_crt ) );
+ mbedtls_test_ca_crt_len );
if( ret != 0 )
{
if( verbose != 0 )
diff --git a/library/x509_crl.c b/library/x509_crl.c
index a915aba..fc4b2df 100644
--- a/library/x509_crl.c
+++ b/library/x509_crl.c
@@ -503,6 +503,11 @@
do
{
mbedtls_pem_init( &pem );
+
+ /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */
+ if( buf[buflen - 1] != '\0' )
+ ret = MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT;
+ else
ret = mbedtls_pem_read_buffer( &pem,
"-----BEGIN X509 CRL-----",
"-----END X509 CRL-----",
@@ -532,7 +537,9 @@
return( ret );
}
}
- while( is_pem && buflen > 0 );
+ /* In the PEM case, buflen is 1 at the end, for the terminated NULL byte.
+ * And a valid CRL cannot be less than 1 byte anyway. */
+ while( is_pem && buflen > 1 );
if( is_pem )
return( 0 );
@@ -556,7 +563,7 @@
ret = mbedtls_x509_crl_parse( chain, buf, n );
- mbedtls_zeroize( buf, n + 1 );
+ mbedtls_zeroize( buf, n );
mbedtls_free( buf );
return( ret );
diff --git a/library/x509_crt.c b/library/x509_crt.c
index 4ebae77..059b60f 100644
--- a/library/x509_crt.c
+++ b/library/x509_crt.c
@@ -852,8 +852,11 @@
* one or more PEM certificates.
*/
#if defined(MBEDTLS_PEM_PARSE_C)
- if( strstr( (const char *) buf, "-----BEGIN CERTIFICATE-----" ) != NULL )
+ if( buf[buflen - 1] == '\0' &&
+ strstr( (const char *) buf, "-----BEGIN CERTIFICATE-----" ) != NULL )
+ {
buf_format = MBEDTLS_X509_FORMAT_PEM;
+ }
#endif
if( buf_format == MBEDTLS_X509_FORMAT_DER )
@@ -865,11 +868,13 @@
int ret;
mbedtls_pem_context pem;
- while( buflen > 0 )
+ /* 1 rather than 0 since the terminating NULL byte is counted in */
+ while( buflen > 1 )
{
size_t use_len;
mbedtls_pem_init( &pem );
+ /* If we get there, we know the string is null-terminated */
ret = mbedtls_pem_read_buffer( &pem,
"-----BEGIN CERTIFICATE-----",
"-----END CERTIFICATE-----",
@@ -953,7 +958,7 @@
ret = mbedtls_x509_crt_parse( chain, buf, n );
- mbedtls_zeroize( buf, n + 1 );
+ mbedtls_zeroize( buf, n );
mbedtls_free( buf );
return( ret );
diff --git a/library/x509_csr.c b/library/x509_csr.c
index f6afa1e..5ec1b86 100644
--- a/library/x509_csr.c
+++ b/library/x509_csr.c
@@ -274,10 +274,15 @@
#if defined(MBEDTLS_PEM_PARSE_C)
mbedtls_pem_init( &pem );
- ret = mbedtls_pem_read_buffer( &pem,
- "-----BEGIN CERTIFICATE REQUEST-----",
- "-----END CERTIFICATE REQUEST-----",
- buf, NULL, 0, &use_len );
+
+ /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */
+ if( buf[buflen - 1] != '\0' )
+ ret = MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT;
+ else
+ ret = mbedtls_pem_read_buffer( &pem,
+ "-----BEGIN CERTIFICATE REQUEST-----",
+ "-----END CERTIFICATE REQUEST-----",
+ buf, NULL, 0, &use_len );
if( ret == 0 )
{
@@ -315,7 +320,7 @@
ret = mbedtls_x509_csr_parse( csr, buf, n );
- mbedtls_zeroize( buf, n + 1 );
+ mbedtls_zeroize( buf, n );
mbedtls_free( buf );
return( ret );