Opened 9 years ago

Closed 9 years ago

#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 9 years ago.
OpenVPN assertion with cipher none
0001-Fix-assertion-error-when-using-cipher-none.patch (3.4 KB) - added by Steffan Karger 9 years ago.
ovpn_ticket_473_crypto_none_additional_fix.patch (510 bytes) - added by sven-ola 9 years ago.
Additional fix needed on running server with Cypto=NONO
0001-Really-fix-cipher-none.patch (2.1 KB) - added by Steffan Karger 9 years ago.

Download all attachments as: .zip

Change History (14)

Changed 9 years ago by deviantintegral

Attachment: openvpn-assertion.txt added

OpenVPN assertion with cipher none

comment:1 Changed 9 years ago by Steffan Karger

Owner: set to Steffan Karger
Status: newassigned

comment:2 Changed 9 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.

Changed 9 years ago by Steffan Karger

comment:3 Changed 9 years ago by Steffan Karger

Resolution: fixed
Status: assignedclosed

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 9 years ago by sven-ola

Additional fix needed on running server with Cypto=NONO

comment:4 Changed 9 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 9 years ago by sven-ola

Resolution: fixed
Status: closedreopened

comment:6 Changed 9 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 9 years ago by sven-ola (previous) (diff)

comment:7 Changed 9 years ago by Steffan Karger

Milestone: release 2.3.6release 2.3.7
Status: reopenedaccepted

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 9 years ago by Steffan Karger

comment:8 Changed 9 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 9 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 9 years ago by Steffan Karger

Resolution: fixed
Status: acceptedclosed

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.

Note: See TracTickets for help on using tickets.