openresty / lua-resty-dns

DNS resolver for the nginx lua module
324 stars 107 forks source link

Comparison to Nginx DNS cache poisioning vulnerability #23

Closed mikz closed 7 years ago

mikz commented 7 years ago

I see lua-resty-dns is using math.random to generate packet ids.

This sounds similar to nginx dns cache poisoning vulnerability: http://blog.zorinaq.com/nginx-resolver-vulns/ (more useful info in https://news.ycombinator.com/item?id=14316430).

Would properly calling math.randomseed prevent this? Would it be reasonable to be able to swap the id generator function to something using secure random generator?

agentzh commented 7 years ago

@mikz Yeah, seeding the random generator yourself with some bytes from /dev/random can definitely improve this. Also, better ensure your network/ISP prohibits IP spoofing. The latter is much more important in general than DNS stuff.

mikz commented 7 years ago

Would seeding it with https://github.com/openresty/lua-resty-string/blob/master/lib/resty/random.lua also work?

What about structuring code in a way, that the whole _gen_id function can be overridden, so I can replace it with resty.random for example?

agentzh commented 7 years ago

@mikz You can just call math.randomseed(seed) in init_worker_by_lua*. No need to hijack the _gen_id function.

agentzh commented 7 years ago

@mikz Cryptographically strong random numbers are expensive to generate. So do not generate it in every _gen_id() call. Otherwise it can be much more expensive than this whole Lua dns client library. Generating it once or periodically (every hour, for example) in a recurring timer from init_worker_by_lua* without touching lua-resty-dns should be good enough already.

mikz commented 7 years ago

Generating it every once in a while is very good point. Thank you.

agentzh commented 7 years ago

@mikz And it will automatically help all the other Lua modules using math.random(), not just this Lua library. This is the best practice in the OpenResty world. Of course, Lua libraries should never call math.randomseed() on its own, since that will override a better seed set previously by the user.

mikz commented 7 years ago

I haven't seen the recommendation to call it by a timer before. Might be a good addition to the lua-resty-dns documentation? Now it says it must be seeded, but it could say it should be seeded from cryptographically strong source and that it is recommended to do it every hour or so via a timer.

agentzh commented 7 years ago

@mikz One does not need to reseed a PRNG periodically. Seeding it from /dev/random at the very beginning should be good enough. Modem PRNG algorithms have extremely large periods.

agentzh commented 7 years ago

Consider it resolved.