mtrojnar / osslsigncode

OpenSSL based Authenticode signing for PE/MSI/Java CAB files
Other
731 stars 124 forks source link

Segmentation Fault when signing with hardware token pkcs11 #316

Closed imaGuru closed 3 months ago

imaGuru commented 8 months ago

Result of my investigation in OpenSC/libp11#520

The segmentation fault occurs because in function read_token there is a call to ENGINE_finish (line 3456) right after finding keys/certificates in the hardware token. This frees up all memory associated with the pkcs11 session and in turn causes a segmentation fault when later accessing session information to sign the requested file.

As a workaround we can remove the ENGINE_finish line from read_token and add this cleanup code at the end of main:

    if((options.p11engine) || (options.p11module)){
        ENGINE* engine = engine_pkcs11();
        if(!engine)
            engine = engine_dynamic(&options);
        if(engine){
            ENGINE_finish(engine);
        }
    }

This fixes the problem but is not very pretty. Please let me know how you want it handled and I can submit a PR.

git-lul commented 7 months ago

Just wanted to add that I also encountered this issue on windows and linux. Suggested fix worked for me.

mtrojnar commented 7 months ago

I tried hard to reproduce this issue using the sc30pkcs11-3.0.5.60-MS.so PKCS#11 module and the GitHub master versions of osslsigncode and libp11, but I failed. Can you try executing "sudo make install" in libp11 to make sure that it installs the pkcs11 in you OpenSSL's default engine directory?

I hoped I fixed this issue back in 2017: https://github.com/OpenSC/libp11/commit/2372b0fdc513fb4625ec0c55135aa3ae88cba69e

joostn commented 7 months ago

I also ran into this issue and imaGuru's fix worked. I'm running Arch linux with libp11 https://archlinux.org/packages/extra/x86_64/libp11/

$ devel/build/sysroots/windows/tools/osslsigncode/bin/osslsigncode \
  -pkcs11engine /usr/lib/engines-3/pkcs11.so \
  -pkcs11module devel/build/sysroots/windows/safenet/usr/lib/libeToken.so \
  -pkcs11cert "pkcs11:model=ID%20Prime%20MD;manufacturer=Gemalto;REDACTEDobject=REDACTED;type=cert" \
  -key "pkcs11:model=ID%20Prime%20MD;manufacturer=Gemalto;REDACTEDtype=public" \
  -in "myapp.exe"   \
  -out "myapp_signed.exe" \
  -readpass /home/joostn/safenetpass.txt
* thread #1, name = 'osslsigncode', stop reason = signal SIGSEGV: invalid address (fault address: 0x0)
    frame #0: 0x00007ffff7f97d6a pkcs11.so`___lldb_unnamed_symbol187 + 58
pkcs11.so`___lldb_unnamed_symbol187:
->  0x7ffff7f97d6a <+58>: movq   (%rdi), %rax
    0x7ffff7f97d6d <+61>: movq   %rcx, 0x20(%rsp)
    0x7ffff7f97d72 <+66>: movq   %rbx, %rdi
    0x7ffff7f97d75 <+69>: movq   $0x0, 0x18(%rsp)
(lldb) bt
* thread #1, name = 'osslsigncode', stop reason = signal SIGSEGV: invalid address (fault address: 0x0)
  * frame #0: 0x00007ffff7f97d6a pkcs11.so`___lldb_unnamed_symbol187 + 58
    frame #1: 0x00007ffff7f98501 pkcs11.so`___lldb_unnamed_symbol194 + 49
    frame #2: 0x00007ffff7f9df6a pkcs11.so`___lldb_unnamed_symbol235 + 138
    frame #3: 0x00007ffff7f9e287 pkcs11.so`___lldb_unnamed_symbol236 + 23
    frame #4: 0x00007ffff7f9c132 pkcs11.so`___lldb_unnamed_symbol219 + 130
    frame #5: 0x00007ffff7f9c1cb pkcs11.so`___lldb_unnamed_symbol220 + 107
    frame #6: 0x00007ffff7bed623 libcrypto.so.3`RSA_sign + 211
    frame #7: 0x00007ffff7bef8bd libcrypto.so.3`___lldb_unnamed_symbol7448 + 573
    frame #8: 0x00007ffff7b8d388 libcrypto.so.3`EVP_DigestSignFinal + 616
    frame #9: 0x00007ffff7bd3dc4 libcrypto.so.3`PKCS7_SIGNER_INFO_sign + 356
    frame #10: 0x00007ffff7bd47c8 libcrypto.so.3`PKCS7_dataFinal + 264
    frame #11: 0x0000555555569b64 osslsigncode`pkcs7_sign_content + 137
    frame #12: 0x0000555555569992 osslsigncode`sign_spc_indirect_data_content + 461
    frame #13: 0x0000555555572fb8 osslsigncode`pe_pkcs7_prepare + 540
    frame #14: 0x0000555555568bf6 osslsigncode`main + 1486
    frame #15: 0x00007ffff775dcd0 libc.so.6`___lldb_unnamed_symbol3187 + 128
    frame #16: 0x00007ffff775dd8a libc.so.6`__libc_start_main + 138
    frame #17: 0x000055555555c2f5 osslsigncode`_start + 37

using a safenet USB token.

Thanks very much for creating this!!

mtrojnar commented 7 months ago

I also ran into this issue and imaGuru's fix worked. I'm running Arch linux with libp11 https://archlinux.org/packages/extra/x86_64/libp11/

What do pacman -Qi libp11 and pacman -Qi openssl print on your Arch Linux?

joostn commented 7 months ago
# pacman -Qi libp11
Name            : libp11
Version         : 0.4.12-2
Description     : A library implementing a small layer on top of the PKCS11 API
Architecture    : x86_64
URL             : https://github.com/OpenSC/libp11/wiki
Licenses        : LGPL
Groups          : None
Provides        : None
Depends On      : openssl
Optional Deps   : None
Required By     : None
Optional For    : None
Conflicts With  : None
Replaces        : None
Installed Size  : 185,23 KiB
Packager        : Felix Yan <felixonmars@archlinux.org>
Build Date      : di 01 nov 2022 14:20:21 CET
Install Date    : di 21 nov 2023 18:02:46 CET
Install Reason  : Explicitly installed
Install Script  : No
Validated By    : SHA-256 Sum

# pacman -Qi openssl
Name            : openssl
Version         : 3.1.4-1
Description     : The Open Source toolkit for Secure Sockets Layer and Transport Layer Security
Architecture    : x86_64
URL             : https://www.openssl.org
Licenses        : Apache
Groups          : None
Provides        : libcrypto.so=3-64  libssl.so=3-64
Depends On      : glibc
Optional Deps   : ca-certificates [installed]
                  perl [installed]
Required By     : apache  argyllcms  aws-c-cal  aws-sdk-cpp  bind  coreutils  cryptsetup  curl  freerdp  git  gst-plugins-bad  jose  kmod  ldns  libarchive  libevent  libgit2  libimobiledevice  libnvme  libp11  libsasl
                  libshout  libssh  libssh2  libvncserver  libzip  minizip-ng  neon  nix  nodejs  openssh  ostree  python  qpdf  rsync  s2n-tls  spice-gtk  srt  sudo  systemd  tpm2-tss  wpa_supplicant  xmlsec
Optional For    : apr-util
Conflicts With  : None
Replaces        : openssl-perl  openssl-doc
Installed Size  : 10,71 MiB
Packager        : Pierre Schmitz <pierre@archlinux.org>
Build Date      : di 24 okt 2023 17:55:17 CEST
Install Date    : di 31 okt 2023 18:28:06 CET
Install Reason  : Installed as a dependency for another package
Install Script  : No
Validated By    : SHA-256 Sum
clst commented 7 months ago

I can confirm this also fixes the same crash when using a YubiKey 5 FIPS with a stored Authenticode Certificate.

Thanks a lot for this. I was just about to start debugging :)

EDIT: I am also on ArchLinux

mrpippy commented 6 months ago

I'm also seeing a crash on macOS (osslsigncode 2.7, libp11 0.4.12, openssl 3.2.0, all from Homebrew) when signing with a YubiKey:

Process 23503 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbeaddc3d40e8)
    frame #0: 0x0000000100378260 pkcs11.dylib`pkcs11_slot_unref + 120
pkcs11.dylib`pkcs11_slot_unref:
->  0x100378260 <+120>: ldr    x8, [x8, #0x78]
    0x100378264 <+124>: ldr    x0, [x19, #0x88]
    0x100378268 <+128>: blr    x8
    0x10037826c <+132>: ldr    x0, [x19, #0x90]
Target 0: (osslsigncode) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbeaddc3d40e8)
  * frame #0: 0x0000000100378260 pkcs11.dylib`pkcs11_slot_unref + 120
    frame #1: 0x0000000100374e48 pkcs11.dylib`pkcs11_object_free + 48
    frame #2: 0x0000000100376b68 pkcs11.dylib`pkcs11_ec_finish + 64
    frame #3: 0x00000001007f7b74 libcrypto.3.dylib`EC_KEY_free + 88
    frame #4: 0x00000001008730b4 libcrypto.3.dylib`evp_pkey_free_legacy + 60
    frame #5: 0x0000000100873120 libcrypto.3.dylib`evp_pkey_free_it + 28
    frame #6: 0x0000000100870d88 libcrypto.3.dylib`EVP_PKEY_free + 72
    frame #7: 0x0000000100002364 osslsigncode`main + 608
    frame #8: 0x000000010003908c dyld`start + 520

The patch from this thread fixes the issue, but doesn't seem like the correct fix. The real bug is probably in libp11 or even openssl.

mrpippy commented 4 months ago

I spent more time tracing this down, using osslsigncode 2.8 (built with AddressSanitizer), git master libp11, and openssl 3.2.0 from homebrew.

As mentioned, ENGINE_finish() in read_token() takes the engine's functional count down to 0, which frees the engine before it should be. But, why does the engine have a functional count of only 1 at that point? ENGINE_init() in read_token() sets the count to 1. ENGINE_load_private_key() then calls load_privkey() in libp11, that calls EVP_PKEY_set1_engine() which also calls ENGINE_init(): count is now 2.

ENGINE_load_private_key() then has code to "enforce check for legacy key" which sort-of makes a copy of the private key, in the process it calls ENGINE_free() but never increases the count to match. Thus, the engine count is now 1 when ENGINE_load_private_key() returns.

It appears there's been ongoing discussion for months about this "legacy key check" code (original PR) being broken: 1 2

The ultimately correct fix is this pull request, which was merged just after OpenSSL 3.0.13/3.1.5/3.2.1 were released. Testing with the tip of the OpenSSL 3.2 branch, I don't see any crash.

So, this was an OpenSSL bug, and should be fixed in the next release of OpenSSL 3.0/3.1/3.2.

mtrojnar commented 4 months ago

@imaGuru @git-lul @joostn @clst Can you confirm that that OpenSSL libraries compiled from the current 3.0, 3.1 or 3.2 GitHub branches fix this issue for you?

git-lul commented 4 months ago

I have built osslsigncode using OpenSSL 3.0.14-dev branch and I can confirm that issue is fixed. I only tested on win.

mtrojnar commented 3 months ago

The following solutions are currently possible:

  1. Wat for the next OpenSSL releases.
  2. Build OpenSSL for its git repository.
  3. Use OpenSSL 3.0.11 (or older) or 3.1.3 (or older) , which do not contain the bug.