ruby / resolv

A thread-aware DNS resolver library written in Ruby
Other
36 stars 28 forks source link

Handle `RCode::ServFail` from nameserver response #26

Open BobSilent opened 1 year ago

BobSilent commented 1 year ago

Fixes #23

Try next nameserver if current nameserver response is RCode::ServFail

BobSilent commented 8 months ago

@hsbt how can we get this merged?

hsbt commented 8 months ago

Can you add test for this at least?

BobSilent commented 8 months ago

Sure i will do so (or at least will give it a try - I am not a ruby pro 😉)

BobSilent commented 8 months ago

@hsbt please have a look at the added test case.

BobSilent commented 7 months ago

@hsbt did you had a chance to look into this?

BobSilent commented 7 months ago

@hanazuki, @hsbt anything else I can add/update here?

hanazuki commented 7 months ago

I have examined the logic but don't know if this works with TCP fallback.

I'm thinking of a situation as follows:

In this case, it is expected that:

  1. Resolv first queries NS1 with UDP. NS1 will respond with NoError with TC=1.
  2. Resolv then falls back to TCP and queries NS1 with TCP. NS1 will respond with ServFail.
  3. Resolv tries the next nameserver with TCP. NS2 will respond with NoError. This answer is returned to the caller.

IIUC, the proposed implementation will:

  1. changes the variable requester to make_tcp_requester(nameserver, port) when it receives a UDP message with TC=1 (so this host/port pair is NS1's),
  2. queries the same server (NS1) and gets a ServFail, and then
  3. uses the same requester to query NS2, which results in an error due to the unmatched host/port pair.

To make this work properly, maybe we have to reset the requester before next.

  when RCode::ServFail
    if Requester::TCP === requester
      requester.close
      requester = nil  # next iteration assigns new TCP requester
    end
    next

Sorry if I'm wrong. I feel confused 😵 .

hanazuki commented 7 months ago

To make this work properly, maybe we have to reset the requester before next.

  when RCode::ServFail
    if Requester::TCP === requester
      requester.close
      requester = nil  # next iteration assigns new TCP requester
    end
    next

I found this was not a sufficient solution. Please forget about it.

We also need to think of what should happen if all of the nameservers return ServFail (This is the expected result when the record set has a bogus DNSSEC signature). In that case, the control goes to https://github.com/ruby/resolv/blob/22153c2a45ecc73232bad89ead5bce3e1b660c9a/lib/resolv.rb#L1149 and a timeout error is raised, which might not be a desired behavior. This means that to handle ServFail correctly, the error needs to be notified to ‎Resolv::DNS::Config#resolv, which conducts retrying.