Browse code

Refactor NCP-negotiable options handling

NCP negotiation can alter options. On reconnect
client sends possibly altered options while server
expects original values. This leads to warnings
in log and, if server uses --opt-verify, breaks
reconnect.

Fix by decouple setting/unsetting NCP options from
the state of TLS context. At startup (and once per sighup)
we load original values to c->c1, which persists over
sigusr1 (restart). When tearing tunnel down we restore
(possibly altered) options back to original values.

Trac: #1105

Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <1537449154-26879-1-git-send-email-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17477.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Lev Stipakov authored on 2018/09/20 22:12:34
Showing 1 changed files
... ...
@@ -612,6 +612,33 @@ uninit_proxy(struct context *c)
612 612
     uninit_proxy_dowork(c);
613 613
 }
614 614
 
615
+/*
616
+ * Assign NCP-negotiable options to context->c1
617
+ * from context->options (initially config values).
618
+ * They persist over sigusr1 restart.
619
+ */
620
+static void
621
+do_set_ncp_options(struct context *c)
622
+{
623
+    c->c1.ciphername = c->options.ciphername;
624
+    c->c1.authname = c->options.authname;
625
+    c->c1.keysize = c->options.keysize;
626
+}
627
+
628
+/*
629
+ * Restore NCP-negotiable options from c->c1 to
630
+ * c->options. The latter ones can be altered by
631
+ * pushed options and therefore need to be restored
632
+ * to original values on sigusr1 restart.
633
+ */
634
+static void
635
+do_unset_ncp_options(struct context *c)
636
+{
637
+    c->options.ciphername = c->c1.ciphername;
638
+    c->options.authname = c->c1.authname;
639
+    c->options.keysize = c->c1.keysize;
640
+}
641
+
615 642
 void
616 643
 context_init_1(struct context *c)
617 644
 {
... ...
@@ -621,6 +648,8 @@ context_init_1(struct context *c)
621 621
 
622 622
     init_connection_list(c);
623 623
 
624
+    do_set_ncp_options(c);
625
+
624 626
 #if defined(ENABLE_PKCS11)
625 627
     if (c->first_time)
626 628
     {
... ...
@@ -2600,10 +2629,6 @@ do_init_crypto_tls_c1(struct context *c)
2600 2600
         /* initialize tls-auth/crypt key */
2601 2601
         do_init_tls_wrap_key(c);
2602 2602
 
2603
-        c->c1.ciphername = options->ciphername;
2604
-        c->c1.authname = options->authname;
2605
-        c->c1.keysize = options->keysize;
2606
-
2607 2603
 #if 0 /* was: #if ENABLE_INLINE_FILES --  Note that enabling this code will break restarts */
2608 2604
         if (options->priv_key_file_inline)
2609 2605
         {
... ...
@@ -2616,11 +2641,6 @@ do_init_crypto_tls_c1(struct context *c)
2616 2616
     {
2617 2617
         msg(D_INIT_MEDIUM, "Re-using SSL/TLS context");
2618 2618
 
2619
-        /* Restore pre-NCP cipher options */
2620
-        c->options.ciphername = c->c1.ciphername;
2621
-        c->options.authname = c->c1.authname;
2622
-        c->options.keysize = c->c1.keysize;
2623
-
2624 2619
         /*
2625 2620
          * tls-auth/crypt key can be configured per connection block, therefore
2626 2621
          * we must reload it as it may have changed
... ...
@@ -4300,6 +4320,8 @@ close_instance(struct context *c)
4300 4300
         /* free key schedules */
4301 4301
         do_close_free_key_schedule(c, (c->mode == CM_P2P || c->mode == CM_TOP));
4302 4302
 
4303
+        do_unset_ncp_options(c);
4304
+
4303 4305
         /* close TCP/UDP connection */
4304 4306
         do_close_link_socket(c);
4305 4307