openssl operations involving pcks11 and softHSM result in segfault on exit

[nhorman]

recently this bug was reported to openssl:
openssl/openssl#22508

Analysis of the problem revealed what is something of an intractable problem.

To summarize the issue:

1. openssl on initalization, loads the pkcs11 library
2. When configured appropriately for this use case, the pkcs11 library loads the SoftHSM library
3. SoftHSM on load creates several global class instances (a SoftHSM object, SecurityManager object, etc)
4. Because SoftHSM is written in C++, the C11 standard mandates that those objects be deleted on exit, so as an implementation detail, the compiler implicitly emits calls to __cxa_atexit, to call the destructor method for each object instance
5. on exit, becuase posix mandates that atexit handlers be run in the reverse order of their registration, the softHSM library deletes the global object instances prior to any previously registered handlers
6. because openssl preforms cleanup in some cases using an atexit handler, it (openssl) makes calls through the pkcs11 engine, which in turn attempts to reference data/code in the softHSM library that has already been deleted via (5), leading to a segfault.

There are a few potential workarounds for this, but it seems to me that the most correct fix would be for softHSM to not delete that data until all references to it were freed from using libraries. While this is more arguably a shortcoming in the C++ standard, I think the best fix would be for softHSM to modify the implementation of the PCKS api in main.cpp such that it can detect when its global object constructors have been called (via a global variable), and return an appropriate status code without attempting to access the now-deleted object data

[nhorman]

FWIW, I believe this issue is related:
#722

[nhorman]

Its untested as of yet, but something like this I think would avoid the issue:
f45b02c

[zandrey]

@nhorman

FTR: I've tried proposed patch, but it unfortunately brings another issue: SoftHSM throws the error on token creation in free slots.

With the patch applied, and following executed:

$ export SOFTHSM2_CONF=/home/zandrey/softhsm2/softhsm2.conf
$ mkdir -p /home/zandrey/softhsm2/softhsm2.tokens
$ softhsm2-util --module /home/zandrey/softhsm2/usr/lib/softhsm/libsofthsm2.so --init-token --free --label test-keys --pin 1111 --so-pin 222222

ERROR: Could not initialize the library.

Slot 0 has a free/uninitialized token.

Content of /home/zandrey/softhsm2/softhsm2.conf:

directories.tokendir = /home/zandrey/softhsm2/softhsm2.tokens
objectstore.backend = db

Strangely enough, there is a new slot that is created by that call. It is just that softhsm2-util throws the error.

-- andrey

[nhorman]

@zandrey sorry about that, I don't have a setup here to test it out, it was really just meant to be a suggestion for a type of fix to the softHSM maintainers. They're going to have to come up with something more robust I'm afraid. Though I've not seen any activity herein some time, so I'm starting to wonder if theres a shortage of people on this project.

[Emantor]

@michaelolbrich and me looked at this again and decided that this particular issue can be fixed by resettting the global variable during Softhsm::reset(). This was tested and fixes the problem reported by @zandrey, Updated commit can be found at Emantor@41968e7.

[zandrey]

@michaelolbrich and me looked at this again and decided that this particular issue can be fixed by resettting the global variable during Softhsm::reset(). This was tested and fixes the problem reported by @zandrey, Updated commit can be found at Emantor@41968e7.

Correct, I've tested the updated patch and both the original SIGSEGV problem and error on token creation are solved with it.

[nhorman]

@michaelweiser Thats good information, thank you. Any chance you have contacts with the SoftHSM maintainers and can get them to incorporate that?

[thesamesam]

@michaelolbrich and me looked at this again and decided that this particular issue can be fixed by resettting the global variable during Softhsm::reset(). This was tested and fixes the problem reported by @zandrey, Updated commit can be found at Emantor@41968e7.

Is there a PR for this?

[Emantor]

@michaelolbrich and me looked at this again and decided that this particular issue can be fixed by resettting the global variable during Softhsm::reset(). This was tested and fixes the problem reported by @zandrey, Updated commit can be found at Emantor@41968e7.

Is there a PR for this?

Sorry for the wait, there is #742 now.