From: Guy Harris Date: Fri, 1 Sep 2017 11:00:38 +0000 (-0700) Subject: Make the ESP decryption not crash with OpenSSL 1.1. X-Git-Tag: tcpdump-4.99-bp~2006 X-Git-Url: https://git.tcpdump.org/tcpdump/commitdiff_plain/6f0750ee0c5e19c6c2586d4d3ffa897853b8341b?ds=inline Make the ESP decryption not crash with OpenSSL 1.1. While we're at it, free the cipher context if we fail to allocate the output buffer for decryption. --- diff --git a/config.h.in b/config.h.in index 6adf589f..db5227e9 100644 --- a/config.h.in +++ b/config.h.in @@ -37,6 +37,9 @@ /* Define to 1 if you have the `ether_ntohost' function. */ #undef HAVE_ETHER_NTOHOST +/* Define to 1 if you have the `EVP_CipherInit_ex' function. */ +#undef HAVE_EVP_CIPHERINIT_EX + /* Define to 1 if you have the `EVP_CIPHER_CTX_new' function. */ #undef HAVE_EVP_CIPHER_CTX_NEW diff --git a/configure b/configure index 64b05e0e..e24e38f8 100755 --- a/configure +++ b/configure @@ -8134,17 +8134,32 @@ fi done # - # OK, do we have EVP_CIPHER_CTX_new? + # OK, then: + # + # 1) do we have EVP_CIPHER_CTX_new? # If so, we use it to allocate an # EVP_CIPHER_CTX, as EVP_CIPHER_CTX may be # opaque; otherwise, we allocate it ourselves. # - for ac_func in EVP_CIPHER_CTX_new + # 2) do we have EVP_CipherInit_ex()? + # If so, we use it, because we need to be + # able to make two "initialize the cipher" + # calls, one with the cipher and key, and + # one with the IV, and, as of OpenSSL 1.1, + # You Can't Do That with EVP_CipherInit(), + # because a call to EVP_CipherInit() will + # unconditionally clear the context, and + # if you don't supply a cipher, it'll + # clear the cipher, rendering the context + # unusable and causing a crash. + # + for ac_func in EVP_CIPHER_CTX_new EVP_CipherInit_ex do : - ac_fn_c_check_func "$LINENO" "EVP_CIPHER_CTX_new" "ac_cv_func_EVP_CIPHER_CTX_new" -if test "x$ac_cv_func_EVP_CIPHER_CTX_new" = xyes; then : + as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh` +ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var" +if eval test \"x\$"$as_ac_var"\" = x"yes"; then : cat >>confdefs.h <<_ACEOF -#define HAVE_EVP_CIPHER_CTX_NEW 1 +#define `$as_echo "HAVE_$ac_func" | $as_tr_cpp` 1 _ACEOF fi diff --git a/configure.in b/configure.in index b342468e..8681daa5 100644 --- a/configure.in +++ b/configure.in @@ -940,12 +940,26 @@ if test "$want_libcrypto" != "no"; then if test "$ac_cv_lib_crypto_DES_cbc_encrypt" = "yes"; then AC_CHECK_HEADERS(openssl/evp.h) # - # OK, do we have EVP_CIPHER_CTX_new? + # OK, then: + # + # 1) do we have EVP_CIPHER_CTX_new? # If so, we use it to allocate an # EVP_CIPHER_CTX, as EVP_CIPHER_CTX may be # opaque; otherwise, we allocate it ourselves. # - AC_CHECK_FUNCS(EVP_CIPHER_CTX_new) + # 2) do we have EVP_CipherInit_ex()? + # If so, we use it, because we need to be + # able to make two "initialize the cipher" + # calls, one with the cipher and key, and + # one with the IV, and, as of OpenSSL 1.1, + # You Can't Do That with EVP_CipherInit(), + # because a call to EVP_CipherInit() will + # unconditionally clear the context, and + # if you don't supply a cipher, it'll + # clear the cipher, rendering the context + # unusable and causing a crash. + # + AC_CHECK_FUNCS(EVP_CIPHER_CTX_new EVP_CipherInit_ex) fi ]) fi diff --git a/print-esp.c b/print-esp.c index f2b4ab3c..d12b97da 100644 --- a/print-esp.c +++ b/print-esp.c @@ -145,6 +145,39 @@ EVP_CIPHER_CTX_free(EVP_CIPHER_CTX *ctx) } #endif +#ifdef HAVE_EVP_CIPHERINIT_EX +/* + * Initialize the cipher by calling EVP_CipherInit_ex(), because + * calling EVP_CipherInit() will reset the cipher context, clearing + * the cipher, so calling it twice, with the second call having a + * null cipher, will clear the already-set cipher. EVP_CipherInit_ex(), + * however, won't reset the cipher context, so you can use it to specify + * the IV oin a second call after a first call to EVP_CipherInit_ex() + * to set the cipher and the key. + * + * XXX - is there some reason why we need to make two calls? + */ +static int +set_cipher_parameters(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *cipher, + const unsigned char *key, + const unsigned char *iv, int enc) +{ + return EVP_CipherInit_ex(ctx, cipher, NULL, key, iv, enc); +} +#else +/* + * Initialize the cipher by calling EVP_CipherInit(), because we don't + * have EVP_CipherInit_ex(); we rely on it not trashing the context. + */ +static int +set_cipher_parameters(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *cipher, + const unsigned char *key, + const unsigned char *iv, int enc) +{ + return EVP_CipherInit(ctx, cipher, key, iv, enc); +} +#endif + /* * this will adjust ndo_packetp and ndo_snapend to new buffer! */ @@ -190,9 +223,9 @@ int esp_print_decrypt_buffer_by_ikev2(netdissect_options *ndo, ctx = EVP_CIPHER_CTX_new(); if (ctx == NULL) return 0; - if (EVP_CipherInit(ctx, sa->evp, sa->secret, NULL, 0) < 0) + if (set_cipher_parameters(ctx, sa->evp, sa->secret, NULL, 0) < 0) (*ndo->ndo_warning)(ndo, "espkey init failed"); - EVP_CipherInit(ctx, NULL, NULL, iv, 0); + set_cipher_parameters(ctx, NULL, NULL, iv, 0); /* * Allocate a buffer for the decrypted data. * The output buffer must be separate from the input buffer, and @@ -203,6 +236,7 @@ int esp_print_decrypt_buffer_by_ikev2(netdissect_options *ndo, output_buffer = (u_char *)malloc(output_buffer_size); if (output_buffer == NULL) { (*ndo->ndo_warning)(ndo, "can't allocate memory for decryption buffer"); + EVP_CIPHER_CTX_free(ctx); return 0; } EVP_Cipher(ctx, output_buffer, buf, len); @@ -735,11 +769,11 @@ esp_print(netdissect_options *ndo, if (sa->evp) { ctx = EVP_CIPHER_CTX_new(); if (ctx != NULL) { - if (EVP_CipherInit(ctx, sa->evp, secret, NULL, 0) < 0) + if (set_cipher_parameters(ctx, sa->evp, secret, NULL, 0) < 0) (*ndo->ndo_warning)(ndo, "espkey init failed"); p = ivoff; - EVP_CipherInit(ctx, NULL, NULL, p, 0); + set_cipher_parameters(ctx, NULL, NULL, p, 0); len = ep - (p + ivlen); /* @@ -753,6 +787,7 @@ esp_print(netdissect_options *ndo, output_buffer = (u_char *)malloc(output_buffer_size); if (output_buffer == NULL) { (*ndo->ndo_warning)(ndo, "can't allocate memory for decryption buffer"); + EVP_CIPHER_CTX_free(ctx); return -1; }