Re: [RFC] domain name matching support for rebound(8)

2016-09-16 Thread Stuart Henderson
On 2016/09/16 11:40, Ted Unangst wrote:
> Dimitris Papastamos wrote:
> > By the way, what do you think about TCP caching support?  I could send
> > a patch to do just that.

Caching sounds complicated, DNS is a bit of a minefield to handle,
you have to cope with things like compression - not that it's all that
hard to do, but it's been responsible for various crashes and worse
bugs over the years, it doesn't really sound like something that's
part of rebound's remit.

Do you mean just persistent TCP connections? That sounds simpler and
potentially quite handy.

> It seems unnecessary. tcp proxy support is there because it's necessary, but
> not because i think it's likely to be used. i'm pretty sure i never use it,
> except when i deliberately test that it's still working.

TCP for DNS is useful, not least because it's very easy to forward over
ssh. If you're stuck on a network that forcibly redirects DNS requests
to a broken local resolver, ssh-forwarding is about the simplest way
to point at a non-broken nameserver. It can also get through certain
types of packet loss (bad wifi networks..) a lot better than UDP.

> rebound isn't meant to be a replacement for unbound. it's just a piece of libc
> that lives somewhere else.



Re: [RFC] domain name matching support for rebound(8)

2016-09-16 Thread Ted Unangst
Dimitris Papastamos wrote:
> By the way, what do you think about TCP caching support?  I could send
> a patch to do just that.

It seems unnecessary. tcp proxy support is there because it's necessary, but
not because i think it's likely to be used. i'm pretty sure i never use it,
except when i deliberately test that it's still working.

rebound isn't meant to be a replacement for unbound. it's just a piece of libc
that lives somewhere else.



Re: [RFC] domain name matching support for rebound(8)

2016-09-16 Thread Dimitris Papastamos
On Fri, Sep 16, 2016 at 09:09:44AM -0400, Ted Unangst wrote:
> Dimitris Papastamos wrote:
> > Hi everyone,
> > 
> > I've put together a patch for 6.0-stable that adds domain name
> > matching support to rebound(8).  The patch is quite rough at the
> > moment.
> > 
> > The config is as follows:
> > 
> > match "local." 10.0.0.53
> > match "." 8.8.8.8
> 
> So this is taking rebound in a rather different direction than planned. It's
> not supposed to do anything complicated. Also, the plan is to remove its
> config file entirely.
> 
> But I think I know why you want this. I have this pf.conf rule on my firewall.
> 
> pass in on cnmac1 proto { udp , tcp } from any to any port 53 rdr-to 10.1.1.1
> port 53

Yes that works for me then.

By the way, what do you think about TCP caching support?  I could send
a patch to do just that.



Re: [RFC] domain name matching support for rebound(8)

2016-09-16 Thread Ted Unangst
Dimitris Papastamos wrote:
> Hi everyone,
> 
> I've put together a patch for 6.0-stable that adds domain name
> matching support to rebound(8).  The patch is quite rough at the
> moment.
> 
> The config is as follows:
> 
>   match "local." 10.0.0.53
>   match "." 8.8.8.8

So this is taking rebound in a rather different direction than planned. It's
not supposed to do anything complicated. Also, the plan is to remove its
config file entirely.

But I think I know why you want this. I have this pf.conf rule on my firewall.

pass in on cnmac1 proto { udp , tcp } from any to any port 53 rdr-to 10.1.1.1
port 53



[RFC] domain name matching support for rebound(8)

2016-09-16 Thread Dimitris Papastamos
Hi everyone,

I've put together a patch for 6.0-stable that adds domain name
matching support to rebound(8).  The patch is quite rough at the
moment.

The config is as follows:

match "local." 10.0.0.53
match "." 8.8.8.8

Requests to foo.local. are sent over to 10.0.0.53, all other requests
go to 8.8.8.8.  In my implementation, the first match wins.

General drawbacks:

- rebound has to parse DNS requests.  I tried to keep the parsing code
  as small as possible to avoid security problems.

Drawbacks in current implementation:

- No caching for DNS requests over TCP.  I am planning to implement
  this via a unified cache that works for both UDP and TCP.
- non-blocking connect(2) support for TCP.  The original code handled
  that but I reworked it because I wanted to get it working first.

What do you think?

===
RCS file: /cvs/src/usr.sbin/rebound/rebound.c,v
retrieving revision 1.65
diff -u -p -r1.65 rebound.c
--- rebound.c   2 Jul 2016 17:09:09 -   1.65
+++ rebound.c   16 Sep 2016 12:29:39 -
@@ -37,6 +37,8 @@
 #include 
 #include 
 
+#define LEN(x) (sizeof (x) / sizeof *(x))
+
 uint16_t randomid(void);
 
 static struct timespec now;
@@ -100,6 +102,13 @@ struct request {
 };
 static TAILQ_HEAD(, request) reqfifo;
 
+struct match {
+   char pat[256];
+   struct sockaddr_storage to;
+   TAILQ_ENTRY(match) entry;
+};
+static TAILQ_HEAD(, match) matches;
+
 static int conncount;
 static int connmax;
 static uint64_t conntotal;
@@ -215,10 +224,94 @@ servfail(int ud, uint16_t id, struct soc
sendto(ud, , sizeof(pkt), 0, fromaddr, fromlen);
 }
 
+static size_t
+readn(int fd, void *buf, size_t n)
+{
+   size_t total = 0;
+   size_t r;
+
+   while (n > 0) {
+   r = read(fd, buf + total, n);
+   if (r == 0 || r == -1)
+   return -1;
+   total += r;
+   n -= r;
+   }
+   return total;
+}
+
+static size_t
+writen(int fd, void *buf, size_t n)
+{
+   size_t total = 0;
+   size_t r;
+
+   while (n > 0) {
+   r = write(fd, buf + total, n);
+   if (r == 0 || r == -1)
+   return -1;
+   total += r;
+   n -= r;
+   }
+   return total;
+}
+
+int
+parsedomain(uint8_t *buf, size_t buflen, char *host, size_t hostlen)
+{
+   uint8_t *bp = [0], *be = [buflen];
+   char *hp = [0], *he = [hostlen];
+
+   bp += sizeof(struct dnspacket);
+   if (bp >= be)
+   return -1;
+   for (;;) {
+   uint8_t len = *bp++;
+   if (len == 0)
+   break;
+   if (bp + len >= be || hp + len >= he)
+   return -1;
+   memcpy(hp, bp, len);
+   bp += len;
+   hp += len;
+   *hp++ = '.';
+   if (hp == he)
+   return -1;
+   }
+   *hp = '\0';
+   return 0;
+}
+
+int
+matchreq(uint8_t *buf, size_t buflen, struct sockaddr_storage *to)
+{
+   char host[65536];
+   struct match *match;
+
+   /* XXX: check flags/qdcount? */
+   if (parsedomain(buf, buflen, host, sizeof(host)) == -1)
+   return -1;
+   TAILQ_FOREACH(match, , entry) {
+   size_t hlen = strlen(host);
+   size_t glen = strlen(match->pat);
+   if (hlen < glen)
+   continue;
+   if (strcmp([hlen - glen], match->pat) == 0) {
+   memcpy(to, >to, sizeof(*to));
+   logmsg(LOG_DEBUG, "matched domain %s with %s",
+  host, match->pat);
+   /* first match wins */
+   return 0;
+   }
+   }
+   return -1;
+}
+
 static struct request *
-newrequest(int ud, struct sockaddr *remoteaddr)
+newrequest(int ud)
 {
-   struct sockaddr from;
+   struct sockaddr_storage remoteaddr;
+   struct sockaddr from, *to;
socklen_t fromlen;
struct request *req;
uint8_t buf[65536];
@@ -271,13 +364,17 @@ newrequest(int ud, struct sockaddr *remo
}
req->cacheent = hit;
 
-   req->s = socket(remoteaddr->sa_family, SOCK_DGRAM, 0);
+   if (matchreq(buf, r, ) == -1)
+   goto fail;
+   to = (struct sockaddr *)
+
+   req->s = socket(to->sa_family, SOCK_DGRAM, 0);
if (req->s == -1)
goto fail;
 
TAILQ_INSERT_TAIL(, req, fifo);
 
-   if (connect(req->s, remoteaddr, remoteaddr->sa_len) == -1) {
+   if (connect(req->s, to, to->sa_len) == -1) {
logmsg(LOG_NOTICE, "failed to connect (%d)", errno);
if (errno == EADDRNOTAVAIL)
servfail(ud, req->clientid, , fromlen);
@@ -335,36 +432,18 @@ sendreply(int ud, struct request *req)
 }
 
 static struct request *
-tcpphasetwo(struct