Browse code

Use separate list for per-client push options

v4:
- fix whitespaces, wrap long lines

v3:
- rebase on master

v2:
- Also move ifconfig and ipv6-ifconfig to separate options list

Move client-specific push options (currently peer-id and cipher) to
separate list, which is deallocated after push_reply
has been send. This makes sure that options fit into buf,
not duplicated nor leak memory on renegotiation.

Signed-off-by: Lev Stipakov <lstipakov@gmail.com>

Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <1476173030-2171-1-git-send-email-lstipakov@gmail.com>
URL: http://www.mail-archive.com/search?l=mid&q=1476173030-2171-1-git-send-email-lstipakov@gmail.com
Signed-off-by: David Sommerseth <davids@openvpn.net>

Lev Stipakov authored on 2016/10/11 17:03:50
Showing 1 changed files
... ...
@@ -40,26 +40,29 @@
40 40
 
41 41
 #if P2MP
42 42
 
43
+static char push_reply_cmd[] = "PUSH_REPLY";
44
+
43 45
 /**
44
- * Add an option to the push list by providing a format string.
46
+ * Add an option to the given push list by providing a format string.
45 47
  *
46 48
  * The string added to the push options is allocated in o->gc, so the caller
47 49
  * does not have to preserve anything.
48 50
  *
49
- * @param o		The current connection's options
50
- * @param msglevel	The message level to use when printing errors
51
- * @param fmt		Format string for the option
52
- * @param ...		Format string arguments
51
+ * @param gc        GC arena where options are allocated
52
+ * @param push_list Push list containing options
53
+ * @param msglevel  The message level to use when printing errors
54
+ * @param fmt       Format string for the option
55
+ * @param ...       Format string arguments
53 56
  *
54 57
  * @return true on success, false on failure.
55 58
  */
56
-static bool push_option_fmt(struct options *o, int msglevel,
57
-    const char *fmt, ...)
59
+static bool push_option_fmt(struct gc_arena *gc, struct push_list *push_list,
60
+			    int msglevel, const char *fmt, ...)
58 61
 #ifdef __GNUC__
59 62
 #if __USE_MINGW_ANSI_STDIO
60
-    __attribute__ ((format (gnu_printf, 3, 4)))
63
+    __attribute__ ((format (gnu_printf, 4, 5)))
61 64
 #else
62
-    __attribute__ ((format (__printf__, 3, 4)))
65
+    __attribute__ ((format (__printf__, 4, 5)))
63 66
 #endif
64 67
 #endif
65 68
     ;
... ...
@@ -295,16 +298,43 @@ send_push_request (struct context *c)
295 295
 /**
296 296
  * Prepare push options, based on local options and available peer info.
297 297
  *
298
- * @param options	Connection options
299
- * @param tls_multi	TLS state structure for the current tunnel
298
+ * @param context	context structure storing data for VPN tunnel
299
+ * @param gc     	gc arena for allocating push options
300
+ * @param push_list	push list to where options are added
300 301
  *
301 302
  * @return true on success, false on failure.
302 303
  */
303 304
 static bool
304
-prepare_push_reply (struct options *o, struct tls_multi *tls_multi)
305
+prepare_push_reply (struct context *c, struct gc_arena *gc,
306
+		    struct push_list *push_list)
305 307
 {
306 308
   const char *optstr = NULL;
309
+  const struct tls_multi *tls_multi = c->c2.tls_multi;
307 310
   const char * const peer_info = tls_multi->peer_info;
311
+  struct options *o = &c->options;
312
+
313
+  /* ipv6 */
314
+  if (c->c2.push_ifconfig_ipv6_defined && !o->push_ifconfig_ipv6_blocked)
315
+    {
316
+      push_option_fmt (gc, push_list, M_USAGE, "ifconfig-ipv6 %s/%d %s",
317
+		       print_in6_addr (c->c2.push_ifconfig_ipv6_local, 0, gc),
318
+		       c->c2.push_ifconfig_ipv6_netbits,
319
+		       print_in6_addr (c->c2.push_ifconfig_ipv6_remote,
320
+				       0, gc));
321
+    }
322
+
323
+  /* ipv4 */
324
+  if (c->c2.push_ifconfig_defined && c->c2.push_ifconfig_local &&
325
+      c->c2.push_ifconfig_remote_netmask)
326
+    {
327
+      in_addr_t ifconfig_local = c->c2.push_ifconfig_local;
328
+      if (c->c2.push_ifconfig_local_alias)
329
+	ifconfig_local = c->c2.push_ifconfig_local_alias;
330
+      push_option_fmt (gc, push_list, M_USAGE, "ifconfig %s %s",
331
+		       print_in_addr_t (ifconfig_local, 0, gc),
332
+		       print_in_addr_t (c->c2.push_ifconfig_remote_netmask,
333
+					0, gc));
334
+    }
308 335
 
309 336
   /* Send peer-id if client supports it */
310 337
   optstr = peer_info ? strstr(peer_info, "IV_PROTO=") : NULL;
... ...
@@ -314,8 +344,8 @@ prepare_push_reply (struct options *o, struct tls_multi *tls_multi)
314 314
       int r = sscanf(optstr, "IV_PROTO=%d", &proto);
315 315
       if ((r == 1) && (proto >= 2))
316 316
 	{
317
-	  push_remove_option(o, "peer-id");
318
-	  push_option_fmt(o, M_USAGE, "peer-id %d", tls_multi->peer_id);
317
+	  push_option_fmt(gc, push_list, M_USAGE, "peer-id %d",
318
+			  tls_multi->peer_id);
319 319
 	}
320 320
     }
321 321
 
... ...
@@ -325,7 +355,7 @@ prepare_push_reply (struct options *o, struct tls_multi *tls_multi)
325 325
       /* if we have already created our key, we cannot change our own
326 326
        * cipher, so disable NCP and warn = explain why
327 327
        */
328
-      struct tls_session *session = &tls_multi->session[TM_ACTIVE];
328
+      const struct tls_session *session = &tls_multi->session[TM_ACTIVE];
329 329
       if ( session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized )
330 330
 	{
331 331
 	   msg( M_INFO, "PUSH: client wants to negotiate cipher (NCP), but "
... ...
@@ -336,87 +366,79 @@ prepare_push_reply (struct options *o, struct tls_multi *tls_multi)
336 336
 	{
337 337
 	  /* Push the first cipher from --ncp-ciphers to the client.
338 338
 	   * TODO: actual negotiation, instead of server dictatorship. */
339
-	  char *push_cipher = string_alloc(o->ncp_ciphers, &o->gc);
339
+	  char *push_cipher = string_alloc(o->ncp_ciphers, gc);
340 340
 	  o->ciphername = strtok (push_cipher, ":");
341
-	  push_remove_option(o, "cipher");
342
-	  push_option_fmt(o, M_USAGE, "cipher %s", o->ciphername);
341
+	  push_option_fmt(gc, push_list, M_USAGE, "cipher %s", o->ciphername);
343 342
 	}
344 343
     }
345 344
   return true;
346 345
 }
347 346
 
348 347
 static bool
349
-send_push_reply (struct context *c)
348
+send_push_options (struct context *c, struct buffer *buf,
349
+		   struct push_list *push_list, int safe_cap,
350
+		   bool *push_sent, bool *multi_push)
350 351
 {
351
-  struct gc_arena gc = gc_new ();
352
-  struct buffer buf = alloc_buf_gc (PUSH_BUNDLE_SIZE, &gc);
353
-  struct push_entry *e = c->options.push_list.head;
354
-  bool multi_push = false;
355
-  static char cmd[] = "PUSH_REPLY";
356
-  const int extra = 84; /* extra space for possible trailing ifconfig and push-continuation */
357
-  const int safe_cap = BCAP (&buf) - extra;
358
-  bool push_sent = false;
359
-
360
-  msg( M_INFO, "send_push_reply(): safe_cap=%d", safe_cap );
361
-
362
-  buf_printf (&buf, "%s", cmd);
363
-
364
-  if ( c->c2.push_ifconfig_ipv6_defined &&
365
-          !c->options.push_ifconfig_ipv6_blocked )
366
-    {
367
-      /* IPv6 is put into buffer first, could be lengthy */
368
-      buf_printf( &buf, ",ifconfig-ipv6 %s/%d %s",
369
-		    print_in6_addr( c->c2.push_ifconfig_ipv6_local, 0, &gc),
370
-		    c->c2.push_ifconfig_ipv6_netbits,
371
-		    print_in6_addr( c->c2.push_ifconfig_ipv6_remote, 0, &gc) );
372
-      if (BLEN (&buf) >= safe_cap)
373
-	{
374
-	  msg (M_WARN, "--push ifconfig-ipv6 option is too long");
375
-	  goto fail;
376
-	}
377
-    }
352
+  struct push_entry *e = push_list->head;
378 353
 
379 354
   while (e)
380 355
     {
381 356
       if (e->enable)
382 357
 	{
383 358
 	  const int l = strlen (e->option);
384
-	  if (BLEN (&buf) + l >= safe_cap)
359
+	  if (BLEN (buf) + l >= safe_cap)
385 360
 	    {
386
-	      buf_printf (&buf, ",push-continuation 2");
387
-	      {
388
-		const bool status = send_control_channel_string (c, BSTR (&buf), D_PUSH);
389
-		if (!status)
390
-		  goto fail;
391
-		push_sent = true;
392
-		multi_push = true;
393
-		buf_reset_len (&buf);
394
-		buf_printf (&buf, "%s", cmd);
395
-	      }
361
+	      buf_printf (buf, ",push-continuation 2");
362
+		{
363
+		  const bool status = send_control_channel_string (c, BSTR (buf), D_PUSH);
364
+		  if (!status)
365
+		    return false;
366
+		  *push_sent = true;
367
+		  *multi_push = true;
368
+		  buf_reset_len (buf);
369
+		  buf_printf (buf, "%s", push_reply_cmd);
370
+		}
396 371
 	    }
397
-	  if (BLEN (&buf) + l >= safe_cap)
372
+	  if (BLEN (buf) + l >= safe_cap)
398 373
 	    {
399 374
 	      msg (M_WARN, "--push option is too long");
400
-	      goto fail;
375
+	      return false;
401 376
 	    }
402
-	  buf_printf (&buf, ",%s", e->option);
377
+	  buf_printf (buf, ",%s", e->option);
403 378
 	}
404 379
       e = e->next;
405 380
     }
381
+  return true;
382
+}
383
+
384
+static bool
385
+send_push_reply (struct context *c, struct push_list *per_client_push_list)
386
+{
387
+  struct gc_arena gc = gc_new ();
388
+  struct buffer buf = alloc_buf_gc (PUSH_BUNDLE_SIZE, &gc);
389
+  bool multi_push = false;
390
+  const int extra = 84; /* extra space for possible trailing ifconfig and push-continuation */
391
+  const int safe_cap = BCAP (&buf) - extra;
392
+  bool push_sent = false;
393
+
394
+  msg( M_INFO, "send_push_reply(): safe_cap=%d", safe_cap );
395
+
396
+  buf_printf (&buf, "%s", push_reply_cmd);
397
+
398
+  /* send options which are common to all clients */
399
+  if (!send_push_options (c, &buf, &c->options.push_list, safe_cap,
400
+			  &push_sent, &multi_push))
401
+    goto fail;
402
+
403
+  /* send client-specific options */
404
+  if (!send_push_options (c, &buf, per_client_push_list, safe_cap,
405
+			  &push_sent, &multi_push))
406
+    goto fail;
406 407
 
407
-  if (c->c2.push_ifconfig_defined && c->c2.push_ifconfig_local && c->c2.push_ifconfig_remote_netmask)
408
-    {
409
-      in_addr_t ifconfig_local = c->c2.push_ifconfig_local;
410
-      if (c->c2.push_ifconfig_local_alias)
411
-	ifconfig_local = c->c2.push_ifconfig_local_alias;
412
-      buf_printf (&buf, ",ifconfig %s %s",
413
-		  print_in_addr_t (ifconfig_local, 0, &gc),
414
-		  print_in_addr_t (c->c2.push_ifconfig_remote_netmask, 0, &gc));
415
-    }
416 408
   if (multi_push)
417 409
     buf_printf (&buf, ",push-continuation 1");
418 410
 
419
-  if (BLEN (&buf) > sizeof(cmd)-1)
411
+  if (BLEN (&buf) > sizeof(push_reply_cmd)-1)
420 412
     {
421 413
       const bool status = send_control_channel_string (c, BSTR (&buf), D_PUSH);
422 414
       if (!status)
... ...
@@ -432,7 +454,7 @@ send_push_reply (struct context *c)
432 432
       bool status = false;
433 433
 
434 434
       buf_reset_len (&buf);
435
-      buf_printf (&buf, "%s", cmd);
435
+      buf_printf (&buf, "%s", push_reply_cmd);
436 436
       status = send_control_channel_string (c, BSTR(&buf), D_PUSH);
437 437
       if (!status)
438 438
 	goto fail;
... ...
@@ -447,7 +469,8 @@ send_push_reply (struct context *c)
447 447
 }
448 448
 
449 449
 static void
450
-push_option_ex (struct options *o, const char *opt, bool enable, int msglevel)
450
+push_option_ex (struct gc_arena *gc, struct push_list *push_list,
451
+		const char *opt, bool enable, int msglevel)
451 452
 {
452 453
   if (!string_class (opt, CC_ANY, CC_COMMA))
453 454
     {
... ...
@@ -456,20 +479,20 @@ push_option_ex (struct options *o, const char *opt, bool enable, int msglevel)
456 456
   else
457 457
     {
458 458
       struct push_entry *e;
459
-      ALLOC_OBJ_CLEAR_GC (e, struct push_entry, &o->gc);
459
+      ALLOC_OBJ_CLEAR_GC (e, struct push_entry, gc);
460 460
       e->enable = true;
461 461
       e->option = opt;
462
-      if (o->push_list.head)
462
+      if (push_list->head)
463 463
 	{
464
-	  ASSERT(o->push_list.tail);
465
-	  o->push_list.tail->next = e;
466
-	  o->push_list.tail = e;
464
+	  ASSERT(push_list->tail);
465
+	  push_list->tail->next = e;
466
+	  push_list->tail = e;
467 467
 	}
468 468
       else
469 469
 	{
470
-	  ASSERT(!o->push_list.tail);
471
-	  o->push_list.head = e;
472
-	  o->push_list.tail = e;
470
+	  ASSERT(!push_list->tail);
471
+	  push_list->head = e;
472
+	  push_list->tail = e;
473 473
 	}
474 474
     }
475 475
 }
... ...
@@ -477,7 +500,7 @@ push_option_ex (struct options *o, const char *opt, bool enable, int msglevel)
477 477
 void
478 478
 push_option (struct options *o, const char *opt, int msglevel)
479 479
 {
480
-  push_option_ex (o, opt, true, msglevel);
480
+  push_option_ex (&o->gc, &o->push_list, opt, true, msglevel);
481 481
 }
482 482
 
483 483
 void
... ...
@@ -489,7 +512,8 @@ clone_push_list (struct options *o)
489 489
       push_reset (o);
490 490
       while (e)
491 491
 	{
492
-	  push_option_ex (o, string_alloc (e->option, &o->gc), true, M_FATAL);
492
+	  push_option_ex (&o->gc, &o->push_list,
493
+			  string_alloc (e->option, &o->gc), true, M_FATAL);
493 494
 	  e = e->next;
494 495
 	}
495 496
     }
... ...
@@ -503,18 +527,18 @@ push_options (struct options *o, char **p, int msglevel, struct gc_arena *gc)
503 503
   push_option (o, opt, msglevel);
504 504
 }
505 505
 
506
-static bool push_option_fmt(struct options *o, int msglevel,
507
-    const char *format, ...)
506
+static bool push_option_fmt(struct gc_arena *gc, struct push_list *push_list,
507
+			    int msglevel, const char *format, ...)
508 508
 {
509 509
   va_list arglist;
510 510
   char tmp[256] = {0};
511
-  int len = -1;
511
+  int len;
512 512
   va_start (arglist, format);
513 513
   len = vsnprintf (tmp, sizeof(tmp), format, arglist);
514 514
   va_end (arglist);
515 515
   if (len > sizeof(tmp)-1)
516 516
     return false;
517
-  push_option (o, string_alloc (tmp, &o->gc), msglevel);
517
+  push_option_ex (gc, push_list, string_alloc (tmp, gc), true, msglevel);
518 518
   return true;
519 519
 }
520 520
 
... ...
@@ -582,12 +606,18 @@ process_incoming_push_request (struct context *c)
582 582
 	}
583 583
       else
584 584
 	{
585
-	  if (prepare_push_reply(&c->options, c->c2.tls_multi) &&
586
-	      send_push_reply (c))
585
+	  /* per-client push options - peer-id, cipher, ifconfig, ipv6-ifconfig */
586
+	  struct push_list push_list;
587
+	  struct gc_arena gc = gc_new ();
588
+
589
+	  CLEAR (push_list);
590
+	  if (prepare_push_reply (c, &gc, &push_list) &&
591
+	      send_push_reply (c, &push_list))
587 592
 	    {
588 593
 	      ret = PUSH_MSG_REQUEST;
589 594
 	      c->c2.sent_push_reply_expiry = now + 30;
590 595
 	    }
596
+	  gc_free(&gc);
591 597
 	}
592 598
     }
593 599
   else