New Subsystem criteria for Match option block in OpenSSH server

Nicola Muto nicola.muto at cryptolab.net
Fri May 18 00:19:36 EST 2012


Hello everybody,

I'm a C/C++ consultant working for Ericsson.

I changed the OpenSSH-Portable code to add a new criteria
into the Match sshd_config option read by the sshd server.

The new criteria is "Subsystem"; so a conditional block based
on subsystem client request can now be added to the sshd_config
configuration server file to override settings in its global
section.

Ericsson requested this new Subsystem criteria because it needs
to "chroot" all the sftp session into a well defined directory
for all users logging into the server from an sftp client.

For details on the Ericsson needs, please refer to the following
OpenSSH mailing list thread

     http://marc.info/?t=132698105300002&r=1&w=2

A little example of the Match Subsystem usage follows

   Match Subsystem sftp
       ChrootDirectory /path/to/sftp/data/jail/on/the/server
       X11Forwarding no
       AllowTcpForwarding no
       ForceCommand internal-sftp

To ensure that the Match Subsystem conditional block will work
properly, that is, the server "chroots" into the directory
specified by the ChrootDirectory directive above, you must also
disable the privilege separation using the config directive

   UsePrivilegeSeparation no

into the sshd_config file. At least I think so; I checked that
the sshd server will skip calling the safely_chroot function if I
keep the privilege separation enabled and login using a no-root user.

I worked on the portable branch of OpenSSH because this feature must be
installed on a SLES (SuSE linux Enterprise Server) distribution.

For the code changes I started from the openssh-6.0p1 released on April
22, 2012, and I followed changes recently made by Darren Tucker to add
other two criteria for the Match option; i.e. LocalAddress and 
LocalPort.
So, my changes are minimized and will be gladly accepted by the OpenSSH
community. :-)
Thank you Darren.

I do not know who should accept and confirm my code changes. Also I 
don't have
an account to check-in the changes into the OpenBSD SVC repository
Hans can you help me?

BRs
\\nm

The diff code list follows

===============================================================================
diff -r /home/qnicmut/Projects/OpenSSH-Portable/openssh-6.0p1/auth.c 
src/auth.c
===============================================================================
546a547
> 	ConnectionInfo connection_info;
548,549c549,554
< 	parse_server_match_config(&options, user,
< 	    get_canonical_hostname(options.use_dns), get_remote_ipaddr());
---
> 	connection_info.user = user;
> 	connection_info.host = get_canonical_hostname(options.use_dns);
> 	connection_info.address = get_remote_ipaddr();
> 	connection_info.subsystem = NULL;
>
> 	parse_server_match_config(&options, &connection_info);

=======================================================================================
diff -r 
/home/qnicmut/Projects/OpenSSH-Portable/openssh-6.0p1/servconf.c 
src/servconf.c
=======================================================================================
601,602c601
< match_cfg_line(char **condition, int line, const char *user, const 
char *host,
<     const char *address)
---
> match_cfg_line(char **condition, int line, ConnectionInfo *ci)
606a606,614
> 	const char *user = NULL;
> 	const char *host = NULL;
> 	const char *address = NULL;
> 	const char *subsystem = NULL;
>
> 	/* Here the condition is hold
> 	 *     ci==NULL ==> user==NULL
> 	 * that is, it is not possible that ci==NULL but user!=NULL
> 	 */
608c616
< 	if (user == NULL)
---
> 	if (ci == NULL)
610,611c618,624
< 	else
< 		debug3("checking match for '%s' user %s host %s addr %s", cp,
---
> 	else {
> 		user = ci->user;
> 		host = ci->host;
> 		address = ci->address;
> 		subsystem = ci->subsystem;
>
> 		debug3("checking match for '%s' user %s host %s addr %s subsystem 
> %s", cp,
613c626,627
< 		    address ? address : "(null)");
---
> 		    address ? address : "(null)", subsystem ? subsystem : 
> "(null)");
> 	}
622c636,639
< 			if (!user) {
---
> 			if (!user) { /* For clarity: it should be "if (!ci || !user) {" 
> but the two expressions have
> 										* exactly the same values under the above condition that
> 										* (ci==NULL) ==> (user==NULL).
> 			 	 	 	 	 	 	 	*/
631a649,652
> 			if (!user) { /* Same comment as above */
> 				result = 0;
> 				continue;
> 			}
639c660
< 			if (!host) {
---
> 			if (!host) { /* Same comment on the condition as user above */
648a670,673
> 			if (!address) { /* Same comment on the condition as user above */
> 				result = 0;
> 				continue;
> 			}
660a686,695
> 		} else if (strcasecmp(attrib, "subsystem") == 0) {
> 			if (!subsystem) {
> 				result = 0;
> 				continue;
> 			}
> 			if (match_pattern_list(subsystem, arg, len, 0) != 1)
> 				result = 0;
> 			else
> 				debug("subsystem %.100s matched 'Subsystem %.100s' at "
> 				    "line %d", subsystem, arg, line);
666c701,702
< 	if (user != NULL)
---
>
> 	if (ci != NULL)
667a704
>
713,714c750
<     const char *filename, int linenum, int *activep, const char 
*user,
<     const char *host, const char *address)
---
>     const char *filename, int linenum, int *activep, ConnectionInfo 
> *conninfo)
745c781
< 		if (user == NULL) {
---
> 		if (conninfo == NULL) {
1316c1352,1354
< 		value = match_cfg_line(&cp, linenum, user, host, address);
---
>
> 		value = match_cfg_line(&cp, linenum, conninfo);
>
1454,1455c1492
< parse_server_match_config(ServerOptions *options, const char *user,
<     const char *host, const char *address)
---
> parse_server_match_config(ServerOptions *options, ConnectionInfo 
> *conninfo)
1460c1497
< 	parse_server_config(&mo, "reprocess config", &cfg, user, host, 
address);
---
> 	parse_server_config(&mo, "reprocess config", &cfg, conninfo);
1463a1501,1536
> int
> parse_server_match_testspec(ConnectionInfo *ci, char *spec)
> {
> 	char *p;
>
> 	while ((p = strsep(&spec, ",")) && *p != '\0') {
> 		if (strncmp(p, "addr=", 5) == 0)
> 			ci->address = xstrdup(p + 5);
> 		else if (strncmp(p, "host=", 5) == 0)
> 			ci->host = xstrdup(p + 5);
> 		else if (strncmp(p, "user=", 5) == 0)
> 			ci->user = xstrdup(p + 5);
> 		else if (strncmp(p, "subsystem=", 10) == 0)
> 			ci->subsystem = xstrdup(p + 10);
> 		else {
> 			fprintf(stderr, "Invalid test mode specification %s\n", p);
> 			return -1;
> 		}
> 	}
> 	return 0;
> }
>
> /*
>  * returns 1 for a complete spec, 0 for partial spec and -1 for an
>  * empty spec.
>  */
> int
> server_match_spec_complete(ConnectionInfo *ci)
> {
> 	if (ci->user && ci->host && ci->address)
> 		return 1;	/* complete */
> 	if (!ci->user && !ci->host && !ci->address)
> 		return -1;	/* empty */
> 	return 0;	/* partial */
> }
>
1537c1610
<     const char *user, const char *host, const char *address)
---
>     ConnectionInfo *conninfo)
1545c1618,1620
< 	active = user ? 0 : 1;
---
>
> 	active = conninfo ? 0 : 1;
>
1549c1624
< 		    linenum++, &active, user, host, address) != 0)
---
> 				linenum++, &active, conninfo) != 0)

=======================================================================================
diff -r 
/home/qnicmut/Projects/OpenSSH-Portable/openssh-6.0p1/servconf.h 
src/servconf.h
=======================================================================================
170a171,178
> /* Information about the incoming connection as used by Match */
> typedef struct {
> 	const char *user;
> 	const char *host;	/* possibly resolved hostname */
> 	const char *address; 	/* remote address */
> 	const char *subsystem; /* Subsystem service requested by the remote 
> client application */
> } ConnectionInfo;
>
186a195
>
188c197,198
< 	     int *, const char *, const char *, const char *);
---
> 	     int *, ConnectionInfo *);
>
191,193c201,207
< 	     const char *, const char *, const char *);
< void	 parse_server_match_config(ServerOptions *, const char *, const 
char *,
< 	     const char *);
---
> 		    ConnectionInfo *);
>
> void	 parse_server_match_config(ServerOptions *, ConnectionInfo *);
>
> int		 parse_server_match_testspec(ConnectionInfo *, char *);
> int		 server_match_spec_complete(ConnectionInfo *);
>

=====================================================================================
diff -r /home/qnicmut/Projects/OpenSSH-Portable/openssh-6.0p1/session.c 
src/session.c
=====================================================================================
571d570
<
825a825
>
2089a2090
> 	/* Searching for subsystem into the options repository */
2091,2105c2092,2120
< 		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 */
> 		ConnectionInfo connection_info;
>
> 		connection_info.user = NULL;
> 		connection_info.host = NULL;
> 		connection_info.address = NULL;
> 		connection_info.subsystem = subsys;
>
> 		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);
>
> 		parse_server_match_config(&options, &connection_info);
>
> 		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);
2106a2122,2123
>
> 		success = do_exec(s, cmd) == 0;

===============================================================================
diff -r /home/qnicmut/Projects/OpenSSH-Portable/openssh-6.0p1/sshd.8 
src/sshd.8
===============================================================================
111c111
< 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
116a117
> .Dq addr ,
118,119c119,127
< .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

===============================================================================
diff -r /home/qnicmut/Projects/OpenSSH-Portable/openssh-6.0p1/sshd.c 
src/sshd.c
===============================================================================
1323d1322
< 	char *test_user = NULL, *test_host = NULL, *test_addr = NULL;
1325c1324
< 	char *line, *p, *cp;
---
> 	char *line;
1330a1330
> 	ConnectionInfo connection_info;
1359a1360,1362
> 	connection_info.address = connection_info.host = 
> connection_info.subsystem =
> 			connection_info.user = NULL;
>
1452,1465c1455,1456
< 			cp = optarg;
< 			while ((p = strsep(&cp, ",")) && *p != '\0') {
< 				if (strncmp(p, "addr=", 5) == 0)
< 					test_addr = xstrdup(p + 5);
< 				else if (strncmp(p, "host=", 5) == 0)
< 					test_host = xstrdup(p + 5);
< 				else if (strncmp(p, "user=", 5) == 0)
< 					test_user = xstrdup(p + 5);
< 				else {
< 					fprintf(stderr, "Invalid test "
< 					    "mode specification %s\n", p);
< 					exit(1);
< 				}
< 			}
---
> 			if (parse_server_match_testspec(&connection_info, optarg) == -1)
> 			exit(1);
1477c1468
< 			    "command-line", 0, NULL, NULL, NULL, NULL) != 0)
---
> 					"command-line", 0, NULL, NULL) != 0)
1533,1540c1524,1529
< 	if (test_flag >= 2 &&
< 	   (test_user != NULL || test_host != NULL || test_addr != NULL)
< 	    && (test_user == NULL || test_host == NULL || test_addr == 
NULL))
< 		fatal("user, host and addr are all required when testing "
< 		   "Match configs");
< 	if (test_flag < 2 && (test_user != NULL || test_host != NULL ||
< 	    test_addr != NULL))
< 		fatal("Config test connection parameter (-C) provided without "
---
> 		if ((test_flag >= 2) && 
> (server_match_spec_complete(&connection_info) == 0))
> 			fatal("user, host and addr are all required when testing "
> 					"Match configs");
>
> 		if ((test_flag < 2) && 
> (server_match_spec_complete(&connection_info) >= 0))
> 			fatal("Config test connection parameter (-C) provided without "
1551c1540
< 	    &cfg, NULL, NULL, NULL);
---
> 			&cfg, NULL);
1713,1715c1702,1703
< 		if (test_user != NULL && test_addr != NULL && test_host != NULL)
< 			parse_server_match_config(&options, test_user,
< 			    test_host, test_addr);
---
> 		if (server_match_spec_complete(&connection_info) == 1)
> 			parse_server_match_config(&options, &connection_info);
2005c1993
<  authenticated:
---
> 	authenticated:

=============================================================================================
diff -r 
/home/qnicmut/Projects/OpenSSH-Portable/openssh-6.0p1/sshd_config.5 
src/sshd_config.5
=============================================================================================
677a678
> .Cm Address ,
679c680
< .Cm Address .
---
> .Cm Subsystem .



More information about the openssh-unix-dev mailing list