SoftHSMv2 port causes free-after-free

[mouse07410 (Mouse)]

MacOS Big Sur 11.6.1, Xcode-13.1

This problem is rather obscure and cumbersome to manifest. It surfaces when multiple OpenSSL engines are installed (like in my case).

The symptom: when the engines finish, during clean OpenSSL crashes with SEGV on attempt to free a NULL-pointer. It happens with Macports-installed OpenSSL-1.1.1, and with OpenSSL-3.1.dev that I build from sources. Surprisingly, it does not happen with Macports-installed OpenSSL-3.0.0.

How do I know it's this port: when I uninstall softhsm and instead build/install SoftHSMv2 from sources, everything works just fine, for all the three installed OpenSSL versions, and all the engines involved.

Here's how I configure SoftHSMv2 myself:

./configure --prefix=/opt/local -enable-64bit --with-openssl=${OSSL_DIR} --with-botan=/opt/local --with-sqlite3=/opt/local --with-objectstore-backend-db

OSSL_DIR is where OpenSSL binaries are installed, so in this case it's /opt/local/libexec/openssl11.

Here's the pointer to a detailed description that includes stack traces: ​https://github.com/OpenSC/libp11/issues/431

[iay (Ian Young)]

Sounds like this might be related to a couple of upstream issues:

​https://github.com/opendnssec/SoftHSMv2/issues/548 ​https://github.com/opendnssec/SoftHSMv2/pull/551 ​https://github.com/opendnssec/SoftHSMv2/issues/635

A fix (and a fix for the fix) was merged in May 2020 but there hasn't been a release since April of that year.

I've poked upstream about whether a new release is likely (it was last discussed in April).

[iay (Ian Young)]

Given the above, one interesting question is whether when you build SoftHSM yourself (which works) whether that's from their _released_ sources or from their current HEAD.

[mouse07410 (Mouse)]

Given the above, one interesting question is whether when you build SoftHSM yourself (which works) whether that's from their released sources or from their current HEAD.

I always build SoftHSMv2 from their develop branch, which, contrary to its name, is supposed to be (relatively) stable, or at least so I was told. As opposed to their master branch.

[iay (Ian Young)]

I am fairly sure their develop branch has the fix for the issues I mentioned earlier, so that would probably account for the difference in behaviour.

If I get some indication that they want to make a new release soon, the right thing is to bump to that when it comes out. If that isn't likely to happen, though, I might try cherry-picking the changes for those issues. I'd prefer not to, though, given the number of _other_ changes those are mixed in with.

[mouse07410 (Mouse)]

I understand, thanks! I concur.

[iay (Ian Young)]

I've had no response from upstream, so although I do worry a bit that SoftHSM2 is becoming defunct, it seems like we should dig into this and see if we can cherry-pick those changes.

The examples you put on the libp11 ticket are a little difficult to reconcile with what I have installed, and in particular I'd like to reproduce the issue without having to be in the process of recompiling some other package, which is what I think you were doing there.

Would it be possible for you to come up with a reproduction of the issue with just what one can install directly from MacPorts today, plus any command-line actions are necessary?

I realise your example required multiple openssl engines and that you were using a Yubikey HSM for your second one; I don't have one of those but I do have a Nitrokey HSM hard token that I think I can substitute.

[iay (Ian Young)]

Incidentally, it may either simplify or complicate things, but the default openssl installed by MacPorts is now 3.0.0, and the versions of softhsm and libp11 installed now use that.

[mouse07410 (Mouse)]

I'd like to reproduce the issue without having to be in the process of recompiling some other package, which is what I think you were doing there.

Yes, I'm recompiling libp11, as I'm on its master branch. But the problem does seem to be with SOftHSMv2 - as demonstrated by successful run after replacing Macports softhsm with the "home-build" one.

Would it be possible for you to come up with a reproduction of the issue with just what one can install directly from MacPorts today, plus any command-line actions are necessary?

I'd love to, but I doubt that...

I realise your example required multiple openssl engines and that you were using a Yubikey HSM for your second one; I don't have one of those but I do have a Nitrokey HSM hard token that I think I can substitute.

Yes, the nature of the other HSM should not matter. As long as they both use PKCS#11 and libp11, it should be sufficient.

. . . the default openssl installed by MacPorts is now 3.0.0, and the versions of softhsm and libp11 installed now use that.

Does not matter. The Macports-installed version of SoftHSMv2 reported in the ticket was 3.0.0-based, as I recall - and the crash report (in the quoted libp11 issue) seems to confirm it.

[iay (Ian Young)]

That's unfortunate, but it is what it is. I will see if I can reproduce your case.

[mouse07410 (Mouse)]

That's unfortunate, but it is what it is.

Yes, I understand and agree.

I will see if I can reproduce your case.

Thank you!

FWIW, I think that it may be sufficient to simply have GOST engine installed, in addition to libp11. You may not need an HSM and its PKCS#11 driver...

[iay (Ian Young)]

I'm not making much progress, I'm afraid. I wanted to start by just having two engines loaded, but I keep running into an inability to configure this with the devices I have available (all of which are PKCS11, which means I've been trying to load multiple PKCS11 engines, which doesn't seem to work at all).

Where does the GOST engine you talk about above come from?

[mouse07410 (Mouse)]

I don't think you need multiple PKCS#11 engines to replicate. You can't (I think) run more than one PKCS#11 engine, but you can have more than one configured (e.g., in ...openssl/etc/openssl.cnf, and/or served by p11-kit (which is what I'm using).

For the GOST engine:

• I use ​https://github.com/mouse07410/engine.git (nicer, because it has scripts to automate and simplify build)
• Upstream is at ​https://github.com/gost-engine/engine.git

[iay (Ian Young)]

I have pinged the upstream developers again. I'll give it a couple of weeks and then I will probably try and do the cherry-pick operation.

[mouse07410 (Mouse)]

@Ian, any luck so far?

[iay (Ian Young)]

Replying to mouse07410:

@Ian, any luck so far?

Sorry, no. I got no response to my query and there has been no activity in that project for a year now, so I think we have to assume that it is defunct. On my side, the immediate need for a fix for this receded a little (the person who would be using it has acquired other commitments) and I let myself drop the ball.

You will recall that one issue was that I couldn't reproduce the issue you were seeing, although I suspect it was the same one I was seeing. I'm a bit nervous about assuming that if I fix my issue that it wouldn't cause problems for other people, including you. One way to satisfy myself about that would be for me to make a topic branch of macports-ports with the cherry-picked changes on it and ask you to verify it. Would you be up for that?

[mouse07410 (Mouse)]

One way to satisfy myself about that would be for me to make a topic branch of macports-ports with the cherry-picked changes on it and ask you to verify it. Would you be up for that?

Sure, if you tell me how to retrieve and compile that branch locally.

[iay (Ian Young)]

I have what I think is a fix for this issue, so if you'd like to try testing it now would be the time.

I'm having to deal with a second-level problem, which is that neither my fixed version nor the current sources without my fix pass the self-test. This appears to be because of the removal of legacy algorithms from openssl: specifically, DES, which the softhsm tests use all over the place. This shouldn't be relevant for your testing, though.

To test, you'd need to go through something like this:

git clone https://github.com/iay/macports-ports.git
git checkout 64036-softhsm-segv
cd security/softhsm
sudo port
[security/softhsm] > uninstall
[security/softhsm] > install
[security/softhsm] > ^D
port installed softhsm
The following ports are currently installed:
  softhsm @2.6.1_2 (active)

The _2 is a packaging revision only, the current released version is _1.

[iay (Ian Young)]

For posterity, it looks like I can get the tests to work by changing the default OpenSSL configuration file in /opt/local/etc/openssl/openssl.cnf to include the legacy provider:

[openssl_init]
providers = provider_sect

# List of providers to load
[provider_sect]
default = default_sect
legacy = legacy_sect

[default_sect]
activate = 1

[legacy_sect]
activate = 1

This is with OpenSSL 3:

port installed openssl\*
The following ports are currently installed:
  openssl @3_10 (active)
  openssl3 @3.1.0_3 (active)
  openssl10 @1.0.2u_4 (active)
  openssl11 @1.1.1t_1 (active)

Ventura 13.3, Xcode 14.3

[mouse07410 (Mouse)]

Unfortunately, enabling legacy provider causes OpenSSL to crash with other (more important) providers, like pkcs11-provider and oqs-provider.

[iay (Ian Young)]

You shouldn't need to enable the legacy provider to build softhsm, or to use it. As far as I can tell, it's only needed if you wanted to run the "port test" command. If you've tried and that's not the case, please let me know what happens and maybe we can work round it.

[mouse07410 (Mouse)]

No, I'm not using the legacy provider. I mentioned it only because your previous post talked about getting the tests run with it enabled.

[mouse07410 (Mouse)]

Also, it would be great if the following patch was applied:

diff --git a/src/lib/crypto/OSSLCryptoFactory.cpp b/src/lib/crypto/OSSLCryptoFactory.cpp
index ace4bcb..b5456d4 100644
--- a/src/lib/crypto/OSSLCryptoFactory.cpp
+++ b/src/lib/crypto/OSSLCryptoFactory.cpp
@@ -175,6 +175,7 @@ OSSLCryptoFactory::OSSLCryptoFactory()
                            OPENSSL_INIT_LOAD_CRYPTO_STRINGS |
                            OPENSSL_INIT_ADD_ALL_CIPHERS |
                            OPENSSL_INIT_ADD_ALL_DIGESTS |
+                            OPENSSL_INIT_NO_ATEXIT |
                            OPENSSL_INIT_LOAD_CONFIG, NULL);
 #endif
 
@@ -238,7 +239,7 @@ OSSLCryptoFactory::~OSSLCryptoFactory()
        // Detect that situation because reinitialisation will fail
        // after OPENSSL_cleanup() has run.
        (void)ERR_set_mark();
-       ossl_shutdown = !OPENSSL_init_crypto(OPENSSL_INIT_ENGINE_RDRAND, NULL);
+       ossl_shutdown = !OPENSSL_init_crypto(OPENSSL_INIT_ENGINE_RDRAND | OPENSSL_INIT_NO_ATEXIT, NULL);
        (void)ERR_pop_to_mark();
 #endif
        if (!ossl_shutdown)

[iay (Ian Young)]

You don't say where the proposed code comes from, or what it does, and I'd need to know that at a minimum. As far as I can tell, it's not part of the upstream codebase, at least on their develop branch.

Is this additional change needed to fix your issue, or do the cherry-picked commits on the branch I asked you to review work for you?

If I can, I'd like to restrict this to cherry-picking from the upstream project to fix a single issue. I don't have a problem with returning for additional fixes once that's out of the way.

[mouse07410 (Mouse)]

You don't say where the proposed code comes from

​https://github.com/openssl/openssl/issues/21023#issuecomment-1558503033

or what it does

Hopefully, prevents the module from initiating OPENSSL_cleanup() upon exit, which apparently causes problems (crashes) when there's more than one module involved.

it's not part of the upstream codebase, at least on their develop branch

Correct, upstream does not have it.

Is this additional change needed to fix your issue

I think it will help with this issue.

If I can, I'd like to restrict this to cherry-picking from the upstream project to fix a single issue

Understood. Somebody needs to bring this up with the upstream maintainers. :-(

[iay (Ian Young)]

If your additional patch was accepted upstream, I'd be happy to include it. If it weren't for the fact that there has been no upstream activity for a year, I'd suggest that as the next step to get some vetting from people who know this code better than I do. You could (and should) try that, but I don't think either of us would have much confidence at this point that it would get a response.

So, in parallel to that, let me try adding your patch to my branch and test that it doesn't break MY use case. Might be a day or so as I have some other stuff on just now.

[mouse07410 (Mouse)]

. . . I don't think either of us would have much confidence at this point that it would get a response.

I'm afraid this is the reality.

. . . let me try adding your patch to my branch and test that it doesn't break MY use case. Might be a day or so as I have some other stuff on just now.

Sounds perfect. I cannot ask for more.

[iay (Ian Young)]

Replying to mouse07410:

. . . let me try adding your patch to my branch and test that it doesn't break MY use case. Might be a day or so as I have some other stuff on just now.

Sounds perfect. I cannot ask for more.

I have now added your change to my branch, and rebased it against the current state of the macports-ports repository.

Can you re-pull that and try it again? The branch, now with your change as well as the two cherry-picked from upstream, still works for my use cases.

If it works for yours as well, I can move forward with getting it merged into the main line (I can't do that directly).

[iay (Ian Young)]

Replying to mouse07410:

Any progress on reviewing?

[mouse07410 (Mouse)]

Any progress on reviewing? [Can you re-pull that and try it again?]

My apologies, I got bogged down with other things.

Could you remind me where to pull it from? Given that I'm not that experienced with Macports "guts" and would prefer not to mess with it..?

[iay (Ian Young)]

You can do this to uninstall the original and build the revised version:

mkdir foo
cd foo
git clone git@github.com:iay/macports-ports
cd macports-ports
git checkout 64036-softhsm-segv
cd security/softhsm
sudo port uninstall softhsm
sudo port install
port installed softhsm

Note that sudo port install does not include the port name; you're building from the current directory.

The last command should return:

The following ports are currently installed:
  softhsm @2.6.1_2 (active)

The _2 indicates that you're using the patched version.

[mouse07410 (Mouse)]

Thanks!

The patched version seems better.

My result is still inconclusive, because I think I'm hitting Apple Xcode-14.3.1 clang bug. But offhand, I'd say - merge/release your patched version. It's definitely not worse, and I think it's better than 2.6.1_1.

[iay (Ian Young)]

Fair enough, I will get that cooking.

@Jakker, any thoughts?

[Jakker (Jaap Akkerhuis)]

I'm pretty neutral at this. I do hope someone takes over the maintenance of this port (running out of time to do that myself). I also hope the problem is reported this bug to the upstream. (​http://bugs.opendnssec.org/).

[iay (Ian Young)]

PR is ​https://github.com/macports/macports-ports/pull/19190

[iay (Ian Young)]

In f63b8bbdb02106107c498271f0c23ea7ae8b4256/macports-ports (master):

softhsm: fix segment violation at application end

Fix a segmentation violation at application termination
through a combination of a couple of commits cherry-picked
from upstream post-2.6.1 and a contribution from mouse07410
directed at the same issue.

Closes: #64036