Restore pre-NCP cipher options on SIGUSR1

As reported by debbie10t on the openvpn-devel list (Message-ID:
<[email protected]...>), an NCP client will attempt to reconnect with the previously pushed cipher, instead of the cipher from the config file, after a sigusr1 restart. This can be a problem when the server is reconfigured (as debbie10t explainted), or when roaming to a differently-configured server. Fix this by restoring the cipher options from the config file after a sigusr1 restart.

This makes the cipher options behaviour different from other pushable options, because those are also cached until a sighup restart. We might want to change this behaviour in general, but for now let's just fix the issue at hand.

---
 src/openvpn/init.c | 4 ++++
 1 file changed, 4 insertions(+)

index 4d106c7..8140292 100644
@@ -2289,6 +2289,10 @@ do_init_crypto_tls_c1 (struct context *c)
   else
     {
       msg (D_INIT_MEDIUM, "Re-using SSL/TLS context");
+
+      /* Restore pre-NCP cipher options */
+      c->options.ciphername = c->c1.ciphername;
+      c->options.authname = c->c1.authname;
     }
 }
 

2.7.4

As reported by debbie10t on the openvpn-devel list (Message-ID:
<[email protected]...>), an NCP client will attempt to reconnect with the previously pushed cipher, instead of the cipher from the config file, after a sigusr1 restart. This can be a problem when the server is reconfigured (as debbie10t explainted), or when roaming to a differently-configured server. Fix this by restoring the cipher options from the config file after a sigusr1 restart.

This makes the cipher options behaviour different from other pushable options, because those are also cached until a sighup restart. We might want to change this behaviour in general, but for now let's just fix the issue at hand.

v2: also cache and restore keysize, as that parameter is relevant too.

---
 src/openvpn/init.c    | 6 ++++++
 src/openvpn/openvpn.h | 1 +
 2 files changed, 7 insertions(+)

index 4d106c7..876141f 100644
@@ -2277,6 +2277,7 @@ do_init_crypto_tls_c1 (struct context *c)
 
       c->c1.ciphername = options->ciphername;
       c->c1.authname = options->authname;
+      c->c1.keysize = options->keysize;
 
 #if 0 /* was: #if ENABLE_INLINE_FILES --  Note that enabling this code will break restarts */
       if (options->priv_key_file_inline)

@@ -2289,6 +2290,11 @@ do_init_crypto_tls_c1 (struct context *c)
   else
     {
       msg (D_INIT_MEDIUM, "Re-using SSL/TLS context");
+
+      /* Restore pre-NCP cipher options */
+      c->options.ciphername = c->c1.ciphername;
+      c->options.authname = c->c1.authname;
+      c->options.keysize = c->c1.keysize;
     }
 }
 

index 1a458f1..a39ff56 100644
@@ -213,6 +213,7 @@ struct context_1
 
   const char *ciphername;	/**< Data channel cipher from config file */
   const char *authname;		/**< Data channel auth from config file */
+  int keysize;			/**< Data channel keysize from config file */
 #endif
 };
 

2.7.4

As reported by debbie10t on the openvpn-devel list (Message-ID:
<[email protected]...>), an NCP client will attempt to reconnect with the previously pushed cipher, instead of the cipher from the config file, after a sigusr1 restart. This can be a problem when the server is reconfigured (as debbie10t explainted), or when roaming to a differently-configured server. Fix this by restoring the cipher options from the config file after a sigusr1 restart.

This makes the cipher options behaviour different from other pushable options, because those are also cached until a sighup restart. We might want to change this behaviour in general, but for now let's just fix the issue at hand.

v2: also cache and restore keysize, as that parameter is relevant too.
v3: inherit cached cipher options from parent context.

---
 src/openvpn/init.c    | 10 ++++++++++
 src/openvpn/openvpn.h |  1 +
 2 files changed, 11 insertions(+)

index ea96f3d..5d25d5f 100644
@@ -2273,6 +2273,7 @@ do_init_crypto_tls_c1 (struct context *c)
 
       c->c1.ciphername = options->ciphername;
       c->c1.authname = options->authname;
+      c->c1.keysize = options->keysize;
 
 #if 0 /* was: #if ENABLE_INLINE_FILES --  Note that enabling this code will break restarts */
       if (options->priv_key_file_inline)

@@ -2285,6 +2286,11 @@ do_init_crypto_tls_c1 (struct context *c)
   else
     {
       msg (D_INIT_MEDIUM, "Re-using SSL/TLS context");
+
+      /* Restore pre-NCP cipher options */
+      c->options.ciphername = c->c1.ciphername;
+      c->options.authname = c->c1.authname;
+      c->options.keysize = c->c1.keysize;
     }
 }
 

@@ -3811,6 +3817,10 @@ inherit_context_child (struct context *dest,
   dest->c1.ks.ssl_ctx = src->c1.ks.ssl_ctx;
   dest->c1.ks.tls_auth_key = src->c1.ks.tls_auth_key;
   dest->c1.ks.tls_auth_key_type = src->c1.ks.tls_auth_key_type;
+  /* inherit pre-NCP ciphers */
+  dest->c1.ciphername = src->c1.ciphername;
+  dest->c1.authname = src->c1.authname;
+  dest->c1.keysize = src->c1.keysize;
 #endif
 
   /* options */

index 5cda7b4..4366a42 100644
@@ -213,6 +213,7 @@ struct context_1
 
   const char *ciphername;	/**< Data channel cipher from config file */
   const char *authname;		/**< Data channel auth from config file */
+  int keysize;			/**< Data channel keysize from config file */
 #endif
 };
 

2.7.4

Hi,

[..]

ACK.

After extensive discussion on #openvpn-devel - the code looks quite harmless, but now I think I understand the flow of things and when and why the SSL/TLS context is re-used on client and server, and what happens if it is *not* re-used (client re-inits from config file, server re-inits from global context which still has the config-file settings).

David, since you currently hold the "not fully pushed" tree (due to sf), can you please merge?

gert

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Your patch has been applied to the master branch

commit 129d2924bb4179b7df4a157a0443c45f2279e92d

Author: Steffan Karger
Date:   Tue Nov 1 20:06:47 2016 +0100

     Restore pre-NCP cipher options on SIGUSR1

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg12869.html
     Signed-off-by: David Sommerseth <[email protected]...>

- --
kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJYLHpGAAoJEIbPlEyWcf3y23UQAIWemHqoz7x7mUa2Ztplpu1h LAaHUsYaltFwo98xzUNc86DrLMVa1xfrehm20hZ6RK+EV2CAXIUM31/3lM31uaCM uSIkqK0L8mIm0LayDgVC33vM7x+akkC+U+4yPhI38HT7HvFA+ptgauAxIhTgHihN IW3m249NdgRasxSqcoCtMIDC0/JrgewBYWCfh6oJ8zX/cJzyLVKDslIeNEWvxz6H BGLUSnQqS+3ajK8PPEflbkxkmzJwQShgA75D+UeuTKJ3PQl5T7SNHpVI+Y11E5Vz SIqlBVvZ4IUoiIEkPF9sllK3v9FGTONwwt3c2Xik0fePtglJsA3lmCygXc1lZ8E9 NJZwFqiXwhUhhmN1me+pM6mmV3YS2/r5s1MVuvDf2WAX4LnPITImSoEKRjFQ2roh MsUv3+LX2GNuuGpdVhjYHwtu0fB4FGD9R1TzgS6fwS43tL/0kJxjcvAnUOnPv5Rp iA7p1Vk9Aa+a/yWTfbmitN3hnXggf5lMzSJ7NP7U0TVxO0r8pvAbtRcqCTXjK2jN EFr4bW2X1AfWDVgoJ4+cn5nRMHlMd4MyuYsZ+fGWBrejzJnHSsNyqQkc8LoB5eWv C5xIqrY2Gkc9NmzmJjuN3SI+c5Jb/6Jyl//ma1tJuFyW0wx3tPZt9N5BJOtMloO8 THQKc6jcaTy0YpTvWbjz
=Y8fp
-----END PGP SIGNATURE-----