varnishadm help missing commands requiring authentication

[nigoroll]

reported by @martin-uplex

• the help command gets handled in the child because of cli commands added in child init functions
• for the child, cli->auth gets set to 1 here https://github.com/varnishcache/varnish-cache/blob/master/bin/varnishd/cache/cache_cli.c#L103 , so basically more-than nothing but less than any real authentication level
• consequently, VCLS_func_help, which checks the authentication level https://github.com/varnishcache/varnish-cache/blob/master/lib/libvarnish/vcli_serve.c#L151 does not output any functions handled in the manager which require authentication

To illustrate, see the difference in output with and without this change:

diff --git a/bin/varnishd/mgt/mgt_cli.c b/bin/varnishd/mgt/mgt_cli.c
index 3677eb620..f5700fdf6 100644
--- a/bin/varnishd/mgt/mgt_cli.c
+++ b/bin/varnishd/mgt/mgt_cli.c
@@ -310,19 +310,13 @@ mcf_auth(struct cli *cli, const char *const *av, void *priv)
 static void v_matchproto_(cli_func_t)
 mcf_help(struct cli *cli, const char * const *av, void *priv)
 {
-       if (cli_o <= 0)
-               VCLS_func_help(cli, av, priv);
-       else
-               mcf_askchild(cli, av, priv);
+       VCLS_func_help(cli, av, priv);
 }

 static void v_matchproto_(cli_func_t)
 mcf_help_json(struct cli *cli, const char * const *av, void *priv)
 {
-       if (cli_o <= 0)
-               VCLS_func_help_json(cli, av, priv);
-       else
-               mcf_askchild(cli, av, priv);
+       VCLS_func_help_json(cli, av, priv);
 }

 static struct cli_proto cli_auth[] = {

->

--- /tmp/help-a.mgt 2018-06-25 16:50:50.019648886 +0200
+++ /tmp/help-a.ori 2018-06-25 16:50:09.915668300 +0200
@@ -1,22 +1,23 @@
 auth <response>
-banner
-debug.panic.master
+backend.list [-p] [<backend_pattern>]
+backend.set_health <backend_pattern> [auto|healthy|sick]
+ban <field> <operator> <arg> [&& <field> <oper> <arg> ...]
+ban.list
+debug.fragfetch
+debug.listen_address
+debug.panic.worker
+debug.srandom
+debug.vmod
+debug.xid
 help [<command>]
-panic.clear [-z]
-panic.show
-param.set <param> <value>
-param.show [-l] [<param>]
 ping [<timestamp>]
 quit
 start
-status
-stop
-storage.list
 vcl.discard <configname|label>
-vcl.inline <configname> <quoted_VCLstring> [auto|cold|warm]
 vcl.label <label> <configname>
 vcl.list
 vcl.load <configname> <filename> [auto|cold|warm]
+vcl.show [-v] <configname>
 vcl.state <configname> [auto|cold|warm]
 vcl.use <configname|label>

lines marked with - are those we are missing to output, lines marked with + are those we would be missing if we did not handle the help command in the child

[nigoroll]

FTR, at this point I do not feel comfortable to submit a patch for this:

• the correct solution would probably be to pass the authentication level to the child
• yet at this point we only have two cases: unauthenticated and authenticated
• mgt either successfully authenticates (via authentication or skipping it because no secret file exists) or fails, so we might as well assume in the child that all commands are authenticated

Anyway, I neither want to hack in a dirty solution nor some overkill

[dridi]

Sounds like a dupe of #2682?

[nigoroll]

Ooops, I had actually commented in that other bug yet still forgot about it. Thank you @Dridi