From d3cd6599088c01b8b7ddbf2d912575ea628c8608 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 11 Jun 2024 08:36:11 -0500 Subject: [PATCH 1/5] Update magick plugin to OpenSSL 1.1.1 API The functions `EVP_MD_CTX_create` and `EVP_MD_CTX_destroy` were renamed to `EVP_MD_CTX_new` and `EVP_MD_CTX_free` in OpenSSL 1.1.0. This renames them in the magick plugin accordingly. This also replaces the individual calls to `EVP_DigestVerifyUpdate` and `EVP_DigestVerifyFinal` with a single call to `EVP_DigestVerify` which is available in OpenSSL 1.1.1. --- plugins/experimental/magick/magick.cc | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/plugins/experimental/magick/magick.cc b/plugins/experimental/magick/magick.cc index 968b0b1889e..d87cbf3db61 100644 --- a/plugins/experimental/magick/magick.cc +++ b/plugins/experimental/magick/magick.cc @@ -176,10 +176,10 @@ struct EVPContext { ~EVPContext() { assert(nullptr != context); - EVP_MD_CTX_destroy(context); + EVP_MD_CTX_free(context); } - EVPContext() : context(EVP_MD_CTX_create()) { assert(nullptr != context); } + EVPContext() : context(EVP_MD_CTX_new()) { assert(nullptr != context); } }; struct EVPKey { @@ -226,19 +226,17 @@ verify(const byte *const msg, const size_t mlen, const byte *const sig, const si } } - { - const int rc = EVP_DigestVerifyUpdate(evp.context, msg, mlen); - assert(1 == rc); - if (1 != rc) { - return false; - } - } - ERR_clear_error(); { - const int rc = EVP_DigestVerifyFinal(evp.context, sig, slen); - return 1 == rc; + const int rc = EVP_DigestVerify(evp.context, sig, slen, msg, mlen); + if (1 == rc) { + return true; + } else if (0 == rc) { + return false; + } else { + return false; + } } return false; From 5b9caac1a9f1f8d4f11a43484211fe15f37be25c Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 11 Jun 2024 12:08:52 -0500 Subject: [PATCH 2/5] Add debug logging for signature verification --- plugins/experimental/magick/magick.cc | 44 +++++++++++++++------------ 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/plugins/experimental/magick/magick.cc b/plugins/experimental/magick/magick.cc index d87cbf3db61..828567d26ed 100644 --- a/plugins/experimental/magick/magick.cc +++ b/plugins/experimental/magick/magick.cc @@ -203,6 +203,20 @@ struct EVPKey { } }; +/** Remove the last error from this thread's error queue and print it. + */ +static void +ssl_error() +{ + if (unsigned long error_code{ERR_get_error()}; 0 != error_code) { + if (char const *reason{ERR_lib_error_string(error_code)}; NULL == reason) { + Dbg(dbg_ctl, "SSL error: error code %lu", error_code); + } else { + Dbg(dbg_ctl, "SSL error: %s", reason); + } + } +} + bool verify(const byte *const msg, const size_t mlen, const byte *const sig, const size_t slen, EVP_PKEY *const pkey) { @@ -218,28 +232,20 @@ verify(const byte *const msg, const size_t mlen, const byte *const sig, const si EVPContext evp; - { - const int rc = EVP_DigestVerifyInit(evp.context, nullptr, EVP_sha256(), nullptr, pkey); - assert(1 == rc); - if (1 != rc) { - return false; - } + if (const int rc = EVP_DigestVerifyInit(evp.context, nullptr, EVP_sha256(), nullptr, pkey); 1 != rc) { + ssl_error(); + return false; } - ERR_clear_error(); - - { - const int rc = EVP_DigestVerify(evp.context, sig, slen, msg, mlen); - if (1 == rc) { - return true; - } else if (0 == rc) { - return false; - } else { - return false; - } + const int rc = EVP_DigestVerify(evp.context, sig, slen, msg, mlen); + if (1 == rc) { + return true; + } else if (0 == rc) { + return false; + } else { + ssl_error(); + return false; } - - return false; } struct Exception { From 32c90352d38547f35f0c1dbda14ddec668bde59a Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 11 Jun 2024 12:52:34 -0500 Subject: [PATCH 3/5] Fix API call to get reason instead of library --- plugins/experimental/magick/magick.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/experimental/magick/magick.cc b/plugins/experimental/magick/magick.cc index 828567d26ed..8604faa1752 100644 --- a/plugins/experimental/magick/magick.cc +++ b/plugins/experimental/magick/magick.cc @@ -209,7 +209,7 @@ static void ssl_error() { if (unsigned long error_code{ERR_get_error()}; 0 != error_code) { - if (char const *reason{ERR_lib_error_string(error_code)}; NULL == reason) { + if (char const *reason{ERR_reason_error_string(error_code)}; NULL == reason) { Dbg(dbg_ctl, "SSL error: error code %lu", error_code); } else { Dbg(dbg_ctl, "SSL error: %s", reason); @@ -237,7 +237,7 @@ verify(const byte *const msg, const size_t mlen, const byte *const sig, const si return false; } - const int rc = EVP_DigestVerify(evp.context, sig, slen, msg, mlen); + if (const int rc = EVP_DigestVerify(evp.context, sig, slen, msg, mlen); 1 == rc) { if (1 == rc) { return true; } else if (0 == rc) { From 3acecd9bfdadad037e9d9ab207b852fd55dec6ad Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 11 Jun 2024 12:54:06 -0500 Subject: [PATCH 4/5] Fix unintentional working directory changes --- plugins/experimental/magick/magick.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/experimental/magick/magick.cc b/plugins/experimental/magick/magick.cc index 8604faa1752..d53395f95a8 100644 --- a/plugins/experimental/magick/magick.cc +++ b/plugins/experimental/magick/magick.cc @@ -237,7 +237,7 @@ verify(const byte *const msg, const size_t mlen, const byte *const sig, const si return false; } - if (const int rc = EVP_DigestVerify(evp.context, sig, slen, msg, mlen); 1 == rc) { + const int rc = EVP_DigestVerify(evp.context, sig, slen, msg, mlen); if (1 == rc) { return true; } else if (0 == rc) { From 80bc025142c50ea5b533d88346763dcf1716efdb Mon Sep 17 00:00:00 2001 From: JosiahWI <41302989+JosiahWI@users.noreply.github.com> Date: Tue, 11 Jun 2024 20:02:36 -0500 Subject: [PATCH 5/5] Collapse EVP_DigestVerify failures into one case --- plugins/experimental/magick/magick.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/plugins/experimental/magick/magick.cc b/plugins/experimental/magick/magick.cc index d53395f95a8..09de27ca23b 100644 --- a/plugins/experimental/magick/magick.cc +++ b/plugins/experimental/magick/magick.cc @@ -237,12 +237,13 @@ verify(const byte *const msg, const size_t mlen, const byte *const sig, const si return false; } - const int rc = EVP_DigestVerify(evp.context, sig, slen, msg, mlen); - if (1 == rc) { + if (const int rc = EVP_DigestVerify(evp.context, sig, slen, msg, mlen); 1 == rc) { return true; - } else if (0 == rc) { - return false; } else { + // The OpenSSL 1.1.1 API distinguishes between a verification failure and a + // more serious error with a specific return value for the former, but we + // have collapsed them into one case because we don't do any special + // handling for serious errors. ssl_error(); return false; }