#812 closed Bug / Defect (fixed)
SIGUSR1 restart does not re-open tun even if the pushed ip address changes
Reported by: | Selva Nair | Owned by: | |
---|---|---|---|
Priority: | critical | Milestone: | release 2.4.1 |
Component: | Generic / unclassified | Version: | OpenVPN 2.4.0 (Community Ed) |
Severity: | Not set (select this one, unless your'e a OpenVPN developer) | Keywords: | |
Cc: | Steffan Karger |
Description
After sitting on this for a day it still looks like a bug so here goes:
An easy way to reproduce:
Have --persist-tun
in client config so tun is preserved unless route, ifconfig etc. changes on restart. Successfully connect to a server and arrange the server to push a new ip address next time (say using ifconfig-push in a ccd file). Restart the client using SIGUSR1 and watch the connection to complete without any tun re-open. That means the old ip and routes remain and the tunnel is broken.
The slightly edited log below is from Linux but looks like this is a cross-platform problem..
First connection:
Tue Jan 3 01:13:59 2017 us=913576 PUSH: Received control message: 'PUSH_REPLY,tun-ipv6,route-gateway 10.9.0.1,topology subnet,ping 10,ping-restart 120,ifconfig 10.9.0.10 255.255.255.0,peer-id 1,cipher AES-256-GCM' Tue Jan 3 01:13:59 2017 us=913893 OPTIONS IMPORT: timers and/or timeouts modified Tue Jan 3 01:13:59 2017 us=913919 OPTIONS IMPORT: --ifconfig/up options modified Tue Jan 3 01:13:59 2017 us=913941 OPTIONS IMPORT: route-related options modified Tue Jan 3 01:13:59 2017 us=913968 OPTIONS IMPORT: peer-id set Tue Jan 3 01:13:59 2017 us=914019 OPTIONS IMPORT: data channel crypto options modified Tue Jan 3 01:13:59 2017 us=914191 Data Channel Encrypt: Cipher 'AES-256-GCM' initialized with 256 bit key Tue Jan 3 01:13:59 2017 us=914222 Data Channel Decrypt: Cipher 'AES-256-GCM' initialized with 256 bit key Tue Jan 3 01:13:59 2017 us=914249 Preserving previous TUN/TAP instance: tun0 Tue Jan 3 01:13:59 2017 us=914291 Initialization Sequence Completed
kill -USR1 client-pid to restart:
Tue Jan 3 01:14:09 2017 us=103647 event_wait : Interrupted system call (code=4) Tue Jan 3 01:14:09 2017 us=103816 TCP/UDP: Closing socket Tue Jan 3 01:14:09 2017 us=103854 SIGUSR1[hard,] received, process restarting Tue Jan 3 01:14:09 2017 us=103873 Restart pause, 5 second(s) Tue Jan 3 01:14:14 2017 us=126645 Re-using SSL/TLS context Tue Jan 3 01:14:14 2017 us=177036 Local Options String (VER=V4): 'V4,dev-type tun,link-mtu 1557,tun-mtu 1500,proto UDPv4,keydir 1,cipher AES-256-CBC,auth SHA1,keysize 256,tls-auth,key-method 2,tls-client' Tue Jan 3 01:14:14 2017 us=177053 Expected Remote Options String (VER=V4): 'V4,dev-type tun,link-mtu 1557,tun-mtu 1500,proto UDPv4,keydir 0,cipher AES-256-CBC,auth SHA1,keysize 256,tls-auth,key-method 2,tls-server' Tue Jan 3 01:14:14 2017 us=177075 TCP/UDP: Preserving recently used remote address: [AF_INET]xx.xx.xx.136:1194 Tue Jan 3 01:14:14 2017 us=220250 TLS: Initial packet from [AF_INET]xx:1194, sid=8ea5f488 c9741ca4 Tue Jan 3 01:14:14 2017 us=265140 VERIFY OK: depth=1, C=CA, ST=ON, ...... Tue Jan 3 01:14:14 2017 us=266554 VERIFY OK: depth=0, C=CA, ST=ON, ...... Tue Jan 3 01:14:14 2017 us=378413 Control Channel: TLSv1.2, cipher TLSv1/SSLv3 ECDHE-RSA-AES256-GCM-SHA384, 1024 bit RSA Tue Jan 3 01:14:14 2017 us=378466 [xxxxx] Peer Connection Initiated with [AF_INET]xx.xx.xx.136:1194 Tue Jan 3 01:14:15 2017 us=520680 SENT CONTROL [sanselvpn]: 'PUSH_REQUEST' (status=1) Tue Jan 3 01:14:15 2017 us=553546 PUSH: Received control message: 'PUSH_REPLY,tun-ipv6,route-gateway 10.9.0.1,topology subnet,ping 10,ping-restart 120,ifconfig 10.9.0.2 255.255.255.0,peer-id 0,cipher AES-256-GCM' Tue Jan 3 01:14:15 2017 us=553723 OPTIONS IMPORT: timers and/or timeouts modified Tue Jan 3 01:14:15 2017 us=553737 OPTIONS IMPORT: --ifconfig/up options modified Tue Jan 3 01:14:15 2017 us=553749 OPTIONS IMPORT: route-related options modified Tue Jan 3 01:14:15 2017 us=553761 OPTIONS IMPORT: peer-id set Tue Jan 3 01:14:15 2017 us=553773 OPTIONS IMPORT: adjusting link_mtu to 1624 Tue Jan 3 01:14:15 2017 us=553784 OPTIONS IMPORT: data channel crypto options modified Tue Jan 3 01:14:15 2017 us=553935 Data Channel Encrypt: Cipher 'AES-256-GCM' initialized with 256 bit key Tue Jan 3 01:14:15 2017 us=553953 Data Channel Decrypt: Cipher 'AES-256-GCM' initialized with 256 bit key Tue Jan 3 01:14:15 2017 us=553970 Preserving previous TUN/TAP instance: tun0 Tue Jan 3 01:14:15 2017 us=554001 Initialization Sequence Completed
Note the pushed ip changed from 10.9.0.10 to 10.9.0.2 but the client doesn't care.
Change History (7)
comment:1 Changed 8 years ago by
comment:2 Changed 8 years ago by
Cc: | Steffan Karger added |
---|
This sounds related to this commit:
commit ec4dff3bbdcc9fedf7844701dc5aa2679d503667 Author: Steffan Karger <steffan@karger.me> Date: Thu Dec 15 22:46:06 2016 +0100 Don't reopen tun if cipher changes When the pulled options change, OpenVPN will attempt to reopen the tun device. That might fail if the process has already dropper privileges, and is not needed unless the tun MTU is changed. This patch therefore ignores the cipher value for the digest if a fixed tun-mtu is used. Additionally, this patch changes the md_ctx_update() call to include the trailing zero byte of each option, to make sure that parsing "foo,bar" results in a different hash than "foobar". (Sorry for not catching that during the review...) The unit tests are a bit lame, but it secretly serves as a way to lower the bar for adding more buffer.c unit tests. Trac: #761 Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: David Sommerseth <davids@openvpn.net> Message-Id: <1481838366-32335-1-git-send-email-steffan@karger.me> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13579.html Signed-off-by: David Sommerseth <davids@openvpn.net>
syzzer? The md_ctx_update() seems to be doing the wrong thing ....
comment:3 Changed 8 years ago by
I see this difference between the mtx_ctx_update() before and after this commit:
- md_ctx_update(ctx, (const uint8_t *) line, strlen(line)); ... + md_ctx_update(ctx, (const uint8_t *) line, strlen(line)+1);
Is it the +1 which causes havoc?
comment:4 Changed 8 years ago by
Not the +1, but the call is moved out of the while loop so its evaluated only after the loop completes at which point all that remains is an empty string. The +1 makes the terminating nul included in the digest computation, so finally we get the digest of just '\0'.
comment:5 Changed 8 years ago by
commit a5dbf8c8dab23c47407c3f833c4f4aae52408af1 (master)
commit bf72ae68c05922c289056f2f851ed36014449cdf (release/2.4)
Author: Selva Nair
Date: Tue Jan 3 16:42:18 2017 -0500
Fix push options digest update
comment:6 Changed 8 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:7 Changed 8 years ago by
Milestone: | → release 2.4.1 |
---|
Oh oh... more debugging shows something interesting.. The push options digest is the same no matter what I push. Here it is
93b885adfe0da089cdf634904fd59f71
,which is the digest of a null character -- strange, not a null string "" but of a nul terminator:
perl -e 'print chr(0)' | md5sum = 93b885adfe0da089cdf634904fd59f71
Unless I'm doing something wrong, the digest is broken.