>Number:         30921
>Category:       pkg
>Synopsis:       tsocks doesn't work with tnftp client (patch included)
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    pkg-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Aug 05 17:19:00 +0000 2005
>Originator:     Noriyuki Soda
>Release:        NetBSD 3.0_BETA
>Organization:
	the NetBSD project
>Environment:
System: NetBSD srapc2586.sra.co.jp 3.0_BETA NetBSD 3.0_BETA (PC) #0: Fri Jul 22 20:47:54 UTC 2005 soda@srapc2586:/usr/src/sys/arch/i386/compile/PC i386
Architecture: i386
Machine: i386

>Description:
tsocks-1.8beta5 doesn't work with NetBSD's stock ftp client (in passive mode)

>How-To-Repeat:
Try the following command, and see it fails.
% tsocks ftp <hostname>

>Fix:

There are 2 serious bugs in poll() implementation and 1 portability
problem in connect() implementation.

1. poll() bug #1
  tsocks assumes that flags in struct connreq::selectevents are
  compatible with flags in struct pollfd::events, because tsocks.c
  line 551 and line 675 directly assign these struct members each other.
  but the assumption is wrong as follows:
	connreq::selectevents	pollfd::events (on Solaris, Linux, NetBSD,)
	READ   0x01		POLLIN  0x1
	WRITE  0x02		POLLOUT 0x4
	EXCEPT 0x04		POLLPRI 0x2
  also, POLLRDNORM should be treated as READ,
  POLLWRNORM and POLLWRBAND should be treated as WRITE,
  and POLLRDBAND should be treated as EXCEPT, as well.
  At least the WRITE case is really critical, because that's refered in
  line 660 of tsocks.c.

  the patch to tsocks.h in the attached file fixes this problem.

2. poll() bug #2
  if connection is established and the client is polling for writing,
  struct pollfd:revents doesn't correctly updated.

  the patch to poll() implementation in the attached file fixes this problem.

3. connect() portability problem
  on NetBSD and some other operating systems, if once connect(2) is
  called with non-blocking mode, subsequent connect(2) call to same
  descriptor doesn't return 0, rather it returns EINPROGRESS
  (Operation now in progress) or EISCONN (Socket is already connected).
  The connect() implementation in tsocks already deals with the
  EINPROGRESS case, but doesn't deal with the EISCONN case.

  the patch to connect_server() function in the attached file fixes
  this problem.
  perhaps it's better to use "#if defined(__NetBSD__)" in this case,
  but I guess this patch isn't so harmful on other OS.

P.S.
I already reported this problem to the tsocks project in sourcefoge 
as follows:
https://sourceforge.net/tracker/index.php?func=detail&aid=1252703&group_id=17338&atid=117338


Index: tsocks.h
*** tsocks.h-	Wed Mar 13 21:58:09 2002
--- tsocks.h	Sat Aug  6 01:01:52 2005
***************
*** 75,83 ****
  #define FAILED 14 
     
  /* Flags to indicate what events a socket was select()ed for */
! #define READ (1<<0)
! #define WRITE (1<<1)
! #define EXCEPT (1<<2)
  #define READWRITE (READ|WRITE)
  #define READWRITEEXCEPT (READ|WRITE|EXCEPT)
  
--- 75,83 ----
  #define FAILED 14 
     
  /* Flags to indicate what events a socket was select()ed for */
! #define READ (POLLIN|POLLRDNORM)
! #define WRITE (POLLOUT|POLLWRNORM|POLLWRBAND)
! #define EXCEPT (POLLRDBAND|POLLPRI)
  #define READWRITE (READ|WRITE)
  #define READWRITEEXCEPT (READ|WRITE|EXCEPT)
  
Index: tsocks.c
*** tsocks.c-	Fri Aug  5 22:03:02 2005
--- tsocks.c	Sat Aug  6 01:05:12 2005
***************
*** 658,664 ****
               * come around again (since we can't flag it for read, we don't know
               * if there is any data to be read and can't be bothered checking) */
              if (conn->selectevents & WRITE) {
!                setevents |= POLLOUT; 
                 nevents++;
              }
           }
--- 658,664 ----
               * come around again (since we can't flag it for read, we don't know
               * if there is any data to be read and can't be bothered checking) */
              if (conn->selectevents & WRITE) {
!                ufds[i].revents |= POLLOUT; 
                 nevents++;
              }
           }
***************
*** 852,858 ****
                      sizeof(conn->serveraddr));
  
     show_msg(MSGDEBUG, "Connect returned %d, errno is %d\n", rc, errno); 
! 	if (rc) {
        if (errno != EINPROGRESS) {
           show_msg(MSGERR, "Error %d attempting to connect to SOCKS "
                    "server (%s)\n", errno, strerror(errno));
--- 852,862 ----
                      sizeof(conn->serveraddr));
  
     show_msg(MSGDEBUG, "Connect returned %d, errno is %d\n", rc, errno); 
! 	if (rc && errno == EISCONN) {
!       rc = 0;
!       show_msg(MSGDEBUG, "Socket %d already connected to SOCKS server\n", conn->sockid);
!       conn->state = CONNECTED;
!    } else if (rc) {
        if (errno != EINPROGRESS) {
           show_msg(MSGERR, "Error %d attempting to connect to SOCKS "
                    "server (%s)\n", errno, strerror(errno));