| Message ID | 20241023134903.66485-1-rzvncj@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Openvpn-devel] Allow setting an empty auth-token in push replies | expand |
Hi, On Wed, Oct 23, 2024 at 04:49:03PM +0300, Razvan Cojocaru wrote: > This in turn allows the server to signal to the client that it > should no longer attempt to reconnect, if it wants to keep the > client out after an AUTH_FAILED. This should not be necessary. After an AUTH_FAILED the token is already invalidated on the client side. Can you describe your use case a bit better? This code is complex enough as it is, so I'm not really willing to add more cases to it (which I do have to test then and forever) if not needed. gert
On 10/23/24 17:23, Gert Doering wrote: > Hi, > > On Wed, Oct 23, 2024 at 04:49:03PM +0300, Razvan Cojocaru wrote: >> This in turn allows the server to signal to the client that it >> should no longer attempt to reconnect, if it wants to keep the >> client out after an AUTH_FAILED. > > This should not be necessary. After an AUTH_FAILED the token is > already invalidated on the client side. > > Can you describe your use case a bit better? This code is complex > enough as it is, so I'm not really willing to add more cases to it > (which I do have to test then and forever) if not needed. Thanks for the quick reply! Of course! The use case pertains to the device posture checks that OpenVPN has rolled out. Long story short, these are meant to allow the server to make decisions on whether the client is allowed to connect (and stay connected, which is the part I'm addressing here) based on information about it. For example, if it has an up-to-date antivirus, or if it has full disk encryption enabled. So a client may connect and be allowed in, then for some reason its antivirus may not be updated for a considerable while, at which point a check will want to disconnect said client. Reconnects will be possible after whoever manages the client machine updates the antivirus. In this case, we want to disconnect the client and it should stay disconnected. A simple AUTH_FAILED for this scenario will have the client attempt another connection. But if we invalidate the token, then the client will not attempt to reconnect. Another reason for this patch is that the OpenVPN 3 client does allow setting the auth-token to an empty string, and when it is empty the logic is to not reconnect - so this would allow both version 2 and version 3 clients to behave the same way. Thanks, Razvan
Hi, On Wed, Oct 23, 2024 at 05:40:43PM +0300, Razvan Cojocaru wrote: > In this case, we want to disconnect the client and it should stay > disconnected. A simple AUTH_FAILED for this scenario will have the client > attempt another connection. But if we invalidate the token, then the client > will not attempt to reconnect. AUTH_FAILED should do this automatically - invalidate the token, that is. Can you show a log where this is (not) happening? gert
On 10/23/24 17:43, Gert Doering wrote: > Hi, > > On Wed, Oct 23, 2024 at 05:40:43PM +0300, Razvan Cojocaru wrote: >> In this case, we want to disconnect the client and it should stay >> disconnected. A simple AUTH_FAILED for this scenario will have the client >> attempt another connection. But if we invalidate the token, then the client >> will not attempt to reconnect. > > AUTH_FAILED should do this automatically - invalidate the token, that is. > Can you show a log where this is (not) happening? Of course: 2024-10-23 14:52:06 us=368754 PUSH: Received control message: 'PUSH_REPLY,auth-token' 2024-10-23 14:52:06 us=368851 UDPv4 WRITE [90] to [AF_INET]69.162.107.71:1194: P_ACK_V1 kid=0 pid=[ #13 ] [ 8 7 6 5 4 3 2 1 ] DATA len=0 2024-10-23 14:52:06 us=368936 UDPv4 READ [163] from [AF_INET]69.162.107.71:1194: P_CONTROL_V1 kid=0 pid=[ #12 ] [ 2 3 4 5 ] pid=9 DATA len=85 2024-10-23 14:52:06 us=368972 AUTH: Received control message: AUTH_FAILED,No Stairway to Heaven allowed in this guitar store 2024-10-23 14:52:06 us=369228 TCP/UDP: Closing socket 2024-10-23 14:52:06 us=369287 SIGUSR1[soft,auth-failure (auth-token)] received, process restarting 2024-10-23 14:52:06 us=369346 Restart pause, 1 second(s) And with this patch: 2024-10-23 17:46:58 us=427109 PUSH: Received control message: 'PUSH_REPLY,auth-token' 2024-10-23 17:46:58 us=427149 UDPv4 WRITE [90] to [AF_INET]69.162.107.71:1194: P_ACK_V1 kid=0 pid=[ #12 ] [ 8 7 6 5 4 3 2 1 ] DATA len=0 2024-10-23 17:46:58 us=427371 UDPv4 READ [163] from [AF_INET]69.162.107.71:1194: P_CONTROL_V1 kid=0 pid=[ #12 ] [ 2 3 4 5 ] pid=9 DATA len=85 2024-10-23 17:46:58 us=427403 AUTH: Received control message: AUTH_FAILED,No Stairway to Heaven allowed in this guitar store 2024-10-23 17:46:58 us=427414 register signal: SIGTERM (auth-failure) 2024-10-23 17:46:58 us=427427 SIGTERM received, sending exit notification to peer 2024-10-23 17:46:58 us=427442 signal_reset: signal UNKNOWN is cleared 2024-10-23 17:46:58 us=427464 UDPv4 WRITE [90] to [AF_INET]69.162.107.71:1194: P_ACK_V1 kid=0 pid=[ #13 ] [ 9 8 7 6 5 4 3 2 ] DATA len=0 2024-10-23 17:46:58 us=427501 UDPv4 WRITE [41] to [AF_INET]69.162.107.71:1194: P_DATA_V2 kid=0 DATA len=40 2024-10-23 17:46:59 us=679084 register signal: SIGTERM (exit-with-notification) 2024-10-23 17:46:59 us=679264 TCP/UDP: Closing socket 2024-10-23 17:46:59 us=679300 net_route_v4_del: 100.96.0.0/11 via 100.96.1.1 dev [NULL] table 0 metric -1 2024-10-23 17:46:59 us=679388 sitnl_send: checking for received messages 2024-10-23 17:46:59 us=679406 sitnl_send: rtnl: received 36 bytes 2024-10-23 17:46:59 us=679437 net_route_v4_del: 100.80.0.0/12 via 100.96.1.1 dev [NULL] table 0 metric -1 2024-10-23 17:46:59 us=679477 sitnl_send: checking for received messages 2024-10-23 17:46:59 us=679500 sitnl_send: rtnl: received 36 bytes 2024-10-23 17:46:59 us=679519 delete_route_ipv6(fd:0:0:8000::/49) 2024-10-23 17:46:59 us=679531 net_route_v6_del: fd:0:0:8000::/49 via :: dev tun1 table 0 metric -1 2024-10-23 17:46:59 us=679618 sitnl_send: checking for received messages 2024-10-23 17:46:59 us=679639 sitnl_send: rtnl: received 36 bytes 2024-10-23 17:46:59 us=679663 delete_route_ipv6(fd:0:0:4000::/50) 2024-10-23 17:46:59 us=679678 net_route_v6_del: fd:0:0:4000::/50 via :: dev tun1 table 0 metric -1 2024-10-23 17:46:59 us=679743 sitnl_send: checking for received messages 2024-10-23 17:46:59 us=679763 sitnl_send: rtnl: received 36 bytes 2024-10-23 17:46:59 us=679801 Closing tun/tap interface 2024-10-23 17:46:59 us=679817 net_addr_v4_del: 100.96.1.2 dev tun1 2024-10-23 17:46:59 us=679930 sitnl_send: checking for received messages 2024-10-23 17:46:59 us=679976 sitnl_send: rtnl: received 36 bytes 2024-10-23 17:46:59 us=679997 net_addr_v6_del: fd:0:0:8100::2/64 dev tun1 2024-10-23 17:46:59 us=680107 sitnl_send: checking for received messages 2024-10-23 17:46:59 us=680129 sitnl_send: rtnl: received 36 bytes 2024-10-23 17:46:59 us=725831 SIGTERM[soft,exit-with-notification] received, process exiting Thanks, Razvan
Hi, On Wed, Oct 23, 2024 at 05:47:51PM +0300, Razvan Cojocaru wrote: > > AUTH_FAILED should do this automatically - invalidate the token, that is. > > Can you show a log where this is (not) happening? > > Of course: > > 2024-10-23 14:52:06 us=368754 PUSH: Received control message: > 'PUSH_REPLY,auth-token' > 2024-10-23 14:52:06 us=368851 UDPv4 WRITE [90] to > [AF_INET]69.162.107.71:1194: P_ACK_V1 kid=0 pid=[ #13 ] [ 8 7 6 5 4 3 2 1 ] > DATA len=0 > 2024-10-23 14:52:06 us=368936 UDPv4 READ [163] from > [AF_INET]69.162.107.71:1194: P_CONTROL_V1 kid=0 pid=[ #12 ] [ 2 3 4 5 ] > pid=9 DATA len=85 > 2024-10-23 14:52:06 us=368972 AUTH: Received control message: AUTH_FAILED,No > Stairway to Heaven allowed in this guitar store > 2024-10-23 14:52:06 us=369228 TCP/UDP: Closing socket > 2024-10-23 14:52:06 us=369287 SIGUSR1[soft,auth-failure (auth-token)] > received, process restarting > 2024-10-23 14:52:06 us=369346 Restart pause, 1 second(s) OK, so I see what is happening - you're sending an AUTH_FAILED "out of the blue", not in response to a client handshake, right? OpenVPN 2 *should* invalidate the token upon the reconnect (and then getting an AUTH_FAILED)... so what happens in this case if you let it reconnect? gert
On 10/23/24 17:50, Gert Doering wrote: > OK, so I see what is happening - you're sending an AUTH_FAILED "out of > the blue", not in response to a client handshake, right? Exactly. In response to a client handshake there's no problem. > OpenVPN 2 *should* invalidate the token upon the reconnect (and then > getting an AUTH_FAILED)... so what happens in this case if you let it > reconnect? If we let it reconnect, the first re-connection attempt will fail at the handshake stage as expected. But OpenVPN GUI tools will tend to show _two_ failed connections (the first "out of the blue" AUTH_FAILED one, and the second actual one), which is confusing for clients. Thanks, Razvan
On Wed, Oct 23, 2024 at 11:03 AM Razvan Cojocaru <rzvncj@gmail.com> wrote: > On 10/23/24 17:50, Gert Doering wrote: > > OK, so I see what is happening - you're sending an AUTH_FAILED "out of > > the blue", not in response to a client handshake, right? > > Exactly. In response to a client handshake there's no problem. > > > OpenVPN 2 *should* invalidate the token upon the reconnect (and then > > getting an AUTH_FAILED)... so what happens in this case if you let it > > reconnect? > > If we let it reconnect, the first re-connection attempt will fail at the > handshake stage as expected. > > But OpenVPN GUI tools will tend to show _two_ failed connections (the > first "out of the blue" AUTH_FAILED one, and the second actual one), > which is confusing for clients. > Wouldn't pushing "HALT" instead of "AUTH_FAILED" work in this case? As in the management command "client-kill {cid} HALT" which calls send_restart() with kill_msg = "HALT". Selva
On 10/23/24 18:25, Selva Nair wrote: > Wouldn't pushing "HALT" instead of "AUTH_FAILED" work in this case? > As in the management command "client-kill {cid} HALT" which calls > send_restart() with kill_msg = "HALT". Possibly, however the intent has always been to use this feature to reject (authorize) clients (so this is a corner case of that, just that we can retract authorization at a later time), and in addition considerable work has already been done that relies on the AUTH_FAILED code paths. Thanks, Razvan
On Wed, Oct 23, 2024 at 11:47 AM Razvan Cojocaru <rzvncj@gmail.com> wrote: > On 10/23/24 18:25, Selva Nair wrote: > > Wouldn't pushing "HALT" instead of "AUTH_FAILED" work in this case? > > As in the management command "client-kill {cid} HALT" which calls > > send_restart() with kill_msg = "HALT". > > Possibly, however the intent has always been to use this feature to > reject (authorize) clients (so this is a corner case of that, just that > we can retract authorization at a later time), and in addition > considerable work has already been done that relies on the AUTH_FAILED > code paths. > > Looks like a misuse of AUTH_FAILED to me. To kill a client while not in the authentication phase, use code paths meant for that purpose. Selva
diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index 70ba5e4d..82ac8056 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -524,6 +524,11 @@ set_auth_token(struct user_pass *tk, const char *token) } protect_user_pass(tk); } + else + { + tk->defined = false; + tk->token_defined = false; + } } void
This in turn allows the server to signal to the client that it should no longer attempt to reconnect, if it wants to keep the client out after an AUTH_FAILED. Signed-off-by: Razvan Cojocaru <rzvncj@gmail.com> --- src/openvpn/misc.c | 5 +++++ 1 file changed, 5 insertions(+)