Oh dear, I'm just getting wised up to the fact that this is the place to
discuss pf issues -- I've been posting on openbsd.org mailing lists.

I've been working under OpenBSD 3.2, but I think the same issues go all
the way to current -- someone please correct me if I'm wrong. The
problem: ftp-proxy does not work correctly with binat. Consider a
situation where you have an interface that I'll describe as being on the
"left", and an interface that I'll describe as being on the "right". On
the right network is an ftp server. Hosts on the left network access
hosts on the right network via binat rules. Out of the box, ftp-proxy is
trying to connect to the *translated* ("left-side") address instead of
the actual address ("right-side").

Enclosed is a patch to util.c that specifically looks up binat rules and
replaces the "real server" address with its actual address. This makes
active ftp work.

When I got this working, I discovered that of all things *passive* ftp
was not working. If I am proxying passive ftp, with the client on the
"left" and the server on the "right", the client should get passed the
IP address of the interface on the *left* as the IP address to connect
to for the data port. Instead, it was getting the IP address of the
interface on the *right* -- to which it has no route. (And this is a
"right-side address", which left-side hosts are never supposed to use.)

Enclosed is a patch to ftp-proxy.c which fixes this problem. In order to
get the patch to work, you need to change your rdr rule from the usual
examples. In the man pages the rdr for ftp-proxy redirects to the IP
address for localhost. With the patch installed, this is the IP address
that gets passed back to the client, and of course that doesn't work.
But it works fine if you change your rdr rule to redirect to the IP
address of the interface facing the client.

E.g. instead of something like

rdr on $mis_if proto tcp from any to any port 21 -> 127.0.0.1 port 8081

you would have something like

rdr on $mis_if proto tcp from any to any port 21 -> $mis_ip port 8081

I believe quite strongly that ftp-proxy should honor binat rules, so I'm
hopeful that these patches can get worked into cvs; but these patches
need to be reviewed by those with more experience with the code than I
have. I will certainly defer to those with more experience if there's a
better way to do this.

A couple of caveats. The util.c patch works "unconditionally". I have
this little birdie telling me that there might be circumstances where
you wouldn't want to do this, though I can't think of what they might
be. It *does* change the behavior of ftp-proxy, so maybe it should be
governed by a command-line option. If the patch to ftp-proxy.c is
accepted, then the man pages should probably also be updated, and I
didn't do any patches for the man pages. Again: these patches were done
against OpenBSD 3.2 stable, so apologies in advance if they don't apply
cleanly against 3.3 or current.

Comments welcome!!

-------------
Patch for util.c:
*** util.c.orig Wed Jul 24 04:42:52 2002
--- util.c      Fri Sep 26 18:02:46 2003
***************
*** 77,83 ****
      struct sockaddr_in *client_sa_ptr)
  {
        struct pfioc_natlook natlook;
!       int slen, fd;
  
        slen = sizeof(*real_server_sa_ptr);
        if (getsockname(connected_fd, (struct sockaddr *)real_server_sa_ptr,
--- 77,84 ----
      struct sockaddr_in *client_sa_ptr)
  {
        struct pfioc_natlook natlook;
!       struct pfioc_binat pb;
!       int slen, fd, nr, mnr, dev_pf;
  
        slen = sizeof(*real_server_sa_ptr);
        if (getsockname(connected_fd, (struct sockaddr *)real_server_sa_ptr,
***************
*** 134,139 ****
--- 135,172 ----
        real_server_sa_ptr->sin_addr.s_addr = natlook.rdaddr.addr32[0];
        real_server_sa_ptr->sin_len = sizeof(struct sockaddr_in);
        real_server_sa_ptr->sin_family = AF_INET;
+ 
+       /*
+        * WART: If the real server is "binatted", we will have the
+        * untranslated address. Look it up (the hard way) and substitute
+        * the translated address.
+        */
+       dev_pf = open("/dev/pf", O_RDWR);
+       if (dev_pf >= 0 && ioctl(fd, DIOCGETBINATS, &pb) == 0) {
+               mnr = pb.nr;
+               for (nr = 0; nr < mnr; ++nr) {
+                       pb.nr = nr;
+                       if (ioctl(fd, DIOCGETBINAT, &pb)) {
+                               syslog(LOG_INFO,
+                                   "get binat failed (%m), connection from %s:%hu",
+                                   inet_ntoa(client_sa_ptr->sin_addr),
+                                   ntohs(client_sa_ptr->sin_port));
+                               close(fd);
+                               return(-1);
+                       }
+                       if (pb.binat.raddr.addr_dyn != NULL)
+                               continue;
+                       if (pb.binat.raddr.addr.addr32[0] ==
+                       real_server_sa_ptr->sin_addr.s_addr) {
+                               real_server_sa_ptr->sin_addr.s_addr =
+                               pb.binat.saddr.addr.addr32[0];
+                               break;
+                       }
+               }
+ 
+       }
+       close(dev_pf);
+ 
        return(0);
  }
  
--------------------
patch for ftp-proxy.c

*** ftp-proxy.c.orig    Fri Oct 10 17:39:17 2003
--- ftp-proxy.c Thu Oct 23 09:41:22 2003
***************
*** 831,837 ****
  void
  do_server_reply(struct csiob *server, struct csiob *client)
  {
!       int code, i, j, rv;
        struct in_addr *iap;
        static int continuing = 0;
        char tbuf[100], *sendbuf, *p;
--- 831,838 ----
  void
  do_server_reply(struct csiob *server, struct csiob *client)
  {
!       int code, i, j, rv, clilen;
!       struct sockaddr_in cli;
        struct in_addr *iap;
        static int continuing = 0;
        char tbuf[100], *sendbuf, *p;
***************
*** 916,922 ****
  
                new_dataconn(0);
                connection_mode = PASV_MODE;
!               iap = &(server->sa.sin_addr);
  
                debuglog(1, "we want client to use %s:%u", inet_ntoa(*iap),
                    htons(client_listen_sa.sin_port));
--- 917,935 ----
  
                new_dataconn(0);
                connection_mode = PASV_MODE;
!               /*
!                * Pass back the "real" IP address of the client socket.
!                * WARNING: If your rdr rule follows the usual examples,
!                * this will produce the IP address of localhost, which
!                * will fail at the client. The pf rule should rdr to
!                * the IP address on the interface facing the client.
!                */
!               clilen = sizeof(cli);
!               if (getsockname(0, (struct sockaddr *)&cli, &clilen) != 0) {
!                       syslog(LOG_INFO, "%s", "getsockname on stdin failed.");
!                       exit(EX_OSERR);
!               }
!               iap =  &cli.sin_addr;
  
                debuglog(1, "we want client to use %s:%u", inet_ntoa(*iap),
                    htons(client_listen_sa.sin_port));

Reply via email to