From e9cac0e277dfe83c455a8f0a36853c69b36d9360 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 27 May 2021 12:59:11 +0200 Subject: [PATCH 1/5] Fix inconsistent documentation of cipher_setup() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - the \internal note said that calling cipher_init() first would be made mandatory later, but the documention of the ctx parameter already said the context had to be initialized... - the documentation was using the word initialize for two different meanings (calling setup() vs calling init()), making the documentation of the ctx parameter quite confusing (you must initialize before you can initialize...) Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/cipher.h | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/include/mbedtls/cipher.h b/include/mbedtls/cipher.h index ffb20a676..36efdee75 100644 --- a/include/mbedtls/cipher.h +++ b/include/mbedtls/cipher.h @@ -437,10 +437,11 @@ void mbedtls_cipher_free( mbedtls_cipher_context_t *ctx ); /** - * \brief This function initializes a cipher context for + * \brief This function prepares a cipher context for * use with the given cipher primitive. * - * \param ctx The context to initialize. This must be initialized. + * \param ctx The context to prepare. This must be initialized by + * a call to mbedtls_cipher_init() first. * \param cipher_info The cipher to use. * * \return \c 0 on success. @@ -448,10 +449,6 @@ void mbedtls_cipher_free( mbedtls_cipher_context_t *ctx ); * parameter-verification failure. * \return #MBEDTLS_ERR_CIPHER_ALLOC_FAILED if allocation of the * cipher-specific context fails. - * - * \internal Currently, the function also clears the structure. - * In future versions, the caller will be required to call - * mbedtls_cipher_init() on the structure first. */ int mbedtls_cipher_setup( mbedtls_cipher_context_t *ctx, const mbedtls_cipher_info_t *cipher_info ); From c42a0be00b7e7ac0a705caadf263d3b5a4a39113 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 31 May 2021 11:13:35 +0200 Subject: [PATCH 2/5] Clarify calling sequence in the Cipher layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/cipher.h | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/cipher.h b/include/mbedtls/cipher.h index 36efdee75..679664450 100644 --- a/include/mbedtls/cipher.h +++ b/include/mbedtls/cipher.h @@ -440,6 +440,18 @@ void mbedtls_cipher_free( mbedtls_cipher_context_t *ctx ); * \brief This function prepares a cipher context for * use with the given cipher primitive. * + * \note After calling this function, you should call + * mbedtls_cipher_setkey() and, if the mode uses padding, + * mbedtls_cipher_set_padding_mode(), then for each + * message to encrypt or decrypt with this key, either: + * - mbedtls_cipher_crypt() for one-shot processing with + * non-AEAD modes; + * - mbedtls_cipher_auth_encrypt_ext() or + * mbedtls_cipher_auth_decrypt_ext() for one-shot + * processing with AEAD modes or NIST_KW; + * - for multi-part processing, see the documentation of + * mbedtls_cipher_reset(). + * * \param ctx The context to prepare. This must be initialized by * a call to mbedtls_cipher_init() first. * \param cipher_info The cipher to use. @@ -684,7 +696,30 @@ int mbedtls_cipher_set_iv( mbedtls_cipher_context_t *ctx, /** * \brief This function resets the cipher state. * - * \param ctx The generic cipher context. This must be initialized. + * \note With non-AEAD ciphers, the order of calls for each message + * is as follows: + * 1. mbedtls_cipher_set_iv() if the mode uses an IV/nonce. + * 2. mbedtls_cipher_reset() + * 3. mbedtls_cipher_update() one or more times + * 4. mbedtls_cipher_finish() + * . + * This sequence can be repeated to encrypt of decrypt multiple + * messages with the same key. + * + * \note With AEAD ciphers, the order of calls for each message + * is as follows: + * 1. mbedtls_cipher_set_iv() if the mode uses an IV/nonce. + * 2. mbedtls_cipher_reset() + * 3. mbedtls_cipher_update_ad() + * 4. mbedtls_cipher_update() one or more times + * 5. mbedtls_cipher_finish() + * 6. mbedtls_cipher_check_tag() (for decryption) or + * mbedtls_cipher_write_tag() (for encryption). + * . + * This sequence can be repeated to encrypt of decrypt multiple + * messages with the same key. + * + * \param ctx The generic cipher context. This must be bound to a key. * * \return \c 0 on success. * \return #MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA on From 42eba1a2742d23f45049d4612ebb157fdecbc10d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 31 May 2021 12:14:02 +0200 Subject: [PATCH 3/5] Fix a typo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/cipher.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/cipher.h b/include/mbedtls/cipher.h index 679664450..06a29e752 100644 --- a/include/mbedtls/cipher.h +++ b/include/mbedtls/cipher.h @@ -703,7 +703,7 @@ int mbedtls_cipher_set_iv( mbedtls_cipher_context_t *ctx, * 3. mbedtls_cipher_update() one or more times * 4. mbedtls_cipher_finish() * . - * This sequence can be repeated to encrypt of decrypt multiple + * This sequence can be repeated to encrypt or decrypt multiple * messages with the same key. * * \note With AEAD ciphers, the order of calls for each message @@ -716,7 +716,7 @@ int mbedtls_cipher_set_iv( mbedtls_cipher_context_t *ctx, * 6. mbedtls_cipher_check_tag() (for decryption) or * mbedtls_cipher_write_tag() (for encryption). * . - * This sequence can be repeated to encrypt of decrypt multiple + * This sequence can be repeated to encrypt or decrypt multiple * messages with the same key. * * \param ctx The generic cipher context. This must be bound to a key. From ee57ebe5530b5d665bd6c069f837f8798050098a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 31 May 2021 12:25:01 +0200 Subject: [PATCH 4/5] Add ChangeLog and migration guide entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- ChangeLog.d/cipher-delayed-output.txt | 6 ++++++ .../cipher-delayed-output.md | 15 +++++++++++++++ 2 files changed, 21 insertions(+) create mode 100644 ChangeLog.d/cipher-delayed-output.txt create mode 100644 docs/3.0-migration-guide.d/cipher-delayed-output.md diff --git a/ChangeLog.d/cipher-delayed-output.txt b/ChangeLog.d/cipher-delayed-output.txt new file mode 100644 index 000000000..0c8f7db99 --- /dev/null +++ b/ChangeLog.d/cipher-delayed-output.txt @@ -0,0 +1,6 @@ +API changes + * For multi-part AEAD operations with the Cipher module, calling + mbedtls_cipher_finish() is now mandatory. Previously the documentation + was unclear on this point, and this function happened to never do + anything with the currently implemented AEADs, so in practice is was + possible to skip calling it, which is no longer supported. diff --git a/docs/3.0-migration-guide.d/cipher-delayed-output.md b/docs/3.0-migration-guide.d/cipher-delayed-output.md new file mode 100644 index 000000000..e376880b8 --- /dev/null +++ b/docs/3.0-migration-guide.d/cipher-delayed-output.md @@ -0,0 +1,15 @@ +Calling `mbedtls_cipher_finish()` is mandatory for all multi-part operations +---------------------------------------------------------------------------- + +This only affect people who use the Cipher module to perform AEAD operations +using the multi-part API. + +Previously, the documentation didn't state explicitly if it was OK to call +`mbedtls_cipher_check_tag()` or `mbedtls_cipher_write_tag()` directly after +the last call to `mbedtls_cipher_update()` - that is, without calling +`mbedtls_cipher_finish()` in-between. If you code was missing that call, +please add it and be prepared to get as much as 15 bytes of output. + +Currently the output is always 0 bytes, but it may be more when alternative +implementations of the underlying primitives are in use, or with future +versions of the library. From c01b87b820260787cd60fd39600124b29bbc5533 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 1 Jun 2021 09:40:53 +0200 Subject: [PATCH 5/5] Fix some typos MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- ChangeLog.d/cipher-delayed-output.txt | 4 ++-- docs/3.0-migration-guide.d/cipher-delayed-output.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ChangeLog.d/cipher-delayed-output.txt b/ChangeLog.d/cipher-delayed-output.txt index 0c8f7db99..4ca3a0cc0 100644 --- a/ChangeLog.d/cipher-delayed-output.txt +++ b/ChangeLog.d/cipher-delayed-output.txt @@ -1,6 +1,6 @@ API changes - * For multi-part AEAD operations with the Cipher module, calling + * For multi-part AEAD operations with the cipher module, calling mbedtls_cipher_finish() is now mandatory. Previously the documentation was unclear on this point, and this function happened to never do - anything with the currently implemented AEADs, so in practice is was + anything with the currently implemented AEADs, so in practice it was possible to skip calling it, which is no longer supported. diff --git a/docs/3.0-migration-guide.d/cipher-delayed-output.md b/docs/3.0-migration-guide.d/cipher-delayed-output.md index e376880b8..18d327152 100644 --- a/docs/3.0-migration-guide.d/cipher-delayed-output.md +++ b/docs/3.0-migration-guide.d/cipher-delayed-output.md @@ -1,7 +1,7 @@ Calling `mbedtls_cipher_finish()` is mandatory for all multi-part operations ---------------------------------------------------------------------------- -This only affect people who use the Cipher module to perform AEAD operations +This only affects people who use the cipher module to perform AEAD operations using the multi-part API. Previously, the documentation didn't state explicitly if it was OK to call