Browse code

Fix file access checks on commands

The current implementation of check_file_access() does not consider that
some options take scripts and executables as input. When some of these
commands are given arguments in the OpenVPN configuration,
check_file_access() would take those arguments as a part of the file name
to the command. Thus the file check would fail.

This patch improves that by introducing a check_cmd_access() function which
first splits out the arguments to the command before checking if the file
with the command is available.

[DS: This patch is splitted out from the options.c.diff patch sent to the
mailing list - where only the function changes is included here]

Signed-off-by: Jonathan K. Bullard <jkbullard@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: CAEsd45RkyJw6yUk1Jwkip70HkCjKYoU+V=do3N7SH7JOaHBZdw@mail.gmail.com
URL: http://article.gmane.org/gmane.network.openvpn.devel/6194
Signed-off-by: David Sommerseth <davids@redhat.com>

Jonathan K. Bullard authored on 2012/03/31 20:47:34
Showing 1 changed files
... ...
@@ -2674,6 +2674,51 @@ check_file_access(const int type, const char *file, const int mode, const char *
2674 2674
 }
2675 2675
 
2676 2676
 /*
2677
+ * Verifies that the path in the "command" that comes after certain script options (e.g., --up) is a
2678
+ * valid file with appropriate permissions.
2679
+ *
2680
+ * "command" consists of a path, optionally followed by a space, which may be
2681
+ * followed by arbitrary arguments. It is NOT a full shell command line -- shell expansion is not
2682
+ * performed.
2683
+ *
2684
+ * The path and arguments in "command" may be single- or double-quoted or escaped.
2685
+ *
2686
+ * The path is extracted from "command", then check_file_access() is called to check it. The
2687
+ * arguments, if any, are ignored.
2688
+ *
2689
+ * Note that the type, mode, and opt arguments to this routine are the same as the corresponding
2690
+ * check_file_access() arguments.
2691
+ */
2692
+static bool
2693
+check_cmd_access(const int type, const char *command, const int mode, const char *opt)
2694
+{
2695
+  struct argv argv;
2696
+  bool return_code;
2697
+
2698
+  /* If no command was set, there are no errors to look for */
2699
+  if (! command)
2700
+      return false;
2701
+
2702
+  /* Extract executable path and arguments */
2703
+  argv = argv_new ();
2704
+  argv_printf (&argv, "%sc", command);
2705
+
2706
+  /* if an executable is specified then check it; otherwise, complain */
2707
+  if (argv.argv[0])
2708
+      return_code = check_file_access(type, argv.argv[0], mode, opt);
2709
+  else
2710
+    {
2711
+      msg (M_NOPREFIX|M_OPTERR, "%s fails with '%s': No path to executable.",
2712
+           opt, command);
2713
+      return_code = true;
2714
+    }
2715
+
2716
+  argv_reset (&argv);
2717
+
2718
+  return return_code;
2719
+}
2720
+
2721
+/*
2677 2722
  * Sanity check of all file/dir options.  Checks that file/dir
2678 2723
  * is accessible by OpenVPN
2679 2724
  */
... ...
@@ -2749,27 +2794,29 @@ options_postprocess_filechecks (struct options *options)
2749 2749
 #if P2MP_SERVER
2750 2750
   errs |= check_file_access (CHKACC_FILE, options->client_config_dir,
2751 2751
                              R_OK|X_OK, "--client-config-dir");
2752
-  /* ** Script hooks ** */
2753
-  errs |= check_file_access (CHKACC_FILE, options->client_connect_script,
2754
-                             R_OK|X_OK, "--client-connect script");
2755
-  errs |= check_file_access (CHKACC_FILE, options->client_disconnect_script,
2756
-                             R_OK|X_OK, "--client-disconnect script");
2757
-  errs |= check_file_access (CHKACC_FILE, options->auth_user_pass_verify_script,
2758
-                             R_OK|X_OK, "--auth-user-pass-verify script");
2759
-  errs |= check_file_access (CHKACC_FILE, options->tls_verify,
2760
-                             R_OK|X_OK, "--tls-verify script");
2761
-  errs |= check_file_access (CHKACC_FILE, options->up_script,
2762
-                             R_OK|X_OK, "--up script");
2763
-  errs |= check_file_access (CHKACC_FILE, options->down_script,
2764
-                             R_OK|X_OK, "--down script");
2765
-  errs |= check_file_access (CHKACC_FILE, options->ipchange,
2766
-                             R_OK|X_OK, "--ipchange script");
2767
-  errs |= check_file_access (CHKACC_FILE, options->route_script,
2768
-                             R_OK|X_OK, "--route-up script");
2769
-  errs |= check_file_access (CHKACC_FILE, options->route_predown_script,
2770
-                             R_OK|X_OK, "--route-pre-down script");
2771
-  errs |= check_file_access (CHKACC_FILE, options->learn_address_script,
2772
-                             R_OK|X_OK, "--learn-address script");
2752
+
2753
+  /* ** Script hooks that accept an optionally quoted and/or escaped executable path, ** */
2754
+  /* ** optionally followed by arguments ** */
2755
+  errs |= check_cmd_access (CHKACC_FILE, options->auth_user_pass_verify_script,
2756
+                            R_OK|X_OK, "--auth-user-pass-verify script");
2757
+  errs |= check_cmd_access (CHKACC_FILE, options->client_connect_script,
2758
+                            R_OK|X_OK, "--client-connect script");
2759
+  errs |= check_cmd_access (CHKACC_FILE, options->client_disconnect_script,
2760
+                            R_OK|X_OK, "--client-disconnect script");
2761
+  errs |= check_cmd_access (CHKACC_FILE, options->tls_verify,
2762
+                            R_OK|X_OK, "--tls-verify script");
2763
+  errs |= check_cmd_access (CHKACC_FILE, options->up_script,
2764
+                            R_OK|X_OK, "--up script");
2765
+  errs |= check_cmd_access (CHKACC_FILE, options->down_script,
2766
+                            R_OK|X_OK, "--down script");
2767
+  errs |= check_cmd_access (CHKACC_FILE, options->ipchange,
2768
+                            R_OK|X_OK, "--ipchange script");
2769
+  errs |= check_cmd_access (CHKACC_FILE, options->route_script,
2770
+                            R_OK|X_OK, "--route-up script");
2771
+  errs |= check_cmd_access (CHKACC_FILE, options->route_predown_script,
2772
+                            R_OK|X_OK, "--route-pre-down script");
2773
+  errs |= check_cmd_access (CHKACC_FILE, options->learn_address_script,
2774
+                            R_OK|X_OK, "--learn-address script");
2773 2775
 #endif /* P2MP_SERVER */
2774 2776
 
2775 2777
   if (errs)