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>

diff --git a/src/openvpn/errlevel.h b/src/openvpn/errlevel.h
index da600ab8a1884793636ec1064bb0cec36f3c66da..ae1f8f497682165608a71045dcf8612e36ba344a 100644 (file)
--- a/src/openvpn/errlevel.h
+++ b/src/openvpn/errlevel.h
@@ -147,6 +147,7 @@
 #define D_PID_DEBUG          LOGLEV(7, 70, M_DEBUG)  /* show packet-id debugging info */
 #define D_PF_DROPPED_BCAST   LOGLEV(7, 71, M_DEBUG)  /* packet filter dropped a broadcast packet */
 #define D_PF_DEBUG           LOGLEV(7, 72, M_DEBUG)  /* packet filter debugging, must also define PF_DEBUG in pf.h */
+#define D_PUSH_DEBUG         LOGLEV(7, 73, M_DEBUG)  /* show push/pull debugging info */
 
 #define D_HANDSHAKE_VERBOSE  LOGLEV(8, 70, M_DEBUG)  /* show detailed description of each handshake */
 #define D_TLS_DEBUG_MED      LOGLEV(8, 70, M_DEBUG)  /* limited info from tls_session routines */

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 9f6099c20e0262b8de8975d57efbd4b9aeecf635..e1ff5849074ea935f7bac72531b86f409417104e 100644 (file)
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5787,6 +5787,7 @@ add_option (struct options *options,
   else if (streq (p[0], "push-remove") && p[1] && !p[2])
     {
       VERIFY_PERMISSION (OPT_P_INSTANCE);
+      msg (D_PUSH, "PUSH_REMOVE '%s'", p[1]);
       push_remove_option (options,p[1]);
     }
   else if (streq (p[0], "ifconfig-pool") && p[1] && p[2] && !p[4])

diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index c0c78a0295580e81f6744d19f3460bd994cc3583..df4f596a1c9673409f196d03b28f62fcbe987b66 100644 (file)
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -314,6 +314,7 @@ prepare_push_reply (struct options *o, struct tls_multi *tls_multi)
       int r = sscanf(optstr, "IV_PROTO=%d", &proto);
       if ((r == 1) && (proto >= 2))
        {
+         push_remove_option(o, "peer-id");
          push_option_fmt(o, M_USAGE, "peer-id %d", tls_multi->peer_id);
        }
     }
@@ -337,6 +338,7 @@ prepare_push_reply (struct options *o, struct tls_multi *tls_multi)
           * TODO: actual negotiation, instead of server dictatorship. */
          char *push_cipher = string_alloc(o->ncp_ciphers, &o->gc);
          o->ciphername = strtok (push_cipher, ":");
+         push_remove_option(o, "cipher");
          push_option_fmt(o, M_USAGE, "cipher %s", o->ciphername);
        }
     }
@@ -525,7 +527,7 @@ push_reset (struct options *o)
 void
 push_remove_option (struct options *o, const char *p)
 {
-  msg( D_PUSH, "PUSH_REMOVE '%s'", p );
+  msg (D_PUSH_DEBUG, "PUSH_REMOVE searching for: '%s'", p);
 
   /* ifconfig-ipv6 is special, as not part of the push list */
   if ( streq( p, "ifconfig-ipv6" ))
@@ -544,7 +546,7 @@ push_remove_option (struct options *o, const char *p)
          if ( e->enable &&
                strncmp( e->option, p, strlen(p) ) == 0 )
            {
-             msg (D_PUSH, "PUSH_REMOVE removing: '%s'", e->option);
+             msg (D_PUSH_DEBUG, "PUSH_REMOVE removing: '%s'", e->option);
              e->enable = false;
            }