The CLI protocol and its implementation is not designed for

transferring huge amounts of data, and it should not be used that
way.

But we have commands like ban.list which potentially can return at
metric shitload of data and this is a problem.

This commit implements a cli_limit paramter which (with its upper
limit) protects from responses that cannot be moved by the CLI
protocol no matter what.

We set the default to a low 4096 bytes, in order to make people
specifically shoot their feet off if they really want to see 200k
bans.

bin/varnishd/cache/cache_cli.c

@@ -236,7 +236,8 @@ CLI_Init(void)
 	Lck_New(&cli_mtx, lck_cli);
 	cli_thread = pthread_self();
 
-	cls = VCLS_New(cli_cb_before, cli_cb_after, cache_param->cli_buffer);
+	cls = VCLS_New(cli_cb_before, cli_cb_after,
+	    &cache_param->cli_buffer, &cache_param->cli_limit);
 	AN(cls);
 
 	CLI_AddFuncs(master_cmds);

bin/varnishd/common/params.h

@@ -107,6 +107,7 @@ struct params {
 
 	/* CLI related */
 	unsigned		cli_timeout;
+	unsigned		cli_limit;
 	unsigned		ping_interval;
 
 	/* LRU list ordering interval */

bin/varnishd/mgt/mgt_cli.c

@@ -336,7 +336,8 @@ static void
 mgt_cli_init_cls(void)
 {
 
-	cls = VCLS_New(mgt_cli_cb_before, mgt_cli_cb_after, mgt_param.cli_buffer);
+	cls = VCLS_New(mgt_cli_cb_before, mgt_cli_cb_after,
+	    &mgt_param.cli_buffer, &mgt_param.cli_limit);
 	AN(cls);
 	AZ(VCLS_AddFunc(cls, MCF_NOAUTH, cli_auth));
 	AZ(VCLS_AddFunc(cls, MCF_AUTH, cli_proto));

bin/varnishd/mgt/mgt_main.c

@@ -349,6 +349,7 @@ main(int argc, char * const *argv)
 	struct cli cli[1];
 	struct vpf_fh *pfh = NULL;
 	char *dirname;
+	unsigned clilim;
 
 	/*
 	 * Start out by closing all unwanted file descriptors we might
@@ -389,9 +390,12 @@ main(int argc, char * const *argv)
 	SHA256_Test();
 
 	memset(cli, 0, sizeof cli);
+	cli[0].magic = CLI_MAGIC;
 	cli[0].sb = VSB_new_auto();
 	XXXAN(cli[0].sb);
 	cli[0].result = CLIS_OK;
+	clilim = 32768;
+	cli[0].limit = &clilim;
 
 	VTAILQ_INIT(&heritage.socks);
 

bin/varnishd/mgt/mgt_param.c

@@ -673,6 +673,19 @@ static const struct parspec input_parspec[] = {
 		"Listen queue depth.",
 		MUST_RESTART,
 		"1024", "connections" },
+	{ "cli_buffer", tweak_uint, &mgt_param.cli_buffer, 4096, UINT_MAX,
+		"Size of buffer for CLI command input."
+		"\nYou may need to increase this if you have big VCL files "
+		"and use the vcl.inline CLI command.\n"
+		"NB: Must be specified with -p to have effect.\n",
+		0,
+		"8192", "bytes" },
+	{ "cli_limit", tweak_uint, &mgt_param.cli_limit, 128, 99999999,
+		"Maximum size of CLI response.  If the response exceeds"
+		" this limit, the reponse code will be 201 instead of"
+		" 200 and the last line will indicate the truncation.",
+		0,
+		"4096", "bytes" },
 	{ "cli_timeout", tweak_timeout, &mgt_param.cli_timeout, 0, 0,
 		"Timeout for the childs replies to CLI requests from "
 		"the mgt_param.",
@@ -805,13 +818,6 @@ static const struct parspec input_parspec[] = {
 		"more sessions take a detour around the waiter.",
 		EXPERIMENTAL,
 		"50", "ms" },
-	{ "cli_buffer", tweak_uint, &mgt_param.cli_buffer, 4096, UINT_MAX,
-		"Size of buffer for CLI input."
-		"\nYou may need to increase this if you have big VCL files "
-		"and use the vcl.inline CLI command.\n"
-		"NB: Must be specified with -p to have effect.\n",
-		0,
-		"8192", "bytes" },
 	{ "log_hashstring", tweak_bool, &mgt_param.log_hash, 0, 0,
 		"Log the hash string components to shared memory log.\n",
 		0,

bin/varnishtest/tests/b00008.vtc

@@ -33,3 +33,7 @@ varnish v1 -cliok "help"
 varnish v1 -cliok "param.set waiter default"
 
 varnish v1 -clierr 106 "param.set waiter HASH(0x8839c4c)"
+
+varnish v1 -cliok "param.set cli_limit 128"
+
+varnish v1 -clierr 201 "param.show"

include/vcli.h

@@ -199,6 +199,7 @@ enum VCLI_status_e {
 	CLIS_PARAM	= 106,
 	CLIS_AUTH	= 107,
 	CLIS_OK		= 200,
+	CLIS_TRUNCATED	= 201,
 	CLIS_CANT	= 300,
 	CLIS_COMMS	= 400,
 	CLIS_CLOSE	= 500

include/vcli_common.h

@@ -42,4 +42,5 @@ struct cli {
 	char			*ident;
 	struct vlu		*vlu;
 	struct VCLS		*cls;
+	volatile unsigned	*limit;
 };

include/vcli_serve.h

@@ -30,7 +30,8 @@
 struct VCLS;
 typedef void cls_cb_f(void *priv);
 typedef void cls_cbc_f(const struct cli*);
-struct VCLS *VCLS_New(cls_cbc_f *before, cls_cbc_f *after, unsigned maxlen);
+struct VCLS *VCLS_New(cls_cbc_f *before, cls_cbc_f *after,
+    volatile unsigned *maxlen, volatile unsigned *limit);
 struct cli *VCLS_AddFd(struct VCLS *cs, int fdi, int fdo, cls_cb_f *closefunc,
     void *priv);
 int VCLS_AddFunc(struct VCLS *cs, unsigned auth, struct cli_proto *clp);

lib/libvarnish/cli_common.c

@@ -43,6 +43,7 @@
 #include <time.h>
 #include <unistd.h>
 
+#include "miniobj.h"
 #include "vas.h"
 #include "vcli.h"
 #include "vcli_common.h"
@@ -56,10 +57,15 @@ VCLI_Out(struct cli *cli, const char *fmt, ...)
 	va_list ap;
 
 	va_start(ap, fmt);
-	if (cli != NULL)
-		(void)VSB_vprintf(cli->sb, fmt, ap);
-	else
+	if (cli != NULL) {
+		CHECK_OBJ_NOTNULL(cli, CLI_MAGIC);
+		if (VSB_len(cli->sb) < *cli->limit)
+			(void)VSB_vprintf(cli->sb, fmt, ap);
+		else if (cli->result == CLIS_OK)
+			cli->result = CLIS_TRUNCATED;
+	} else {
 		(void)vfprintf(stdout, fmt, ap);
+	}
 	va_end(ap);
 }
 
@@ -68,6 +74,7 @@ void
 VCLI_Quote(struct cli *cli, const char *s)
 {
 
+	CHECK_OBJ_NOTNULL(cli, CLI_MAGIC);
 	VSB_quote(cli->sb, s, -1, 0);
 }
 
@@ -75,10 +82,13 @@ void
 VCLI_SetResult(struct cli *cli, unsigned res)
 {
 
-	if (cli != NULL)
-		cli->result = res;	/*lint !e64 type mismatch */
-	else
+	if (cli != NULL) {
+		CHECK_OBJ_NOTNULL(cli, CLI_MAGIC);
+		if (cli->result != CLIS_TRUNCATED || res != CLIS_OK)
+			cli->result = res;	/*lint !e64 type mismatch */
+	} else {
 		printf("CLI result = %u\n", res);
+	}
 }
 
 int

lib/libvarnish/cli_serve.c

@@ -81,7 +81,8 @@ struct VCLS {
 	unsigned			nfd;
 	VTAILQ_HEAD(,VCLS_func)		funcs;
 	cls_cbc_f			*before, *after;
-	unsigned			maxlen;
+	volatile unsigned		*maxlen;
+	volatile unsigned		*limit;
 };
 
 /*--------------------------------------------------------------------*/
@@ -245,6 +246,10 @@ cls_vlu2(void *priv, char * const *av)
 	struct VCLS_func *cfn;
 	struct cli *cli;
 	unsigned na;
+	ssize_t len;
+	char *s;
+	unsigned lim;
+	const char *trunc = "!\n[response was truncated]\n";
 
 	CAST_OBJ_NOTNULL(cfd, priv, VCLS_FD_MAGIC);
 	cs = cfd->cls;
@@ -297,7 +302,16 @@ cls_vlu2(void *priv, char * const *av)
 
 	cli->cls = NULL;
 
-	if (VCLI_WriteResult(cfd->fdo, cli->result, VSB_data(cli->sb)) ||
+	s = VSB_data(cli->sb);
+	len = VSB_len(cli->sb);
+	lim = *cs->limit;
+	if (len > lim) {
+		if (cli->result == CLIS_OK)
+			cli->result = CLIS_TRUNCATED;
+		strcpy(s + (lim - strlen(trunc)), trunc);
+		assert(strlen(s) <= lim);
+	}
+	if (VCLI_WriteResult(cfd->fdo, cli->result, s) ||
 	    cli->result == CLIS_CLOSE)
 		return (1);
 
@@ -380,7 +394,8 @@ cls_vlu(void *priv, const char *p)
 }
 
 struct VCLS *
-VCLS_New(cls_cbc_f *before, cls_cbc_f *after, unsigned maxlen)
+VCLS_New(cls_cbc_f *before, cls_cbc_f *after, volatile unsigned *maxlen,
+    volatile unsigned *limit)
 {
 	struct VCLS *cs;
 
@@ -391,6 +406,7 @@ VCLS_New(cls_cbc_f *before, cls_cbc_f *after, unsigned maxlen)
 	cs->before = before;
 	cs->after = after;
 	cs->maxlen = maxlen;
+	cs->limit = limit;
 	return (cs);
 }
 
@@ -409,8 +425,9 @@ VCLS_AddFd(struct VCLS *cs, int fdi, int fdo, cls_cb_f *closefunc, void *priv)
 	cfd->fdo = fdo;
 	cfd->cli = &cfd->clis;
 	cfd->cli->magic = CLI_MAGIC;
-	cfd->cli->vlu = VLU_New(cfd, cls_vlu, cs->maxlen);
+	cfd->cli->vlu = VLU_New(cfd, cls_vlu, *cs->maxlen);
 	cfd->cli->sb = VSB_new_auto();
+	cfd->cli->limit = cs->limit;
 	cfd->closefunc = closefunc;
 	cfd->priv = priv;
 	AN(cfd->cli->sb);