you need to change len to be sizeof(saddr) instead of sizeof(struct sockaddr) in unprivproc_pop. likewise, the memcpy in source_addresses needs to use res->ai_addrlen instead of sizeof(struct sockaddr).
with those fixes its ok by me. On 20 Dec 2013, at 9:12 pm, Florian Obser <flor...@openbsd.org> wrote: > On Fri, Dec 20, 2013 at 01:17:08PM +1000, David Gwynne wrote: >> 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 >> > > done, I also nuked this from the man page, that was a brain-fart: > +This has to be the IP to which an accompanying nat-to > +.Xr pf 4 > +rule translates outgoing packets. > > Additionally a pass out rule is generated, so it actually does work. > > ( I was wondering why the return traffic was passed and I thought it was > the nat-to rule. Turns out I had a stall pass out rule from a previous > tftp-proxy run where it created pass rules instead of rdr rules and > then crashed and thus the anchor was never cleared. Since I never > restarted the tftp client those rules still matched. ) > > Thanks, > Florian > > diff --git tftp-proxy.8 tftp-proxy.8 > index f0dc9a4..fb06ba04 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 and / 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,10 @@ 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. > +Instead of a pass in rule a rdr rule will be generated. > .It Fl d > Do not daemonize. > If this option is specified, > diff --git tftp-proxy.c tftp-proxy.c > index bcbecd7..7a85646 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,15 @@ struct proxy_child { > struct proxy_child *child = NULL; > TAILQ_HEAD(, proxy_listener) proxy_listeners; > > +struct src_addr { > + TAILQ_ENTRY(src_addr) entry; > + struct sockaddr_storage addr; > + socklen_t addrlen; > +}; > +TAILQ_HEAD(, src_addr) src_addrs; > + > +void source_addresses(const char*, int); > + > int > main(int argc, char *argv[]) > { > @@ -190,12 +199,15 @@ main(int argc, char *argv[]) > struct passwd *pw; > > char *addr = "localhost"; > + char *saddr = NULL; > char *port = "6969"; > int family = AF_UNSPEC; > > int pair[2]; > > - 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 +215,9 @@ main(int argc, char *argv[]) > case '6': > family = AF_INET6; > break; > + case 'a': > + saddr = optarg; > + break; > case 'd': > verbose = debug = 1; > break; > @@ -239,6 +254,9 @@ main(int argc, char *argv[]) > if (pw == NULL) > lerrx(1, "no %s user", NOPRIV_USER); > > + if (saddr != NULL) > + source_addresses(saddr, family); > + > switch (fork()) { > case -1: > lerr(1, "fork"); > @@ -312,6 +330,30 @@ main(int argc, char *argv[]) > return(0); > } > > +void > +source_addresses(const char* name, int family) > +{ > + struct addrinfo hints, *res, *res0; > + struct src_addr *saddr; > + int error; > + > + memset(&hints, 0, sizeof(hints)); > + hints.ai_family = family; > + hints.ai_socktype = SOCK_DGRAM; > + hints.ai_flags = AI_PASSIVE; > + error = getaddrinfo(name, "0", &hints, &res0); > + if (error) > + lerrx(1, "%s:%s: %s", name, "0", gai_strerror(error)); > + for (res = res0; res != NULL; res = res->ai_next) { > + if ((saddr = calloc(1, sizeof(struct src_addr))) == NULL) > + lerrx(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); > +} > > void > proxy_privproc(int s, struct passwd *pw) > @@ -360,7 +402,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 +450,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.ss_family == > + req.src.ss_family) { > + if (bind(rep->fd, (struct sockaddr*) > + &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 +785,13 @@ unprivproc_pop(int fd, short events, void *arg) > } cmsgbuf; > struct cmsghdr *cmsg; > struct iovec iov; > + struct sockaddr_storage saddr; > + socklen_t len; > int result; > int s; > > + len = sizeof(struct sockaddr); > + > do { > memset(&msg, 0, sizeof(msg)); > iov.iov_base = &result; > @@ -787,17 +849,34 @@ 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, (struct sockaddr*)&saddr, &len) == > -1) > + lerr(1, "%s: getsockname", __func__); > + if (add_rdr(r->id, (struct sockaddr *)&r->addrs.dst, > + (struct sockaddr*)&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 (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 (do_commit() == -1) > lerr(1, "%s: couldn't commit rules", __func__); > > > -- > I'm not entirely sure you are real.