Browse code

Implement client side handling of AUTH_PENDING message

This allows a client to extend the timeout of pull-request response
while waiting for the user to complete a pending authentication. A
timeout of 60s for a normal authentication might still works for a
simple 2FA (but still challenging). With a sophisticated (or overly
complicated) web based authentication 60s are quite short.

To avoid not detecting network problem in this phase, we use the
constant sending of PUSH_REQUEST/AUTH_PENDING as keepalive signal
and still timeout the session after the handshake window time.

patch v2: typo fixes, invert if for sscanf

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20210125125628.30364-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21491.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Arne Schwabe authored on 2021/01/25 21:56:19
Showing 9 changed files
... ...
@@ -442,6 +442,10 @@ fast hardware. SSL/TLS authentication must be used in this mode.
442 442
     - bit 1: The peer supports peer-id floating mechanism
443 443
     - bit 2: The client expects a push-reply and the server may
444 444
       send this reply without waiting for a push-request first.
445
+    - bit 3: The client is capable of doing key derivation using
446
+      RFC5705 key material exporter.
447
+    - bit 4: The client is capable of accepting additional arguments
448
+      to the `AUTH_PENDING` message.
445 449
 
446 450
   :code:`IV_NCP=2`
447 451
         Negotiable ciphers, client supports ``--cipher`` pushed by
... ...
@@ -610,14 +610,30 @@ to signal a pending authenticating to the client. A pending auth means
610 610
 that the connecting requires extra authentication like a one time
611 611
 password or doing a single sign one via web.
612 612
 
613
-    client-pending-auth {CID} {EXTRA}
614
-
615
-The server will send AUTH_PENDING and INFO_PRE,{EXTRA} to the client.
616
-The client is expected to inform the user that authentication is pending and
617
-display the extra information. For the format of EXTRA see below
618
-For the OpenVPN server this is stateless operation and needs to be
619
-followed by a client-deny/client-auth[-nt] command (that is the result of the
620
-out of band authentication).
613
+    client-pending-auth {CID} {EXTRA} {TIMEOUT}
614
+
615
+The server will send AUTH_PENDING and INFO_PRE,{EXTRA} to the client. If the
616
+client supports accepting keywords to AUTH_PENDING (announced via IV_PROTO),
617
+TIMEOUT parameter will be also be announced to the client to allow it to modify
618
+its own timeout. The client is expected to inform the user that authentication
619
+is pending and display the extra information and also show the user the
620
+remaining time to complete the auth if applicable.
621
+
622
+Receiving an AUTH_PENDING message will make the client change its timeout to
623
+the timeout proposed by the server, even if the timeout is shorter.
624
+If the client does not receive a packet from the server for hand-window the
625
+connection times out regardless of the timeout. This ensures that the connection
626
+still times out relatively quickly in case of network problems. The client will
627
+continously send PULL_REQUEST messages to the server until the timeout is reached.
628
+This message also triggers an ACK message from the server that resets the
629
+hand-window based timeout.
630
+
631
+Both client and server limit the maximum timeout to the smaller value of half the
632
+--tls-reneg minimum time and --hand-window time (defaults to 60s).
633
+
634
+For the format of EXTRA see below. For the OpenVPN server this is a stateless
635
+operation and needs to be followed by a client-deny/client-auth[-nt] command
636
+(that is the result of the out of band authentication).
621 637
 
622 638
 Before issuing a client-pending-auth to a client instead of a
623 639
 client-auth/client-deny, the server should check the IV_SSO
... ...
@@ -630,7 +646,7 @@ set
630 630
     setenv IV_SSO openurl,crtext
631 631
 
632 632
 The variable name IV_SSO is historic as AUTH_PENDING was first used
633
-to signal single sign on support. To keep compatiblity with existing
633
+to signal single sign on support. To keep compatibility with existing
634 634
 implementations the name IV_SSO is kept in lieu of a better name.
635 635
 
636 636
 openurl
... ...
@@ -646,6 +662,11 @@ The space in a control message is limited, so this url should be kept
646 646
 short to avoid issues. If a loger url is required a URL that redirects
647 647
 to the longer URL should be sent instead.
648 648
 
649
+A complete documentation how URLs should be handled on the client is available
650
+in the openvpn3 repository:
651
+
652
+https://github.com/OpenVPN/openvpn3/blob/master/doc/webauth.md
653
+
649 654
 url_proxy
650 655
 ========
651 656
 To avoid issues with OpenVPN connection persist-tun and not able
... ...
@@ -240,6 +240,10 @@ check_incoming_control_channel(struct context *c)
240 240
         {
241 241
             receive_cr_response(c, &buf);
242 242
         }
243
+        else if (buf_string_match_head_str(&buf, "AUTH_PENDING"))
244
+        {
245
+            receive_auth_pending(c, &buf);
246
+        }
243 247
         else
244 248
         {
245 249
             msg(D_PUSH_ERRORS, "WARNING: Received unknown control message: %s", BSTR(&buf));
... ...
@@ -299,7 +303,12 @@ check_connection_established(struct context *c)
299 299
             }
300 300
 #endif
301 301
             /* fire up push request right away (already 1s delayed) */
302
-            c->c2.push_request_timeout = now + c->options.handshake_window;
302
+            /* We might receive a AUTH_PENDING request before we armed this
303
+             * timer. In that case we don't change the value */
304
+            if (c->c2.push_request_timeout < now)
305
+            {
306
+                c->c2.push_request_timeout = now + c->options.handshake_window;
307
+            }
303 308
             event_timeout_init(&c->c2.push_request_interval, 0, now);
304 309
             reset_coarse_timers(c);
305 310
         }
... ...
@@ -39,6 +39,31 @@
39 39
 /*
40 40
  * min/max functions
41 41
  */
42
+static inline unsigned int
43
+max_uint(unsigned int x, unsigned int y)
44
+{
45
+    if (x > y)
46
+    {
47
+        return x;
48
+    }
49
+    else
50
+    {
51
+        return y;
52
+    }
53
+}
54
+
55
+static inline unsigned int
56
+min_uint(unsigned int x, unsigned int y)
57
+{
58
+    if (x < y)
59
+    {
60
+        return x;
61
+    }
62
+    else
63
+    {
64
+        return y;
65
+    }
66
+}
42 67
 
43 68
 static inline int
44 69
 max_int(int x, int y)
... ...
@@ -232,6 +232,62 @@ receive_cr_response(struct context *c, const struct buffer *buffer)
232 232
 }
233 233
 
234 234
 /**
235
+ * Parse the keyword for the AUTH_PENDING request
236
+ * @param buffer                buffer containing the keywords, the buffer's
237
+ *                              content will be modified by this function
238
+ * @param server_timeout        timeout pushed by the server or unchanged
239
+ *                              if the server does not push a timeout
240
+ */
241
+static void
242
+parse_auth_pending_keywords(const struct buffer *buffer,
243
+                            unsigned int *server_timeout)
244
+{
245
+    struct buffer buf = *buffer;
246
+
247
+    /* does the buffer start with "AUTH_PENDING," ? */
248
+    if (!buf_advance(&buf, strlen("AUTH_PENDING"))
249
+        || !(buf_read_u8(&buf) == ',') || !BLEN(&buf))
250
+    {
251
+        return;
252
+    }
253
+
254
+    /* parse the keywords in the same way that push options are parsed */
255
+    char line[OPTION_LINE_SIZE];
256
+
257
+    while (buf_parse(&buf, ',', line, sizeof(line)))
258
+    {
259
+        if (sscanf(line, "timeout %u", server_timeout) != 1)
260
+        {
261
+            msg(D_PUSH, "ignoring AUTH_PENDING parameter: %s", line);
262
+        }
263
+    }
264
+}
265
+
266
+void
267
+receive_auth_pending(struct context *c, const struct buffer *buffer)
268
+{
269
+    if (!c->options.pull)
270
+        return;
271
+
272
+    /* Cap the increase at the maximum time we are willing stay in the
273
+     * pending authentication state */
274
+    unsigned int max_timeout = max_uint(c->options.renegotiate_seconds/2,
275
+                               c->options.handshake_window);
276
+
277
+    /* try to parse parameter keywords, default to hand-winow timeout if the
278
+     * server does not supply a timeout */
279
+    unsigned int server_timeout = c->options.handshake_window;
280
+    parse_auth_pending_keywords(buffer, &server_timeout);
281
+
282
+    msg(D_PUSH, "AUTH_PENDING received, extending handshake timeout from %us "
283
+                "to %us", c->options.handshake_window,
284
+                min_uint(max_timeout, server_timeout));
285
+
286
+    struct key_state *ks = &c->c2.tls_multi->session[TM_ACTIVE].key[KS_PRIMARY];
287
+    c->c2.push_request_timeout = ks->established + min_uint(max_timeout, server_timeout);
288
+}
289
+
290
+/**
235 291
  * Add an option to the given push list by providing a format string.
236 292
  *
237 293
  * The string added to the push options is allocated in o->gc, so the caller
... ...
@@ -372,7 +428,17 @@ send_push_request(struct context *c)
372 372
     struct tls_session *session = &c->c2.tls_multi->session[TM_ACTIVE];
373 373
     struct key_state *ks = &session->key[KS_PRIMARY];
374 374
 
375
-    if (c->c2.push_request_timeout > now)
375
+    /* We timeout here under two conditions:
376
+     * a) we reached the hard limit of push_request_timeout
377
+     * b) we have not seen anything from the server in hand_window time
378
+     *
379
+     * for non auth-pending scenario, push_request_timeout is the same as
380
+     * hand_window timeout. For b) every PUSH_REQUEST is a acknowledged by
381
+     * the server by a P_ACK_V1 packet that reset the keepalive timer
382
+     */
383
+
384
+    if (c->c2.push_request_timeout > now
385
+        && (now - ks->peer_last_packet) < c->options.handshake_window)
376 386
     {
377 387
         return send_control_channel_string(c, "PUSH_REQUEST", D_PUSH);
378 388
     }
... ...
@@ -89,5 +89,14 @@ void send_restart(struct context *c, const char *kill_msg);
89 89
  */
90 90
 void send_push_reply_auth_token(struct tls_multi *multi);
91 91
 
92
+/**
93
+ * Parses an AUTH_PENDING message and if in pull mode extends the timeout
94
+ *
95
+ * @param c             The context struct
96
+ * @param buffer        Buffer containing the control message with AUTH_PENDING
97
+ */
98
+void
99
+receive_auth_pending(struct context *c, const struct buffer *buffer);
100
+
92 101
 #endif /* if P2MP */
93 102
 #endif /* ifndef PUSH_H */
... ...
@@ -2268,6 +2268,7 @@ push_peer_info(struct buffer *buf, struct tls_session *session)
2268 2268
         if (session->opt->pull)
2269 2269
         {
2270 2270
             iv_proto |= IV_PROTO_REQUEST_PUSH;
2271
+            iv_proto |= IV_PROTO_AUTH_PENDING_KW;
2271 2272
         }
2272 2273
 
2273 2274
         /* support for Negotiable Crypto Parameters */
... ...
@@ -3733,6 +3734,8 @@ tls_pre_decrypt(struct tls_multi *multi,
3733 3733
             }
3734 3734
         }
3735 3735
     }
3736
+    /* Remember that we received a valid control channel packet */
3737
+    ks->peer_last_packet = now;
3736 3738
 
3737 3739
 done:
3738 3740
     buf->len = 0;
... ...
@@ -119,6 +119,9 @@
119 119
 /** Supports key derivation via TLS key material exporter [RFC5705] */
120 120
 #define IV_PROTO_TLS_KEY_EXPORT (1<<3)
121 121
 
122
+/** Supports signaling keywords with AUTH_PENDING, e.g. timeout=xy */
123
+#define IV_PROTO_AUTH_PENDING_KW (1<<4)
124
+
122 125
 /* Default field in X509 to be username */
123 126
 #define X509_USERNAME_FIELD_DEFAULT "CN"
124 127
 
... ...
@@ -178,6 +178,7 @@ struct key_state
178 178
     time_t established;         /* when our state went S_ACTIVE */
179 179
     time_t must_negotiate;      /* key negotiation times out if not finished before this time */
180 180
     time_t must_die;            /* this object is destroyed at this time */
181
+    time_t peer_last_packet;    /* Last time we received a packet in this control session */
181 182
 
182 183
     int initial_opcode;         /* our initial P_ opcode */
183 184
     struct session_id session_id_remote; /* peer's random session ID */