Updated to v2.0.3pre2

AUDIT

@@ -13,7 +13,7 @@ filestr.c                    3
 ftpcmdio.c                   3
 ftpdataio.c                  2
 hash.c                       1
-ipv6parse.c                  1
+ipaddrparse.c                1
 logging.c                    3
 ls.c                         2
 main.c                       3
@@ -28,6 +28,7 @@ privparent.c                 3
 privsock.c                   3
 secbuf.c                     3
 secutil.c                    3
+ssl.c                        1
 standalone.c                 1
 str.c                        2
 strlist.c                    2

Changelog

@@ -855,6 +855,25 @@ separate files for the certificates and private keys.
 Better fix (which reports timeout to client properly) to follow.
 - Add which setsockopt option failed to die("setsockopt") calls.
 - Fix when running on recent OpenBSDs - OpenBSD change broke vsftpd. Lower
-linger timeout from INT_MAX to 32767 (SHORT_MAX).
+linger timeout from INT_MAX to 32767 (SHORT_MAX). Reported by
+Ewoud van der Vliet <e.c.vandervliet@student.utwente.nl> and Ed Vazquez
+<ed.vazquez@dhha.org>.
 (v2.0.3pre1)
+- Fix error with IPv4 connections to IPv6 listeners and PORT type data
+connections when connect_from_port_20 is set. RedHat bugzilla 134541. Reported
+by Joe Orton <jorton@redhat.com>, Radek Vokal <rvokal@redhat.com> and
+Andreas Kupfer <kupfer@42h.de>.
+- Remove vsf_sysutil_sockaddr_same_family (unused).
+- Support protocol 1 (IPv4) in EPRT.
+- Add ssl.c to AUDIT.
+- Allow config file to use "ssl_ciphers=" to use default OpenSSL cipher list.
+- Allow "EPSV 1" to mean IPv4 EPSV.
+- Report dummy IP but correct port with IPv6 / PASV.
+- Handle SSL_WANT_READ and SSL_WANT_WRITE retries in SSL_read and SSL_write;
+fixes SSL upload failures when data timeouts are in use with some clients.
+Specifically, I used the test case FileZilla 2.2.12a on Windows XP. Reported
+by Lee Lawrence <leel@aspin.co.uk> (using CuteFTP and BackupEdge) and
+Christian DELAIR <christian@cognition.fr> (using lftp, FileZilla and
+SmartFTP). Thanks to these two people for valuable help.
+(v2.0.3pre2)
 

Makefile

@@ -13,7 +13,7 @@ OBJS	=	main.o utility.o prelogin.o ftpcmdio.o postlogin.o privsock.o \
 		postprivparent.o logging.o str.o netstr.o sysstr.o strlist.o \
     banner.o filestr.o parseconf.o secutil.o \
     ascii.o oneprocess.o twoprocess.o privops.o standalone.o hash.o \
-    tcpwrap.o ipv6parse.o access.o features.o readwrite.o \
+    tcpwrap.o ipaddrparse.o access.o features.o readwrite.o \
     ssl.o sysutil.o sysdeputil.o
 
 

TODO

@@ -4,10 +4,9 @@ CRITICAL
 - Improve FAQ, docs (ongoing..)
 - Integrated test suite (I'm so lazy..)
 - Sweedish, Russian etc. characters showing as ? in the log - many complaints.
-
-- SIGALRM at prelogin stage with SSL not safe w.r.t. re-entrancy.
+- SIGALRM at prelogin stage with SSL not safe w.r.t. re-entrancy?
 - Check re-entrancy of usage of vsf_sysutil_exit_func.
-- And session timeout too :( (not working with SSL).
+- Fix SSL session timeout more gracefully.
 - Send until EOF is really reached (not the case with sendfile mode).
 
 NOT SO CRITICAL

ftpdataio.c

@@ -130,17 +130,14 @@ vsf_ftpdataio_get_port_fd(struct vsf_session* p_sess)
   }
   else
   {
-    remote_fd = vsf_sysutil_get_ipsock(p_sess->p_port_sockaddr);
-    if (vsf_sysutil_sockaddr_same_family(p_sess->p_port_sockaddr,
-                                         p_sess->p_local_addr))
-    {
-      static struct vsf_sysutil_sockaddr* s_p_addr;
-      vsf_sysutil_sockaddr_clone(&s_p_addr, p_sess->p_local_addr);
-      retval = vsf_sysutil_bind(remote_fd, s_p_addr);
-      if (retval != 0)
-      {
-        die("vsf_sysutil_bind");
-      }
+    static struct vsf_sysutil_sockaddr* s_p_addr;
+    remote_fd = vsf_sysutil_get_ipsock(p_sess->p_local_addr);
+    vsf_sysutil_sockaddr_clone(&s_p_addr, p_sess->p_local_addr);
+    vsf_sysutil_sockaddr_set_port(s_p_addr, 0);
+    retval = vsf_sysutil_bind(remote_fd, s_p_addr);
+    if (retval != 0)
+    { 
+      die("vsf_sysutil_bind");
     }
   }
   retval = vsf_sysutil_connect_timeout(remote_fd, p_sess->p_port_sockaddr,

ipaddrparse.c

@@ -2,13 +2,13 @@
  * Part of Very Secure FTPd
  * Licence: GPL v2
  * Author: Chris Evans
- * ipv6parse.c
+ * ipaddrparse.c
  *
- * A routine to parse ipv6 addresses. I'm paranoid and don't want to use
+ * A routine to parse ip addresses. I'm paranoid and don't want to use
  * inet_pton.
  */
 
-#include "ipv6parse.h"
+#include "ipaddrparse.h"
 #include "sysutil.h"
 #include "str.h"
 
@@ -58,6 +58,48 @@ vsf_sysutil_parse_ipv6(const struct mystr* p_str)
   return str_getbuf(&s_ret);
 }
 
+const unsigned char*
+vsf_sysutil_parse_ipv4(const struct mystr* p_str)
+{
+  static unsigned char items[4];
+  return vsf_sysutil_parse_uchar_string_sep(p_str, '.', items, sizeof(items));
+}
+
+const unsigned char*
+vsf_sysutil_parse_uchar_string_sep(
+  const struct mystr* p_str, char sep, unsigned char* p_items,
+  unsigned int items)
+{
+  static struct mystr s_tmp_str;
+  unsigned int i;
+  str_copy(&s_tmp_str, p_str);
+  for (i=0; i<items; i++)
+  {
+    static struct mystr s_rhs_sep_str;
+    int this_number;
+    /* This puts a single separator delimited field in tmp_str */
+    str_split_char(&s_tmp_str, &s_rhs_sep_str, sep);
+    /* Sanity - check for too many or two few dots! */
+    if ( (i < (items-1) && str_isempty(&s_rhs_sep_str)) ||
+         (i == (items-1) && !str_isempty(&s_rhs_sep_str)))
+    {
+      return 0;
+    }
+    this_number = str_atoi(&s_tmp_str);
+    if (this_number < 0 || this_number > 255)
+    {
+      return 0;
+    }
+    /* If this truncates from int to uchar, we don't care */
+    p_items[i] = (unsigned char) this_number;
+    /* The right hand side of the comma now becomes the new string to
+     * breakdown
+     */
+    str_copy(&s_tmp_str, &s_rhs_sep_str);
+  }
+  return p_items;
+}
+
 static int
 ipv6_parse_main(struct mystr* p_out_str, const struct mystr* p_in_str)
 {

ipaddrparse.h

@@ -0,0 +1,20 @@
+#ifndef VSF_IPADDRPARSE_H
+#define VSF_IPADDRPARSE_H
+
+struct mystr;
+
+/* Effectively doing the same sort of job as inet_pton. Since inet_pton does
+ * a non-trivial amount of parsing, we'll do it ourselves for maximum security
+ * and safety.
+ */
+
+const unsigned char* vsf_sysutil_parse_ipv6(const struct mystr* p_str);
+
+const unsigned char* vsf_sysutil_parse_ipv4(const struct mystr* p_str);
+
+const unsigned char* vsf_sysutil_parse_uchar_string_sep(
+  const struct mystr* p_str, char sep, unsigned char* p_items,
+  unsigned int items);
+
+#endif /* VSF_IPADDRPARSE_H */
+

ipv6parse.h

@@ -1,14 +0,0 @@
-#ifndef VSF_IPV6PARSE_H
-#define VSF_IPV6PARSE_H
-
-struct mystr;
-
-/* Effectively doing the same sort of job as inet_pton. Since inet_pton does
- * a non-trivial amount of parsing, we'll do it ourselves for maximum security
- * and safety.
- */
-
-const unsigned char* vsf_sysutil_parse_ipv6(const struct mystr* p_str);
-
-#endif /* VSF_IPV6PARSE_H */
-

postlogin.c

@@ -21,7 +21,7 @@
 #include "sysutil.h"
 #include "logging.h"
 #include "sysdeputil.h"
-#include "ipv6parse.h"
+#include "ipaddrparse.h"
 #include "access.h"
 #include "features.h"
 #include "ssl.h"
@@ -511,7 +511,7 @@ handle_pasv(struct vsf_session* p_sess, int is_epsv)
       return;
     }
     argval = vsf_sysutil_atoi(str_getbuf(&p_sess->ftp_arg_str));
-    if (!is_ipv6 || argval != 2)
+    if (argval < 1 || argval > 2 || (!is_ipv6 && argval == 2))
     {
       vsf_cmdio_write(p_sess, FTP_EPSVBAD, "Bad network protocol.");
       return;
@@ -519,7 +519,7 @@ handle_pasv(struct vsf_session* p_sess, int is_epsv)
   }
   pasv_cleanup(p_sess);
   port_cleanup(p_sess);
-  if (is_epsv && is_ipv6)
+  if (is_ipv6)
   {
     p_sess->pasv_listen_fd = vsf_sysutil_get_ipv6_sock();
   }
@@ -596,6 +596,10 @@ handle_pasv(struct vsf_session* p_sess, int is_epsv)
     {
       str_append_text(&s_pasv_res_str, vsf_sysutil_inet_ntoa(p_v4addr));
     }
+    else
+    {
+      str_append_text(&s_pasv_res_str, "0,0,0,0");
+    }
   }
   str_replace_char(&s_pasv_res_str, '.', ',');
   str_append_text(&s_pasv_res_str, ",");
@@ -888,42 +892,21 @@ handle_type(struct vsf_session* p_sess)
 static void
 handle_port(struct vsf_session* p_sess)
 {
-  static struct mystr s_tmp_str;
   unsigned short the_port;
   unsigned char vals[6];
-  int i;
+  const unsigned char* p_raw;
   pasv_cleanup(p_sess);
   port_cleanup(p_sess);
-  str_copy(&s_tmp_str, &p_sess->ftp_arg_str);
-  for (i=0; i<6; i++)
+  p_raw = vsf_sysutil_parse_uchar_string_sep(&p_sess->ftp_arg_str, ',', vals,
+                                             sizeof(vals));
+  if (p_raw == 0)
   {
-    static struct mystr s_rhs_comma_str;
-    int this_number;
-    /* This puts a single , delimited field in tmp_str */
-    str_split_char(&s_tmp_str, &s_rhs_comma_str, ',');
-    /* Sanity - check for too many or two few commas! */
-    if ( (i<5 && str_isempty(&s_rhs_comma_str)) ||
-         (i==5 && !str_isempty(&s_rhs_comma_str)))
-    {
-      vsf_cmdio_write(p_sess, FTP_BADCMD, "Illegal PORT command.");
-      return;
-    }
-    this_number = str_atoi(&s_tmp_str);
-    if (this_number < 0 || this_number > 255)
-    {
-      vsf_cmdio_write(p_sess, FTP_BADCMD, "Illegal PORT command.");
-      return;
-    }
-    /* If this truncates from int to uchar, we don't care */
-    vals[i] = (unsigned char) this_number;
-    /* The right hand side of the comma now becomes the new string to
-     * breakdown
-     */
-    str_copy(&s_tmp_str, &s_rhs_comma_str);
+    vsf_cmdio_write(p_sess, FTP_BADCMD, "Illegal PORT command.");
+    return;
   }
   the_port = vals[4] << 8;
   the_port |= vals[5];
-  vsf_sysutil_sockaddr_alloc_ipv4(&p_sess->p_port_sockaddr);
+  vsf_sysutil_sockaddr_clone(&p_sess->p_port_sockaddr, p_sess->p_local_addr);
   vsf_sysutil_sockaddr_set_ipv4addr(p_sess->p_port_sockaddr, vals);
   vsf_sysutil_sockaddr_set_port(p_sess->p_port_sockaddr, the_port);
   /* SECURITY:
@@ -1312,6 +1295,10 @@ handle_sigurg(void* p_private)
   if (str_equal_text(&real_cmd_str, "ABOR"))
   {
     p_sess->abor_received = 1;
+    /* This is failok because of a small race condition; the SIGURG might
+     * be raised after the data socket is closed, but before data_fd is
+     * set to -1.
+     */
     vsf_sysutil_shutdown_failok(p_sess->data_fd);
   }
   else
@@ -1586,14 +1573,21 @@ handle_eprt(struct vsf_session* p_sess)
   /* Split out the protocol and check it */
   str_split_char(&s_part2_str, &s_part1_str, '|');
   proto = str_atoi(&s_part2_str);
-  if (!is_ipv6 || proto != 2)
+  if (proto < 1 || proto > 2 || (!is_ipv6 && proto == 2))
   {
     vsf_cmdio_write(p_sess, FTP_BADCMD, "Bad EPRT protocol.");
     return;
   }
   /* Split out address and parse it */
   str_split_char(&s_part1_str, &s_part2_str, '|');
-  p_raw_addr = vsf_sysutil_parse_ipv6(&s_part1_str);
+  if (proto == 2)
+  {
+    p_raw_addr = vsf_sysutil_parse_ipv6(&s_part1_str);
+  }
+  else
+  {
+    p_raw_addr = vsf_sysutil_parse_ipv4(&s_part1_str);
+  }
   if (!p_raw_addr)
   {
     goto bad_eprt;
@@ -1609,9 +1603,16 @@ handle_eprt(struct vsf_session* p_sess)
   {
     goto bad_eprt;
   }
-  vsf_sysutil_sockaddr_alloc_ipv6(&p_sess->p_port_sockaddr);
-  vsf_sysutil_sockaddr_set_ipv6addr(p_sess->p_port_sockaddr, p_raw_addr);
-  vsf_sysutil_sockaddr_set_port(p_sess->p_port_sockaddr, (unsigned short)port);
+  vsf_sysutil_sockaddr_clone(&p_sess->p_port_sockaddr, p_sess->p_local_addr);
+  if (proto == 2)
+  {
+    vsf_sysutil_sockaddr_set_ipv6addr(p_sess->p_port_sockaddr, p_raw_addr);
+  }
+  else
+  {
+    vsf_sysutil_sockaddr_set_ipv4addr(p_sess->p_port_sockaddr, p_raw_addr);
+  }
+  vsf_sysutil_sockaddr_set_port(p_sess->p_port_sockaddr, (unsigned short) port);
   /* SECURITY:
    * 1) Reject requests not connecting to the control socket IP
    * 2) Reject connects to privileged ports

ssl.c

@@ -97,7 +97,8 @@ ssl_init(struct vsf_session* p_sess)
         die("SSL: cannot load DSA private key");
       }
     }
-    if (SSL_CTX_set_cipher_list(p_ctx, tunable_ssl_ciphers) != 1)
+    if (tunable_ssl_ciphers &&
+        SSL_CTX_set_cipher_list(p_ctx, tunable_ssl_ciphers) != 1)
     {
       die("SSL: could not set cipher list");
     }
@@ -211,18 +212,34 @@ ssl_getline(const struct vsf_session* p_sess, struct mystr* p_str,
 int
 ssl_read(void* p_ssl, char* p_buf, unsigned int len)
 {
-  int retval = SSL_read((SSL*) p_ssl, p_buf, len);
+  int retval;
+  int err;
   int fd = SSL_get_fd((SSL*) p_ssl);
-  vsf_sysutil_check_pending_actions(kVSFSysUtilIO, retval, fd);
+  do
+  {
+    retval = SSL_read((SSL*) p_ssl, p_buf, len);
+    err = SSL_get_error((SSL*) p_ssl, retval);
+    vsf_sysutil_check_pending_actions(kVSFSysUtilIO, retval, fd);
+  }
+  while (retval < 0 && (err == SSL_ERROR_WANT_READ ||
+                        err == SSL_ERROR_WANT_WRITE));
   return retval;
 }
 
 int
 ssl_write(void* p_ssl, const char* p_buf, unsigned int len)
 {
-  int retval = SSL_write((SSL*) p_ssl, p_buf, len);
+  int retval;
+  int err;
   int fd = SSL_get_fd((SSL*) p_ssl);
-  vsf_sysutil_check_pending_actions(kVSFSysUtilIO, retval, fd);
+  do
+  {
+    retval = SSL_write((SSL*) p_ssl, p_buf, len);
+    err = SSL_get_error((SSL*) p_ssl, retval);
+    vsf_sysutil_check_pending_actions(kVSFSysUtilIO, retval, fd);
+  }
+  while (retval < 0 && (err == SSL_ERROR_WANT_READ ||
+                        err == SSL_ERROR_WANT_WRITE));
   return retval;
 }
 

standalone.c

@@ -17,7 +17,7 @@
 #include "defs.h"
 #include "hash.h"
 #include "str.h"
-#include "ipv6parse.h"
+#include "ipaddrparse.h"
 
 static unsigned int s_children;
 static struct hash* s_p_ip_count_hash;

sysutil.c

@@ -1920,17 +1920,6 @@ vsf_sysutil_sockaddr_is_ipv6(const struct vsf_sysutil_sockaddr* p_sockaddr)
   return 0;
 }
 
-int
-vsf_sysutil_sockaddr_same_family(const struct vsf_sysutil_sockaddr* p1,
-                                 const struct vsf_sysutil_sockaddr* p2)
-{
-  if (p1->u.u_sockaddr.sa_family == p2->u.u_sockaddr.sa_family)
-  {
-    return 1;
-  }
-  return 0;
-}
-
 void
 vsf_sysutil_sockaddr_set_ipv4addr(struct vsf_sysutil_sockaddr* p_sockptr,
                                   const unsigned char* p_raw)
@@ -1940,6 +1929,16 @@ vsf_sysutil_sockaddr_set_ipv4addr(struct vsf_sysutil_sockaddr* p_sockptr,
     vsf_sysutil_memcpy(&p_sockptr->u.u_sockaddr_in.sin_addr, p_raw,
                        sizeof(p_sockptr->u.u_sockaddr_in.sin_addr));
   }
+  else if (p_sockptr->u.u_sockaddr.sa_family == AF_INET6)
+  {
+    static struct vsf_sysutil_sockaddr* s_p_sockaddr;
+    vsf_sysutil_sockaddr_alloc_ipv4(&s_p_sockaddr);
+    vsf_sysutil_memcpy(&s_p_sockaddr->u.u_sockaddr_in.sin_addr, p_raw,
+                       sizeof(&s_p_sockaddr->u.u_sockaddr_in.sin_addr));
+    vsf_sysutil_memcpy(&p_sockptr->u.u_sockaddr_in6.sin6_addr,
+                       vsf_sysutil_sockaddr_ipv4_v6(s_p_sockaddr),
+                       sizeof(p_sockptr->u.u_sockaddr_in6.sin6_addr));
+  }
   else
   {
     bug("bad family");
@@ -1978,6 +1977,18 @@ vsf_sysutil_sockaddr_ipv6_v4(const struct vsf_sysutil_sockaddr* p_addr)
   return &p_addr_start[12];
 }
 
+const void*
+vsf_sysutil_sockaddr_ipv4_v6(const struct vsf_sysutil_sockaddr* p_addr)
+{
+  static char ret[16] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xFF, 0xFF };
+  if (p_addr->u.u_sockaddr.sa_family != AF_INET)
+  {
+    return 0;
+  }
+  vsf_sysutil_memcpy(&ret[12], &p_addr->u.u_sockaddr_in.sin_addr, 4);
+  return ret;
+}
+
 void*
 vsf_sysutil_sockaddr_get_raw_addr(struct vsf_sysutil_sockaddr* p_sockptr)
 {

sysutil.h

@@ -215,9 +215,6 @@ int vsf_sysutil_sockaddr_addr_equal(const struct vsf_sysutil_sockaddr* p1,
                                     const struct vsf_sysutil_sockaddr* p2);
 int vsf_sysutil_sockaddr_is_ipv6(
   const struct vsf_sysutil_sockaddr* p_sockaddr);
-int vsf_sysutil_sockaddr_same_family(
-  const struct vsf_sysutil_sockaddr* p1,
-  const struct vsf_sysutil_sockaddr* p2);
 void vsf_sysutil_sockaddr_set_ipv4addr(struct vsf_sysutil_sockaddr* p_sockptr,
                                        const unsigned char* p_raw);
 void vsf_sysutil_sockaddr_set_ipv6addr(struct vsf_sysutil_sockaddr* p_sockptr,
@@ -232,6 +229,8 @@ void* vsf_sysutil_sockaddr_get_raw_addr(
   struct vsf_sysutil_sockaddr* p_sockaddr);
 const void* vsf_sysutil_sockaddr_ipv6_v4(
   const struct vsf_sysutil_sockaddr* p_sockaddr);
+const void* vsf_sysutil_sockaddr_ipv4_v6(
+  const struct vsf_sysutil_sockaddr* p_sockaddr);
 int vsf_sysutil_get_ipv4_sock(void);
 int vsf_sysutil_get_ipv6_sock(void);
 struct vsf_sysutil_socketpair_retval