Subject: Patch for passive mode in Beta12

From: [email-masked] (Pedro Melo)
Date: Tue, 04 Feb 1997 17:32:23 GMT

Hi!

I've made a patch (check the attachment) for Beta12 that some of you might
find interesting. So far it is working ok.

When the client ask for PASV, the server must bind to a port and wait the
connection from the client. With the original code, the port would randomly
selected. This patch makes the ftpd bind to a port in a certain range
(defined at compile time for now).

What's the benefit? Basically I only have to open a certain range of ports
in my firewall to allow PASV mode to work.

If you find anything wrong with this patch, please let me know.

Thanks!

Melo

PS: the patch is to the file src/ftpd.c in beta12.
PS2: to change the range of port numbers you want, change PASSIVE_BASE 
     (base port) and PASSIVE_SPAN (number of ports to open). In my patch, I

     open 500 ports from 3000 to 3499
--
************** Pedro Melo ([email-masked]) BOFH ******************
*   TELENET, Servicos de Telecomunicacoes, SA - Tel +351 1 3139190   *
* finger [email-masked] or search key servers for PGP key *
*           It's management's job to support the geniuses,           *
*                      not to tell them what to do                   *
**********************************************************************
*** ftpd.c.orig	Tue Feb  4 17:11:09 1997
--- ftpd.c	Tue Feb  4 17:09:25 1997
***************
*** 396,401 ****
--- 396,403 ----
      struct sockaddr_in *virtual_ptr;
  #endif
  
+     srandom(getpid());
+ 
  #ifdef AUX
      setcompat(COMPAT_POSIX | COMPAT_BSDSETUGID);
  #endif
***************
*** 3212,3217 ****
--- 3214,3222 ----
   * response by Jon Postel in a telephone conversation with Rick Adams on 25
   * Jan 89. */
  
+ #define PASSIVE_BASE 3000
+ #define PASSIVE_SPAN 500
+ 
  void
  #ifdef __STDC__
  passive(void)
***************
*** 3219,3225 ****
  passive()
  #endif
  {
!     int len;
      register char *p,
       *a;
  
--- 3224,3230 ----
  passive()
  #endif
  {
!     int len, bind_error;
      register char *p,
       *a;
  
***************
*** 3234,3247 ****
          return;
      }
      pasv_addr = ctrl_addr;
!     pasv_addr.sin_port = 0;
      delay_signaling(); /* we can't allow any signals while euid==0: kinch */
      (void) seteuid((uid_t) 0);		/* XXX: not needed if > 1024 */
!     if (bind(pdata, (struct sockaddr *) &pasv_addr, sizeof(pasv_addr)) < 0) {
!         (void) seteuid((uid_t) pw->pw_uid);
!         enable_signaling(); /* we can allow signals once again: kinch */
!         goto pasv_error;
      }
      (void) seteuid((uid_t) pw->pw_uid);
      enable_signaling(); /* we can allow signals once again: kinch */
      len = sizeof(pasv_addr);
--- 3239,3259 ----
          return;
      }
      pasv_addr = ctrl_addr;
! 
      delay_signaling(); /* we can't allow any signals while euid==0: kinch */
      (void) seteuid((uid_t) 0);		/* XXX: not needed if > 1024 */
!     do {
! 	pasv_addr.sin_port = PASSIVE_BASE + random() % PASSIVE_SPAN;
! 	if (log_commands) syslog(LOG_DEBUG, "PASV trying %d", pasv_addr.sin_port);
! 	bind_error = bind(pdata, (struct sockaddr *) &pasv_addr, sizeof(pasv_addr));
!     } while(errno==EADDRINUSE);
!     if (bind_error < 0) {
!        	(void) seteuid((uid_t) pw->pw_uid);
!        	enable_signaling(); /* we can allow signals once again: kinch */
!        	goto pasv_error;
      }
+     if (log_commands) syslog(LOG_DEBUG, "PASV got %d", pasv_addr.sin_port);
+ 
      (void) seteuid((uid_t) pw->pw_uid);
      enable_signaling(); /* we can allow signals once again: kinch */
      len = sizeof(pasv_addr);

---

From: "Karl O. Pinc" <[email-masked]>
Date: Tue, 4 Feb 1997 12:10:12 -0600

>If you find anything wrong with this patch, please let me know.

At first glance, sounds like a good idea.  But I'm unclear what the point
is.  If you only open 500 ports instead of 65536 (or whatever), then only
500 hackers can break into/out of your site at one time?

Karl

May the Legos (TM) always be swept from your path in the night.

---

From: [email-masked] (Pedro Melo)
Date: Tue, 04 Feb 1997 19:02:12 GMT

>500 hackers can break into/out of your site at one time?

:)

My ideia is that I have some ports above 1024 open (solaris stuff) that I
dont want to make available. And I also want to make PASV ftp work. So, I
choose a range of ports that are not being used by any service and open
those in the firewall.

The range of ports you open should be unused.

Melo

---

From: Alain Magloire <[email-masked]>
Date: Tue, 4 Feb 1997 14:16:33 -0500 (EST)

According to `Karl O. Pinc':
...
> 500 hackers can break into/out of your site at one time?

:-)
I think Karl, that the advantage is when you have a firewall.
So you can open a specific range of ports in your firewall
for FTP.

The problem that I can think of is; if the range is too small
the ports may be easily guessable. So an attacker can steal
the port before the legitimate client, since there is no
authentication for the PASV nor the PORT command.

-- 
Bonne anne'e, alain

---

From: [email-masked] (Pedro Melo)
Date: Wed, 05 Feb 1997 16:40:09 GMT

>authentication for the PASV nor the PORT command.

Not quite. If you use PARANOID, it already deals with that. He only allows
connections from the same IP address as the control sockect.

Melo

---

From: Alain Magloire <[email-masked]>
Date: Wed, 5 Feb 1997 12:21:14 -0500 (EST)

According to `Pedro Melo':

...

> connections from the same IP address as the control sockect.

To quote you .. "not quite"
1-
  PARANOID doesn't deal with IP checking
  it disallows some questionnable practice of wu-ftpd (rename, 
  site exec, overwrite etc...). Check the source code again
2-
  the new betas-xx(since *Hobbit* fixes) restrict the PORT
  command to port > 1024 and #IP to be the same as the client.
  Althought this violates the spirit of rfc959 and a some
  ftp clients. Nothing to do with PARANOID
3-
  There is no restriction on the PASV directive.
  except from your patch.

Althought checking PORT seems to stop ftp bounce attacks
It doesn't do anything about stealing ports. There is
an IETF-draft on the work about the client and the server
exchanging cookies for authentication.

-- 
Bonne anne'e, alain

---

From: [email-masked] (Pedro Melo)
Date: Wed, 05 Feb 1997 22:57:02 GMT

On Wed, 5 Feb 1997 12:21:14 -0500 (EST), Alain Magloire
<[email-masked]> wrote:

> According to someone (lost the quote)

*paf*. Yep, you're right... I though that PASV had the same checks that
PORT has... hmms.. Not that it would be difficult to had, though :).
If I ask for PASV, then only I should be allowed to bind to that port
anyway... Right? 

>  ftp clients. Nothing to do with PARANOID

hmms.. Yes, it breaks the spirit of the rfc but it should not break any
clients because they receive the port they should bind and wait there for
the ftpd to connect to them.

>  There is no restriction on the PASV directive.

Yes, but if we restrict the IP of the client to the same one used on the
command sockect, does that break anything? I think not (although I should
check the RFC again... Weekend reading) and it would be a nice (although
not perfect) tool to prevent port stealing.

>exchanging cookies for authentication.

That would be a more eficient solution, but still open to sniffing.. But I
starting to go off topic... :)

In short and to summarize: The advantage of this patch is to restrict the
ports ued in PASV by the server to a more limited range, therefore making
it easier on the firewalls at the server site. This could make it easier to
do port stealing kind of attacks. This problem is only solved by a
modification on the protocol and therefore not a valid solution rigth now
(without breaking a lot of things). A patch to make ftpd only accept a PASV
connection if they come from the same IP address as the control sockect
could help minimize the risk. I'm not sure if that breaks anything in the
protocol spec, but I'll check this weekend.

Melo