[Openvpn-devel,v2,3/3] Restore also ping related options on a reconnect
Commit Message
This fixes the issue that if a client reconnects the next connection
entries inherits the keepalive settings that were pushed or set by
the previous entry. Since UDP+PULL entries have an implicit 120s
timeout, this timeout also got applied to a TCP session after an
UDP entry.
Patch v2: rebase on master
Reported-By: Jan Just Keijser <janjust@nikhef.nl>
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
src/openvpn/options.c | 9 +++++++++
src/openvpn/options.h | 4 ++++
2 files changed, 13 insertions(+)
Comments
Hi all,
On 17/03/2021 17:00, Arne Schwabe wrote:
> This fixes the issue that if a client reconnects the next connection
> entries inherits the keepalive settings that were pushed or set by
> the previous entry. Since UDP+PULL entries have an implicit 120s
> timeout, this timeout also got applied to a TCP session after an
> UDP entry.
>
> Patch v2: rebase on master
>
> Reported-By: Jan Just Keijser <janjust@nikhef.nl>
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>
This patch is actually fixing a bug, as reported by JJK, however it is
such a coerner case that we may not want to backport this to 2.4
(backporting requires rewriting the patch).
This patch was reviewed with the Arne's guidance.
Acked-by: Antonio Quartulli <antonio@openvpn.net>
The patch is visually fine, and I was actually successfull in testing
pre/post behaviour. As in: client with multiple <connection> blocks,
and using the management interface to make it connect to server A
("push ping, ping-restart") first, then server B ("do not push anything
ping-related"). Without the patch, the client would die with --ping-restart
timeout, with the patch it happily does "no ping restart".
It needs SIGUSR1 restarts to trigger the issue. SIGHUP restarts do a
"full config reload" and never display "lingering pushed config" problems.
Testing has uncovered more save & restore bugs, related to compression
and "--route-gateway". At least...
Your patch has been applied to the master and release/2.5 branch (bugfix),
but I'm not sure I want to really backport it to 2.4 - it's not a crucial
crashbug, not easy to trigger, and if you're annoyed by it, just upgrade.
I have not tested the 2.5 backport in full depth, just a basic t_client
run (but the change is straightforward enough, and it applied without
much friction).
commit 5a2ed714d14acb2312d27fe40e300be96d970c27 (master)
commit a0e844c892d6f67977bf8e9162cbc43a3f64ba46 (release/2.5)
Author: Arne Schwabe
Date: Wed Mar 17 17:00:38 2021 +0100
Restore also ping related options on a reconnect
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210317160038.25828-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21675.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
The patch is visually fine, and I was actually successfull in testing
pre/post behaviour. As in: client with multiple <connection> blocks,
and using the management interface to make it connect to server A
("push ping, ping-restart") first, then server B ("do not push anything
ping-related"). Without the patch, the client would die with --ping-restart
timeout, with the patch it happily does "no ping restart".
It needs SIGUSR1 restarts to trigger the issue. SIGHUP restarts do a
"full config reload" and never display "lingering pushed config" problems.
Testing has uncovered more save & restore bugs, related to compression
and "--route-gateway". At least...
Your patch has been applied to the master and release/2.5 branch (bugfix),
but I'm not sure I want to really backport it to 2.4 - it's not a crucial
crashbug, not easy to trigger, and if you're annoyed by it, just upgrade.
I have not tested the 2.5 backport in full depth, just a basic t_client
run (but the change is straightforward enough, and it applied without
much friction).
commit 5a2ed714d14acb2312d27fe40e300be96d970c27 (master)
commit a0e844c892d6f67977bf8e9162cbc43a3f64ba46 (release/2.5)
Author: Arne Schwabe
Date: Wed Mar 17 17:00:38 2021 +0100
Restore also ping related options on a reconnect
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210317160038.25828-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21675.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
@@ -3540,6 +3540,11 @@ pre_pull_save(struct options *o)
o->pre_pull->ciphername = o->ciphername;
o->pre_pull->authname = o->authname;
o->pre_pull->keysize = o->keysize;
+
+ /* Ping related options should be reset to the config values on reconnect */
+ o->pre_pull->ping_rec_timeout = o->ping_rec_timeout;
+ o->pre_pull->ping_rec_timeout_action = o->ping_rec_timeout_action;
+ o->pre_pull->ping_send_timeout = o->ping_send_timeout;
}
}
@@ -3591,6 +3596,10 @@ pre_pull_restore(struct options *o, struct gc_arena *gc)
o->ciphername = pp->ciphername;
o->authname = pp->authname;
o->keysize = pp->keysize;
+
+ o->ping_rec_timeout = pp->ping_rec_timeout;
+ o->ping_rec_timeout_action = pp->ping_rec_timeout_action;
+ o->ping_send_timeout = pp->ping_send_timeout;
}
o->push_continuation = 0;
@@ -79,6 +79,10 @@ struct options_pre_pull
const char* authname;
int keysize;
+ int ping_send_timeout;
+ int ping_rec_timeout;
+ int ping_rec_timeout_action;
+
int foreign_option_index;
};