Opened 7 years ago
Closed 7 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)
Change History (8)
Changed 7 years ago by
| Attachment: | openvpn-2.4.0-tls-crypt_segfault.patch added |
|---|
comment:1 Changed 7 years ago by
| Owner: | set to Steffan Karger |
|---|---|
| Status: | new → assigned |
comment:2 Changed 7 years ago by
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 7 years ago by
| Status: | assigned → accepted |
|---|
D'oh indeed.
The patch is right; the {{kt.hmac_length = md_kt_size(kt.digest);}} will fail in a similar way if HNMAC-SHA-256 is not availeble.
So, ACK to the patch.
comment:4 Changed 7 years ago by
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 7 years ago by
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 7 years ago by
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 7 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | accepted → closed |
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")
Thanks for spotting this and providing a patch. "Doh"... - to syzzer, for review and ACKing.