Show HN: MassDNS – A high-performance DNS stub resolver in C | Hacker News Reader

Show HN: MassDNS – A high-performance DNS stub resolver in C

github.com

35 pointsblechschmidt53 comments

// SECURITY IMPROVEMENTS REQUIRED. VULNERABLE TO INTEGER AND BUFFER OVERFLOWS.

well at least it's pretty clear up front. No malloc result checks, invalid format in printfs, probably more.

(I don't mean to be disparaging, please don't take it that way OP. We clearly need more/better tooling in this area in C).

(Edited for clarity)

Malloc results are checked by the safe_malloc function in security.h. Could you point out a line that is prone to integer overflows?

stream_buffer_init() calls malloc directly, so it's not protected by safe_malloc(). (Unless there is some funky re-define stuff I didn't see).

%x takes unsigned int, not a char. (Although I guess you could just rely on promotion here if you can rely on the unsignedness of char).

> Could you point out a line that is prone to integer overflows?

Never said that it was an overflow (just C&P that comment from the file). Just said its invalid use. Which I'm totally willing to agree on the compiler mostly getting correct behavior. Just not really something I'd want to see landing in something that we all need a really good, secure implementation of.

Best of luck though, I hope you iterate on this to the point we can have a really good DNS implementation in C, in user-space, with optional async processing.

Thank you for pointing this out. This file actually contained some unused code that was not supposed to make it into the repository so I just removed it. The "%x" format specifiers were located in debug comments that have been commented out. They have also been removed now.

Although I guess you could just rely on promotion here if you can rely on the unsignedness of char

If compilers did not do promotion, they would be very obviously not compliant with the C standard and unable to compile correctly most existing code, so I'm willing to bet that is something you can rely on. It's defined by the standard and widely used.

Sure, but in this case it was signed to unsigned promotion.

Why should that matter in this case? The worst case would have been the output of a wrong value and it was some commented out debug output which I have used in a very specific case.

Totally, just wrong value. This thread is already making a bigger deal of it than I expected it to.

Malloc result checks are generally a bad idea; they're error prone. Better just to run with the allocator configured to abort on failure.

Unless, of course, one can recover from malloc failing.

Even if you can in theory, it's still a bad idea to try to do that. Statistically speaking, you will get it wrong.

It may be a bad idea for an average developer to try it, sure. Plenty of advanced things fall in to that category though.

It is a bad idea for any developer to try it. Statistically speaking, it is the best programmers who create the most widespread security flaws.

Which statistics are you referring to?

Well, there can't be any objective quantification of "best", so it's impossible to prove this without being subjective. But the most widespread vulnerabilities tend to be in software that a lot of people use. Software that a lot of people use is usually written by good programmers, because good programmers (for typical definitions of "good") are the most productive ones.

The people who write the Linux kernel, for example, are by and large good programmers, because kernel programming is difficult and becoming a prolific contributor even more so. They've been responsible for the majority of the security bugs in the Linux kernel. That doesn't make them bad programmers.

Correlation vs causation.

Good programmers work on popular software because most people work on popular software.

Most security bugs are found in popular software because most people care about (use, audit, exploit) popular software.

Nothing here backs the assertion that good programmers are more likely to create more bugs per line of code written.

I'd argue that the reverse is most likely true. Good programmers know their limits, the limits of their tools, and have the experience to recognize problematic patterns.

Saying no one should try to recover from allocation failure is like saying no one should attempt surgery because only surgeons kill people occasionally during operations.

  > Nothing here backs the assertion that good programmers 
  > are more likely to create more bugs per line of code 
  > written.

This is moving the goalposts. Here's your original statement:

"It may be a bad idea for an average developer to try it"

And here's pcwalton's rebuttal:

"It is a bad idea for any developer to try it"

The relative frequency of bugs between average developers and advanced developers isn't the issue. Very good developers create bugs that lead to security flaws all the time. That the number of bugs they create is less than the number of bugs that would be created by less experienced developers does not matter in this context, because we have empirical evidence that both these numbers are very far from zero.

  > Good programmers know their limits, the limits of 
  > their tools, and have the experience to recognize 
  > problematic patterns

Sharing such experience is precisely what tptacek and pcwalton are doing here.

I didn't move the goalposts very far at all. If you quoted the rest of pcwalton's comment you would have had a more appropriate context in which to understand my reply.

Here we have a claim that encompasses 100% of a population, which is refuted by someone in that population who doesn't agree with the claim (and whose direct experience contradicts it).

And it isn't just my experience which contradicts it. Many of my colleagues would dismiss such an assertion as well.

Now if his claim was about "many" or even "most" programmers then sure, perhaps there is nothing to refute. But then his claim would have added almost nothing to what he replied to.

Saying all? That's either a dramatic device which I don't think adds to a healthy discussion, or a genuine belied that I think should be refuted since I'm in a position to provide counter evidence.

> Nothing here backs the assertion that good programmers are more likely to create more bugs per line of code written.

That isn't what I said. I said that good programmers create more bugs on the whole.

> Saying no one should try to recover from allocation failure is like saying no one should attempt surgery because only surgeons kill people occasionally during operations.

We need surgery so that people don't die. You probably don't need to recover from malloc failing.

> > Nothing here backs the assertion that good programmers are more likely to create more bugs per line of code written.

> I said that good programmers create more bugs on the whole.

Well if good programmers write more code, then sure, that follows quite nicely. If you don't normalize for lines of code written, though, then I'm not sure I follow what the point of that assertion is.

If a good programmer writes 10x the code but 1.5x the bugs, I don't think it's fair or meaningful for this discussion to claim that the good programmer created more bugs on the whole.

> We need surgery so that people don't die. You probably don't need to recover from malloc failing.

Software definitely runs in places where lives can be affected and lost.

Generally agree, (and that's what we usually do in GLib). However, for a general purpose DNS user-space implementation I think it would be good to have something that can be used in both scenarios.

That's a coherent argument, but that argument is also, I think, probably responsible for virtually all of the exploitable offset-from-NULL vulnerabilities shipped in C code.

No malloc result checks

The last time I worked with the DNS protocol, I seem to remember that everything had a well-defined (and small) limit on its size, and it wasn't really necessary to use dynamic allocation at all. Strings are length-prefixed, not null-terminated, and the longest length fields are only 16 bits.

A look at the RFC shows the strict limits on size:

https://www.ietf.org/rfc/rfc1035.txt

In comparison to text protocols like HTTP, I see far less opportunities to make errors like buffer overflows with DNS. It's a simple and relatively easy to parse binary protocol.

Just because strings are length prefixed, doesn't mean that they don't need to be dynamically allocated. The reason they can be put on the stack is that the prefix is just one byte and thus the maximum length 255 bytes for the strings.

Prefixed strings can even be a source of buffer overflows, if you don't check if the length is smaller than the actual length of the rest of the data. In fact this was actually the source of the heartbleed bug.

    We clearly need more/better tooling in this area in C).

I think the tooling we have nowadays is better than we give it credit for. Between clang's analyser, ASAN, Infer and tis-interpreter, I often find when someone points out a vulnerability, I can get one of these tools to make an issue of it.

Note I'm not suggesting this is enough to make C safe.

Edit: A quick demonstration, based on 30 seconds with valgrind. If I run with ./bin/massdns --help, it makes a malloc() on line 595, then exits on 605 without free()ing. And yes I know this isn't really an issue in practice, but it demonstrates that the tools can see issues.

Ugh, so public resolvers are flooded with requests? Wouldn't it make much more sense to set up a local caching resolver like unbound and feed your queries to it? It would be much more considerate towards the public resolvers and also use less of your own bandwidth.

There are valid use cases for this. For example, web crawling needs to do millions of resolutions one time which completely nullifies the need for a cache.

For this to be correct, all records to be resolved would need to have completely disjunct labels from the root down. Which they cannot if they are indeed in the millions.

I have not yet managed to setup a single local recursor, such as PowerDNS recursor, to deliver the same performance as the list consisting of multiple open resolvers, although bandwidth does not seem to be the limiting factor. Testing with dnsperf, the best result I am currently getting is about 6,000 resolves per second with bind, for PDNS the figure is even lower for some reason. I will have to dig a bit deeper in order to find the reason for that cap.

I'm not surprised at all that BIND performs poorly, look at those graphs (granted this is for authoritative servers, but says a lot about BIND's performance in general): https://www.nlnetlabs.nl/blog/2013/07/05/nsd4-performance-me...

I'd stick with Unbound. There are a lot of knobs to fiddle with in the config. Be sure to compile against libevent so that you can use the highly scalable epoll as a backend (assuming you're on Linux). Turn up all the limits for cache size etc. Disable DNSSEC validation if you don't care about spoofed records. Ask on the mailing list if you need help, Wouter and his colleagues are very nice and respond very quickly.

"A high-performance non-recursive DNS resolver"

I believe a better description for this program would be a "stub resolver".

This is already discussed below and you are right. It was an issue with the perspective of the term recursion from my side. Unfortunately, I cannot change the HN title anymore. (Maybe some moderator can?) The GitHub project description has been changed.

Some of the resolvers might ban/rate-limit you indeed and even send abuse complaints to your ISP.

EDIT: You might have a look at the newly implemented --norecurse option.

"--norecurse Useful for DNS cache snooping"

Not if the resolver rejects non-recursive queries, as do dnscache and dqcache.

There is a way to resolve DNS domainnames to IP addresses using only non-recursive queries. I do it everyday.

But I've never seen anyone release any program that did this. Your program does not even attempt to do this -- you need to send the queries to authoritative nameservers not public resolvers. But you used the term "non-recursive" so I thought maybe someone had finally tried.

One of the shortcomings of DNS IMO is that the specification allows for the possibility of including more than one name in a query. But no one has ever implemented this, as far as I know.

Despite the design of the DNS, most of the information stored in it is more static than dynamic, and much of it is centralized. Most dommainnames do not change IP addresses very often and there are very large numbers of domainnames sharing the same authoritative nameservers.

For DNS cache snooping the usage would of course be different. You would supply the tool with one resolver which does not reject non-recursive queries. Theoretically, one could even perform traffic analyses of DNS resolvers by snooping.

Having implemented the --norecurse option, the title is at least not wrong anymore. One can have non-recursive, non-iterative resolver (which is what you use when you want to perform DNS cache snooping) and the title does not suggest that the tool supports iterative lookups.

Handling multiple questions within one packet is difficult because response codes such as NXDOMAIN are only included once per packet. AFAIK, bind does not support handling such queries.

"AKAIK, bind does not support handling such queries."

AKAIK, there is not a server in existence that handles such queries.

Cache snooping is not very reliable. I tested the 999 open resolvers listed in resolvers.txt for example.com and only 643 of them returned an answer. 13 of those answers were fake.

Moreover, there are at least hundreds of thousands of domainnames that will not be in any given cache.

Finally, most internet users are probably not using open resolvers. They are more likely using the ones provided by their ISP, which are not open.

Anyway, I doubt anyone is ever going to release a non-recursive resolver.

I have my own solution which is too embarrasing to release but I can tell you it is usually faster than a recursive resolver if the cache is not already primed with the right records.

To me, "non-recursive" resolution means something specific -- no use of DNS caches. And because of that there's no risk of "cache poisoning".

Thanks, we updated the title.

Out of curiosity, what is the difference in a blurb? I was reading about people bemoaning the addition of systemd-resolved as stub resolver by default in Ubuntu 16.10. I get it, but do not have time for the full RFC.

https://lobste.rs/s/edrihm/sad_news_today_systemd_resolved_b...

http://seclists.org/oss-sec/2014/q4/595

(Please understand, I use systemd, just not resolved yet; this is not meant to start a tangential flame; I just realize even though I am one of those idiots who preaches better security and runs his own dnsmasq settings per recommendations of known bloggers, I need more detailed background on DNS.)

What are the benefits of resolving DNS entries in a non-recrusive manner? A recursive resolve means that the final resolution of the domain is cached by the original DNS server that was queried along with all the servers it took to resolve. My understanding is that this behavior was ideal.

What I mean by non-recursive in this case is that the program does not perform recursion by itself as it relies on a list of DNS resolvers. When querying these resolvers, the recursion bit is actually set. However, I agree that the title might be misleading but I am not sure how I could express that better.

Yeah, most titles I just brainstormed still suck in some way. Yours is technically true but only locally. Perhaps add something like you just said to the opening paragraph. That reduces confusion on what non-recursive means along with saving reader any wasted time if they were seeking something else.

Well, in order to make it correct, I have implemented a --norecurse option.

    if (packet->flags & DNS_RESPONSE_FLAG == 0 || packet->questioncount != 1)

I'm not sure that it does what you want.

This is the line of code that pre-checks whether an incoming packet should be parsed at all. If it is not a response packet or if the number of questions is not one, it should not be parsed, simply because the packet parsing function expects a reply packet with exactly one question, namely the one that has been queried for.

Operator precedence causes it to be interpreted as:

  if ((packet->flags & (DNS_RESPONSE_FLAG == 0)) || (packet->questioncount != 1))

which at least does not read as intended.

You are correct. Has been fixed.

You may want to test it with http://www.verisign.com/en_US/channel-resources/domain-regis...

For easy comparability, request handling rates of tools like this are best described in terms of requests per second, not requests per hour. Also the hardware on which the benchmarks were performed needs to be described in detail.

Are you sure tinydns, which has existed for ages without security vulnerabilities, can't trivially handle 27k requests/second on modern hardware?

I have not heard about tindydns before but it seems to be a DNS server, not a client.

The tool has mainly been tested on a Hetzner EX41 server. (Ubuntu, Intel® Core™ i7-6700, 32 GB RAM)

Ah, you are correct. There is also dnscache, which is part of the same suite. However, it does not do what you are attempting to do.

Have you tried c-ares? http://c-ares.haxx.se/

Not yet. I have had a quick look at ldns (https://www.nlnetlabs.nl/projects/ldns/) which supports parsing DNS packets from wire. I will probably replace the DNS implementation with either libldns or c-ares.

ldns is synchronous so all your queries will block. libunbound (https://www.unbound.net/documentation/libunbound.html), like c-ares, is an async C resolver library from the authors of ldns.

Ah, that would mean that I would have the libraries handle the resolving. I was currently only thinking about keeping the single socket which is used in order to only use the parsing functions from the libraries.

Reply on news.ycombinator.com