Opened 3 years ago
Closed 3 years ago
#1448 closed Bug / Defect ( fixed )
--inactive bytes not working
| Reported by: | Gert Döring | Owned by: | Gert Döring |
|---|---|---|---|
| Priority: | major | Milestone: | release 2.5.6 |
| Component: | Generic / unclassified | Version: | OpenVPN 2.5.4 (Community Ed) |
| Severity: | Not set (select this one, unless your'e a OpenVPN developer) | Keywords: | msvc , --inactive , overflow |
| Cc: | stipa , Gert Döring |
Description
Server pushes
inactive 900 107374182400
which, according to the manpage, means "if there is no activity in 900s *or* "exit if less than bytes of combined in/out traffic are produced on the tun/tap device in n seconds". This is "10Gbyte in 15 minutes".
OpenVPN 2.5.x on Linux (all versions) do not abort here, neither do mingw-built Windows versions (up to 2.5.3).
MSVC built Windows versions (2.5.4 and up) do exit after 900s with
2022-02-04 09:28:53 us=781000 Inactivity timeout (--inactive), exiting
which seem to be the correct thing to do.
Now the interesting question is "why is MSVC-built windows behaving differently, and where is the real bug?"...
Change History (5)
comment:3 Changed 3 years ago by
| Owner: | set to Gert Döring |
|---|---|
| Status: | new → accepted |
comment:4 Changed 3 years ago by
| Keywords: | --inactive overflow added; inactive removed |
|---|---|
| Milestone: | → release 2.5.6 |
commit cae1a7fcf14e6ded34ab5a1e8842c3034cc89608 (master)
commit 1e573aa9b31d9270bd43d8c5a448314508a3311f (release/2.5)
Author: Gert Doering
Date: Fri Feb 4 12:42:01 2022 +0100
Repair --inactive with 'bytes' argument larger 2Gbytes.
Fix will be in 2.5.6 (... breaking all the Linux installations as well!) and in 2.6.0
comment:5 Changed 3 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | accepted → closed |
The magic of integer overflows at work here.
static int positive_atoi(const char *str) { const int i = atoi(str); return i < 0 ? 0 : i; } ... else if (streq(p[0], "inactive") && p[1] && !p[3]) { VERIFY_PERMISSION(OPT_P_TIMER); options->inactivity_timeout = positive_atoi(p[1]); if (p[2]) { options->inactivity_minimum_bytes = positive_atoi(p[2]); }... and this number is too big for an "int", so it overflows. On Linux, the result of the overflow seems to be "0" (and on MinGW-built Windows), so it does "nothing". On MSVC built, the result is (2 31 )-1 (214748364), which is about 2Gbyte - hard to achieve in 15minutes normally, so "it does as instructed".
Now, what to do about it? Go for a long for option parsing? And then what? Stay with a "long" (and update all the code paths to "long") or warn/abort if there is an integer overlflow, keeping the existing behaviour but making clear what happens?