Opened 6 years ago

Closed 6 years ago

#408 closed Bug / Defect (fixed)

Assertion failed on clients with --tcp-nodelay

Reported by: 0xa Owned by:
Priority: minor Milestone: release 2.3.7
Component: Generic / unclassified Version: OpenVPN 2.3.4 (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords:
Cc:

Description

Hi,

I tried to run OpenVPN as a client with --tcp-nodelay and it exited:
Assertion failed at helper.c:533

However, it is documented to ignore it without --server, and it should do so, or print a meaningful error message.

Attachments (1)

bug-408-remove-assert.patch (1.3 KB) - added by JoshC 6 years ago.

Download all attachments as: .zip

Change History (5)

Changed 6 years ago by JoshC

Attachment: bug-408-remove-assert.patch added

comment:1 Changed 6 years ago by JoshC

In options_postprocess(), options_postprocess_mutate() is called before options_postprocess_verify(). In this case, the mutate caused an ASSERT(0) to be hit before the verification would have caught the use of --tcp-nodelay outside or server mode.

The initial patch removes the assert to expose the more useful error message from the verify step.

We still need to decide if --tcp-nodelay is a server-only option (and update the manpage to indicate this) or choose to ignore or warn rather than die when called from non-server context.

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

This type of option ("set x, push y") is really only useful as a server option - but indeed, it should only warn, if anything at all. Maybe warn is good, because otherwise I can already see the bug report "I did tcp-nodelay and nothing changes!!!"...

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

This popped up again in #429.

So what's the state of affairs here? JoshC?

Looking at the man page again, I think it's fairly clear that this is intended as a server option: "This macro sets the TCP_NODELAY socket flag on the server as well as pushes it to connecting clients..." - so your patch doesn't sound too bad for me as a first step.

The alternative would be to change the helper to set the socket flag always, and if it's a server, also push. But that would need changes to the manpage to explain the different behaviour. Discuss it on the openvpn-devel list?

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

Milestone: release 2.3.7
Resolution: fixed
Status: newclosed

Reported again in #489.

Fixed in the code now (thanks, David)

commit 706283d3765d1ee62dbd913fbfc191855b92528d (master)
commit 333566fd9d1951be97397f5f19d8bc78fb025af9 (release/2.3)

Author: David Sommerseth
Date: Tue Dec 9 10:52:44 2014 +0100

sockets: Remove the limitation of --tcp-nodelay to be server-only

Signed-off-by: David Sommerseth <davids@…>
Acked-by: Arne Schwabe <arne@…>
Message-Id: 1418118764-17846-1-git-send-email-openvpn.list@…

Note: See TracTickets for help on using tickets.