tftp-proxy(8) with nat-to

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

tftp-proxy(8) with nat-to

Florian Obser-2
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 | Threaded
Open this post in threaded view
|

Re: tftp-proxy(8) with nat-to

David Gwynne-5
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 <[hidden email]> 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 | Threaded
Open this post in threaded view
|

Re: tftp-proxy(8) with nat-to

Florian Obser-2
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.

Reply | Threaded
Open this post in threaded view
|

Re: tftp-proxy(8) with nat-to

David Gwynne-5
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 <[hidden email]> 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.