lib/45007: rcmd_af(3) and thusly rsh(1) ignore requested address family

[neitzel%marshlabs.gaertner.de@localhost]

>Number:         45007
>Category:       lib
>Synopsis:       rcmd_af(3) and thusly rsh(1) ignore requested address family
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue May 31 05:10:00 +0000 2011
>Originator:     Martin Neitzel
>Release:        5.1 and anything prior; 5-STABLE and 5.99 as of 2011-05-30
>Organization:
        Gaertner Datensysteme, Marshlabs
>Environment:
System: NetBSD nguyen.marshlabs.gaertner.de 5.1_STABLE NetBSD 5.1_STABLE 
(NGUYEN) #0: Tue May 24 22:39:32 CEST 2011 
neitzel%nguyen.marshlabs.gaertner.de@localhost:/u1/scratch/obj/sys/arch/i386/compile/NGUYEN
 i386
Architecture: any
Machine: any
>Description:
        The rsh(1) options -4 and -6 are supposed to select explicitely
        between the IPv4/IPv6 protocols but either option is effectively
        ignored.  Between a dual-stacked rsh client and server, network
        traffic will still silently flow via the system-preferred protocol,
        perhaps evading assumed crypto policies on the requested protocol
        or clashing against access restrictions on the actually used one..

        It turns out that the actual bug is in the underlying rcmd_af(3)
        library routine (part of libc).  The routine ignores its "af"
        parameter when calling out to the shell service.

        [I am torn between categories "lib" and "bin" while submitting
        this PR.  Please re-assign the category if I'm wrong with "lib".]

>How-To-Repeat:
        From some dual-stacked client,
        $ rsh -4 to.some.dualstacked.server date
        while running
        # tcpdump -tnn port 514

        You need a server name with both A and AAAA records.
        Instead of expected IPv4 traffic, you will see IPv6 traffic.

        (If you do not have the "shell" service activated: you'll
        see the SYN/RST packets using the wrong protocol.   Thusly,
        "rsh -4 www.netbsd.org date" does nicely to prove the bug
        if you just have a dual-stacked client.  Do not try to
        repeat the the bug with "localhost" as server -- rsh(1)
        will shortcut this into "/bin/sh -c ...", avoiding any
        network traffic.  A temporary, additional name for the
        v4 and v6 localhost lines in /etc/hosts would allow you
        to avoid the "localhost" shortcut and to watch the bug
        on the lo0 interface on a isolated box.)

>Fix:
        Work around on buggy systems: rcmd(1) instead of rsh(1).

        While the rcmd(1) man page doesn't mention them, it will
        handle the -4 and -6 options just fine.  (A separate PR
        will fix that documentation.)

        Real fix:

        Both rsh(1) and rcmd(1) build from the same source.
        This is the control flow:

        rsh(1) -> rcmd_af(3) -> rcmd(1)[setuid root] -> orcmd_af(3)

        rsh(1) is doing the right, turning the none/-4/-6 option
        into an AF_UNSPEC/AF_INET/AF_INET6 parameter for rcmd_af(3).
        rmcd_af(3) drops track of its "af" parameter when the target
        service is "shell" (514), however.

        The following patch (against NETBSD-5 and -current as of today,
        2011-05-30) corrects this.  The AF_whatever will be passed
        onwards and turned back into an "-4" or "-6" option to the
        rcmd(1) to be exec()ed.

        (To be frank: all this to and fro in the name of privilege
        separation doesn't make very much sense to me.  But who am
        I to tell... perhaps someone is relying on having $IN_RCMD
        pluggable, so I'm sticking with the current approach, too.)


Index: lib/libc/net/rcmd.c
===================================================================
RCS file: /cvsroot/src/lib/libc/net/rcmd.c,v
retrieving revision 1.65
diff -u -r1.65 rcmd.c
--- lib/libc/net/rcmd.c 3 Jan 2007 11:46:22 -0000       1.65
+++ lib/libc/net/rcmd.c 30 May 2011 23:54:36 -0000
@@ -77,8 +77,8 @@
 int    __ivaliduser __P((FILE *, u_int32_t, const char *, const char *));
 int    __ivaliduser_sa __P((FILE *, const struct sockaddr *, socklen_t,
            const char *, const char *));
-static int rshrcmd __P((char **, u_int32_t, const char *, const char *,
-           const char *, int *, const char *));
+static int rshrcmd __P((int, char **, u_int32_t, const char *,
+           const char *, const char *, int *, const char *));
 static int resrcmd __P((struct addrinfo *, char **, u_int32_t, const char *,
            const char *, const char *, int *));
 static int __icheckhost __P((const struct sockaddr *, socklen_t,
@@ -142,7 +142,7 @@
         */
        sp = getservbyname("shell", "tcp");
        if (sp != NULL && sp->s_port == rport)
-               error = rshrcmd(ahost, (u_int32_t)rport,
+               error = rshrcmd(af, ahost, (u_int32_t)rport,
                    locuser, remuser, cmd, fd2p, getenv("RCMD_CMD"));
        else
                error = resrcmd(res, ahost, (u_int32_t)rport,
@@ -379,7 +379,8 @@
  */
 /* ARGSUSED */
 static int
-rshrcmd(ahost, rport, locuser, remuser, cmd, fd2p, rshcmd)
+rshrcmd(af, ahost, rport, locuser, remuser, cmd, fd2p, rshcmd)
+       int     af;
        char    **ahost;
        u_int32_t       rport;
        const   char *locuser, *remuser, *cmd;
@@ -479,9 +480,26 @@
                        p = strrchr(rshcmd, '/');
                        execlp(rshcmd, p ? p + 1 : rshcmd, "-c", cmd, NULL);
                } else {
-                       p = strrchr(rshcmd, '/');
-                       execlp(rshcmd, p ? p + 1 : rshcmd, *ahost, "-l",
-                           remuser, cmd, NULL);
+                       const char *program;
+                       program = strrchr(rshcmd, '/');
+                       program = program ? program + 1 : rshcmd;
+                       switch (af) {
+                       case AF_INET:
+                               execlp(rshcmd, program, "-4", "-l", remuser,
+                                   *ahost, cmd, NULL);
+                               break;
+
+                       case AF_INET6:
+                               execlp(rshcmd, program, "-6", "-l", remuser,
+                                   *ahost, cmd, NULL);
+                               break;
+
+                       default:
+                               /* typically AF_UNSPEC, plus whatever */
+                               execlp(rshcmd, program,       "-l", remuser,
+                                   *ahost, cmd, NULL);
+                               break;
+                       }
                }
                warn("rshrcmd: exec %s", rshcmd);
                _exit(1);