New Subsystem criteria for Match option block in OpenSSH server

Darren Tucker dtucker at zip.com.au
Tue May 22 21:23:07 EST 2012


OK, so the way this works is that it reparses the config again when it
does the subsystem lookup.  I'm not convinced this is a good idea, and
there are a couple of problems with the existing implementation.

On Mon, May 21, 2012 at 04:15:16PM +0200, Nicola Muto wrote:
[...]
> +		connection_info.user = NULL;
[...]
> +		logit("subsystem request: Parsing server match config options: user == '%s', host == '%s', address == '%s', subsystem == '%s'",
> +				connection_info.user, connection_info.host,
> +				connection_info.address, connection_info.subsystem);

This will deref null pointers on most platforms.  glibc might save you,
but most libcs won't.

> +		parse_server_match_config(&options, &connection_info);

Because everything except the subsystem is nulled out a this point,
config lines like "Match User foo subsystem sftp" are syntactically
valid, but don't do what you'd think.

This reparsing could also change the server state in unexpected ways,
for example:

AllowTcpForwarding yes
Match Subsystem sftp
	AllowTcpForwarding no

would allow port fowarding until you sent an sftp subsystem request.

> +		prog = options.subsystem_command[i]

In order to understand it, I forward-ported the patch to HEAD and
removed all of the unrelated changes.  (I haven't tested it, I'm only
including it in case saves someone the work).

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
    Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.
-------------- next part --------------
Index: servconf.c
===================================================================
RCS file: /home/dtucker/openssh/cvs/openssh/servconf.c,v
retrieving revision 1.223
diff -u -p -r1.223 servconf.c
--- servconf.c	19 May 2012 09:37:01 -0000	1.223
+++ servconf.c	22 May 2012 02:34:23 -0000
@@ -553,6 +553,7 @@ get_connection_info(int populate, int us
 	ci.address = get_remote_ipaddr();
 	ci.laddress = get_local_ipaddr(packet_get_connection_in());
 	ci.lport = get_local_port();
+	ci.subsystem = NULL;
 	return &ci;
 }
 
@@ -632,10 +633,12 @@ match_cfg_line(char **condition, int lin
 		debug3("checking syntax for 'Match %s'", cp);
 	else
 		debug3("checking match for '%s' user %s host %s addr %s "
-		    "laddr %s lport %d", cp, ci->user ? ci->user : "(null)",
+		    "laddr %s subsys %s lport %d", cp,
+		    ci->user ? ci->user : "(null)",
 		    ci->host ? ci->host : "(null)",
 		    ci->address ? ci->address : "(null)",
-		    ci->laddress ? ci->laddress : "(null)", ci->lport);
+		    ci->laddress ? ci->laddress : "(null)",
+		    ci->subsystem ? ci->subsystem : "(null)", ci->lport);
 
 	while ((attrib = strdelim(&cp)) && *attrib != '\0') {
 		if ((arg = strdelim(&cp)) == NULL || *arg == '\0') {
@@ -709,6 +712,17 @@ match_cfg_line(char **condition, int lin
 			case -2:
 				return -1;
 			}
+		} else if (strcasecmp(attrib, "subsystem") == 0) {
+			if (ci == NULL || ci->subsystem == NULL) {
+				result = 0;
+				continue;
+			}
+			if (match_pattern_list(ci->subsystem, arg, len, 0) != 1)
+				result = 0;
+			else
+				debug("subsystem %.100s matched 'Subsystem "
+				    "'%.100s' line %d", ci->subsystem, arg,
+				    line);
 		} else if (strcasecmp(attrib, "localport") == 0) {
 			if ((port = a2port(arg)) == -1) {
 				error("Invalid LocalPort '%s' on Match line",
@@ -1566,6 +1580,8 @@ int parse_server_match_testspec(struct c
 			ci->user = xstrdup(p + 5);
 		} else if (strncmp(p, "laddr=", 6) == 0) {
 			ci->laddress = xstrdup(p + 6);
+		} else if (strncmp(p, "subsystem=", 10) == 0) {
+			ci->subsystem = xstrdup(p + 10);
 		} else if (strncmp(p, "lport=", 6) == 0) {
 			ci->lport = a2port(p + 6);
 			if (ci->lport == -1) {
Index: servconf.h
===================================================================
RCS file: /home/dtucker/openssh/cvs/openssh/servconf.h,v
retrieving revision 1.93
diff -u -p -r1.93 servconf.h
--- servconf.h	19 May 2012 09:37:01 -0000	1.93
+++ servconf.h	22 May 2012 01:37:23 -0000
@@ -176,6 +176,7 @@ struct connection_info {
 	const char *host;	/* possibly resolved hostname */
 	const char *address; 	/* remote address */
 	const char *laddress;	/* local address */
+	const char *subsystem; /* Subsystem service requested by the remote client application */
 	int lport;		/* local port */
 };
 
Index: session.c
===================================================================
RCS file: /home/dtucker/openssh/cvs/openssh/session.c,v
retrieving revision 1.414
diff -u -p -r1.414 session.c
--- session.c	22 Apr 2012 01:08:10 -0000	1.414
+++ session.c	22 May 2012 01:55:02 -0000
@@ -2087,23 +2087,39 @@ session_subsystem_req(Session *s)
 	logit("subsystem request for %.100s by user %s", subsys,
 	    s->pw->pw_name);
 
+	/* Searching for subsystem into the options repository */
 	for (i = 0; i < options.num_subsystems; i++) {
-		if (strcmp(subsys, options.subsystem_name[i]) == 0) {
-			prog = options.subsystem_command[i];
-			cmd = options.subsystem_args[i];
-			if (strcmp(INTERNAL_SFTP_NAME, prog) == 0) {
-				s->is_subsystem = SUBSYSTEM_INT_SFTP;
-				debug("subsystem: %s", prog);
-			} else {
-				if (stat(prog, &st) < 0)
-					debug("subsystem: cannot stat %s: %s",
-					    prog, strerror(errno));
-				s->is_subsystem = SUBSYSTEM_EXT;
-				debug("subsystem: exec() %s", cmd);
-			}
-			success = do_exec(s, cmd) == 0;
-			break;
+		if (strcmp(subsys, options.subsystem_name[i]) == 0)	break;
+	}
+
+	if (i < options.num_subsystems) { /* subsystem found */
+		struct connection_info ci;
+
+		ci.user = NULL;
+		ci.host = NULL;
+		ci.address = NULL;
+		ci.subsystem = subsys;
+
+		logit("subsystem request: Parsing server match config options: user == '%s', host == '%s', address == '%s', subsystem == '%s'",
+		    ci.user, ci.host, ci.address, ci.subsystem);  /* XXX: SEGFAULT */
+
+		parse_server_match_config(&options, &ci);
+
+		prog = options.subsystem_command[i];
+		cmd = options.subsystem_args[i];
+
+		if (strcmp(INTERNAL_SFTP_NAME, prog) == 0) {
+			s->is_subsystem = SUBSYSTEM_INT_SFTP;
+			debug("subsystem: %s", prog);
+		} else {
+			if (stat(prog, &st) < 0)
+				debug("subsystem: cannot stat %s: %s",
+				    prog, strerror(errno));
+			s->is_subsystem = SUBSYSTEM_EXT;
+			debug("subsystem: exec() %s", cmd);
 		}
+
+		success = do_exec(s, cmd) == 0;
 	}
 
 	if (!success)
Index: sshd.8
===================================================================
RCS file: /home/dtucker/openssh/cvs/openssh/sshd.8,v
retrieving revision 1.226
diff -u -p -r1.226 sshd.8
--- sshd.8	19 May 2012 09:37:01 -0000	1.226
+++ sshd.8	22 May 2012 01:56:37 -0000
@@ -108,17 +108,24 @@ extended test mode.
 If provided, any
 .Cm Match
 directives in the configuration file
-that would apply to the specified user, host, and address will be set before
+that would apply to the specified user, host, address, and subsystem will be set before
 the configuration is written to standard output.
 The connection parameters are supplied as keyword=value pairs.
 The keywords are
+.Dq addr,
 .Dq user ,
-.Dq host ,
-.Dq laddr ,
+.Dq host , ,
 .Dq lport ,
 and
-.Dq addr .
-All are required and may be supplied in any order, either with multiple
+.Dq subsystem .
+Only
+.Dq user ,
+.Dq host ,
+and
+.Dq addr 
+keywords are required;
+.Dq subsystem 
+is optional. These parameters may be supplied in any order, either with multiple
 .Fl C
 options or as a comma-separated list.
 .It Fl c Ar host_certificate_file
Index: sshd_config.5
===================================================================
RCS file: /home/dtucker/openssh/cvs/openssh/sshd_config.5,v
retrieving revision 1.147
diff -u -p -r1.147 sshd_config.5
--- sshd_config.5	19 May 2012 09:37:33 -0000	1.147
+++ sshd_config.5	22 May 2012 11:03:38 -0000
@@ -677,10 +677,11 @@ The available criteria are
 .Cm User ,
 .Cm Group ,
 .Cm Host ,
+.Cm Address ,
 .Cm LocalAddress ,
 .Cm LocalPort ,
 and
-.Cm Address .
+.Cm Subsystem .
 The match patterns may consist of single entries or comma-separated
 lists and may use the wildcard and negation operators described in the
 .Sx PATTERNS


More information about the openssh-unix-dev mailing list