Opened 3 years ago

Closed 3 years ago

#825 closed Bug / Defect (fixed)

Segfault with --tls-crypt on RHEL5/CentOS5

Reported by: Simon Matter Owned by: Steffan Karger
Priority: major Milestone: release 2.4.1
Component: Crypto Version: OpenVPN 2.4.0 (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords:
Cc:

Description

Openvpn segfaults on RHEL5/CentOS5 when using --tls-crypt, because it doesn't have AES-256-CTR support:

openvpn[15330]: OpenVPN 2.4.0 x86_64-redhat-linux-gnu [SSL (OpenSSL)] [LZO] [LZ4] [EPOLL] [MH/PKTINFO] built on Jan 17 2017
openvpn[15330]: library versions: OpenSSL 0.9.8e-fips-rhel5 01 Jul 2008, LZO 2.09, LZ4 1.7.5
openvpn[15331]: NOTE: the current --script-security setting may allow this configuration to call user-defined scripts
kernel: openvpn[15331]: segfault at 0000000000000008 rip 000000000040ebe0 rsp 00007fffdcfc5738 error 4

attached patch fixes it so it shows:

openvpn[424]: ERROR: --tls-crypt requires AES-256-CTR support.
openvpn[424]: Exiting due to fatal error

Regards,
Simon

Attachments (1)

openvpn-2.4.0-tls-crypt_segfault.patch (778 bytes) - added by Simon Matter 3 years ago.

Download all attachments as: .zip

Change History (8)

Changed 3 years ago by Simon Matter

comment:1 Changed 3 years ago by Gert Döring

Owner: set to Steffan Karger
Status: newassigned

Thanks for spotting this and providing a patch. "Doh"... - to syzzer, for review and ACKing.

comment:2 Changed 3 years ago by Gert Döring

As for the patch, you're moving the "hmac_length" setting as well - which is unrelated.

Maybe just move the if ( !kt.cipher ) check right up after the kt.cipher= assignment, and leave the rest as it is. Feels a bit more logical, does the same thing.

(But anyway, the patch looks very reasonable - without even testing, it is clear that this is a working fix)

comment:3 Changed 3 years ago by Steffan Karger

Status: assignedaccepted

D'oh indeed.

The patch is right; the kt.hmac_length = md_kt_size(kt.digest); will fail in a similar way if HMAC-SHA-256 is not available.

So, ACK to the patch.

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

comment:4 Changed 3 years ago by Gert Döring

So, going through my backlog - is this patch already on the list? Preferrably with the comment:2 above addressed? This one can go in quicky, and fixes a real bug...

comment:5 Changed 3 years ago by Steffan Karger

Simon, we'd like to apply your patch, but would prefer to do so with proper attribution. Would you be willing to provide us with a full name and email address?

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

Simon sent me email as he was unable to comment here. It turned out the culprit was one of our external spam filters. He're his comment:

It is Simon Matter and simon.matter and the domain is invoca.ch, thanks.

comment:7 Changed 3 years ago by Gert Döring

Resolution: fixed
Status: acceptedclosed

Your patch has been applied to the master and release/2.4 branches.

commit 2fe5547c1df854d41611633ea533649fe88e3031 (master)
commit c9b4313eae6fc59f7d075edf23a7f59b137ba11f (release/2.4)
Author: Simon Matter
Date: Tue Feb 21 20:34:15 2017 +0100

Fix segfault when using crypto lib without AES-256-CTR or SHA256

thanks!

(planned release for 2.4.1, which will have the patch, is "in about two weeks")

Note: See TracTickets for help on using tickets.