twisted.names dns-spoofing vulnerability

[jknight]

See:  http://www.kb.cert.org/vuls/id/800113

We apparently need to use a cryptographically secure random source port number.

[exarkun]

I think this came up once before. Last time, someone (probably me, I don't really remember) decided that using a random (hopefully cryptographically so, but the code will fall back to a non-cryptographically secure random source if a cryptographically secure random source is unavailable on the system) message identifier would be a good enough fix. Looking at the vulnerabilities linked from that page, I see three pieces of software which failed to use cryptographically secure random sources, but no vulnerabilities based on a server not selecting source port numbers with a cryptographic random number generator. I agree with the page's assertion that using a random port for each message increases security, but it is harder to implement, slower at runtime (perhaps not significantly so), and likely not practically more secure.

Thoughts?

[jknight]

That's an incorrect reading of the vulnerability report. That first section talks about previous vulnerabilities that had already been fixed a year ago.

t.names already (I think...) uses a secure random source for its message ids. Hm, actually, it looks like it's being really stupid and cutting down the effective number of bits to 10 instead of 16. That seems really bad....

But anyways, the new issue is about randomizing the port used. It has not yet been disclosed why this is so important, only that it is a relatively effective mitigation technique for the so far undisclosed vulnerability.

And t.names does not randomize the port used for each query. It should.

[jknight]

The first comment here:  http://blog.netherlabs.nl/articles/2008/07/09/some-thoughts-on-the-recent-dns-vulnerability has speculation as to the reason for the newly discovered incredible level of brokenness in not randomizing the port.

[exarkun]

(In [24306]) Branching to 'dns-random-source-port-3342'

[jknight]

Probably #3217 will get fixed as a side-effect of fixing this.

[exarkun]

Ready for review. I think #3217 is indeed fixed here. I didn't change any APIs or add any tests for this in particular, but I did delete the cleanup code in the tests which reached into the resolver and disconnected its port - that happens by itself now, unless the deprecated API is used.

For the rest, it's pretty straightforward - client.Resolver tries not to use its protocol attribute anymore, instead of it binds a UDP port for each query it needs to do and unbinds it afterwards.

Build results at  http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/dns-random-source-port-3342

[therve]

Please update copyright of test_names, and merge.

[exarkun]

(In [24401]) Merge dns-random-source-port-3342

Author: exarkun Reviewer: therve Fixes: #3342

Change twisted.names.client.Resolver to use a new random UDP source port for each DNS request it makes. Also the function responsible for generating DNS message identifier to use the full 16 bit range instead of only using 10 bits of it.

See  http://www.kb.cert.org/vuls/id/800113