On Wed, Mar 28, 2012 at 2:11 PM, David Sommerseth < openvpn.l...@topphemmelig.net> wrote: <snip>
> > Attached is a heavily revised version of my original patch. It uses > > argv_printf() to __check__ an option's commands, so it accepts exactly > > the same input as the parts of OpenVPN that __use__ the options' > > commands. It also makes all the relevant changes suggested by Gert > > except having the argument following --iproute checked. The --iproute > > code is handled differently than the other options, and I think it is > > OK that we don't do checking in 2.3 on something that wasn't checked > > in 2.2 and (apparently) might go away sometime soon. > > Thanks a lot! I have one more comment to what Gert and Fabian has > already covered. > Instead of adding wrapper function, check_cmd_access(), would it be > possible to > integrate this with check_file_access() and add another type flag, f.ex: > > #define CHKACC_EXEC (1<<5) /** Filename is an executable, ignore exec > args */ > > Then you can just flip the type flag from CHKACC_FILE to CHKACC_EXEC. If > this > type is checked for, enforcing an X_OK mode check in addition is probably > reasonable > too. Thanks for this suggestion, David, but I believe it is better to create a separate routine for this: 1. The input is not a path, but a "command". A "command" is processed (single- and double-quote and backslashes are processed and leading spaces are removed), and it consists not only of a path, but may include arguments. 2. Having two separate functions makes two cleaner and more readable (to me) functions, instead of a single function that accepts strings with two different formats and is more complicated. The first attached patch incorporates Fabian's comments. But if the consensus is that I should incorporate David's suggestion, I will do that. The first patch also updates the usage message to clarify what a "cmd" is. (That is, it is *not* a shell script or path.) The second patch updates the man page to: - Clarify what a "cmd" is; - Change the descriptions of several options to note that they accept a "command"; - Change the description of --client-connect and --client-disconnect indicate that the temporary file's path is passed as the *last* argument to the command, not the *first* argument; and - Adds a description of --route-pre-down to the descriptions of the other --route options. Thanks again to Fabian, Gert, and David for their help.
options.c.diff
Description: Binary data
openvpn.8.diff
Description: Binary data
