Browse code

Fix file checks when --chroot is being used

Commit 0f2bc0dd92f43c9 started to introduce some file sanity
checking before OpenVPN started to avoid harder to explain issues
due to missing files or directories later on. But that commit
did not consider --chroot at all. Which would basically cause
OpenVPN to complain on non-missing files, because it would not
consider that the files where inside a chroot.

This patch is based on the thoughts in a patch by Josh Cepek [1],
but trying to simplify it at bit.

[1] <http://thread.gmane.org/gmane.network.openvpn.devel/7873>,
(Message-ID: l142b7$15v$1@ger.gmane.org)

[v2 - Simplify the changes in check_cmd_access(), let the chroot
tackling happen only in check_file_access_chroot() only]

Trac-ticket: 330
Signed-off-by: David Sommerseth <davids@redhat.com>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <1385382778-4723-1-git-send-email-dazo@users.sourceforge.net>
URL: http://article.gmane.org/gmane.network.openvpn.devel/8060
Signed-off-by: Gert Doering <gert@greenie.muc.de>

David Sommerseth authored on 2013/11/25 21:32:58
Showing 1 changed files
... ...
@@ -5,7 +5,7 @@
5 5
  *             packet encryption, packet authentication, and
6 6
  *             packet compression.
7 7
  *
8
- *  Copyright (C) 2002-2010 OpenVPN Technologies, Inc. <sales@openvpn.net>
8
+ *  Copyright (C) 2002-2013 OpenVPN Technologies, Inc. <sales@openvpn.net>
9 9
  *  Copyright (C) 2008-2013 David Sommerseth <dazo@users.sourceforge.net>
10 10
  *
11 11
  *  This program is free software; you can redistribute it and/or modify
... ...
@@ -2599,6 +2599,44 @@ check_file_access(const int type, const char *file, const int mode, const char *
2599 2599
   return (errcode != 0 ? true : false);
2600 2600
 }
2601 2601
 
2602
+/* A wrapper for check_file_access() which also takes a chroot directory.
2603
+ * If chroot is NULL, behaviour is exactly the same as calling check_file_access() directly,
2604
+ * otherwise it will look for the file inside the given chroot directory instead.
2605
+ */
2606
+static bool
2607
+check_file_access_chroot(const char *chroot, const int type, const char *file, const int mode, const char *opt)
2608
+{
2609
+  bool ret = false;
2610
+
2611
+  /* If no file configured, no errors to look for */
2612
+  if (!file)
2613
+      return false;
2614
+
2615
+  /* If chroot is set, look for the file/directory inside the chroot */
2616
+  if( chroot )
2617
+    {
2618
+      struct gc_arena gc = gc_new();
2619
+      struct buffer chroot_file;
2620
+      int len = 0;
2621
+
2622
+      /* Build up a new full path including chroot directory */
2623
+      len = strlen(chroot) + strlen(PATH_SEPARATOR_STR) + strlen(file) + 1;
2624
+      chroot_file = alloc_buf_gc(len, &gc);
2625
+      buf_printf(&chroot_file, "%s%s%s", chroot, PATH_SEPARATOR_STR, file);
2626
+      ASSERT (chroot_file.len > 0);
2627
+
2628
+      ret = check_file_access(type, BSTR(&chroot_file), mode, opt);
2629
+      gc_free(&gc);
2630
+    }
2631
+  else
2632
+    {
2633
+      /* No chroot in play, just call core file check function */
2634
+      ret = check_file_access(type, file, mode, opt);
2635
+    }
2636
+  return ret;
2637
+}
2638
+
2639
+
2602 2640
 /*
2603 2641
  * Verifies that the path in the "command" that comes after certain script options (e.g., --up) is a
2604 2642
  * valid file with appropriate permissions.
... ...
@@ -2616,7 +2654,7 @@ check_file_access(const int type, const char *file, const int mode, const char *
2616 2616
  * check_file_access() arguments.
2617 2617
  */
2618 2618
 static bool
2619
-check_cmd_access(const char *command, const char *opt)
2619
+check_cmd_access(const char *command, const char *opt, const char *chroot)
2620 2620
 {
2621 2621
   struct argv argv;
2622 2622
   bool return_code;
... ...
@@ -2635,7 +2673,7 @@ check_cmd_access(const char *command, const char *opt)
2635 2635
      * only requires X_OK to function on Unix - a scenario not unlikely to
2636 2636
      * be seen on suid binaries.
2637 2637
      */
2638
-    return_code = check_file_access(CHKACC_FILE, argv.argv[0], X_OK, opt);
2638
+    return_code = check_file_access_chroot(chroot, CHKACC_FILE, argv.argv[0], X_OK, opt);
2639 2639
   else
2640 2640
     {
2641 2641
       msg (M_NOPREFIX|M_OPTERR, "%s fails with '%s': No path to executable.",
... ...
@@ -2661,7 +2699,7 @@ options_postprocess_filechecks (struct options *options)
2661 2661
 #ifdef ENABLE_SSL
2662 2662
   errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->dh_file, R_OK, "--dh");
2663 2663
   errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->ca_file, R_OK, "--ca");
2664
-  errs |= check_file_access (CHKACC_FILE, options->ca_path, R_OK, "--capath");
2664
+  errs |= check_file_access_chroot (options->chroot_dir, CHKACC_FILE, options->ca_path, R_OK, "--capath");
2665 2665
   errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->cert_file, R_OK, "--cert");
2666 2666
   errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->extra_certs_file, R_OK,
2667 2667
                              "--extra-certs");
... ...
@@ -2674,10 +2712,10 @@ options_postprocess_filechecks (struct options *options)
2674 2674
                              "--pkcs12");
2675 2675
 
2676 2676
   if (options->ssl_flags & SSLF_CRL_VERIFY_DIR)
2677
-    errs |= check_file_access (CHKACC_FILE, options->crl_file, R_OK|X_OK,
2677
+    errs |= check_file_access_chroot (options->chroot_dir, CHKACC_FILE, options->crl_file, R_OK|X_OK,
2678 2678
                                "--crl-verify directory");
2679 2679
   else
2680
-    errs |= check_file_access (CHKACC_FILE, options->crl_file, R_OK,
2680
+    errs |= check_file_access_chroot (options->chroot_dir, CHKACC_FILE, options->crl_file, R_OK,
2681 2681
                                "--crl-verify");
2682 2682
 
2683 2683
   errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->tls_auth_file, R_OK,
... ...
@@ -2719,13 +2757,13 @@ options_postprocess_filechecks (struct options *options)
2719 2719
 
2720 2720
   /* ** Config related ** */
2721 2721
 #ifdef ENABLE_SSL
2722
-  errs |= check_file_access (CHKACC_FILE, options->tls_export_cert,
2722
+  errs |= check_file_access_chroot (options->chroot_dir, CHKACC_FILE, options->tls_export_cert,
2723 2723
                              R_OK|W_OK|X_OK, "--tls-export-cert");
2724 2724
 #endif /* ENABLE_SSL */
2725 2725
 #if P2MP_SERVER
2726
-  errs |= check_file_access (CHKACC_FILE, options->client_config_dir,
2726
+  errs |= check_file_access_chroot (options->chroot_dir, CHKACC_FILE, options->client_config_dir,
2727 2727
                              R_OK|X_OK, "--client-config-dir");
2728
-  errs |= check_file_access (CHKACC_FILE, options->tmp_dir,
2728
+  errs |= check_file_access_chroot (options->chroot_dir, CHKACC_FILE, options->tmp_dir,
2729 2729
                              R_OK|W_OK|X_OK, "Temporary directory (--tmp-dir)");
2730 2730
 
2731 2731
 #endif /* P2MP_SERVER */
... ...
@@ -3990,7 +4028,8 @@ static void
3990 3990
 set_user_script (struct options *options,
3991 3991
 		 const char **script,
3992 3992
 		 const char *new_script,
3993
-		 const char *type)
3993
+		 const char *type,
3994
+		 bool in_chroot)
3994 3995
 {
3995 3996
   if (*script) {
3996 3997
     msg (M_WARN, "Multiple --%s scripts defined.  "
... ...
@@ -4005,8 +4044,9 @@ set_user_script (struct options *options,
4005 4005
     openvpn_snprintf (script_name, sizeof(script_name),
4006 4006
                       "--%s script", type);
4007 4007
 
4008
-    if (check_cmd_access (*script, script_name))
4008
+    if (check_cmd_access (*script, script_name, (in_chroot ? options->chroot_dir : NULL)))
4009 4009
       msg (M_USAGE, "Please correct this error.");
4010
+
4010 4011
   }
4011 4012
 #endif
4012 4013
 }
... ...
@@ -4502,7 +4542,7 @@ add_option (struct options *options,
4502 4502
       set_user_script (options,
4503 4503
 		       &options->ipchange,
4504 4504
 		       string_substitute (p[1], ',', ' ', &options->gc),
4505
-		       "ipchange");
4505
+		       "ipchange", true);
4506 4506
     }
4507 4507
   else if (streq (p[0], "float"))
4508 4508
     {
... ...
@@ -4548,14 +4588,14 @@ add_option (struct options *options,
4548 4548
       VERIFY_PERMISSION (OPT_P_SCRIPT);
4549 4549
       if (!no_more_than_n_args (msglevel, p, 2, NM_QUOTE_HINT))
4550 4550
 	goto err;
4551
-      set_user_script (options, &options->up_script, p[1], "up");
4551
+      set_user_script (options, &options->up_script, p[1], "up", false);
4552 4552
     }
4553 4553
   else if (streq (p[0], "down") && p[1])
4554 4554
     {
4555 4555
       VERIFY_PERMISSION (OPT_P_SCRIPT);
4556 4556
       if (!no_more_than_n_args (msglevel, p, 2, NM_QUOTE_HINT))
4557 4557
 	goto err;
4558
-      set_user_script (options, &options->down_script, p[1], "down");
4558
+      set_user_script (options, &options->down_script, p[1], "down", true);
4559 4559
     }
4560 4560
   else if (streq (p[0], "down-pre"))
4561 4561
     {
... ...
@@ -5230,7 +5270,7 @@ add_option (struct options *options,
5230 5230
       VERIFY_PERMISSION (OPT_P_SCRIPT);
5231 5231
       if (!no_more_than_n_args (msglevel, p, 2, NM_QUOTE_HINT))
5232 5232
 	goto err;
5233
-      set_user_script (options, &options->route_script, p[1], "route-up");
5233
+      set_user_script (options, &options->route_script, p[1], "route-up", false);
5234 5234
     }
5235 5235
   else if (streq (p[0], "route-pre-down") && p[1])
5236 5236
     {
... ...
@@ -5240,7 +5280,7 @@ add_option (struct options *options,
5240 5240
       set_user_script (options,
5241 5241
 		       &options->route_predown_script,
5242 5242
 		       p[1],
5243
-		       "route-pre-down");
5243
+		       "route-pre-down", true);
5244 5244
     }
5245 5245
   else if (streq (p[0], "route-noexec"))
5246 5246
     {
... ...
@@ -5609,7 +5649,7 @@ add_option (struct options *options,
5609 5609
 	}
5610 5610
       set_user_script (options,
5611 5611
 		       &options->auth_user_pass_verify_script,
5612
-		       p[1], "auth-user-pass-verify");
5612
+		       p[1], "auth-user-pass-verify", true);
5613 5613
     }
5614 5614
   else if (streq (p[0], "client-connect") && p[1])
5615 5615
     {
... ...
@@ -5617,7 +5657,7 @@ add_option (struct options *options,
5617 5617
       if (!no_more_than_n_args (msglevel, p, 2, NM_QUOTE_HINT))
5618 5618
 	goto err;
5619 5619
       set_user_script (options, &options->client_connect_script,
5620
-		       p[1], "client-connect");
5620
+		       p[1], "client-connect", true);
5621 5621
     }
5622 5622
   else if (streq (p[0], "client-disconnect") && p[1])
5623 5623
     {
... ...
@@ -5625,7 +5665,7 @@ add_option (struct options *options,
5625 5625
       if (!no_more_than_n_args (msglevel, p, 2, NM_QUOTE_HINT))
5626 5626
 	goto err;
5627 5627
       set_user_script (options, &options->client_disconnect_script,
5628
-		       p[1], "client-disconnect");
5628
+		       p[1], "client-disconnect", true);
5629 5629
     }
5630 5630
   else if (streq (p[0], "learn-address") && p[1])
5631 5631
     {
... ...
@@ -5633,7 +5673,7 @@ add_option (struct options *options,
5633 5633
       if (!no_more_than_n_args (msglevel, p, 2, NM_QUOTE_HINT))
5634 5634
 	goto err;
5635 5635
       set_user_script (options, &options->learn_address_script,
5636
-		       p[1], "learn-address");
5636
+		       p[1], "learn-address", true);
5637 5637
     }
5638 5638
   else if (streq (p[0], "tmp-dir") && p[1])
5639 5639
     {
... ...
@@ -6584,7 +6624,7 @@ add_option (struct options *options,
6584 6584
 	goto err;
6585 6585
       set_user_script (options, &options->tls_verify,
6586 6586
 		       string_substitute (p[1], ',', ' ', &options->gc),
6587
-		       "tls-verify");
6587
+		       "tls-verify", true);
6588 6588
     }
6589 6589
 #ifndef ENABLE_CRYPTO_POLARSSL
6590 6590
   else if (streq (p[0], "tls-export-cert") && p[1])