Opened 10 years ago
Closed 10 years ago
#3342 defect closed fixed (fixed)
twisted.names dns-spoofing vulnerability
| Reported by: | jknight | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | |
| Component: | names | Keywords: | |
| Cc: | jknight | Branch: |
branches/dns-random-source-port-3342
branch-diff, diff-cov, branch-cov, buildbot |
| Author: | exarkun |
Description
See: http://www.kb.cert.org/vuls/id/800113
We apparently need to use a cryptographically secure random source port number.
Change History (10)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
| Cc: | jknight added |
|---|
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.
comment:3 Changed 10 years ago by
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.
comment:4 Changed 10 years ago by
| author: | → exarkun |
|---|---|
| Branch: | → branches/dns-random-source-port-3342 |
(In [24306]) Branching to 'dns-random-source-port-3342'
comment:6 Changed 10 years ago by
| Keywords: | review added |
|---|
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
comment:7 Changed 10 years ago by
| Owner: | Jean-Paul Calderone deleted |
|---|
comment:8 Changed 10 years ago by
| Keywords: | review removed |
|---|---|
| Owner: | set to Jean-Paul Calderone |
Please update copyright of test_names, and merge.
comment:9 Changed 10 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
(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.
comment:10 Changed 7 years ago by
| Owner: | Jean-Paul Calderone deleted |
|---|
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?