From c6a7b7bebafea37fc8d9bcb2768e7da14b5e9462 Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Wed, 25 Jan 2023 10:15:05 +0100 Subject: Add notes about ignoring initialization failures on contexts Fixes #20130 Reviewed-by: Matt Caswell Reviewed-by: Dmitry Belyavskiy (Merged from https://github.com/openssl/openssl/pull/20136) (cherry picked from commit d4c5d8ff483d99f94d649fb67f1f26fce9694c92) --- doc/man3/EVP_DigestInit.pod | 32 ++++++++++++++++++++++++++++---- doc/man3/EVP_DigestSignInit.pod | 4 ++++ doc/man3/EVP_DigestVerifyInit.pod | 4 ++++ doc/man3/EVP_EncryptInit.pod | 26 ++++++++++++++++++++++---- 4 files changed, 58 insertions(+), 8 deletions(-) diff --git a/doc/man3/EVP_DigestInit.pod b/doc/man3/EVP_DigestInit.pod index dd04ff54cd..c720c196ea 100644 --- a/doc/man3/EVP_DigestInit.pod +++ b/doc/man3/EVP_DigestInit.pod @@ -573,6 +573,7 @@ Returns 1 for success or 0 for failure. EVP_Digest(), EVP_DigestInit_ex2(), EVP_DigestInit_ex(), +EVP_DigestInit(), EVP_DigestUpdate(), EVP_DigestFinal_ex(), EVP_DigestFinalXOF(), and @@ -651,6 +652,13 @@ are still in common use. For most applications the I parameter to EVP_DigestInit_ex() will be set to NULL to use the default digest implementation. +Ignoring failure returns of EVP_DigestInit_ex(), EVP_DigestInit_ex2(), or +EVP_DigestInit() can lead to undefined behavior on subsequent calls +updating or finalizing the B such as the EVP_DigestUpdate() or +EVP_DigestFinal() functions. The only valid calls on the B +when initialization fails are calls that attempt another initialization of +the context or release the context. + The functions EVP_DigestInit(), EVP_DigestFinal() and EVP_MD_CTX_copy() are obsolete but are retained to maintain compatibility with existing code. New applications should use EVP_DigestInit_ex(), EVP_DigestFinal_ex() and @@ -698,10 +706,26 @@ digest name passed on the command line. } mdctx = EVP_MD_CTX_new(); - EVP_DigestInit_ex2(mdctx, md, NULL); - EVP_DigestUpdate(mdctx, mess1, strlen(mess1)); - EVP_DigestUpdate(mdctx, mess2, strlen(mess2)); - EVP_DigestFinal_ex(mdctx, md_value, &md_len); + if (!EVP_DigestInit_ex2(mdctx, md, NULL)) { + printf("Message digest initialization failed.\n"); + EVP_MD_CTX_free(mdctx); + exit(1); + } + if (!EVP_DigestUpdate(mdctx, mess1, strlen(mess1))) { + printf("Message digest update failed.\n"); + EVP_MD_CTX_free(mdctx); + exit(1); + } + if (!EVP_DigestUpdate(mdctx, mess2, strlen(mess2))) { + printf("Message digest update failed.\n"); + EVP_MD_CTX_free(mdctx); + exit(1); + } + if (!EVP_DigestFinal_ex(mdctx, md_value, &md_len)) { + printf("Message digest finalization failed.\n"); + EVP_MD_CTX_free(mdctx); + exit(1); + } EVP_MD_CTX_free(mdctx); printf("Digest is: "); diff --git a/doc/man3/EVP_DigestSignInit.pod b/doc/man3/EVP_DigestSignInit.pod index fd2a64999e..7506381633 100644 --- a/doc/man3/EVP_DigestSignInit.pod +++ b/doc/man3/EVP_DigestSignInit.pod @@ -172,6 +172,10 @@ multiple times on a context and the parameters set by previous calls should be preserved if the I parameter is NULL. The call then just resets the state of the I. +Ignoring failure returns of EVP_DigestSignInit() and EVP_DigestSignInit_ex() +functions can lead to subsequent undefined behavior when calling +EVP_DigestSignUpdate(), EVP_DigestSignFinal(), or EVP_DigestSign(). + The use of EVP_PKEY_get_size() with these functions is discouraged because some signature operations may have a signature length which depends on the parameters set. As a result EVP_PKEY_get_size() would have to return a value diff --git a/doc/man3/EVP_DigestVerifyInit.pod b/doc/man3/EVP_DigestVerifyInit.pod index 7498e2f852..cb454835ac 100644 --- a/doc/man3/EVP_DigestVerifyInit.pod +++ b/doc/man3/EVP_DigestVerifyInit.pod @@ -161,6 +161,10 @@ multiple times on a context and the parameters set by previous calls should be preserved if the I parameter is NULL. The call then just resets the state of the I. +Ignoring failure returns of EVP_DigestVerifyInit() and EVP_DigestVerifyInit_ex() +functions can lead to subsequent undefined behavior when calling +EVP_DigestVerifyUpdate(), EVP_DigestVerifyFinal(), or EVP_DigestVerify(). + =head1 SEE ALSO L, diff --git a/doc/man3/EVP_EncryptInit.pod b/doc/man3/EVP_EncryptInit.pod index 3a6b326a1b..63b42ddc51 100644 --- a/doc/man3/EVP_EncryptInit.pod +++ b/doc/man3/EVP_EncryptInit.pod @@ -1500,6 +1500,12 @@ removed, and it is especially important for the B flag treated specially in EVP_CipherInit_ex(). +Ignoring failure returns of the B initialization functions can +lead to subsequent undefined behavior when calling the functions that update or +finalize the context. The only valid calls on the B when +initialization fails are calls that attempt another initialization of the +context or release the context. + EVP_get_cipherbynid(), and EVP_get_cipherbyobj() are implemented as macros. =head1 BUGS @@ -1532,7 +1538,11 @@ Encrypt a string using IDEA: FILE *out; ctx = EVP_CIPHER_CTX_new(); - EVP_EncryptInit_ex2(ctx, EVP_idea_cbc(), key, iv, NULL); + if (!EVP_EncryptInit_ex2(ctx, EVP_idea_cbc(), key, iv, NULL)) { + /* Error */ + EVP_CIPHER_CTX_free(ctx); + return 0; + } if (!EVP_EncryptUpdate(ctx, outbuf, &outlen, intext, strlen(intext))) { /* Error */ @@ -1590,13 +1600,21 @@ with a 128-bit key: /* Don't set key or IV right away; we want to check lengths */ ctx = EVP_CIPHER_CTX_new(); - EVP_CipherInit_ex2(ctx, EVP_aes_128_cbc(), NULL, NULL, - do_encrypt, NULL); + if (!EVP_CipherInit_ex2(ctx, EVP_aes_128_cbc(), NULL, NULL, + do_encrypt, NULL)) { + /* Error */ + EVP_CIPHER_CTX_free(ctx); + return 0; + } OPENSSL_assert(EVP_CIPHER_CTX_get_key_length(ctx) == 16); OPENSSL_assert(EVP_CIPHER_CTX_get_iv_length(ctx) == 16); /* Now we can set key and IV */ - EVP_CipherInit_ex2(ctx, NULL, key, iv, do_encrypt, NULL); + if (!EVP_CipherInit_ex2(ctx, NULL, key, iv, do_encrypt, NULL)) { + /* Error */ + EVP_CIPHER_CTX_free(ctx); + return 0; + } for (;;) { inlen = fread(inbuf, 1, 1024, in); -- cgit v1.2.3