im glad you wrote a diff rather than simply complain that nat and tftp doesnt 
work. the moving parts generally look good to me apart from the struct src_addr 
and getopt chunks.

please use sockaddr_storage instead of sockaddr in the src_addr struct.

could you split the resolution of the argument to -a out into a separate 
function and call it after the getopt loop, and pass the value of the family 
variable to it too?

you might want to use sockaddr_storage in unprivproc_pop too.

cheers,
dlg

On 20 Dec 2013, at 5:03 am, Florian Obser <[email protected]> wrote:

> tftp-proxy(8) doesn't work if there is a nat gateway in the path from
> the client to the server.
> I considered changing the location of the tftp server in our network
> but that's not possibly because of reasons. (6) of RFC 1925
> applies...
> With this tftp-proxy(8) grows another knob (-a) like ftp-proxy.
> I'm cautiously optimistic that this doesn't change current behaviour
> at all but would appriciate tests from tftp-proxy(8) users.
> 
> Comments / OKs?
> 
> diff --git tftp-proxy.8 tftp-proxy.8
> index f0dc9a4..286ca37 100644
> --- tftp-proxy.8
> +++ tftp-proxy.8
> @@ -34,6 +34,7 @@
> .Sh SYNOPSIS
> .Nm tftp-proxy
> .Op Fl 46dv
> +.Op Fl a Ar address
> .Op Fl l Ar address
> .Op Fl p Ar port
> .Op Fl w Ar transwait
> @@ -51,7 +52,7 @@ a rule with divert-reply set.
> .Pp
> The proxy inserts
> .Xr pf 4
> -pass rules using the
> +pass or rdr rules using the
> .Ar anchor
> facility to allow payload packets between the client and the server.
> Once the rules are inserted,
> @@ -76,6 +77,13 @@ to use IPv4 addresses only.
> Forces
> .Nm
> to use IPv6 addresses only.
> +.It Fl a Ar address
> +The proxy will use this as the source address for the initial request from
> +the client to the server for nat traversal.
> +This has to be the IP to which an accompanying nat-to
> +.Xr pf 4
> +rule translates outgoing packets.
> +Instead of pass rules rdr rules will be generated.
> .It Fl d
> Do not daemonize.
> If this option is specified,
> diff --git tftp-proxy.c tftp-proxy.c
> index bcbecd7..4b7588d1 100644
> --- tftp-proxy.c
> +++ tftp-proxy.c
> @@ -141,7 +141,7 @@ __dead void
> usage(void)
> {
>       extern char *__progname;
> -     fprintf(stderr, "usage: %s [-46dv] [-l address] [-p port]"
> +     fprintf(stderr, "usage: %s [-46dv] [-a address] [-l address] [-p port]"
>           " [-w transwait]\n", __progname);
>       exit(1);
> }
> @@ -179,6 +179,13 @@ struct proxy_child {
> struct proxy_child *child = NULL;
> TAILQ_HEAD(, proxy_listener) proxy_listeners;
> 
> +struct src_addr {
> +     TAILQ_ENTRY(src_addr)   entry;
> +     struct sockaddr         addr;
> +     socklen_t               addrlen;
> +};
> +TAILQ_HEAD(, src_addr) src_addrs;
> +
> int
> main(int argc, char *argv[])
> {
> @@ -187,15 +194,19 @@ main(int argc, char *argv[])
>       int c;
>       const char *errstr;
> 
> +     struct addrinfo hints, *res, *res0;
>       struct passwd *pw;
> +     struct src_addr *saddr;
> 
>       char *addr = "localhost";
>       char *port = "6969";
>       int family = AF_UNSPEC;
> 
> -     int pair[2];
> +     int pair[2], error;
> 
> -     while ((c = getopt(argc, argv, "46dvl:p:w:")) != -1) {
> +     TAILQ_INIT(&src_addrs);
> +
> +     while ((c = getopt(argc, argv, "46a:dvl:p:w:")) != -1) {
>               switch (c) {
>               case '4':
>                       family = AF_INET;
> @@ -203,6 +214,26 @@ main(int argc, char *argv[])
>               case '6':
>                       family = AF_INET6;
>                       break;
> +             case 'a':
> +                     memset(&hints, 0, sizeof(hints));
> +                     hints.ai_family = AF_UNSPEC;
> +                     hints.ai_socktype = SOCK_DGRAM;
> +                     hints.ai_flags = AI_PASSIVE;
> +                     error = getaddrinfo(optarg, "0", &hints, &res0);
> +                     if (error)
> +                             errx(1, "%s:%s: %s", optarg, "0",
> +                                 gai_strerror(error));
> +                     for (res = res0; res != NULL; res = res->ai_next) {
> +                             if ((saddr = calloc(1, sizeof(struct src_addr)))
> +                                 == NULL)
> +                                     errx(1, "calloc");
> +                             memcpy(&(saddr->addr), res->ai_addr,
> +                                 sizeof(struct sockaddr));
> +                             saddr->addrlen = res->ai_addrlen;
> +                             TAILQ_INSERT_TAIL(&src_addrs, saddr, entry);
> +                     }
> +                     freeaddrinfo(res0);
> +                     break;
>               case 'd':
>                       verbose = debug = 1;
>                       break;
> @@ -360,7 +391,8 @@ privproc_pop(int fd, short events, void *arg)
>       struct addr_pair req;
>       struct privproc *p = arg;
>       struct fd_reply *rep;
> -     int add = 0;
> +     struct src_addr *saddr;
> +     int add = 0, found = 0;
> 
>       switch (evbuffer_read(p->buf, fd, sizeof(req))) {
>       case 0:
> @@ -407,9 +439,24 @@ privproc_pop(int fd, short events, void *arg)
>                   &on, sizeof(on)) == -1)
>                       lerr(1, "privproc setsockopt(REUSEPORT)");
> 
> -             if (bind(rep->fd, (struct sockaddr *)&req.src,
> -                 req.src.ss_len) == -1)
> -                     lerr(1, "privproc bind");
> +             if (TAILQ_EMPTY(&src_addrs)) {
> +                     if (bind(rep->fd, (struct sockaddr *)&req.src,
> +                         req.src.ss_len) == -1)
> +                             lerr(1, "privproc bind");
> +             } else {
> +                     TAILQ_FOREACH(saddr, &src_addrs, entry)
> +                             if (saddr->addr.sa_family ==
> +                                 req.src.ss_family) {
> +                                     if (bind(rep->fd, &saddr->addr,
> +                                         saddr->addrlen) == -1)
> +                                             lerr(1, "privproc bind");
> +                                     found = 1;
> +                                     break;
> +                             }
> +
> +                     if (found == 0)
> +                             lerr(1, "no source address found");
> +             }
> 
>               if (TAILQ_EMPTY(&p->replies))
>                       add = 1;
> @@ -727,9 +774,13 @@ unprivproc_pop(int fd, short events, void *arg)
>       } cmsgbuf;
>       struct cmsghdr *cmsg;
>       struct iovec iov;
> +     struct sockaddr saddr;
> +     socklen_t len;
>       int result;
>       int s;
> 
> +     len = sizeof(struct sockaddr);
> +
>       do {
>               memset(&msg, 0, sizeof(msg));
>               iov.iov_base = &result;
> @@ -787,17 +838,28 @@ unprivproc_pop(int fd, short events, void *arg)
>               if (prepare_commit(r->id) == -1)
>                       lerr(1, "%s: prepare_commit", __func__);
> 
> -             if (add_filter(r->id, PF_IN, (struct sockaddr *)&r->addrs.dst,
> -                 (struct sockaddr *)&r->addrs.src,
> -                 ntohs(((struct sockaddr_in *)&r->addrs.src)->sin_port),
> -                 IPPROTO_UDP) == -1)
> -                     lerr(1, "%s: couldn't add pass in", __func__);
> -
> -             if (add_filter(r->id, PF_OUT, (struct sockaddr *)&r->addrs.dst,
> -                 (struct sockaddr *)&r->addrs.src,
> -                 ntohs(((struct sockaddr_in *)&r->addrs.src)->sin_port),
> -                 IPPROTO_UDP) == -1)
> -                     lerr(1, "%s: couldn't add pass out", __func__);
> +             if (TAILQ_EMPTY(&src_addrs)) {
> +                     if (add_filter(r->id, PF_IN, (struct sockaddr *)
> +                         &r->addrs.dst, (struct sockaddr *)&r->addrs.src,
> +                         ntohs(((struct sockaddr_in *)&r->addrs.src)
> +                         ->sin_port), IPPROTO_UDP) == -1)
> +                             lerr(1, "%s: couldn't add pass in", __func__);
> +
> +                     if (add_filter(r->id, PF_OUT, (struct sockaddr *)
> +                         &r->addrs.dst, (struct sockaddr *)&r->addrs.src,
> +                         ntohs(((struct sockaddr_in *)&r->addrs.src)
> +                         ->sin_port), IPPROTO_UDP) == -1)
> +                             lerr(1, "%s: couldn't add pass out", __func__);
> +             } else {
> +                     if (getsockname(s, &saddr, &len) == -1)
> +                             lerr(1, "%s: getsockname", __func__);
> +                     if (add_rdr(r->id, (struct sockaddr *)&r->addrs.dst,
> +                         &saddr, ntohs(((struct sockaddr_in *)&saddr)
> +                         ->sin_port), (struct sockaddr *)&r->addrs.src,
> +                         ntohs(((struct sockaddr_in *)&r->addrs.src)->
> +                         sin_port), IPPROTO_UDP ) == -1)
> +                             lerr(1, "%s: couldn't add rdr rule", __func__);
> +             }
> 
>               if (do_commit() == -1)
>                       lerr(1, "%s: couldn't commit rules", __func__);
> 
> -- 
> I'm not entirely sure you are real.
> 


Reply via email to