Browse code

Fix duplicate PUSH_REPLY options

As reported by Lev Stipakov, starting from 3a5a46cf we add peer-id and
cipher values to context->options->push_list instead of adding those
directly to buf. Since push_list is preserved over sigusr1 restarts,
we add duplicate values for peer-id and cipher.

Fixed by removing the previous values from the list before adding new ones.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <CAA1Abx+1GQKipc1O1D2BXjDgrtDAFTa5GB2GUZKrT+-J-QsuNA@mail.gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg12642.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Steffan Karger authored on 2016/09/30 02:48:29
Showing 3 changed files
... ...
@@ -147,6 +147,7 @@
147 147
 #define D_PID_DEBUG          LOGLEV(7, 70, M_DEBUG)  /* show packet-id debugging info */
148 148
 #define D_PF_DROPPED_BCAST   LOGLEV(7, 71, M_DEBUG)  /* packet filter dropped a broadcast packet */
149 149
 #define D_PF_DEBUG           LOGLEV(7, 72, M_DEBUG)  /* packet filter debugging, must also define PF_DEBUG in pf.h */
150
+#define D_PUSH_DEBUG         LOGLEV(7, 73, M_DEBUG)  /* show push/pull debugging info */
150 151
 
151 152
 #define D_HANDSHAKE_VERBOSE  LOGLEV(8, 70, M_DEBUG)  /* show detailed description of each handshake */
152 153
 #define D_TLS_DEBUG_MED      LOGLEV(8, 70, M_DEBUG)  /* limited info from tls_session routines */
... ...
@@ -5787,6 +5787,7 @@ add_option (struct options *options,
5787 5787
   else if (streq (p[0], "push-remove") && p[1] && !p[2])
5788 5788
     {
5789 5789
       VERIFY_PERMISSION (OPT_P_INSTANCE);
5790
+      msg (D_PUSH, "PUSH_REMOVE '%s'", p[1]);
5790 5791
       push_remove_option (options,p[1]);
5791 5792
     }
5792 5793
   else if (streq (p[0], "ifconfig-pool") && p[1] && p[2] && !p[4])
... ...
@@ -314,6 +314,7 @@ 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");
317 318
 	  push_option_fmt(o, M_USAGE, "peer-id %d", tls_multi->peer_id);
318 319
 	}
319 320
     }
... ...
@@ -337,6 +338,7 @@ prepare_push_reply (struct options *o, struct tls_multi *tls_multi)
337 337
 	   * TODO: actual negotiation, instead of server dictatorship. */
338 338
 	  char *push_cipher = string_alloc(o->ncp_ciphers, &o->gc);
339 339
 	  o->ciphername = strtok (push_cipher, ":");
340
+	  push_remove_option(o, "cipher");
340 341
 	  push_option_fmt(o, M_USAGE, "cipher %s", o->ciphername);
341 342
 	}
342 343
     }
... ...
@@ -525,7 +527,7 @@ push_reset (struct options *o)
525 525
 void
526 526
 push_remove_option (struct options *o, const char *p)
527 527
 {
528
-  msg( D_PUSH, "PUSH_REMOVE '%s'", p );
528
+  msg (D_PUSH_DEBUG, "PUSH_REMOVE searching for: '%s'", p);
529 529
 
530 530
   /* ifconfig-ipv6 is special, as not part of the push list */
531 531
   if ( streq( p, "ifconfig-ipv6" ))
... ...
@@ -544,7 +546,7 @@ push_remove_option (struct options *o, const char *p)
544 544
 	  if ( e->enable &&
545 545
                strncmp( e->option, p, strlen(p) ) == 0 )
546 546
 	    {
547
-	      msg (D_PUSH, "PUSH_REMOVE removing: '%s'", e->option);
547
+	      msg (D_PUSH_DEBUG, "PUSH_REMOVE removing: '%s'", e->option);
548 548
 	      e->enable = false;
549 549
 	    }
550 550