Browse code

Make sure options->ciphername and options->authname are always defined

The NCP code does a strcmp(options->ciphername, ...) without first checking
whether options->ciphername is NULL. This could cause a crash when using
"--cipher none". This patch fixes that problem by ensuring that
options->ciphername (and options->authname) are never NULL. Ensuring that
options->ciphername is never null prevents us from having to write null
checks everywhere.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1475055231-1778-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg12576.html

Signed-off-by: Gert Doering <gert@greenie.muc.de>

Steffan Karger authored on 2016/09/28 19:40:51
Showing 3 changed files
... ...
@@ -759,8 +759,11 @@ init_key_type (struct key_type *kt, const char *ciphername,
759 759
 {
760 760
   bool aead_cipher = false;
761 761
 
762
+  ASSERT(ciphername);
763
+  ASSERT(authname);
764
+
762 765
   CLEAR (*kt);
763
-  if (ciphername)
766
+  if (strcmp (ciphername, "none") != 0)
764 767
     {
765 768
       kt->cipher = cipher_kt_get (translate_cipher_name_from_openvpn(ciphername));
766 769
       kt->cipher_length = cipher_kt_key_size (kt->cipher);
... ...
@@ -785,7 +788,7 @@ init_key_type (struct key_type *kt, const char *ciphername,
785 785
       if (warn)
786 786
 	msg (M_WARN, "******* WARNING *******: null cipher specified, no encryption will be used");
787 787
     }
788
-  if (authname)
788
+  if (strcmp (authname, "none") != 0)
789 789
     {
790 790
       if (!aead_cipher) { /* Ignore auth for AEAD ciphers */
791 791
 	kt->digest = md_kt_get (authname);
... ...
@@ -2266,7 +2266,7 @@ do_init_crypto_tls_c1 (struct context *c)
2266 2266
 
2267 2267
 	  /* Initialize key_type for tls-auth with auth only */
2268 2268
 	  CLEAR (c->c1.ks.tls_auth_key_type);
2269
-	  if (options->authname)
2269
+	  if (!streq (options->authname, "none"))
2270 2270
 	    {
2271 2271
 	      c->c1.ks.tls_auth_key_type.digest = md_kt_get (options->authname);
2272 2272
 	      c->c1.ks.tls_auth_key_type.hmac_length =
... ...
@@ -6659,19 +6659,11 @@ add_option (struct options *options,
6659 6659
     {
6660 6660
       VERIFY_PERMISSION (OPT_P_GENERAL);
6661 6661
       options->authname = p[1];
6662
-      if (streq (options->authname, "none"))
6663
-	{
6664
-	  options->authname = NULL;
6665
-	}
6666 6662
     }
6667 6663
   else if (streq (p[0], "cipher") && p[1] && !p[2])
6668 6664
     {
6669 6665
       VERIFY_PERMISSION (OPT_P_NCP);
6670 6666
       options->ciphername = p[1];
6671
-      if (streq (options->ciphername, "none"))
6672
-	{
6673
-	  options->ciphername = NULL;
6674
-	}
6675 6667
     }
6676 6668
   else if (streq (p[0], "ncp-ciphers") && p[1] && !p[2])
6677 6669
     {