#473 closed Bug / Defect ( fixed )

cipher none causes "Assertion failed at crypto_openssl.c:523"

Reported by: deviantintegral Owned by: Steffan Karger
Priority: major 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

I'm using OpenVPN on Ubuntu 12.04, using the OpenVPN repository.

After upgrading to 2.3.5, using "cipher none" throws an assertion the VPN fails to start. Downgrading to OpenVPN 2.3.4 fixes the issue. See the attached log for details.

Attachments (4)

openvpn-assertion.txt ​ ( 18.1 KB ) - added by deviantintegral 10 years ago .
OpenVPN assertion with cipher none
0001-Fix-assertion-error-when-using-cipher-none.patch ​ ( 3.4 KB ) - added by Steffan Karger 10 years ago .
ovpn_ticket_473_crypto_none_additional_fix.patch ​ ( 510 bytes ) - added by sven-ola 10 years ago .
Additional fix needed on running server with Cypto=NONO
0001-Really-fix-cipher-none.patch ​ ( 2.1 KB ) - added by Steffan Karger 10 years ago .

.zip

Change History (14)

Changed 10 years ago by deviantintegral

OpenVPN assertion with cipher none

comment:1 Changed 10 years ago by Steffan Karger

Owner: set to Steffan Karger
Status: new → assigned

comment:2 Changed 10 years ago by Steffan Karger

Milestone: → release 2.3.6

Sorry for that. I sent a patch to the mailinglist to fix it:
​ http://article.gmane.org/gmane.network.openvpn.devel/9216

Also attached the patch file to this ticket.

comment:3 Changed 10 years ago by Steffan Karger

Resolution: → fixed
Status: assigned → closed

The fix has been applied to the release/2.3 and master branches, and will be included in the next release:
​ https://github.com/OpenVPN/openvpn/commit/4429456 (release/2.3)
​ https://github.com/OpenVPN/openvpn/commit/4e93e6d (master)

Changed 10 years ago by sven-ola

Additional fix needed on running server with Cypto=NONO

comment:4 Changed 10 years ago by sven-ola

With a running Openvpn instance using Crypto=NONE for speed reasons, the additional patch (see above) is required. This seems to be true for at least two instances, one running on Debian/Squeeze? , the other running on Debian/Wheezy? .

Also, I got an additional compiler quirks. Which requires me to add "-fno-tree-vrp" to CFLAGS, at least when compiling under Debian/Squeeze? with gcc-4.4.real (Debian 4.4.5-8) 4.4. The compiler optimizes out the NULL==cipher test in crypto_openssl.c::cipher_kt_mode_ofb_cfb() which in turn again rises the assert which triggers this bug report. Maybe the crypto_polarssl.c is also affcected, haven't checked that. I'm not sure *why* the compiler is damn sure, that the *cipher never is NULL, maybe a bug with this specific compiler version only.

Reasons enough to re-open this bug.

comment:5 Changed 10 years ago by sven-ola

Resolution: fixed
Status: closed → reopened

comment:6 Changed 10 years ago by sven-ola

The -fno-tree-vrp is also required with Debian/Wheezy? and gcc (Debian 4.7.2-5) 4.7.2. Tested with

cd src/openvpn;make CFLAGS="-ggdb -O2" clean all

Otherwise triggers Assertion failed at crypto_openssl.c:523 also on an AMD64 machine (ovpn client with cipher-none). Source and patchset avail under ​ http://sven-ola.dyndns.org/repo/pool/source/freifunk-openvpn_2.3.6-2.dsc

Last edited 10 years ago by sven-ola ( previous ) ( diff )

comment:7 Changed 10 years ago by Steffan Karger

Milestone: release 2.3.6 → release 2.3.7
Status: reopened → accepted

Aargh, I forgot to remove the __attribute__((nonnull)) on the functions, making gcc rightfully optimizing out the check in the cipher_kt_mode_ofb_cfb() , which will still result in the assertion. Sorry. Attached a patch that removes the incorrent attributes, and adds a test for --cipher none to the make check tests, to prevent future regressions.

Changed 10 years ago by Steffan Karger

comment:8 Changed 10 years ago by Gert Döring

The "really fix cipher none" patch has been merged to release/2.3 and master:

commit 785838614afc20d362b64907b0212e9a779e2287 (release/2.3)
commit 98156e90e1e83133a6a6a020db8e7333ada6156b (master)
Author: Steffan Karger <steffan@…>
Date: Tue Dec 2 21:42:00 2014 +0100

Really fix '--cipher none' regression

so this should be really done for good now :-) - sven-ola, could you please re-test?

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

Pinging again - sven-ola, is this working for you now? The patch is only in git (release/2.3 branch or master) for now, but we'll ship 2.3.7 "soonish" which will have it - and it would be good to know that it actually works.

comment:10 Changed 10 years ago by Steffan Karger

Resolution: → fixed
Status: accepted → closed

I'm pretty convinced this second attempt did fix it, so I'm closing the ticket. If I somehow messed up again with this, just reopen the ticket.