OpenSC / pkcs11-helper

Library that simplifies the interaction with PKCS#11 providers for end-user applications using a simple API and optional OpenSSL engine
Other
66 stars 43 forks source link

Deadlock inside fork when used with OpenVPN #24

Closed loskutov closed 5 years ago

loskutov commented 5 years ago

By default, OpenVPN uses systemd-ask-password to query for passwords; however, it fails to fork because of the handlers set by pkcs11-helper. Full backtrace attached. Latest revisions of pkcs11-helper and opensc-pkcs11 used. backtrace.txt

alonbl commented 5 years ago

Hi,

You should not let PKCS#11 provider to be loaded as root, this is a bad practice, it makes much more sense to run it as non-run under your own user[1][2]

OpenVPN should not use fork() within pin-entry of PKCS#11-helper, as the fork must be consistent. However, if openvpn developers know what they are doing, they can setup pkcs11-helper to be unsafe fork post daemon fork[3].

In any case, the proper solution is to use the management interface to query credentials[4] and not fork.

The best method if you want to help them is to remove the entire PKCS#11 handling in OpenVPN and delegate the entire private key usage into the management interface, this will allow OpenVPN daemon to be loaded in privileged context, be much more simple and secured, while the the device access happens at the context of the interactive user.

I am closing this as there are many options to resolve this, and loading PKCS#11 provider in OpenVPN should be deprecated after 10 years of effort and the management interface is the way to go or worse case setup pkcs11-helper to perform unsafe forks.

[1] https://community.openvpn.net/openvpn/wiki/UnprivilegedUser [2] http://alonbl.shoutwiki.com/wiki/Gentoo/OpenVPN_Non_Root [3] https://github.com/OpenSC/pkcs11-helper/blob/master/include/pkcs11-helper-1.0/pkcs11h-core.h#L338 [4] https://sites.google.com/site/alonbarlev/openvpn-pkcs11

frankmorgner commented 5 years ago

@loskutov , see https://github.com/OpenVPN/openvpn/pull/121