[master] 6cb21b4 Get rid of the cli_buffer parameter for good.

Poul-Henning Kamp phk at FreeBSD.org
Tue Nov 7 19:21:08 UTC 2017


commit 6cb21b47a16be6b97b8197dd3cb79cef42999273
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Tue Nov 7 19:17:54 2017 +0000

    Get rid of the cli_buffer parameter for good.
    
    Rewrite the CLI command acceptance to not use the VLU functions,
    but a small state machine.
    
    Unauthenticated CLI sessions have a hard limit of 80 char cmds
    and cannot use the "<< nonce" construct.

diff --git a/bin/varnishd/cache/cache_cli.c b/bin/varnishd/cache/cache_cli.c
index ee542ec..bdaa4ae 100644
--- a/bin/varnishd/cache/cache_cli.c
+++ b/bin/varnishd/cache/cache_cli.c
@@ -100,6 +100,7 @@ CLI_Run(void)
 	cli = VCLS_AddFd(cache_cls,
 	    heritage.cli_in, heritage.cli_out, NULL, NULL);
 	AN(cli);
+	cli->auth = 1;	// Non-zero to disable paranoia in vcli_serve
 
 	do {
 		i = VCLS_Poll(cache_cls, cli, -1);
@@ -126,8 +127,9 @@ CLI_Init(void)
 	Lck_New(&cli_mtx, lck_cli);
 	cli_thread = pthread_self();
 
-	cache_cls = heritage.cls;
+	cache_cls = VCLS_New(heritage.cls);
 	AN(cache_cls);
+	VCLS_SetLimit(cache_cls, &cache_param->cli_limit);
 	VCLS_SetHooks(cache_cls, cli_cb_before, cli_cb_after);
 
 	CLI_AddFuncs(cli_cmds);
diff --git a/bin/varnishd/mgt/mgt_cli.c b/bin/varnishd/mgt/mgt_cli.c
index 0d5f777..a25cc92 100644
--- a/bin/varnishd/mgt/mgt_cli.c
+++ b/bin/varnishd/mgt/mgt_cli.c
@@ -364,8 +364,9 @@ void
 mgt_cli_init_cls(void)
 {
 
-	mgt_cls = VCLS_New(&mgt_param.cli_buffer, &mgt_param.cli_limit);
+	mgt_cls = VCLS_New(NULL);
 	AN(mgt_cls);
+	VCLS_SetLimit(mgt_cls, &mgt_param.cli_limit);
 	VCLS_SetHooks(mgt_cls, mgt_cli_cb_before, mgt_cli_cb_after);
 	VCLS_AddFunc(mgt_cls, MCF_NOAUTH, cli_auth);
 	VCLS_AddFunc(mgt_cls, MCF_AUTH, cli_proto);
diff --git a/include/vcli_serve.h b/include/vcli_serve.h
index a7b3471..eff8a4b 100644
--- a/include/vcli_serve.h
+++ b/include/vcli_serve.h
@@ -90,8 +90,9 @@ void VCLI_SetResult(struct cli *cli, unsigned r);
 
 typedef void cls_cb_f(void *priv);
 typedef void cls_cbc_f(const struct cli*);
-struct VCLS *VCLS_New(volatile unsigned *maxlen, volatile unsigned *limit);
+struct VCLS *VCLS_New(struct VCLS *);
 void VCLS_SetHooks(struct VCLS *, cls_cbc_f *, cls_cbc_f *);
+void VCLS_SetLimit(struct VCLS *, volatile unsigned *);
 struct cli *VCLS_AddFd(struct VCLS *cs, int fdi, int fdo, cls_cb_f *closefunc,
     void *priv);
 void VCLS_AddFunc(struct VCLS *cs, unsigned auth, struct cli_proto *clp);
diff --git a/lib/libvarnish/vcli_serve.c b/lib/libvarnish/vcli_serve.c
index 9cde8b1..71c2544 100644
--- a/lib/libvarnish/vcli_serve.c
+++ b/lib/libvarnish/vcli_serve.c
@@ -49,7 +49,6 @@
 
 #include "vav.h"
 #include "vcli_serve.h"
-#include "vlu.h"
 #include "vsb.h"
 
 struct VCLS_fd {
@@ -62,9 +61,9 @@ struct VCLS_fd {
 	cls_cb_f			*closefunc;
 	void				*priv;
 	struct vsb			*last_arg;
-	int				last_idx;
 	char				**argv;
-	struct vlu			*vlu;
+	int				argc;
+	char				*match;
 };
 
 struct VCLS {
@@ -74,7 +73,6 @@ struct VCLS {
 	unsigned			nfd;
 	VTAILQ_HEAD(,cli_proto)		funcs;
 	cls_cbc_f			*before, *after;
-	volatile unsigned		*maxlen;
 	volatile unsigned		*limit;
 	struct cli_proto		*wildcard;
 };
@@ -244,6 +242,7 @@ cls_exec(struct VCLS_fd *cfd, char * const *av)
 	ssize_t len;
 	char *s;
 	unsigned lim;
+	int retval = 0;
 	const char *trunc = "!\n[response was truncated]\n";
 
 	CHECK_OBJ_NOTNULL(cfd, VCLS_FD_MAGIC);
@@ -314,105 +313,140 @@ cls_exec(struct VCLS_fd *cfd, char * const *av)
 	}
 	if (VCLI_WriteResult(cfd->fdo, cli->result, s) ||
 	    cli->result == CLIS_CLOSE)
-		return (1);
+		retval = 1;
 
-	return (0);
+	/*
+	 * In unauthenticated mode we are very intolerant, and close the
+	 * connection at the least provocation.
+	 */
+	if (cli->auth == 0 && cli->result != CLIS_OK)
+		retval = 1;
+
+	return (retval);
 }
 
 static int
-cls_vlu(void *priv, const char *p)
+cls_feed(struct VCLS_fd *cfd, const char *p, const char *e)
 {
-	struct VCLS_fd *cfd;
 	struct cli *cli;
-	int i, ac;
-	char **av;
+	int i, retval = 0, ac;
+	char **av, *q;
 
-	CAST_OBJ_NOTNULL(cfd, priv, VCLS_FD_MAGIC);
+	CHECK_OBJ_NOTNULL(cfd, VCLS_FD_MAGIC);
 	AN(p);
+	assert(e > p);
 
 	cli = cfd->cli;
 	CHECK_OBJ_NOTNULL(cli, CLI_MAGIC);
 
-	if (cfd->argv == NULL) {
-		/*
-		 * Lines with only whitespace are simply ignored, in order
-		 * to not complicate CLI-client side scripts and TELNET users
-		 */
-		for (; isspace(*p); p++)
+	for(;p < e; p++) {
+		if (cli->cmd == NULL && isspace(*p)) {
+			/* Ignore all leading space before cmd */
 			continue;
-		if (*p == '\0')
-			return (0);
-		AN(p);	/* for FlexeLint */
-
-		cli->cmd = VSB_new(NULL, NULL, strlen(p) + 1, 0);
-		AN(cli->cmd);
-		VSB_cat(cli->cmd, p);
-		AZ(VSB_finish(cli->cmd));
-
-		/* We ignore a single leading '-' (for -I cli_file) */
-		if (p[0] == '-')
-			av = VAV_Parse(p + 1, &ac, 0);
-		else
-			av = VAV_Parse(p, &ac, 0);
-		AN(av);
-		if (av[0] != NULL) {
-			i = cls_exec(cfd, av);
-			VAV_Free(av);
-			VSB_destroy(&cli->cmd);
-			return (i);
-		}
-		if (ac < 3 || cli->auth == 0 || strcmp(av[ac - 2], "<<")) {
-			i = cls_exec(cfd, av);
-			VAV_Free(av);
-			VSB_destroy(&cli->cmd);
-			return (i);
 		}
-		cfd->argv = av;
-		cfd->last_idx = ac - 2;
-		cfd->last_arg = VSB_new_auto();
-		AN(cfd->last_arg);
-		return (0);
-	} else {
-		AN(cfd->argv[cfd->last_idx]);
-		AZ(strcmp(cfd->argv[cfd->last_idx], "<<"));
-		AN(cfd->argv[cfd->last_idx + 1]);
-		if (strcmp(p, cfd->argv[cfd->last_idx + 1])) {
-			VSB_cat(cfd->last_arg, p);
-			VSB_cat(cfd->last_arg, "\n");
-			return (0);
+		if (cfd->argv == NULL) {
+
+			/* Collect first line up to \n or \r */
+			if (cli->cmd == NULL) {
+				cli->cmd = VSB_new_auto();
+				AN(cli->cmd);
+			}
+
+			/* Until authenticated, limit length hard */
+			if (*p != '\n' && *p != '\r' &&
+			    (cli->auth > 0 || VSB_len(cli->cmd) < 80)) {
+				VSB_putc(cli->cmd, *p);
+				continue;
+			}
+
+			AZ(VSB_finish(cli->cmd));
+
+			/* Ignore leading '-' */
+			q = VSB_data(cli->cmd);
+			if (*q == '-')
+				q++;
+			av = VAV_Parse(q, &ac, 0);
+			AN(av);
+
+			if (cli->auth > 0 &&
+			    av[0] == NULL &&
+			    ac >= 3 &&
+			    !strcmp(av[ac-2], "<<") &&
+			    *av[ac - 1] != '\0') {
+				/* Go to "<< nonce" mode */
+				cfd->argv = av;
+				cfd->argc = ac;
+				cfd->match = av[ac - 1];
+				cfd->last_arg = VSB_new_auto();
+				AN(cfd->last_arg);
+			} else {
+				/* Plain command */
+				i = cls_exec(cfd, av);
+				VAV_Free(av);
+				VSB_destroy(&cli->cmd);
+				if (i)
+					return(i);
+			}
+		} else {
+			/* "<< nonce" mode */
+			AN(cfd->argv);
+			AN(cfd->argc);
+			AN(cfd->match);
+			AN(cfd->last_arg);
+			if (*cfd->match == '\0' && (*p == '\r' || *p == '\n')) {
+				AZ(VSB_finish(cfd->last_arg));
+				// NB: VAV lib internals trusted
+				REPLACE(cfd->argv[cfd->argc - 1], NULL);
+				REPLACE(cfd->argv[cfd->argc - 2], NULL);
+				cfd->argv[cfd->argc - 2] =
+				    VSB_data(cfd->last_arg);
+				i = cls_exec(cfd, cfd->argv);
+				cfd->argv[cfd->argc - 2] = NULL;
+				VAV_Free(cfd->argv);
+				cfd->argv = NULL;
+				VSB_destroy(&cfd->last_arg);
+				VSB_destroy(&cli->cmd);
+				if (i)
+					return (i);
+			} else if (*p == *cfd->match) {
+				cfd->match++;
+			} else if (cfd->match != cfd->argv[cfd->argc - 1]) {
+				q = cfd->argv[cfd->argc - 1];
+				VSB_bcat(cfd->last_arg, q, cfd->match - q);
+				cfd->match = q;
+				VSB_putc(cfd->last_arg, *p);
+			} else {
+				VSB_putc(cfd->last_arg, *p);
+			}
 		}
-		AZ(VSB_finish(cfd->last_arg));
-		free(cfd->argv[cfd->last_idx]);
-		cfd->argv[cfd->last_idx] = NULL;
-		free(cfd->argv[cfd->last_idx + 1]);
-		cfd->argv[cfd->last_idx + 1] = NULL;
-		cfd->argv[cfd->last_idx] = VSB_data(cfd->last_arg);
-		i = cls_exec(cfd, cfd->argv);
-		cfd->argv[cfd->last_idx] = NULL;
-		VAV_Free(cfd->argv);
-		cfd->argv = NULL;
-		VSB_destroy(&cli->cmd);
-		VSB_destroy(&cfd->last_arg);
-		cfd->last_idx = 0;
-		return (i);
 	}
+	return (retval);
 }
 
 struct VCLS *
-VCLS_New(volatile unsigned *maxlen, volatile unsigned *limit)
+VCLS_New(struct VCLS *model)
 {
 	struct VCLS *cs;
 
+	CHECK_OBJ_ORNULL(model, VCLS_MAGIC);
+
 	ALLOC_OBJ(cs, VCLS_MAGIC);
 	AN(cs);
 	VTAILQ_INIT(&cs->fds);
 	VTAILQ_INIT(&cs->funcs);
-	cs->maxlen = maxlen;
-	cs->limit = limit;
+	if (model != NULL)
+		VTAILQ_CONCAT(&cs->funcs, &model->funcs, list);
 	return (cs);
 }
 
 void
+VCLS_SetLimit(struct VCLS *cs, volatile unsigned *limit)
+{
+	CHECK_OBJ_NOTNULL(cs, VCLS_MAGIC);
+	cs->limit = limit;
+}
+
+void
 VCLS_SetHooks(struct VCLS *cs, cls_cbc_f *before, cls_cbc_f *after)
 {
 
@@ -436,8 +470,6 @@ 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->vlu = VLU_New(cls_vlu, cfd, *cs->maxlen);
-	AN(cfd->vlu);
 	cfd->cli->sb = VSB_new_auto();
 	AN(cfd->cli->sb);
 	cfd->cli->limit = cs->limit;
@@ -458,7 +490,6 @@ cls_close_fd(struct VCLS *cs, struct VCLS_fd *cfd)
 
 	VTAILQ_REMOVE(&cs->fds, cfd, list);
 	cs->nfd--;
-	VLU_Destroy(&cfd->vlu);
 	VSB_destroy(&cfd->cli->sb);
 	if (cfd->closefunc == NULL) {
 		(void)close(cfd->fdi);
@@ -510,6 +541,7 @@ VCLS_Poll(struct VCLS *cs, const struct cli *cli, int timeout)
 	struct VCLS_fd *cfd;
 	struct pollfd pfd[1];
 	int i, j, k;
+	char buf[BUFSIZ];
 
 	CHECK_OBJ_NOTNULL(cs, VCLS_MAGIC);
 	if (cs->nfd == 0) {
@@ -536,8 +568,13 @@ VCLS_Poll(struct VCLS *cs, const struct cli *cli, int timeout)
 		return (j);
 	if (pfd[0].revents & POLLHUP)
 		k = 1;
-	else
-		k = VLU_Fd(cfd->vlu, cfd->fdi);
+	else {
+		i = read(cfd->fdi, buf, sizeof buf);
+		if (i <= 0)
+			k = 1;
+		else
+			k = cls_feed(cfd, buf, buf + i);
+	}
 	if (k)
 		cls_close_fd(cs, cfd);
 	return (k);


More information about the varnish-commit mailing list