Opened 9 years ago

Closed 9 years ago

Last modified 6 years ago

#480 closed Bug / Defect (fixed)

[PATCH] Openvpn with crytpodev on FreeBSD does not work

Reported by: ermal.luci Owned by:
Priority: blocker Milestone: release 2.3.7
Component: Crypto Version: OpenVPN 2.3.5 (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords:
Cc:

Description

Basically the issue was identified as described here https://redmine.pfsense.org/issues/3966.

Whenever the aesni.ko module is loaded openvpn cannot work.
This seems to be the effect that crypto descriptors are prepared before doing fork.
After this the crypto operations do not work.

The patch below fixes the issue reported.

--- src/openvpn/init.c  2014-10-20 10:51:43.000000000 +0200
+++ /root/init.c        2014-11-14 12:40:43.000000000 +0100
@@ -3301,6 +3301,9 @@
     init_query_passwords (c);
 #endif

+  /* do one-time inits, and possibily become a daemon here */
+  do_init_first_time (c);
+
   /* initialize context level 2 --verb/--mute parms */
   init_verb_mute (c, IVM_LEVEL_2);

@@ -3423,8 +3426,6 @@
   if (c->mode == CM_P2P)
     do_init_traffic_shaper (c);

-  /* do one-time inits, and possibily become a daemon here */
-  do_init_first_time (c);

 #ifdef ENABLE_PLUGIN
   /* initialize plugins */

Basically it just makes the fork before the init of crypto.

Attachments (3)

150322-Reload-OpenSSL-engines-after-forking.patch (4.4 KB) - added by Steffan Karger 9 years ago.
150406-Reload-OpenSSL-engines-after-forking-v2.patch (4.5 KB) - added by Steffan Karger 9 years ago.
150413-Call-daemon-before-initializing-crypto-library.patch (3.0 KB) - added by Steffan Karger 9 years ago.

Download all attachments as: .zip

Change History (35)

comment:1 Changed 9 years ago by cbuechler

Confirmed this issue, and this patch does fix it.

comment:2 Changed 9 years ago by Steffan Karger

Thanks for reporting. Patch looks reasonable, but I will need to check whether changing the init order does not have any unwanted side effects. I will come back to this.

comment:3 Changed 9 years ago by Steffan Karger

I'm having a hard time estimating the full implications of this change. At first glance it looks innocent, but there are a lot of dependencies in the init code, which vary per platform (as you've noticed), and I don't have the means to test them all. Is this patch yours? Can you elaborate on why do_init_first_time (c) should be moved specifically here? Or would any spot before the crypto init suffice?

comment:4 Changed 9 years ago by ermal.luci

It should be before any forking happening.
I put it there as the easiest place i could identify before any forking was done from openvpn.

Basically the crypto(9) dev protects accessing the session by the pid who triggered it on FreeBSD and i think on other BSDs derivations as well.

comment:5 Changed 9 years ago by Steffan Karger

Unfortunately, moving the fork before the crypto init will break configs using relative paths for key, cert, ca, etc, since daemon() will chdir to /, unless cd is set to the correct path. We discussed this during an IRC meeting today, and the "thou shalt not break user configs" applies in this case too. So, we'll have to figure out another way to work around the problem.

I am not familiar with /dev/crypto and the working of it in the BSDs, but I will put this on my list for when I find some spare time. In the mean time, any suggestions are welcome of course.

comment:6 Changed 9 years ago by Gert Döring

Is there some additional pointer how OpenVPN ends up talking to /dev/crypto? I assume this is happening somewhere inside OpenSSL, right? Is there a way to tell OpenSSL or crypto(9) "hey, I've just forked, it's allright?" (we have such code for pkcs11 already)

comment:7 Changed 9 years ago by ermal.luci

Sure EVP API does that automagically and there is way to disable that at least as things stand today in OpenVPN.

Look at crypto(4) interface if you can find anything useful but i am not aware of such thing since at the very end still it would be a problem.

Though seems you are looking for an workaround here no?

Changed 9 years ago by Steffan Karger

comment:8 Changed 9 years ago by Steffan Karger

So I finally found some time to look into a workaround. See the attached patch. Instead of changing the init order, I just reload the engines. Unfortunately, I do not have a proper FreeBSD+cryptodev test setup available, nor do I have to time to set one up. So all I could do is verify that this does not break anything for my test setups. Could someone with a FreeBSD+cryptodev setup please check that the attached patch (150322-Reload-OpenSSL-engines-after-forking.patch) indeed fixes this issue? As soon as I have confirmation, I'll send the patch to the openvpn-devel mailinglist for processing.

Last edited 9 years ago by Steffan Karger (previous) (diff)

comment:9 Changed 9 years ago by mandree

I personally don't have such a FreeBSD+cryptodev setup either, but am asking the FreeBSD reporter to test drive the patch. https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=195004

comment:10 Changed 9 years ago by Samuli Seppänen

Milestone: release 2.3.7

Syzzer will send mail to openvpn-devel and openvpn-user lists asking for help in testing the fix.

comment:11 Changed 9 years ago by garga

We replaced previous fix with this one on pfsense snapshots and the result was not good. Here are some logs

Apr 2 10:35:06 	openvpn[74272]: OpenVPN 2.3.6 amd64-portbld-freebsd10.1 [SSL (OpenSSL)] [LZO] [MH] [IPv6] built on Mar 31 2015
Apr 2 10:35:06 	openvpn[74272]: library versions: OpenSSL 1.0.1l-freebsd 15 Jan 2015, LZO 2.09
Apr 2 10:35:06 	openvpn[74272]: NOTE: the current --script-security setting may allow this configuration to call user-defined scripts
Apr 2 10:35:06 	openvpn[74272]: TUN/TAP device ovpns2 exists previously, keep at program end
Apr 2 10:35:06 	openvpn[74272]: TUN/TAP device /dev/tun2 opened
Apr 2 10:35:06 	openvpn[74272]: ioctl(TUNSIFMODE): Device busy: Device busy (errno=16)
Apr 2 10:35:06 	openvpn[74272]: do_ifconfig, tt->ipv6=1, tt->did_ifconfig_ipv6_setup=0
Apr 2 10:35:06 	openvpn[74272]: /sbin/ifconfig ovpns2 10.0.14.1 10.0.14.2 mtu 1500 netmask 255.255.255.255 up
Apr 2 10:35:06 	openvpn[74272]: /usr/local/sbin/ovpn-linkup ovpns2 1500 1560 10.0.14.1 10.0.14.2 init
Apr 2 10:35:06 	openvpn[75323]: UDPv4 link local (bound): [AF_INET]192.168.20.74:1195
Apr 2 10:35:06 	openvpn[75323]: UDPv4 link remote: [undef]
Apr 2 10:35:06 	openvpn[75323]: Assertion failed at crypto.c:169
Apr 2 10:35:06 	openvpn[75323]: Exiting due to fatal error
Apr 2 10:35:06 	openvpn[75323]: /usr/local/sbin/ovpn-linkdown ovpns2 1500 1560 10.0.14.1 10.0.14.2 init

And here you can see 2 configs, server1.conf works well but server2.conf dies before accept a connection:

: cat server1.conf 
dev ovpns1
verb 1
dev-type tun
tun-ipv6
dev-node /dev/tun1
writepid /var/run/openvpn_server1.pid
#user nobody
#group nobody
script-security 3
daemon
keepalive 10 60
ping-timer-rem
persist-tun
persist-key
proto udp
cipher AES-128-CBC
auth SHA1
up /usr/local/sbin/ovpn-linkup
down /usr/local/sbin/ovpn-linkdown
client-connect /usr/local/sbin/openvpn.attributes.sh
client-disconnect /usr/local/sbin/openvpn.attributes.sh
local 192.168.20.74
tls-server
server 10.0.10.0 255.255.255.0
client-config-dir /var/etc/openvpn-csc
username-as-common-name
auth-user-pass-verify "/usr/local/sbin/ovpn_auth_verify user 'Local Database' false server1" via-env
tls-verify "/usr/local/sbin/ovpn_auth_verify tls 'vpn-server' 1"
lport 1194
management /var/etc/openvpn/server1.sock unix
push "route 10.42.42.0 255.255.255.0"
client-to-client
ca /var/etc/openvpn/server1.ca 
cert /var/etc/openvpn/server1.cert 
key /var/etc/openvpn/server1.key 
dh /etc/dh-parameters.2048
tls-auth /var/etc/openvpn/server1.tls-auth 0
persist-remote-ip
float


: cat server2.conf 
dev ovpns2
verb 1
dev-type tun
tun-ipv6
dev-node /dev/tun2
writepid /var/run/openvpn_server2.pid
#user nobody
#group nobody
script-security 3
daemon
keepalive 10 60
ping-timer-rem
persist-tun
persist-key
proto udp
cipher AES-128-CBC
auth SHA1
up /usr/local/sbin/ovpn-linkup
down /usr/local/sbin/ovpn-linkdown
local 192.168.20.74
ifconfig 10.0.14.1 10.0.14.2
lport 1195
management /var/etc/openvpn/server2.sock unix
secret /var/etc/openvpn/server2.secret


comment:12 Changed 9 years ago by Steffan Karger

Interesting. This is on the same machine? Do both of these configs fail without the patch?

Changed 9 years ago by Steffan Karger

comment:13 Changed 9 years ago by Steffan Karger

See the attachments for a v2 patch. I'm not yet a 100% sure this is the right fix, but (again) I expect this to resolve this issue. Before I dig deeper, could someone please check whether this one does indeed resolve the issue in static key mode too?

comment:14 Changed 9 years ago by garga

Unfortunately the second patch failed the same way. A simple config using shared key is enough to make it crash with the same log message:

Assertion failed at crypto.c:169

We restored original patch sent by Ermal on pfSense and it back to work

comment:15 Changed 9 years ago by Samuli Seppänen

Quoting today's IRC meeting summary:

Syzzer will provide a patch that will change the init order from "openssl init, then daemon(with chdir)" to "daemon(without chdir), openssl init, chdir()". This should fix the issue in a clean way without nasty side-effects.

comment:16 Changed 9 years ago by Gert Döring

JFTR, running FreeBSD 10.1 in a VM on top of vmware ESX on "recent enough" Intel hardware provides AESNI and one can test this by loading cryptodev.ko, crypto.ko, aesni.ko kernel modules.

One thing I noticed is that openssl is not exactly well-behaving here - it will use /dev/crypto even if "-engine cryptodev" is not requested (run "truss openssl speed -evp aes-256-cbc" and see the CIOCCRYPT ioctl()s scroll by...). This is the reason why reloading the *engines* after fork() doesn't work - the file descriptor is not released/reopened... I'd call that a bug in the provided openssl, it shouldn't go for cryptodev unless being told so. But we'll have to cope with that.

comment:17 Changed 9 years ago by Steffan Karger

The attached patch (150413-Call-daemon-before-initializing-crypto-library.patch) does what samuli promised two posts ago. Since I now have access to a FreeBSD machine with the faulty cryptodev behaviour (thanks cron2), I could verify this workaround fixes the issue for the static key setup that triggered the issue before applying this patch.

Please, let me know if this fixes the bug for you too. (I don't dare to claim anything anymore, after the previous two patches did not work out... ;) )

comment:18 Changed 9 years ago by garga

I'll make a test and report back. Thanks!

comment:19 Changed 9 years ago by ermal.luci

One thing I noticed is that openssl is not exactly well-behaving here - it will use /dev/crypto even if "-engine cryptodev" is not requested (run "truss openssl speed -evp aes-256-cbc" and see the CIOCCRYPT ioctl()s scroll by...). This is the reason why reloading the *engines* after fork() doesn't work - the file descriptor is not released/reopened... I'd call that a bug in the provided openssl, it shouldn't go for cryptodev unless being told so. But we'll have to cope with that.

Just some feedback since i traced this at the time.
This is not the openssl fault.
It is the fault of how openvpn uses openssl EVP API.
It calls the API simply and it tries by default to use any engine present provided by the OS implementation.
Normally openvpn should forbid the EVP API to use any engine if one is not configured.

This will become more important when AEAD ciphers are supported by OpenVPN since for those the userland implementation might be faster than using a 'not forced' engine provided by the OS.

comment:20 in reply to:  19 Changed 9 years ago by Steffan Karger

Replying to ermal.luci:

One thing I noticed is that openssl is not exactly well-behaving here - it will use /dev/crypto even if "-engine cryptodev" is not requested (run "truss openssl speed -evp aes-256-cbc" and see the CIOCCRYPT ioctl()s scroll by...). This is the reason why reloading the *engines* after fork() doesn't work - the file descriptor is not released/reopened... I'd call that a bug in the provided openssl, it shouldn't go for cryptodev unless being told so. But we'll have to cope with that.

Just some feedback since i traced this at the time.
This is not the openssl fault.
It is the fault of how openvpn uses openssl EVP API.
It calls the API simply and it tries by default to use any engine present provided by the OS implementation.
Normally openvpn should forbid the EVP API to use any engine if one is not configured.

I'd disagree. I think OpenSSL, FreeBSD ánd OpenVPN are at fault here.

First OpenSSL. OpenVPN calls OpenSSL_add_all_algorithms() because it needs to fill OpenSSL internal search tables for ciphers and digests. A side effect of this function (that is not controllable) is that OpenSSL loads the cryptodev engine. Of course I could copy-paste the contents of that function minus the cryptodev part into OpenVPN code, but then I would have keep track of upstream changes to keep my copy-paste consistent with the OpenSSL version. I consider that bad practice.

Then FreeBSD (or actually, OpenSSL on FreeBSD). The current default to prefer AES-NI through cryptodev over user-land AES-NI makes no sense at all. Yes, I can avoid that by not calling OpenSSL_add_all_algorithms(), but I think openssl users should not be bothered with that. Defaults should be sane.

Finally OpenVPN previously forked *after* initializing OpenSSL, which is arguably a bad choice.

We'll fix the init order in OpenVPN. FreeBSD and/or OpenSSL should fix the weird default AES-NI/cryptodev behaviour, instead of asking all their users to work around it.

This will become more important when AEAD ciphers are supported by OpenVPN since for those the userland implementation might be faster than using a 'not forced' engine provided by the OS.

Totally agree. But I believe the default behaviour of OpenSSL should be to prefer AES-NI from user-land, so this should be fixed outside of OpenVPN.

comment:21 Changed 9 years ago by Steffan Karger

Just sent the (what I believe should be) final patch to the ml:

http://article.gmane.org/gmane.network.openvpn.devel/9609

comment:22 Changed 9 years ago by garga

This last patch version doesn't apply to 2.3.6, is it expected?

comment:23 Changed 9 years ago by Steffan Karger

That is expected, you'll need to apply this patch first:
http://article.gmane.org/gmane.network.openvpn.devel/9555

comment:24 Changed 9 years ago by garga

Yeah, I applied this one and also ac1c2f259b44d1229a65a3e639b09d57a4e2a53b and it's now building.

comment:25 Changed 9 years ago by mandree

I have added the three patches from comment:21, comment:23, comment:24 to the FreeBSD port as of 2.3.6_4.

comment:26 Changed 9 years ago by garga

We are testing it for some time on pfSense and this last version works as expected. Thanks!!

comment:27 Changed 9 years ago by Gert Döring

Thanks for testing and for your patience :-)

I have now merged Syzzer's patch from comment:21 to both git master and release/2.3

commit da9b292733e929a2900dc32d37f0424c3d588366 (master)
commit f025de005d719201a69ad0313d545a1ddd244752 (release/2.3)

Author: Steffan Karger
Date: Mon Apr 27 16:28:57 2015 +0200

Call daemon() before initializing crypto library

and it will be in the upcoming 2.3.7 release (expected to show up some time next week)

Syzzer, I leave the honour of closing this ticket to you :-)

comment:28 Changed 9 years ago by garga

Thank you guys!

comment:29 in reply to:  27 Changed 9 years ago by Steffan Karger

Resolution: fixed
Status: newclosed

Replying to cron2:

Syzzer, I leave the honour of closing this ticket to you :-)

Let's do that!

comment:30 Changed 8 years ago by Olivier

This ticket comes up quite often when looking for information on configuring OpenVPN on FreeBSD, so I'll just add some useful tips:

  • Do not load cryptodev in your FreeBSD kernel. Make it optional
  • Only load the cryptodev module if you have encryption acceleration hardware and do not plan on using AES-NI
  • It is not necessary to load the aesni module for OpenSSL to use the CPU extensions
  • You can still load the aesni module (without cryptodev!) if you have other software which can use it (ipsec). It won't hurt OpenSSL performance

comment:31 Changed 6 years ago by patrickjane

Hi,

I am facing similar issue of OPENVPN compatibility with FreeBSD cryptodev and came across this post. Followed all the instructions suggested in above post and ensured I am on track, but unfortunately no success. I have confirmed the functionality of HW accelerators using Openssl speed tests. We are using OCF cryptodev framework in kernel(ocf.ko, cryptosoft.ko, cryptodev.ko)

We are using OPENVPN 2.3.6 and applied the above suggested patches. Please let me know if anything i am missing. Any guidance will be helpful.

Thanks
Patrick

comment:32 in reply to:  31 Changed 6 years ago by Gert Döring

Replying to patrickjane:

We are using OPENVPN 2.3.6

please try something not multiple years old - namely, OpenVPN 2.4.4, which is what comes in the FreeBSD ports.

This should work nicely and out of the box with cryptodev.

(Though I'm curious what you're using cryptodev for - as documented, AESNI is way faster without cryptodev)

Note: See TracTickets for help on using tickets.