New Subsystem criteria for Match option block in OpenSSH server
Dan Kaminsky
dan at doxpara.com
Fri May 18 00:54:48 EST 2012
Is it a problem that scp and shell are un-chrooted? Or are they handled elsewhere?
Sent from my iPhone
On May 17, 2012, at 7:19 AM, Nicola Muto <nicola.muto at cryptolab.net> wrote:
> 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 .
>
> _______________________________________________
> openssh-unix-dev mailing list
> openssh-unix-dev at mindrot.org
> https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
More information about the openssh-unix-dev
mailing list