Permalink
Browse files

Drop duplicate query ID handling

Protection against the Kaminsky attack requires query IDs to be
random, but it doesn't require them to be unique. And since net-dns
opens a new socket for each query it performs, it doesn't rely on the
uniqueness of the query ID to map requests to responses.

Requiring unique query IDs limits net-dns to only making 65535 lookups
before it starts spinning in circles, trying to generate a query ID it
hasn't seen before, when in fact they have all been generated.

So instead, generate random query IDs, but don't require them to be
unique.
  • Loading branch information...
1 parent b5cfb32 commit 3a22dbe096d86ecc62f9bb8e486641278dad49a2 @evan-stripe evan-stripe committed Jul 12, 2012
Showing with 1 addition and 17 deletions.
  1. +1 −14 lib/net/dns/header.rb
  2. +0 −3 test/header_test.rb
View
@@ -63,10 +63,6 @@ class WrongOpcodeError < ArgumentError
class Error < StandardError
end
- # The requested ID is already in use.
- class DuplicateIDError < Error
- end
-
#
# = Name
@@ -180,8 +176,6 @@ def to_s
# Array with given strings
OPARR = %w[QUERY IQUERY STATUS]
- @@id_arr = []
-
# Reader for +id+ attribute
attr_reader :id
# Reader for the operational code
@@ -357,12 +351,8 @@ def data
# performing security tests.
#
def id=(val)
- if @@id_arr.include? val
- raise DuplicateIDError, "ID `#{val}' already used"
- end
if (1..65535).include? val
@id = val
- @@id_arr.push val
else
raise ArgumentError, "ID `#{val}' out of range"
end
@@ -729,10 +719,7 @@ def new_from_hash(hash)
end
def genID
- while (@@id_arr.include?(q = rand(65535)))
- end
- @@id_arr.push(q)
- q
+ rand(65535)
end
end
View
@@ -119,9 +119,6 @@ def test_simple
assert_raises(ArgumentError) do
Header.parse("aa")
end
- assert_raises(Header::DuplicateIDError) do
- @default.id = 441
- end
assert_raises(ArgumentError) do
@default.id = 1000000
end

0 comments on commit 3a22dbe

Please sign in to comment.