From 6af45ec53e2c2e87e86e10079bc3d921b1ebde39 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 19 Dec 2018 17:52:05 +0100 Subject: [PATCH] PK: document context validity requirements Document when a context must be initialized or not, when it must be set up or not, and whether it needs a private key or a public key will do. The implementation is sometimes more liberal than the documentation, accepting a non-set-up context as a context that can't perform the requested information. This preserves backward compatibility. --- include/mbedtls/pk.h | 77 +++++++++++++++++------------ library/pk.c | 5 ++ tests/suites/test_suite_pk.function | 6 ++- 3 files changed, 55 insertions(+), 33 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index 716070454..91950f940 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -207,7 +207,7 @@ void mbedtls_pk_init( mbedtls_pk_context *ctx ); /** * \brief Free the components of a #mbedtls_pk_context. * - * \param ctx The context to clear. + * \param ctx The context to clear. It must have been initialized. * If this is \c NULL, this function does nothing. */ void mbedtls_pk_free( mbedtls_pk_context *ctx ); @@ -224,7 +224,7 @@ void mbedtls_pk_restart_init( mbedtls_pk_restart_ctx *ctx ); /** * \brief Free the components of a restart context * - * \param ctx The context to clear. + * \param ctx The context to clear. It must have been initialized. * If this is \c NULL, this function does nothing. */ void mbedtls_pk_restart_free( mbedtls_pk_restart_ctx *ctx ); @@ -234,7 +234,8 @@ void mbedtls_pk_restart_free( mbedtls_pk_restart_ctx *ctx ); * \brief Initialize a PK context with the information given * and allocates the type-specific PK subcontext. * - * \param ctx Context to initialize. Must be empty (type NONE). + * \param ctx Context to initialize. It must not have been set + * up yet (type #MBEDTLS_PK_NONE). * \param info Information to use * * \return 0 on success, @@ -250,7 +251,8 @@ int mbedtls_pk_setup( mbedtls_pk_context *ctx, const mbedtls_pk_info_t *info ); /** * \brief Initialize an RSA-alt context * - * \param ctx Context to initialize. Must be empty (type NONE). + * \param ctx Context to initialize. It must not have been set + * up yet (type #MBEDTLS_PK_NONE). * \param key RSA key pointer * \param decrypt_func Decryption function * \param sign_func Signing function @@ -270,7 +272,7 @@ int mbedtls_pk_setup_rsa_alt( mbedtls_pk_context *ctx, void * key, /** * \brief Get the size in bits of the underlying key * - * \param ctx Context to use + * \param ctx The context to query. It must have been initialized. * * \return Key size in bits, or 0 on error */ @@ -278,7 +280,8 @@ size_t mbedtls_pk_get_bitlen( const mbedtls_pk_context *ctx ); /** * \brief Get the length in bytes of the underlying key - * \param ctx Context to use + * + * \param ctx The context to query. It must have been initialized. * * \return Key length in bytes, or 0 on error */ @@ -290,18 +293,21 @@ static inline size_t mbedtls_pk_get_len( const mbedtls_pk_context *ctx ) /** * \brief Tell if a context can do the operation given by type * - * \param ctx Context to test - * \param type Target type + * \param ctx The context to query. It must have been initialized. + * \param type The desired type. * - * \return 0 if context can't do the operations, - * 1 otherwise. + * \return 1 if the context can do operations on the given type. + * \return 0 if the context cannot do the operations on the given + * type. This is always the case for a context that has + * been initialized but not set up, or that has been + * cleared with mbedtls_pk_free(). */ int mbedtls_pk_can_do( const mbedtls_pk_context *ctx, mbedtls_pk_type_t type ); /** * \brief Verify signature (including padding if relevant). * - * \param ctx PK context to use + * \param ctx The PK context to use. It must have been set up. * \param md_alg Hash algorithm used (see notes) * \param hash Hash of the message to sign * \param hash_len Hash length or 0 (see notes) @@ -334,7 +340,7 @@ int mbedtls_pk_verify( mbedtls_pk_context *ctx, mbedtls_md_type_t md_alg, * \c mbedtls_ecp_set_max_ops() to reduce blocking for ECC * operations. For RSA, same as \c mbedtls_pk_verify(). * - * \param ctx PK context to use + * \param ctx The PK context to use. It must have been set up. * \param md_alg Hash algorithm used (see notes) * \param hash Hash of the message to sign * \param hash_len Hash length or 0 (see notes) @@ -358,7 +364,7 @@ int mbedtls_pk_verify_restartable( mbedtls_pk_context *ctx, * * \param type Signature type (inc. possible padding type) to verify * \param options Pointer to type-specific options, or NULL - * \param ctx PK context to use + * \param ctx The PK context to use. It must have been set up. * \param md_alg Hash algorithm used (see notes) * \param hash Hash of the message to sign * \param hash_len Hash length or 0 (see notes) @@ -389,7 +395,8 @@ int mbedtls_pk_verify_ext( mbedtls_pk_type_t type, const void *options, /** * \brief Make signature, including padding if relevant. * - * \param ctx PK context to use - must hold a private key + * \param ctx The PK context to use. It must have been set up + * with a private key. * \param md_alg Hash algorithm used (see notes) * \param hash Hash of the message to sign * \param hash_len Hash length or 0 (see notes) @@ -423,7 +430,8 @@ int mbedtls_pk_sign( mbedtls_pk_context *ctx, mbedtls_md_type_t md_alg, * \c mbedtls_ecp_set_max_ops() to reduce blocking for ECC * operations. For RSA, same as \c mbedtls_pk_sign(). * - * \param ctx PK context to use - must hold a private key + * \param ctx The PK context to use. It must have been set up + * with a private key. * \param md_alg Hash algorithm used (see notes) * \param hash Hash of the message to sign * \param hash_len Hash length or 0 (see notes) @@ -447,7 +455,8 @@ int mbedtls_pk_sign_restartable( mbedtls_pk_context *ctx, /** * \brief Decrypt message (including padding if relevant). * - * \param ctx PK context to use - must hold a private key + * \param ctx The PK context to use. It must have been set up + * with a private key. * \param input Input to decrypt * \param ilen Input size * \param output Decrypted output @@ -468,7 +477,7 @@ int mbedtls_pk_decrypt( mbedtls_pk_context *ctx, /** * \brief Encrypt message (including padding if relevant). * - * \param ctx PK context to use + * \param ctx The PK context to use. It must have been set up. * \param input Message to encrypt * \param ilen Message size * \param output Encrypted output @@ -499,7 +508,7 @@ int mbedtls_pk_check_pair( const mbedtls_pk_context *pub, const mbedtls_pk_conte /** * \brief Export debug information * - * \param ctx Context to use + * \param ctx The PK context to use. It must have been initialized. * \param items Place to write debug items * * \return 0 on success or MBEDTLS_ERR_PK_BAD_INPUT_DATA @@ -509,7 +518,7 @@ int mbedtls_pk_debug( const mbedtls_pk_context *ctx, mbedtls_pk_debug_item *item /** * \brief Access the type name * - * \param ctx Context to use + * \param ctx The PK context to use. It must have been initialized. * * \return Type name on success, or "invalid PK" */ @@ -518,9 +527,10 @@ const char * mbedtls_pk_get_name( const mbedtls_pk_context *ctx ); /** * \brief Get the key type * - * \param ctx Context to use + * \param ctx The PK context to use. It must have been initialized. * - * \return Type on success, or MBEDTLS_PK_NONE + * \return Type on success. + * \return #MBEDTLS_PK_NONE for a context that has not been set up. */ mbedtls_pk_type_t mbedtls_pk_get_type( const mbedtls_pk_context *ctx ); @@ -529,7 +539,8 @@ 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 ctx The PK context to fill. It must have been initialized + * but not set up. * \param key Input buffer to parse. * The buffer must contain the input exactly, with no * extra trailing material. For PEM, the buffer must @@ -561,7 +572,8 @@ 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 ctx The PK context to fill. It must have been initialized + * but not set up. * \param key Input buffer to parse. * The buffer must contain the input exactly, with no * extra trailing material. For PEM, the buffer must @@ -586,7 +598,8 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, /** * \brief Load and parse a private key * - * \param ctx key to be initialized + * \param ctx The PK context to fill. It must have been initialized + * but not set up. * \param path filename to read the private key from * \param password Optional password to decrypt the file. * Pass \c NULL if expecting a non-encrypted key. @@ -609,7 +622,8 @@ int mbedtls_pk_parse_keyfile( mbedtls_pk_context *ctx, /** * \brief Load and parse a public key * - * \param ctx key to be initialized + * \param ctx The PK context to fill. It must have been initialized + * but not set up. * \param path filename to read the public key from * * \note On entry, ctx must be empty, either freshly initialised @@ -632,7 +646,7 @@ int mbedtls_pk_parse_public_keyfile( mbedtls_pk_context *ctx, const char *path ) * return value to determine where you should start * using the buffer * - * \param ctx private to write away + * \param ctx PK context which must contain a valid private key. * \param buf buffer to write to * \param size size of the buffer * @@ -647,7 +661,7 @@ int mbedtls_pk_write_key_der( mbedtls_pk_context *ctx, unsigned char *buf, size_ * return value to determine where you should start * using the buffer * - * \param ctx public key to write away + * \param ctx PK context which must contain a valid public or private key. * \param buf buffer to write to * \param size size of the buffer * @@ -660,7 +674,7 @@ 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 Context containing the public key to write. + * \param ctx PK context which must contain a valid public or private key. * \param buf Buffer to write to. The output includes a * terminating null byte. * \param size Size of the buffer in bytes. @@ -672,7 +686,7 @@ 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 Context containing the private key to write. + * \param ctx PK context which must contain a valid private key. * \param buf Buffer to write to. The output includes a * terminating null byte. * \param size Size of the buffer in bytes. @@ -694,7 +708,8 @@ int mbedtls_pk_write_key_pem( mbedtls_pk_context *ctx, unsigned char *buf, size_ * * \param p the position in the ASN.1 data * \param end end of the buffer - * \param pk the key to fill + * \param pk The PK context to fill. It must have been initialized + * but not set up. * * \return 0 if successful, or a specific PK error code */ @@ -709,7 +724,7 @@ int mbedtls_pk_parse_subpubkey( unsigned char **p, const unsigned char *end, * * \param p reference to current position pointer * \param start start of the buffer (for bounds-checking) - * \param key public key to write away + * \param key PK context which must contain a valid public or private key. * * \return the length written or a negative error code */ diff --git a/library/pk.c b/library/pk.c index 66301ee2d..2658627c4 100644 --- a/library/pk.c +++ b/library/pk.c @@ -183,6 +183,9 @@ int mbedtls_pk_setup_rsa_alt( mbedtls_pk_context *ctx, void * key, */ int mbedtls_pk_can_do( const mbedtls_pk_context *ctx, mbedtls_pk_type_t type ) { + /* A context with null pk_info is not set up yet and can't do anything. + * For backward compatibility, also accept NULL instead of a context + * pointer. */ if( ctx == NULL || ctx->pk_info == NULL ) return( 0 ); @@ -493,6 +496,8 @@ int mbedtls_pk_check_pair( const mbedtls_pk_context *pub, const mbedtls_pk_conte */ size_t mbedtls_pk_get_bitlen( const mbedtls_pk_context *ctx ) { + /* For backward compatibility, accept NULL or a context that + * isn't set up yet, and return a fake value that should be safe. */ if( ctx == NULL || ctx->pk_info == NULL ) return( 0 ); diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index bf3cf5dac..8a09171e6 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -90,10 +90,12 @@ void valid_parameters( ) TEST_ASSERT( mbedtls_pk_setup( &pk, NULL ) == MBEDTLS_ERR_PK_BAD_INPUT_DATA ); + /* In informational functions, we accept NULL where a context pointer + * is expected because that's what the library has done forever. + * We do not document that NULL is accepted, so we may wish to change + * the behavior in a future version. */ TEST_ASSERT( mbedtls_pk_get_bitlen( NULL ) == 0 ); - TEST_ASSERT( mbedtls_pk_get_len( NULL ) == 0 ); - TEST_ASSERT( mbedtls_pk_can_do( NULL, MBEDTLS_PK_NONE ) == 0 ); TEST_ASSERT( mbedtls_pk_sign_restartable( &pk,