Given that pasta supports specifying a command to be executed on the command line, even without the usual -- separator as long as there's no ambiguity, we shouldn't eat up options that are not meant for us. Paul reports, for instance, that with: pasta --config-net ip -6 route -6 is taken by pasta to mean --ipv6-only, and we execute 'ip route'. That's because getopt_long(), by default, shuffles the argument list to shift non-option arguments at the end. Avoid that by adding '-' at the beginning of 'optstring', and mark the position of the first non-option argument (getopt_long() will now return the character code 1 once we hit it), so that we can use that as command to run, or as PID for the target namespace. Reported-by: Paul Holzinger <pholzing(a)redhat.com> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: Instead of overriding 'name' in the getopt_long() loop, to force exiting the loop, adjust the exit condition conf.c | 23 +++++++++++++++-------- util.c | 2 +- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/conf.c b/conf.c index be5259e..1a30b47 100644 --- a/conf.c +++ b/conf.c @@ -1240,22 +1240,22 @@ void conf(struct ctx *c, int argc, char **argv) enum fwd_ports_mode fwd_default = FWD_NONE; bool v4_only = false, v6_only = false; struct fqdn *dnss = c->dns_search; + int name, ret, first_nonopt = -1; unsigned int ifi4 = 0, ifi6 = 0; const char *logfile = NULL; const char *optstring; size_t logsize = 0; char *runas = NULL; long fd_tap_opt; - int name, ret; uid_t uid; gid_t gid; if (c->mode == MODE_PASTA) { c->no_dhcp_dns = c->no_dhcp_dns_search = 1; fwd_default = FWD_AUTO; - optstring = "dqfel:hF:I:p:P:m:a:n:M:g:i:o:D:S:46t:u:T:U:"; + optstring = "-dqfel:hF:I:p:P:m:a:n:M:g:i:o:D:S:46t:u:T:U:"; } else { - optstring = "dqfel:hs:F:p:P:m:a:n:M:g:i:o:D:S:461t:u:"; + optstring = "-dqfel:hs:F:p:P:m:a:n:M:g:i:o:D:S:461t:u:"; } c->tcp.fwd_in.mode = c->tcp.fwd_out.mode = FWD_UNSET; @@ -1269,6 +1269,9 @@ void conf(struct ctx *c, int argc, char **argv) case -1: case 0: break; + case 1: + first_nonopt = optind - 1; + break; case 2: if (c->mode != MODE_PASTA) die("--userns is for pasta mode only"); @@ -1629,7 +1632,10 @@ void conf(struct ctx *c, int argc, char **argv) usage(argv[0], stderr, EXIT_FAILURE); break; } - } while (name != -1); + } while (name != -1 && name != 1); + + if (first_nonopt == -1) + first_nonopt = optind; if (c->mode == MODE_PASTA && !c->pasta_conf_ns) { if (copy_routes_opt) @@ -1689,9 +1695,9 @@ void conf(struct ctx *c, int argc, char **argv) } while (name != -1); if (c->mode == MODE_PASTA) - conf_pasta_ns(&netns_only, userns, netns, optind, argc, argv); - else if (optind != argc) - die("Extra non-option argument: %s", argv[optind]); + conf_pasta_ns(&netns_only, userns, netns, first_nonopt, argc, argv); + else if (first_nonopt != argc) + die("Extra non-option argument: %s", argv[first_nonopt]); conf_open_files(c); /* Before any possible setuid() / setgid() */ @@ -1705,7 +1711,8 @@ void conf(struct ctx *c, int argc, char **argv) pasta_open_ns(c, netns); } else { pasta_start_ns(c, uid, gid, - argc - optind, argv + optind); + argc - first_nonopt, + argv + first_nonopt); } } diff --git a/util.c b/util.c index 9c6be6a..13524e6 100644 --- a/util.c +++ b/util.c @@ -710,7 +710,7 @@ void close_open_files(int argc, char **argv) int name; do { - name = getopt_long(argc, argv, ":F", optfd, NULL); + name = getopt_long(argc, argv, "-:F", optfd, NULL); if (name == 'F') { errno = 0; -- 2.43.0
On Wed, Aug 07, 2024 at 01:28:40PM +0200, Stefano Brivio wrote:Given that pasta supports specifying a command to be executed on the command line, even without the usual -- separator as long as there's no ambiguity, we shouldn't eat up options that are not meant for us. Paul reports, for instance, that with: pasta --config-net ip -6 route -6 is taken by pasta to mean --ipv6-only, and we execute 'ip route'. That's because getopt_long(), by default, shuffles the argument list to shift non-option arguments at the end. Avoid that by adding '-' at the beginning of 'optstring', and mark the position of the first non-option argument (getopt_long() will now return the character code 1 once we hit it), so that we can use that as command to run, or as PID for the target namespace. Reported-by: Paul Holzinger <pholzing(a)redhat.com> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>Eh... I'm kind of ambivalent about the idea. I tend to think that accepting options in any position is generally expected behaviour, and anything programmatically adding commands to pasta should routinely insert a "--" (that's certainly what I do in testing code). But, that's not a particularly strong opinion, so whatever. The implementation looks more complex than necessary, though. AFAICT if you just add a '+' to the front of the optstring it will do exactly what you want without having to juggle the first_nonopt and other variables. Quoting getopt_long(3) | By default, getopt() permutes the contents of argv as it scans, so | that eventually all the nonoptions are at the end. Two other | scanning modes are also implemented. If the first character of | optstring is '+' or the environment variable POSIXLY_CORRECT is set, | then option processing stops as soon as a nonoption argument is | encountered. If '+' is not the first character of opt‐ string, it | is treated as a normal option. If POSIXLY_CORRECT behaviour is | required in this case optstring will contain two '+' symbols. If | the first character of optstring is '-', then each nonoption | argv-element is handled as if it were the argument of an option with | character code 1. (This is used by programs that were written to | expect options and other argv-elements in any order and that care | about the ordering of the two.) The special argument "--" forces an | end of option-scanning regardless of the scanning mode. -- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson
On Thu, 8 Aug 2024 10:57:24 +1000 David Gibson <david(a)gibson.dropbear.id.au> wrote:On Wed, Aug 07, 2024 at 01:28:40PM +0200, Stefano Brivio wrote:Sure, on the other hand it's convenient when you're quickly trying out ip -6 route show, or some command where "--" would add 10% of typing or so.Given that pasta supports specifying a command to be executed on the command line, even without the usual -- separator as long as there's no ambiguity, we shouldn't eat up options that are not meant for us. Paul reports, for instance, that with: pasta --config-net ip -6 route -6 is taken by pasta to mean --ipv6-only, and we execute 'ip route'. That's because getopt_long(), by default, shuffles the argument list to shift non-option arguments at the end. Avoid that by adding '-' at the beginning of 'optstring', and mark the position of the first non-option argument (getopt_long() will now return the character code 1 once we hit it), so that we can use that as command to run, or as PID for the target namespace. Reported-by: Paul Holzinger <pholzing(a)redhat.com> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>Eh... I'm kind of ambivalent about the idea. I tend to think that accepting options in any position is generally expected behaviour, and anything programmatically adding commands to pasta should routinely insert a "--" (that's certainly what I do in testing code).But, that's not a particularly strong opinion, so whatever. The implementation looks more complex than necessary, though. AFAICT if you just add a '+' to the front of the optstring it will do exactly what you want without having to juggle the first_nonopt and other variables.Oops, that's actually the first approach I considered, but I didn't see any guarantee that optind would assume the expected value after the loop. However, it does... sending v3. -- Stefano
On Thu, Aug 08, 2024 at 06:02:28AM +0200, Stefano Brivio wrote:On Thu, 8 Aug 2024 10:57:24 +1000 David Gibson <david(a)gibson.dropbear.id.au> wrote:Ok, sure.On Wed, Aug 07, 2024 at 01:28:40PM +0200, Stefano Brivio wrote:Sure, on the other hand it's convenient when you're quickly trying out ip -6 route show, or some command where "--" would add 10% of typing or so.Given that pasta supports specifying a command to be executed on the command line, even without the usual -- separator as long as there's no ambiguity, we shouldn't eat up options that are not meant for us. Paul reports, for instance, that with: pasta --config-net ip -6 route -6 is taken by pasta to mean --ipv6-only, and we execute 'ip route'. That's because getopt_long(), by default, shuffles the argument list to shift non-option arguments at the end. Avoid that by adding '-' at the beginning of 'optstring', and mark the position of the first non-option argument (getopt_long() will now return the character code 1 once we hit it), so that we can use that as command to run, or as PID for the target namespace. Reported-by: Paul Holzinger <pholzing(a)redhat.com> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>Eh... I'm kind of ambivalent about the idea. I tend to think that accepting options in any position is generally expected behaviour, and anything programmatically adding commands to pasta should routinely insert a "--" (that's certainly what I do in testing code).I don't really see how it could be anything else, after all you still need to parse the remaining non-option arguments in this case. -- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibsonBut, that's not a particularly strong opinion, so whatever. The implementation looks more complex than necessary, though. AFAICT if you just add a '+' to the front of the optstring it will do exactly what you want without having to juggle the first_nonopt and other variables.Oops, that's actually the first approach I considered, but I didn't see any guarantee that optind would assume the expected value after the loop. However, it does... sending v3.