Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/libexec/ftpd * make checkportcmd the default. this breaks th...



details:   https://anonhg.NetBSD.org/src/rev/0a089aa2fe2a
branches:  trunk
changeset: 495161:0a089aa2fe2a
user:      lukem <lukem%NetBSD.org@localhost>
date:      Sun Jul 23 14:40:48 2000 +0000

description:
* make checkportcmd the default. this breaks third-party proxy ftp but
  prevents the ftp bounce attack, and we should be secure out of the
  box, not require users to tweak obscure stuff.
* allow the version string reported to clients to be changed with '-V vers'.
  if vers is empty or `-', don't report a version.
* if -r is given, permanently drop root privs
* if not a REAL user (i.e, GUEST or CHROOT), and ftpd is running on a port
  > IPPORT_RESERVED+1, permanently drop root privs
* don't bother reverting to root privs to logout of wtmp/utmp; since the
  file descriptor is already open this isn't necessary.
* fix the binding of the port for the PORT/LPRT/EPRT connection to be the
  ctrl_addr.su_port-1, not hardcoded to `20' (this was broken in the ipv6
  merge). if root privs have been dropped, and this would be a port <
  IPPORT_RESERVED, use a random port instead (which isn't RFC959 compliant
  but it doesn't appear that many clients care).
* prevent login of a new user if privs have been dropped and already logged
  in as a REAL user (existing check already stops GUEST & CHROOT users).
* move the port check stuff into a separate port_check() function, and use
  for PORT, LPRT, and EPRT checks. inspired by freebsd
* minor KNF
* minor man page cleanup

diffstat:

 libexec/ftpd/conf.c      |    6 +-
 libexec/ftpd/extern.h    |    3 +-
 libexec/ftpd/ftpcmd.y    |  207 ++++++++++++++++------------------------------
 libexec/ftpd/ftpd.8      |  128 ++++++++++++++++++++++++----
 libexec/ftpd/ftpd.c      |  139 +++++++++++++++++++++---------
 libexec/ftpd/ftpd.conf.5 |    6 +-
 libexec/ftpd/version.h   |    4 +-
 7 files changed, 288 insertions(+), 205 deletions(-)

diffs (truncated from 960 to 300 lines):

diff -r f04124faf5c9 -r 0a089aa2fe2a libexec/ftpd/conf.c
--- a/libexec/ftpd/conf.c       Sun Jul 23 14:34:14 2000 +0000
+++ b/libexec/ftpd/conf.c       Sun Jul 23 14:40:48 2000 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: conf.c,v 1.33 2000/07/17 02:30:52 lukem Exp $  */
+/*     $NetBSD: conf.c,v 1.34 2000/07/23 14:40:48 lukem Exp $  */
 
 /*-
  * Copyright (c) 1997-2000 The NetBSD Foundation, Inc.
@@ -38,7 +38,7 @@
 
 #include <sys/cdefs.h>
 #ifndef lint
-__RCSID("$NetBSD: conf.c,v 1.33 2000/07/17 02:30:52 lukem Exp $");
+__RCSID("$NetBSD: conf.c,v 1.34 2000/07/23 14:40:48 lukem Exp $");
 #endif /* not lint */
 
 #include <sys/types.h>
@@ -88,7 +88,7 @@
                free(conv);
        }
 
-       curclass.checkportcmd = 0;
+       curclass.checkportcmd = 1;
        REASSIGN(curclass.chroot, NULL);
        REASSIGN(curclass.classname, NULL);
        curclass.conversions =  NULL;
diff -r f04124faf5c9 -r 0a089aa2fe2a libexec/ftpd/extern.h
--- a/libexec/ftpd/extern.h     Sun Jul 23 14:34:14 2000 +0000
+++ b/libexec/ftpd/extern.h     Sun Jul 23 14:40:48 2000 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: extern.h,v 1.30 2000/07/17 02:30:53 lukem Exp $        */
+/*     $NetBSD: extern.h,v 1.31 2000/07/23 14:40:48 lukem Exp $        */
 
 /*-
  * Copyright (c) 1992, 1993
@@ -259,6 +259,7 @@
 GLOBAL sig_atomic_t    transflag;
 GLOBAL int             type;
 GLOBAL int             usedefault;             /* for data transfers */
+GLOBAL const char     *version;
 
                                                /* total file data bytes */
 GLOBAL off_t           total_data_in,  total_data_out,  total_data;
diff -r f04124faf5c9 -r 0a089aa2fe2a libexec/ftpd/ftpcmd.y
--- a/libexec/ftpd/ftpcmd.y     Sun Jul 23 14:34:14 2000 +0000
+++ b/libexec/ftpd/ftpcmd.y     Sun Jul 23 14:40:48 2000 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ftpcmd.y,v 1.51 2000/07/17 02:30:53 lukem Exp $        */
+/*     $NetBSD: ftpcmd.y,v 1.52 2000/07/23 14:40:48 lukem Exp $        */
 
 /*-
  * Copyright (c) 1997-2000 The NetBSD Foundation, Inc.
@@ -83,7 +83,7 @@
 #if 0
 static char sccsid[] = "@(#)ftpcmd.y   8.3 (Berkeley) 4/6/94";
 #else
-__RCSID("$NetBSD: ftpcmd.y,v 1.51 2000/07/17 02:30:53 lukem Exp $");
+__RCSID("$NetBSD: ftpcmd.y,v 1.52 2000/07/23 14:40:48 lukem Exp $");
 #endif
 #endif /* not lint */
 
@@ -254,91 +254,20 @@
 
        | PORT check_login SP host_port CRLF
                {
-                       if ($2) {
-                                       /* be paranoid, if told so */
-                       if (curclass.checkportcmd &&
-                           ((ntohs(data_dest.su_port) < IPPORT_RESERVED) ||
-                           memcmp(&data_dest.su_sin.sin_addr,
-                           &his_addr.su_sin.sin_addr,
-                           sizeof(data_dest.su_sin.sin_addr)) != 0)) {
-                               reply(500,
-                                   "Illegal PORT command rejected");
-                       } else if (epsvall) {
-                               reply(501, "PORT disallowed after EPSV ALL");
-                       } else {
-                               usedefault = 0;
-                               if (pdata >= 0) {
-                                       (void) close(pdata);
-                                       pdata = -1;
-                               }
-                               reply(200, "PORT command successful.");
-                       }
-
-                       }
+                       if ($2)
+                               port_check("PORT", AF_INET);
                }
 
        | LPRT check_login SP host_long_port4 CRLF
                {
-                       if ($2) {
-
-                       /* reject invalid host_long_port4 */
-                       if (data_dest.su_family != AF_INET) {
-                               reply(500, "Illegal LPRT command rejected");
-                               return (NULL);
-                       }
-                       /* be paranoid, if told so */
-                       if (curclass.checkportcmd &&
-                           ((ntohs(data_dest.su_port) < IPPORT_RESERVED) ||
-                            memcmp(&data_dest.su_sin.sin_addr,
-                                   &his_addr.su_sin.sin_addr,
-                            sizeof(data_dest.su_sin.sin_addr)) != 0)) {
-                               reply(500, "Illegal LPRT command rejected");
-                               return (NULL);
-                       }
-                       if (epsvall)
-                               reply(501, "LPRT disallowed after EPSV ALL");
-                       else {
-                               usedefault = 0;
-                               if (pdata >= 0) {
-                                       (void) close(pdata);
-                                       pdata = -1;
-                               }
-                               reply(200, "LPRT command successful.");
-                       }
-
-                       }
+                       if ($2)
+                               port_check("LPRT", AF_INET);
                }
 
        | LPRT check_login SP host_long_port6 CRLF
                {
-                       if ($2) {
-
-                       /* reject invalid host_long_port6 */
-                       if (data_dest.su_family != AF_INET6) {
-                               reply(500, "Illegal LPRT command rejected");
-                               return (NULL);
-                       }
-                       /* be paranoid, if told so */
-                       if (curclass.checkportcmd &&
-                           ((ntohs(data_dest.su_port) < IPPORT_RESERVED) ||
-                            memcmp(&data_dest.su_sin6.sin6_addr,
-                                   &his_addr.su_sin6.sin6_addr,
-                            sizeof(data_dest.su_sin6.sin6_addr)) != 0)) {
-                               reply(500, "Illegal LPRT command rejected");
-                               return (NULL);
-                       }
-                       if (epsvall)
-                               reply(501, "LPRT disallowed after EPSV ALL");
-                       else {
-                               usedefault = 0;
-                               if (pdata >= 0) {
-                                       (void) close(pdata);
-                                       pdata = -1;
-                               }
-                               reply(200, "LPRT command successful.");
-                       }
-
-                       }
+                       if ($2)
+                               port_check("LPRT", AF_INET6);
                }
 
        | EPRT check_login SP STRING CRLF
@@ -353,16 +282,6 @@
 
                        if ($2) {
 
-                       if (epsvall) {
-                               reply(501, "EPRT disallowed after EPSV ALL");
-                               goto eprt_done;
-                       }
-                       usedefault = 0;
-                       if (pdata >= 0) {
-                               (void) close(pdata);
-                               pdata = -1;
-                       }
-
                        tmp = xstrdup($4);
                        p = tmp;
                        delim = p[0];
@@ -371,7 +290,7 @@
                        for (i = 0; i < 3; i++) {
                                q = strchr(p, delim);
                                if (!q || *q != delim) {
-               parsefail:
+ parsefail:
                                        reply(500,
                                            "Invalid argument, rejected.");
                                        usedefault = 1;
@@ -402,55 +321,19 @@
                        if (atoi(result[0]) == 2)
                                hints.ai_family = PF_INET6;
                        else
-                               hints.ai_family = PF_UNSPEC;    /*XXX*/
+                               hints.ai_family = PF_UNSPEC;    /* XXX */
                        hints.ai_socktype = SOCK_STREAM;
                        if (getaddrinfo(result[1], result[2], &hints, &res))
                                goto parsefail;
                        memcpy(&data_dest, res->ai_addr, res->ai_addrlen);
-                       if (his_addr.su_family == AF_INET6
-                        && data_dest.su_family == AF_INET6) {
+                       if (his_addr.su_family == AF_INET6 &&
+                           data_dest.su_family == AF_INET6) {
                                /* XXX more sanity checks! */
                                data_dest.su_sin6.sin6_scope_id =
                                        his_addr.su_sin6.sin6_scope_id;
                        }
-                       /* be paranoid, if told so */
-                       if (curclass.checkportcmd) {
-                               int fail;
-                               fail = 0;
-                               if (ntohs(data_dest.su_port) < IPPORT_RESERVED)
-                                       fail++;
-                               if (data_dest.su_family != his_addr.su_family)
-                                       fail++;
-                               if (data_dest.su_len != his_addr.su_len)
-                                       fail++;
-                               switch (data_dest.su_family) {
-                               case AF_INET:
-                                       fail += memcmp(
-                                           &data_dest.su_sin.sin_addr,
-                                           &his_addr.su_sin.sin_addr,
-                                           sizeof(data_dest.su_sin.sin_addr));
-                                       break;
-                               case AF_INET6:
-                                       fail += memcmp(
-                                           &data_dest.su_sin6.sin6_addr,
-                                           &his_addr.su_sin6.sin6_addr,
-                                           sizeof(data_dest.su_sin6.sin6_addr));
-                                       break;
-                               default:
-                                       fail++;
-                               }
-                               if (fail) {
-                                       reply(500,
-                                           "Illegal EPRT command rejected");
-                                       return (NULL);
-                               }
-                       }
-                       if (pdata >= 0) {
-                               (void) close(pdata);
-                               pdata = -1;
-                       }
-                       reply(200, "EPRT command successful.");
-               eprt_done:;
+                       port_check("EPRT", -1);
+ eprt_done:
                        if (tmp != NULL)
                                free(tmp);
 
@@ -855,8 +738,11 @@
 
        | SYST CRLF
                {
-                       reply(215, "UNIX Type: L%d Version: %s", NBBY,
-                           FTPD_VERSION);
+                       if (EMPTYSTR(version))
+                               reply(215, "UNIX Type: L%d", NBBY);
+                       else
+                               reply(215, "UNIX Type: L%d Version: %s", NBBY,
+                                   version);
                }
 
        | STAT check_login SP pathname CRLF
@@ -1478,6 +1364,7 @@
 };
 
 static void            help(struct tab *, const char *);
+static void            port_check(const char *, int);
 static void            toolong(int);
 static int             yylex(void);
 
@@ -1911,3 +1798,57 @@
                reply(214, "%s%-*s\t%s; not implemented.", type, width,
                    c->name, c->help);
 }
+
+/*
+ * Check that the structures used for a PORT, LPRT or EPRT command are
+ * valid (data_dest, his_addr), and if necessary, detect ftp bounce attacks.
+ * If family != -1 check that his_addr.su_family == family.
+ */
+static void
+port_check(const char *cmd, int family)
+{
+
+       if (epsvall) {
+               reply(501, "%s disallowed after EPSV ALL", cmd);
+               return;
+       }
+
+       if (family != -1 && his_addr.su_family != family) {
+ port_check_fail:
+               reply(500, "Illegal %s command rejected", cmd);
+               return;
+       }
+
+       if (data_dest.su_family != his_addr.su_family)
+               goto port_check_fail;
+
+                       /* be paranoid, if told so */
+       if (curclass.checkportcmd) {
+               if ((ntohs(data_dest.su_port) < IPPORT_RESERVED) ||



Home | Main Index | Thread Index | Old Index