Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No RD bit in request - DNS Query Randomly Fails #29

Closed
fishcharlie opened this issue Jan 6, 2021 · 11 comments · Fixed by #37
Closed

No RD bit in request - DNS Query Randomly Fails #29

fishcharlie opened this issue Jan 6, 2021 · 11 comments · Fixed by #37

Comments

@fishcharlie
Copy link

After discussing with a member of the 1.1.1.1 team at Cloudflare, the reason for this problem is that dns2 is not passing in the RD bit in the request header.

I added the following line right above the client.send call in client/udp.js, and it seems to have fixed the problem completely.

query.header.rd = 1;

According to the member at Cloudflare, the RD bit is required by the Cloudflare resolver, and is also required by most other resolvers.

I'd be happy to submit a PR to fix this @song940, however, I'm not sure we want to hard code it to 1. Should this be a dynamic option passed in by the user? If so, how should the syntax/API look for this?

Original report below.


If you run the following code.

(async () => {
	const name = "avatars3.githubusercontent.com";

	// dns2
	const DNS = require('dns2');
	const dnsClient = new DNS({
		"nameServers": ["1.1.1.1"]
	});
	console.log(await dnsClient.resolveA(name));

	// Node dns
	const { Resolver } = require('dns');
	const resolver = new Resolver();
	resolver.setServers(['1.1.1.1']);
	resolver.resolve(name, "A", (err, records) => {
		console.log(records);
	});
})();

It results in:

Packet {
  header: {
    id: 3559,
    qr: 1,
    opcode: 0,
    aa: 0,
    tc: 0,
    rd: 0,
    ra: 1,
    z: 0,
    rcode: 2,
    qdcount: 1,
    nscount: 0,
    arcount: 0,
    ancount: 0
  },
  questions: [ { name: 'avatars3.githubusercontent.com', type: 1, class: 1 } ],
  answers: [],
  authorities: [],
  additionals: []
}
[
  '151.101.128.133',
  '151.101.64.133',
  '151.101.0.133',
  '151.101.192.133'
]

As you can see, when running the command through the native Node.js DNS client I get 4 answers back, whereas when running it through dns2 I get 0 answers back in the array.

It looks like part of the problem is the ancount is 0 for some reason. The rcode is also 2. Which according to the spec means:

Server failure - The name server was unable to process this query due to a problem with the name server.

However, this doesn't make sense. I'm hitting the same name server (1.1.1.1) for both the native Node.js DNS client (where it's obviously successful), and the dns2 query.

Is there a problem with the request being made in dns2 where the name server can't understand it? Or is there a problem with parsing the response in dns2?

Any ideas what is going on here?

I will also mention, this issue occurs the majority of the time. But every once in a while (rarely). It succeeds and works just fine (with no code changes on my part).

@fishcharlie
Copy link
Author

@song940 Any thoughts on how the syntax/API should look for this? I'd be happy to submit a PR once I get some guidance on how it should work.

@eviltik
Copy link
Contributor

eviltik commented Feb 5, 2021

@fishcharlie just curious, is it possible to have a screenshot of a tcpdump, one with native node dns code, the other one with dns2 (with npm install, not with a git clone of the current master branch, see #31) ?

I'd like to see if you have a FailErr in the reply, to compare with a potential problem i have on my side. Thank you.

@eviltik
Copy link
Contributor

eviltik commented Mar 8, 2021

After testing, the purposale of @fishcharlie solve #33. Let me PR.

@fishcharlie
Copy link
Author

@eviltik #33 should fix the issue. One thing that I mentioned above:

I'd be happy to submit a PR to fix this @song940, however, I'm not sure we want to hard code it to 1. Should this be a dynamic option passed in by the user? If so, how should the syntax/API look for this?

For my personal use I hard code it to 1 (just like your PR), but that is a question worth asking I think.

@eviltik
Copy link
Contributor

eviltik commented Mar 8, 2021

Good question !

Let's go deeper, https://tools.ietf.org/html/rfc1034

The actual algorithm used by the name server will depend on the local OS
and data structures used to store RRs. The following algorithm assumes
that the RRs are organized in several tree structures, one for each
zone, and another for the cache:

  1. Set or clear the value of recursion available in the response
    depending on whether the name server is willing to provide
    recursive service. If recursive service is available and
    requested via the RD bit in the query, go to step 5,
    otherwise step 2.
  1. Using the local resolver or a copy of its algorithm (see
    resolver section of this memo) to answer the query. Store
    the results, including any intermediate CNAMEs, in the answer
    section of the response.

I'm not sure to perfectly understand, but i think that having RD bit with value 1 by default is OK : it only change the server side behavior depending if it support recursive service or not.

@fishcharlie perhaps you can double check wit Cloudflare ? On my side, it fix Virtualbox issue too.

@eviltik
Copy link
Contributor

eviltik commented Mar 8, 2021

Mhh well ... perhaps a PR with an option to enable RD bit is better, so we can implement this without effect side and document it. Give me few minutes ;)

@eviltik
Copy link
Contributor

eviltik commented Mar 8, 2021

https://tools.ietf.org/html/bcp123

2.10. Misdirected Recursive Queries

The root name servers receive a significant number of recursive
queries (i.e., queries with the Recursion Desired (RD) bit set in the
header). Since none of the root servers offers recursion, the
servers' response in such a situation ignores the request for
recursion and the response probably does not contain the data the
querier anticipated. Some of these queries result from users
configuring stub resolvers to query a root server. (This situation
is not hypothetical: we have received complaints from users when this
configuration does not work as hoped.) Of course, users should not
direct stub resolvers to use name servers that do not offer
recursion, but we are not aware of any stub resolver implementation
that offers any feedback to the user when so configured, aside from
simply "not working".

@eviltik
Copy link
Contributor

eviltik commented Mar 8, 2021

Seem's that RD flag is 1 in nslookup.c

https://github.com/abhisg/nslookup/blob/master/nslookup.c

@eviltik
Copy link
Contributor

eviltik commented Mar 8, 2021

Got it. It's enable in dig and nslookup by default, but can be disabled.

dig man page

  +[no]rdflag
      A synonym for +[no]recurse.

  +[no]recurse
      Toggle the setting of the RD (recursion desired) bit in the query. This bit is set by default, 
       which means dig normally sends recursive queries. Recursion is automatically disabled
       when the +nssearch or +trace query options are used.

@eviltik
Copy link
Contributor

eviltik commented Mar 8, 2021

Dropping my PR, working on a new one

@fishcharlie
Copy link
Author

@eviltik I think you are correct that 1 being the default is fine. I just think we should consider allowing the user to change it to 0. Just like dig, where you can set it to have recursion disabled (0).

It's more of a question about how the syntax and API looks for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants