From a050bcef9cf71e7479551677b116879e6c563bd5 Mon Sep 17 00:00:00 2001 From: "Jonathan K. Bullard" Date: Sat, 31 Mar 2012 07:47:34 -0400 Subject: 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 Acked-by: Gert Doering Message-Id: CAEsd45RkyJw6yUk1Jwkip70HkCjKYoU+V=do3N7SH7JOaHBZdw@mail.gmail.com URL: http://article.gmane.org/gmane.network.openvpn.devel/6194 Signed-off-by: David Sommerseth --- src/openvpn/options.c | 89 +++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 68 insertions(+), 21 deletions(-) diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 019be57..076b229 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -2673,6 +2673,51 @@ check_file_access(const int type, const char *file, const int mode, const char * return (errcode != 0 ? true : false); } +/* + * Verifies that the path in the "command" that comes after certain script options (e.g., --up) is a + * valid file with appropriate permissions. + * + * "command" consists of a path, optionally followed by a space, which may be + * followed by arbitrary arguments. It is NOT a full shell command line -- shell expansion is not + * performed. + * + * The path and arguments in "command" may be single- or double-quoted or escaped. + * + * The path is extracted from "command", then check_file_access() is called to check it. The + * arguments, if any, are ignored. + * + * Note that the type, mode, and opt arguments to this routine are the same as the corresponding + * check_file_access() arguments. + */ +static bool +check_cmd_access(const int type, const char *command, const int mode, const char *opt) +{ + struct argv argv; + bool return_code; + + /* If no command was set, there are no errors to look for */ + if (! command) + return false; + + /* Extract executable path and arguments */ + argv = argv_new (); + argv_printf (&argv, "%sc", command); + + /* if an executable is specified then check it; otherwise, complain */ + if (argv.argv[0]) + return_code = check_file_access(type, argv.argv[0], mode, opt); + else + { + msg (M_NOPREFIX|M_OPTERR, "%s fails with '%s': No path to executable.", + opt, command); + return_code = true; + } + + argv_reset (&argv); + + return return_code; +} + /* * Sanity check of all file/dir options. Checks that file/dir * is accessible by OpenVPN @@ -2749,27 +2794,29 @@ options_postprocess_filechecks (struct options *options) #if P2MP_SERVER errs |= check_file_access (CHKACC_FILE, options->client_config_dir, R_OK|X_OK, "--client-config-dir"); - /* ** Script hooks ** */ - errs |= check_file_access (CHKACC_FILE, options->client_connect_script, - R_OK|X_OK, "--client-connect script"); - errs |= check_file_access (CHKACC_FILE, options->client_disconnect_script, - R_OK|X_OK, "--client-disconnect script"); - errs |= check_file_access (CHKACC_FILE, options->auth_user_pass_verify_script, - R_OK|X_OK, "--auth-user-pass-verify script"); - errs |= check_file_access (CHKACC_FILE, options->tls_verify, - R_OK|X_OK, "--tls-verify script"); - errs |= check_file_access (CHKACC_FILE, options->up_script, - R_OK|X_OK, "--up script"); - errs |= check_file_access (CHKACC_FILE, options->down_script, - R_OK|X_OK, "--down script"); - errs |= check_file_access (CHKACC_FILE, options->ipchange, - R_OK|X_OK, "--ipchange script"); - errs |= check_file_access (CHKACC_FILE, options->route_script, - R_OK|X_OK, "--route-up script"); - errs |= check_file_access (CHKACC_FILE, options->route_predown_script, - R_OK|X_OK, "--route-pre-down script"); - errs |= check_file_access (CHKACC_FILE, options->learn_address_script, - R_OK|X_OK, "--learn-address script"); + + /* ** Script hooks that accept an optionally quoted and/or escaped executable path, ** */ + /* ** optionally followed by arguments ** */ + errs |= check_cmd_access (CHKACC_FILE, options->auth_user_pass_verify_script, + R_OK|X_OK, "--auth-user-pass-verify script"); + errs |= check_cmd_access (CHKACC_FILE, options->client_connect_script, + R_OK|X_OK, "--client-connect script"); + errs |= check_cmd_access (CHKACC_FILE, options->client_disconnect_script, + R_OK|X_OK, "--client-disconnect script"); + errs |= check_cmd_access (CHKACC_FILE, options->tls_verify, + R_OK|X_OK, "--tls-verify script"); + errs |= check_cmd_access (CHKACC_FILE, options->up_script, + R_OK|X_OK, "--up script"); + errs |= check_cmd_access (CHKACC_FILE, options->down_script, + R_OK|X_OK, "--down script"); + errs |= check_cmd_access (CHKACC_FILE, options->ipchange, + R_OK|X_OK, "--ipchange script"); + errs |= check_cmd_access (CHKACC_FILE, options->route_script, + R_OK|X_OK, "--route-up script"); + errs |= check_cmd_access (CHKACC_FILE, options->route_predown_script, + R_OK|X_OK, "--route-pre-down script"); + errs |= check_cmd_access (CHKACC_FILE, options->learn_address_script, + R_OK|X_OK, "--learn-address script"); #endif /* P2MP_SERVER */ if (errs) -- cgit