src: bump OpenSSL, EKU check

README.md

@@ -49,7 +49,7 @@ complete.
 ## Building
 
 *μthenticode* depends on [pe-parse](https://github.com/trailofbits/pe-parse)
-and OpenSSL 1.1.0, which are installed via `vcpkg` by following these steps:
+and OpenSSL 3.0 or higher, which are installed via `vcpkg` by following these steps:
 
 ```bash
 cmake -B build -S . -DCMAKE_TOOLCHAIN_FILE=<vcpkg-path>/scripts/buildsystems/vcpkg.cmake

src/CMakeLists.txt

@@ -4,7 +4,7 @@ project(uthenticode)
 
 find_package(pe-parse REQUIRED)
 find_package(
-  OpenSSL 1.1
+  OpenSSL 3.0
   COMPONENTS Crypto
   REQUIRED
 )

src/uthenticode.cpp

@@ -3,6 +3,7 @@
 #include <openssl/crypto.h>
 #include <openssl/err.h>
 #include <openssl/x509.h>
+#include <openssl/x509v3.h>
 
 #include <algorithm>
 #include <array>
@@ -209,6 +210,32 @@ bool SignedData::verify_signature() const {
     return false;
   }
 
+  auto *signers_stack_ptr = PKCS7_get0_signers(p7_, nullptr, 0);
+  if (signers_stack_ptr == nullptr) {
+    return false;
+  }
+  auto signers_stack = impl::STACK_OF_X509_ptr(signers_stack_ptr, impl::SK_X509_free);
+
+  /* NOTE(ww): Authenticode specification, page 13: the signer must have the
+   * codeSigning EKU, **or** no member of the signer's chain may have it.
+   *
+   * The check below is more strict than that: **every** signer must have
+   * the codeSigning EKU, and we don't check the embedded chain (since
+   * we can't do full chain verification anyways).
+   */
+  for (auto i = 0; i < sk_X509_num(signers_stack.get()); ++i) {
+    auto *signer = sk_X509_value(signers_stack.get(), i);
+
+    /* NOTE(ww): Ths should really be X509_check_purpose with
+     * X509_PURPOSE_CODE_SIGN, but this is inexplicably not present
+     * in even the latest releases of OpenSSL as of 2023-05.
+     */
+    auto xku_flags = X509_get_extended_key_usage(signer);
+    if (!(xku_flags & XKU_CODE_SIGN)) {
+      return false;
+    }
+  }
+
   /* NOTE(ww): What happens below is a bit dumb: we convert our SpcIndirectDataContent back
    * into DER form so that we can unwrap its ASN.1 sequence and pass the underlying data
    * to PKCS7_verify for verification. This displays our intent a little more clearly than

test/helpers.h

@@ -95,3 +95,19 @@ class AuthNest32PlusTest : public ::testing::Test {
 
   peparse::parsed_pe *pe{nullptr};
 };
+
+class MissingEKUTest : public ::testing::Test {
+ protected:
+  void SetUp() override {
+    auto *file = UTHENTICODE_TEST_ASSETS "/A_SSLippery_Slope.exe";
+
+    pe = peparse::ParsePEFromFile(file);
+    ASSERT_TRUE(pe != nullptr);
+  }
+
+  void TearDown() override {
+    peparse::DestructParsedPE(pe);
+  }
+
+  peparse::parsed_pe *pe{nullptr};
+};

test/signeddata-test.cpp

@@ -116,3 +116,10 @@ TEST_F(AuthNest32PlusTest, SignedData_properties_nested) {
   ASSERT_STRCASEEQ(checksumstr.c_str(),
                    "ddc5b39c4292120745eb86a67eaa331032cc05a0dafaf6e28ec9aa0f189c408d");
 }
+
+TEST_F(MissingEKUTest, SignedData_missing_codesigning_EKU) {
+  auto certs = uthenticode::read_certs(pe);
+  auto signed_data = certs[0].as_signed_data();
+
+  ASSERT_FALSE(signed_data->verify_signature());
+}