Opened 4 years ago

Last modified 6 weeks ago

#538 assigned Bug / Defect

no PIN prompt with PKCS11 when systemd is enabled

Reported by: coudot Owned by: David Sommerseth
Priority: major Milestone: release 2.5
Component: Generic / unclassified Version: OpenVPN 2.3.4 (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords: pkcs11 PIN password console prompt systemd
Cc: Steffan Karger

Description

When systemd is enabled, and PKCS11 auth is used, openvpn hangs just before PIN prompt. Last log line is:

Thu Apr  2 10:22:03 2015 us=593664 PKCS#11: Calling pin_prompt hook for 'CF.NET P11'

As described here, this is due to the fact that openvpn tries to fork to launch systemd-ask-password in a pkcs11-helper hook that does not allow fork() to be safely used:

/**
 * @brief Set a pin prompt callback.
 * @param hook	Callback.
 * @param global_data	Data to send to callback.
 * @return CK_RV.
 * @attention
 * If @ref pkcs11h_setForkMode() is true, you cannot fork while in hook.
 */
CK_RV
pkcs11h_setPINPromptHook (
	IN const pkcs11h_hook_pin_prompt_t hook,
	IN void * const global_data
);

As a result, openvpn hangs (infinite wait loop in the pkcs11-helper pthread_atfork() handler), and can only be stopped with kill -9.


OS: Debian jessie
configure line: ./configure $(shell dpkg-buildflags --export=configure) --enable-password-save --enable-pkcs11 --enable-x509-alt-username --enable-systemd
$ openvpn --version
OpenVPN 2.3.4 x86_64-unknown-linux-gnu [SSL (OpenSSL)] [LZO] [EPOLL] [PKCS11] [MH] [IPv6] built on Apr 1 2015
library versions: OpenSSL 1.0.1k 8 Jan 2015, LZO 2.08
Originally developed by James Yonan
Copyright (C) 2002-2010 OpenVPN Technologies, Inc. <sales@…>
Compile time defines: enable_crypto=yes enable_debug=yes enable_def_auth=yes enable_dlopen=unknown enable_dlopen_self=unknown enable_dlopen_self_static=unknown enable_fast_install=yes enable_fragment=yes enable_http_proxy=yes enable_iproute2=no enable_libtool_lock=yes enable_lzo=yes enable_lzo_stub=no enable_management=yes enable_multi=yes enable_multihome=yes enable_pam_dlopen=no enable_password_save=yes enable_pedantic=no enable_pf=yes enable_pkcs11=yes enable_plugin_auth_pam=yes enable_plugin_down_root=yes enable_plugins=yes enable_port_share=yes enable_selinux=no enable_server=yes enable_shared=yes enable_shared_with_static_runtimes=no enable_small=no enable_socks=yes enable_ssl=yes enable_static=yes enable_strict=no enable_strict_options=no enable_systemd=yes enable_win32_dll=yes enable_x509_alt_username=yes with_crypto_library=openssl with_gnu_ld=yes with_mem_check=no with_plugindir='$(libdir)/openvpn/plugins' with_sysroot=no

Attachments (6)

work-around-for-bug538.patch (676 bytes) - added by Bjoern Voigt 2 years ago.
Patch disables SystemD PIN prompt and enables fall-back to getpass()
openvpn-2.4-work-around-for-bug538.patch (571 bytes) - added by Bjoern Voigt 17 months ago.
Patch for OpenVPN 2.4 disables SystemD PIN prompt and enables fall-back to query_user_exec_builtin()
openvpn-2.4.2-pkcs11_pin_prompt.patch (1.4 KB) - added by naf 16 months ago.
Another patch, to use query_user_exec_builtin() for PKCS11 PIN prompt only.
bug538-workaround.patch (7.8 KB) - added by n 12 months ago.
Working workaround
openvpn-2.4.5_bug-538.patch (8.2 KB) - added by tim427 7 months ago.
Based on "n"'s patch: patch file for openvpn-2.4.5 :)
0001-pkcs11-Workaround-to-make-PKCS-11-PIN-token-work-wit.patch (7.6 KB) - added by David Sommerseth 6 weeks ago.
[Alternative patch] pkcs11: Workaround to make PKCS#11 PIN token work with systemd

Download all attachments as: .zip

Change History (21)

comment:1 Changed 3 years ago by dwmw2

It's worse than that though. Even the fork() callbacks at other times are failing because of the way pkcs11-helper calls C_Initialize() in the child for every provider module which is active in the parent.

When that happens, the OpenSC PKCS#11 module tends to confuse pcscd because it now has two open handles to it, leading to the problem described at http://sourceforge.net/p/opensc/mailman/message/34073754/ — and the p11-kit-proxy.so module gets an internal deadlock as described at https://www.mail-archive.com/p11-glue@lists.freedesktop.org/msg00126.html

I'm going to assume that pkcs11-helper is "correct" to do what it's doing, and that the provider modules in question are buggy. But I suspect they are the most *common* provider modules we want to use with OpenVPN, so it's hard to just blame them and say it's not our fault.

The cheap way to "fix" this is to use vfork() in openvpn_execve() instead of fork(). Then the atfork handlers don't get run, and the problem doesn't occur.

comment:2 Changed 3 years ago by dwmw2

Since the systemd-ask-password helper is spawned with openvpn_popen() rather than openvpn_execve(), we cannot use vfork() there. We cannot do anything between vfork() and execve(), and we need to close file descriptors in that case. So the patch I posted does not solve this issue, although it does solve the other issues with fork(). I've filed a separate ticket #549 for those, making it clear that it's a workaround for issues in the common PKCS#11 provider modules we are likely to be using.

This issue, on the other hand, is very clearly an OpenVPN bug — as stated above, we are spawning the systemd-ask-password helper from a callback which clearly documents that it does not allow fork() to be used.

comment:3 Changed 3 years ago by David Sommerseth

At the hackathon in Munich last autumn, we discussed a better API for quering for user input. The current model ("ask instantly when I need something") isn't optimal, and the current systemd implementation has some issues too - like in this case.

I did post a bigger patch-set in November to the openvpn-devel mailing list, which we agreed in the developer meeting yesterday I should rebase and re-submit for review.

Some review help here and some testing (I'm lacking proper PKCS11 devices for real testing) would be great.

comment:4 Changed 3 years ago by dwmw2

I can certainly help with the "lacking proper PKCS#11 devices for real testing" part — you don't need real hardware. PKCS#11 providers can also be done purely in software. If you are on a system with NSS or GNOME keyring, you have PKCS#11 devices which you can use for testing. There's also SoftHSM.

GNOME keyring probably isn't the best one to use for this particular case, since it has a 'special' way of handling login and doesn't demand a PIN from the application. So let's use NSS.

Let's assume you're on a modern Linux or *BSD distribution with p11-kit installed and working correctly. The instructions below were tested on Fedora 22, but it should work fairly much anywhere.

First, create a new NSS database directory (you could use the standard per-user one in ~/.pki/nssdb if you want, but for this test you're going to set a passphrase on it and that might be a PITA for Chrome and other things that use it, so let's use a separate one).

$ cd
$ mkdir ${HOME}/.openvpn-nssdb
$ certutil -N -d sql:$HOME/.openvpn-nssdb
Enter a password which will be used to encrypt your keys.
The password should be at least 8 characters long,
and should contain at least one non-alphabetic character.

Enter new password: 
Re-enter password: 

Use a real password when it asks for one instead of just hitting 'Enter', of course. But you don't actually have to obey its complexity rules.

Now, you need to configure the system to make this database available by default (in certain processes, since we're just testing):

$ mkdir -p $HOME/.config/pkcs11/modules
$ cat > $HOME/.config/pkcs11/modules/openvpn.module <<EOF
module: /usr/lib64/libsoftokn3.so
x-init-reserved: configdir='sql:$HOME/.openvpn-nssdb' certPrefix='' keyPrefix='' secmod='secmod.db'
enable-in: p11tool openvpn seahorse
EOF

Now if you run the 'seahorse' application you should see the 'NSS Certificate DB' show up under 'Certificates'. You can use 'File... Import' to import a certificate from a PKCS#12 file into it.

You can now find the ID for this certificate by running openvpn --show-pkcs11-ids, and use that ID with that --pkcs-id option instead of a cert from a file. Voilà — now you have a PKCS#11 device for real testing ☺

(Note that until this p11-kit bug is fixed, you'll want to apply my workaround in ticket #549)

comment:5 Changed 3 years ago by Samuli Seppänen

Milestone: release 2.5
Owner: set to David Sommerseth
Status: newassigned

comment:6 Changed 2 years ago by Bjoern Voigt

Even with a fixed version of p11-kit (tested with version 0.23.2) the PIN prompt is invisible in SystemD more. systemd-ask-password hangs for unknown reasons.

I found an work-around for OpenVPN PIN prompt in interactive mode (no --daemon) option. The work-around simply disables get_console_input_systemd, so that a simple getpass() method can show the PIN prompt and read the password.

Changed 2 years ago by Bjoern Voigt

Patch disables SystemD PIN prompt and enables fall-back to getpass()

comment:7 Changed 2 years ago by dwmw2

Is this still because of https://github.com/OpenSC/pkcs11-helper/issues/5 ?

We really need to ditch pkcs11-helper.

Last edited 2 years ago by dwmw2 (previous) (diff)

comment:8 Changed 20 months ago by nfournil

Patch & workaround doesn't work in ubuntu 16.04 + pcks11-helper trunk + openvpn trunk :-(

Changed 17 months ago by Bjoern Voigt

Patch for OpenVPN 2.4 disables SystemD PIN prompt and enables fall-back to query_user_exec_builtin()

comment:9 Changed 17 months ago by Bjoern Voigt

The new work-around patch should work for OpenVPN 2.4 (tested with 2.4.2).

comment:10 Changed 17 months ago by Steffan Karger

Cc: Steffan Karger added

comment:11 Changed 17 months ago by David Sommerseth

That patch is effectively just replacing query_user_exec() in console_systemd.c with query_user_exec_builtin(). The patch works in this regard, but could be done "cleaner" in regards to coding style.

That said ... This hack will have consequences if you then try to start openvpn during boot on systemd systems and the configuration will need to ask the user for information.

But I agree with dwm2, we need to look into getting pkcs11-helper replaced with p11-kit, which is a more complicated task.

1) Implement p11-kit suppprt in mbed TLS
2) Implement mbed TLS support in p11-kit
3) Migrate pkcs11-helper calls to use p11-kit instead

Task 1 + 2 is can run in parallel, and probably must do so too. And they both require that patches/support are accepted upstream. Once that is done, we can seriously start fixing this in OpenVPN.

Unless we can manage to figure out a side-channel implementation for using smart-cards, which OpenVPN can use via some generic API (type D-Bus os similar). However, the D-Bus approach will also have some dependency challenges during boot currently, as it needs to be started after the D-Bus daemon have been started (at least until bus1 (former kdbus) is accepted into the kernel and landing in all Linux distros)

Changed 16 months ago by naf

Another patch, to use query_user_exec_builtin() for PKCS11 PIN prompt only.

comment:12 Changed 16 months ago by Bjoern Voigt

Unfortunately OpenVPN with the latest patch openvpn-2.4.2-pkcs11_pin_prompt.patch​ does not ask anymore for my PKCS11 PIN.

openvpn --cd /etc/openvpn --config client.ovpn
[...]
[hangs forever]

comment:13 Changed 12 months ago by n

The last patch doesn't work because it only changes the password in _pkcs11_openvpn_show_pkcs11_ids_pin_prompt. That's probably only used with --show-pkcs11-ids, not while connecting. There's another function, _pkcs11_openvpn_pin_prompt, which calls get_user_pass, which calls get_user_pass_cr, which calls query_user_SINGLE. That's probably what needs to be changed.

Changed 12 months ago by n

Attachment: bug538-workaround.patch added

Working workaround

Changed 7 months ago by tim427

Attachment: openvpn-2.4.5_bug-538.patch added

Based on "n"'s patch: patch file for openvpn-2.4.5 :)

comment:14 Changed 4 months ago by Gert Döring

I can see what the patch does, but I do not like it - it's changing way too many code for a single if().

Introducing GET_USER_PASS_FORCE_BUILTIN sounds useful, and forcing the builtin passwort prompt if called from a pkcs11 context also sounds like the least bad workaround for "we need to get rid of pkcs11-helper". But to have four different places duplicate the "if(flag) { call this; } else { call that; } is not good.

Introducing a flag argument to query_user_SINGLE() which will then either run the builtin functions or the normal processing sounds like an approach with less and better isolated code changes. (Not sure if it will work out nicely, haven't looked into it in full detail).

Also, there is a change that I cannot easily explain

@ -941,7 +950,7 @@
                 strcpy(up->password, "ok");
             }
         }
-        else if (flags & GET_USER_PASS_INLINE_CREDS)
+        else if (flags & GET_USER_PASS_NEED_OK)
         {
             struct buffer buf;
             buf_set_read(&buf, (uint8_t *) auth_file, strlen(auth_file) + 1);

this seems to be an unrelated change?

Changed 6 weeks ago by David Sommerseth

[Alternative patch] pkcs11: Workaround to make PKCS#11 PIN token work with systemd

comment:15 Changed 6 weeks ago by David Sommerseth

So I'm joining in with an alternative patch to try to resolve this issue. This patch is very lightly tested, so I only post it here for initial review and testing.

This patch extends query_user_exec() with a bool argument, to force the built-in query mechanism. Further query_user_SINGLE() gets the echo bool argument replaced with a flag type instead. And this flag type is then used elsewhere. And PKCS#11 related queries indicates it wants the built-in query mechanism.

Note: See TracTickets for help on using tickets.