Attachment 3399 Details for Bug 3122 – proposed patch for this & #3169

[patch] proposed patch for this & #3169

file_3122.txt (text/plain), 8.24 KB, created by Jakub Jelen on 2020-05-27 00:38:17 AEST

(hide)

Description:

Filename:

MIME Type:

Creator: Jakub Jelen

Created: 2020-05-27 00:38:17 AEST

Size: 8.24 KB

patch

obsolete

>From 3caa40f40c7f97ecf46969e050e530338864033e Mon Sep 17 00:00:00 2001
>From: Jakub Jelen <jjelen@redhat.com>
>Date: Mon, 25 May 2020 15:46:51 +0200
>Subject: [PATCH 1/3] regress: Add more test cases
>
>---
> regress/servcfginclude.sh | 36 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 35 insertions(+), 1 deletion(-)
>
>diff --git a/regress/servcfginclude.sh b/regress/servcfginclude.sh
>index b25c8faa..b6a9a248 100644
>--- a/regress/servcfginclude.sh
>+++ b/regress/servcfginclude.sh
>@@ -146,9 +146,43 @@ Include
> _EOF
> 
> trace "disallow invalid with no argument"
>-${SUDO} ${REAL_SSHD} -f $OBJ/sshd_config.i.x \
>+${SUDO} ${REAL_SSHD} -f $OBJ/sshd_config.i.x -T \
>     -C "host=x,user=test,addr=127.0.0.1" 2>/dev/null && \
> 	fail "sshd allowed Include with no argument"
> 
>+# Ensure the Include before any Match block works as expected (bug #3122)
>+cat > $OBJ/sshd_config.i << _EOF
>+Banner /xx
>+HostKey $OBJ/host.ssh-ed25519
>+Include $OBJ/sshd_config.i.2
>+Match host a
>+	Banner /aaaa
>+_EOF
>+cat > $OBJ/sshd_config.i.2 << _EOF
>+Match host a
>+	Banner /aa
>+_EOF
>+
>+trace "Include before match blocks"
>+trial a /aa "included file before match blocks is properly evaluated"
>+
>+# Port in included file is correctly interpretted (bug #3169)
>+cat > $OBJ/sshd_config.i << _EOF
>+Include $OBJ/sshd_config.i.2
>+Port 7722
>+_EOF
>+cat > $OBJ/sshd_config.i.2 << _EOF
>+HostKey $OBJ/host.ssh-ed25519
>+_EOF
>+
>+trace "Port after included files"
>+${SUDO} ${REAL_SSHD} -f $OBJ/sshd_config.i -T \
>+    -C "host=x,user=test,addr=127.0.0.1" > $OBJ/sshd_config.out || \
>+	fail "failed to parse Port after included files"
>+_port=`grep -i '^port ' $OBJ/sshd_config.out | awk '{print $2}'`
>+if test "x7722" != "x$_port" ; then
>+	fail "The Port in included file was intertepretted wrongly. Expected 7722, got $_port"
>+fi
>+
> # cleanup
> rm -f $OBJ/sshd_config.i $OBJ/sshd_config.i.* $OBJ/sshd_config.out
>-- 
>2.25.4
>
>
>From 924922fcb8f34fb4a156367de2ee33ad92a68a6a Mon Sep 17 00:00:00 2001
>From: Jakub Jelen <jjelen@redhat.com>
>Date: Mon, 25 May 2020 16:56:39 +0200
>Subject: [PATCH 2/3] Do not call process_queued_listen_addrs() for every
> included file
>
>Fixes #3169
>---
> servconf.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/servconf.c b/servconf.c
>index 5bb4b1f8..78a7d87d 100644
>--- a/servconf.c
>+++ b/servconf.c
>@@ -74,7 +74,7 @@ static void add_listen_addr(ServerOptions *, const char *,
>     const char *, int);
> static void add_one_listen_addr(ServerOptions *, const char *,
>     const char *, int);
>-void parse_server_config_depth(ServerOptions *options, const char *filename,
>+static void parse_server_config_depth(ServerOptions *options, const char *filename,
>     struct sshbuf *conf, struct include_list *includes,
>     struct connection_info *connectinfo, int flags, int *activep, int depth);
> 
>@@ -2580,7 +2580,7 @@ copy_set_server_options(ServerOptions *dst, ServerOptions *src, int preauth)
> #undef M_CP_STRARRAYOPT
> 
> #define SERVCONF_MAX_DEPTH	16
>-void
>+static void
> parse_server_config_depth(ServerOptions *options, const char *filename,
>     struct sshbuf *conf, struct include_list *includes,
>     struct connection_info *connectinfo, int flags, int *activep, int depth)
>@@ -2606,7 +2606,6 @@ parse_server_config_depth(ServerOptions *options, const char *filename,
> 	if (bad_options > 0)
> 		fatal("%s: terminating, %d bad configuration options",
> 		    filename, bad_options);
>-	process_queued_listen_addrs(options);
> }
> 
> void
>@@ -2617,6 +2616,7 @@ parse_server_config(ServerOptions *options, const char *filename,
> 	int active = connectinfo ? 0 : 1;
> 	parse_server_config_depth(options, filename, conf, includes,
> 	    connectinfo, 0, &active, 0);
>+	process_queued_listen_addrs(options);
> }
> 
> static const char *
>-- 
>2.25.4
>
>
>From 26d970b4fb373cb7bd99286e41dd095cd1eadbd0 Mon Sep 17 00:00:00 2001
>From: Jakub Jelen <jjelen@redhat.com>
>Date: Tue, 26 May 2020 16:25:24 +0200
>Subject: [PATCH 3/3] servconf: Fix parsing of Match blocks in included files
> (#3122)
>
>---
> servconf.c | 28 +++++++++++++++++++---------
> 1 file changed, 19 insertions(+), 9 deletions(-)
>
>diff --git a/servconf.c b/servconf.c
>index 78a7d87d..a8541514 100644
>--- a/servconf.c
>+++ b/servconf.c
>@@ -554,6 +554,7 @@ typedef enum {
> #define SSHCFG_MATCH		0x02	/* allowed inside a Match section */
> #define SSHCFG_ALL		(SSHCFG_GLOBAL|SSHCFG_MATCH)
> #define SSHCFG_NEVERMATCH	0x04  /* Match never matches; internal only */
>+#define SSHCFG_MATCH_ONLY	0x08  /* Match only in conditional blocks; internal only */
> 
> /* Textual representation of the tokens. */
> static struct {
>@@ -1265,7 +1266,7 @@ static const struct multistate multistate_tcpfwd[] = {
> static int
> process_server_config_line_depth(ServerOptions *options, char *line,
>     const char *filename, int linenum, int *activep,
>-    struct connection_info *connectinfo, int inc_flags, int depth,
>+    struct connection_info *connectinfo, int *inc_flags, int depth,
>     struct include_list *includes)
> {
> 	char ch, *cp, ***chararrayptr, **charptr, *arg, *arg2, *p;
>@@ -2012,7 +2013,9 @@ process_server_config_line_depth(ServerOptions *options, char *line,
> 					parse_server_config_depth(options,
> 					    item->filename, item->contents,
> 					    includes, connectinfo,
>-					    (oactive ? 0 : SSHCFG_NEVERMATCH),
>+					    (*inc_flags & SSHCFG_MATCH_ONLY
>+					        ? SSHCFG_MATCH_ONLY : (oactive
>+					            ? 0 : SSHCFG_NEVERMATCH)),
> 					    activep, depth + 1);
> 				}
> 				found = 1;
>@@ -2060,7 +2063,9 @@ process_server_config_line_depth(ServerOptions *options, char *line,
> 				parse_server_config_depth(options,
> 				    item->filename, item->contents,
> 				    includes, connectinfo,
>-				    (oactive ? 0 : SSHCFG_NEVERMATCH),
>+				    (*inc_flags & SSHCFG_MATCH_ONLY
>+				        ? SSHCFG_MATCH_ONLY : (oactive
>+				            ? 0 : SSHCFG_NEVERMATCH)),
> 				    activep, depth + 1);
> 				*activep = oactive;
> 				TAILQ_INSERT_TAIL(includes, item, entry);
>@@ -2078,11 +2083,14 @@ process_server_config_line_depth(ServerOptions *options, char *line,
> 		if (cmdline)
> 			fatal("Match directive not supported as a command-line "
> 			   "option");
>-		value = match_cfg_line(&cp, linenum, connectinfo);
>+		value = match_cfg_line(&cp, linenum,
>+		    (*inc_flags & SSHCFG_NEVERMATCH ? NULL : connectinfo));
> 		if (value < 0)
> 			fatal("%s line %d: Bad Match condition", filename,
> 			    linenum);
>-		*activep = (inc_flags & SSHCFG_NEVERMATCH) ? 0 : value;
>+		*activep = (*inc_flags & SSHCFG_NEVERMATCH) ? 0 : value;
>+		/* The MATCH_ONLY is applicable only until the first match block */
>+		*inc_flags &= ~SSHCFG_MATCH_ONLY;
> 		break;
> 
> 	case sPermitListen:
>@@ -2385,8 +2393,9 @@ process_server_config_line(ServerOptions *options, char *line,
>     const char *filename, int linenum, int *activep,
>     struct connection_info *connectinfo, struct include_list *includes)
> {
>+	int inc_flags = 0;
> 	return process_server_config_line_depth(options, line, filename,
>-	    linenum, activep, connectinfo, 0, 0, includes);
>+	    linenum, activep, connectinfo, &inc_flags, 0, includes);
> }
> 
> 
>@@ -2591,14 +2600,15 @@ parse_server_config_depth(ServerOptions *options, const char *filename,
> 	if (depth < 0 || depth > SERVCONF_MAX_DEPTH)
> 		fatal("Too many recursive configuration includes");
> 
>-	debug2("%s: config %s len %zu", __func__, filename, sshbuf_len(conf));
>+	debug2("%s: config %s len %zu%s", __func__, filename, sshbuf_len(conf),
>+	    (flags & SSHCFG_NEVERMATCH ? " [checking syntax only]" : ""));
> 
> 	if ((obuf = cbuf = sshbuf_dup_string(conf)) == NULL)
> 		fatal("%s: sshbuf_dup_string failed", __func__);
> 	linenum = 1;
> 	while ((cp = strsep(&cbuf, "\n")) != NULL) {
> 		if (process_server_config_line_depth(options, cp,
>-		    filename, linenum++, activep, connectinfo, flags,
>+		    filename, linenum++, activep, connectinfo, &flags,
> 		    depth, includes) != 0)
> 			bad_options++;
> 	}
>@@ -2615,7 +2625,7 @@ parse_server_config(ServerOptions *options, const char *filename,
> {
> 	int active = connectinfo ? 0 : 1;
> 	parse_server_config_depth(options, filename, conf, includes,
>-	    connectinfo, 0, &active, 0);
>+	    connectinfo, (connectinfo ? SSHCFG_MATCH_ONLY : 0), &active, 0);
> 	process_queued_listen_addrs(options);
> }
> 
>-- 
>2.25.4
>

Actions: View | Diff

Attachments on bug 3122: 3384 | 3399