From 159171b72ac7080c3692587565b19142ac9fd32f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 19 Dec 2018 17:03:28 +0100 Subject: [PATCH] PK parse/write: support keylen=0 correctly A 0-length buffer for the key is a legitimate edge case. Ensure that it works, even with buf=NULL. Document the key and keylen parameters. There are already test cases for parsing an empty buffer. A subsequent commit will add tests for writing to an empty buffer. --- include/mbedtls/pk.h | 34 ++++++++++++++++++++++------------ library/pkparse.c | 22 +++++++++++----------- library/pkwrite.c | 8 ++++++-- 3 files changed, 39 insertions(+), 25 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index e2529e4ce..310aeef5f 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -530,9 +530,13 @@ mbedtls_pk_type_t mbedtls_pk_get_type( const mbedtls_pk_context *ctx ); * \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 key Input buffer to parse. + * The buffer must contain the input exactly, with no + * extra trailing material. For PEM, the buffer must + * contain a null-terminated string. + * \param keylen Size of \b key in bytes. + * For PEM data, this includes the terminating null byte, + * so \p keylen must be equal to `strlen(key) + 1`. * \param pwd password for decryption (optional) * \param pwdlen size of the password * @@ -553,9 +557,13 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *ctx, * \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) + * \param key Input buffer to parse. + * The buffer must contain the input exactly, with no + * extra trailing material. For PEM, the buffer must + * contain a null-terminated string. + * \param keylen Size of \b key in bytes. + * For PEM data, this includes the terminating null byte, + * so \p keylen must be equal to `strlen(key) + 1`. * * \note On entry, ctx must be empty, either freshly initialised * with mbedtls_pk_init() or reset with mbedtls_pk_free(). If you need a @@ -643,9 +651,10 @@ int mbedtls_pk_write_pubkey_der( mbedtls_pk_context *ctx, unsigned char *buf, si /** * \brief Write a public key to a PEM string * - * \param ctx public key to write away - * \param buf buffer to write to - * \param size size of the buffer + * \param ctx Context containing the public key to write. + * \param buf Buffer to write to. The output includes a + * terminating null byte. + * \param size Size of the buffer in bytes. * * \return 0 if successful, or a specific error code */ @@ -654,9 +663,10 @@ int mbedtls_pk_write_pubkey_pem( mbedtls_pk_context *ctx, unsigned char *buf, si /** * \brief Write a private key to a PKCS#1 or SEC1 PEM string * - * \param ctx private to write away - * \param buf buffer to write to - * \param size size of the buffer + * \param ctx Context containing the private key to write. + * \param buf Buffer to write to. The output includes a + * terminating null byte. + * \param size Size of the buffer in bytes. * * \return 0 if successful, or a specific error code */ diff --git a/library/pkparse.c b/library/pkparse.c index 7c14e34ec..127f9b840 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -1170,15 +1170,16 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk, #endif PK_VALIDATE_RET( pk != NULL ); - PK_VALIDATE_RET( key != NULL || keylen == 0 ); - PK_VALIDATE_RET( pwd != NULL || pwdlen == 0 ); + if( keylen == 0 ) + return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT ); + PK_VALIDATE_RET( key != NULL ); #if defined(MBEDTLS_PEM_PARSE_C) mbedtls_pem_init( &pem ); #if defined(MBEDTLS_RSA_C) /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */ - if( keylen == 0 || key[keylen - 1] != '\0' ) + if( key[keylen - 1] != '\0' ) ret = MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT; else ret = mbedtls_pem_read_buffer( &pem, @@ -1209,7 +1210,7 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk, #if defined(MBEDTLS_ECP_C) /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */ - if( keylen == 0 || key[keylen - 1] != '\0' ) + if( key[keylen - 1] != '\0' ) ret = MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT; else ret = mbedtls_pem_read_buffer( &pem, @@ -1239,7 +1240,7 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk, #endif /* MBEDTLS_ECP_C */ /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */ - if( keylen == 0 || key[keylen - 1] != '\0' ) + if( key[keylen - 1] != '\0' ) ret = MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT; else ret = mbedtls_pem_read_buffer( &pem, @@ -1262,7 +1263,7 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk, #if defined(MBEDTLS_PKCS12_C) || defined(MBEDTLS_PKCS5_C) /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */ - if( keylen == 0 || key[keylen - 1] != '\0' ) + if( key[keylen - 1] != '\0' ) ret = MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT; else ret = mbedtls_pem_read_buffer( &pem, @@ -1300,9 +1301,6 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk, { unsigned char *key_copy; - if( keylen == 0 ) - return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT ); - if( ( key_copy = mbedtls_calloc( 1, keylen ) ) == NULL ) return( MBEDTLS_ERR_PK_ALLOC_FAILED ); @@ -1387,13 +1385,15 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, #endif PK_VALIDATE_RET( ctx != NULL ); + if( keylen == 0 ) + return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT ); PK_VALIDATE_RET( key != NULL || keylen == 0 ); #if defined(MBEDTLS_PEM_PARSE_C) mbedtls_pem_init( &pem ); #if defined(MBEDTLS_RSA_C) /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */ - if( keylen == 0 || key[keylen - 1] != '\0' ) + if( key[keylen - 1] != '\0' ) ret = MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT; else ret = mbedtls_pem_read_buffer( &pem, @@ -1424,7 +1424,7 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, #endif /* MBEDTLS_RSA_C */ /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */ - if( keylen == 0 || key[keylen - 1] != '\0' ) + if( key[keylen - 1] != '\0' ) ret = MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT; else ret = mbedtls_pem_read_buffer( &pem, diff --git a/library/pkwrite.c b/library/pkwrite.c index 51d0c56f1..8d1da2f75 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -186,7 +186,9 @@ int mbedtls_pk_write_pubkey_der( mbedtls_pk_context *key, unsigned char *buf, si const char *oid; PK_VALIDATE_RET( key != NULL ); - PK_VALIDATE_RET( buf != NULL || size == 0 ); + if( size == 0 ) + return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); + PK_VALIDATE_RET( buf != NULL ); c = buf + size; @@ -236,7 +238,9 @@ int mbedtls_pk_write_key_der( mbedtls_pk_context *key, unsigned char *buf, size_ size_t len = 0; PK_VALIDATE_RET( key != NULL ); - PK_VALIDATE_RET( buf != NULL || size == 0 ); + if( size == 0 ) + return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); + PK_VALIDATE_RET( buf != NULL ); c = buf + size;