New Subsystem criteria for Match option block in OpenSSH server

John Olsson M john.m.olsson at ericsson.com
Mon May 21 17:12:41 EST 2012


Hi,

> Is it a problem that scp and shell are un-chrooted?
> Or are they handled elsewhere?

I'm not sure what part you are referring to. What we need to be able to do is to place just the SFTP server in a chroot jail. But I'll give you some background so you can understand better what it is we want to do

We have a box/node/machine/server/whatever that runs SLES as Nicola wrote. This server has two SSH servers running; one on port 22 and one on port 4422. When logging in to port 22 credentials are first verified against /etc/passwd and then an LDAP server is checked if /etc/passwd failed, for port 4422 it is only /etc/passwd that is checked and SFTP subsystem is disabled.

If you just do a "normal" ssh login (without specifying any subsystem) you end up in what we call "COM CLI" which can be thought of as a text-based interface to an information model (you can navigate a tree structure, modify attributes and create objects, etc). This information model will also provide a branch showing a virtual filesystem. Thus this is a completely different animal compared to a Bash shell, it is purely a high level management interface for the server.

Now when connecting to port 22 using an SFTP client, this client may only see the same virtual filesystem that is also visible via the information model. Thus the SFTP server must run in a chrooted environment, but SSH (COM CLI) may not since the COM CLI must be able to access the complete system.

Connecting to port 4422 gives you a standard Bash shell.


Did this answer your question, or did I answer some other question which you did not ask? :)


/John

-----Original Message-----
From: Dan Kaminsky [mailto:dan at doxpara.com]
Sent: den 17 maj 2012 16:55
To: Nicola Muto
Cc: <openssh-unix-dev at mindrot.org>; Nicola Muto; John Olsson M; Hans Insulander
Subject: Re: New Subsystem criteria for Match option block in OpenSSH server

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