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

dnsdist: downstream response protocol can be confusing #11501

Closed
phonedph1 opened this issue Apr 4, 2022 · 0 comments · Fixed by #11545
Closed

dnsdist: downstream response protocol can be confusing #11501

phonedph1 opened this issue Apr 4, 2022 · 0 comments · Fixed by #11545

Comments

@phonedph1
Copy link
Contributor

  • Program: dnsdist
  • Issue type: Bug report, I think.

Short description

Client DoT or TCP which result in a TCP downstream query end up recording the protocol as DoUDP in the response ring.

Environment

  • Operating system: Debian
  • Software version: master
  • Software source: compiled yourself

Steps to reproduce

Do a dig @127.0.0.1 google.com +tcp query and verify the downstream shows DoUDP instead of DoTCP

Time    Client                                          Protocol     Server       ID    Name                      Type  Lat.   TC RD AA Rcode
-2.1    127.0.0.1:43359                                 DoTCP                     32009 google.com.               A               RD    Question
-2.1    127.0.0.1:43359                                 DoUDP        8.8.8.8:53   32009 google.com.               A     64.3      RD    No Error. 1 answers

Expected behaviour

DoTCP shown for downstream TCP connections.

Actual behaviour

It looks like the default is to show DoUDP unless it's a tcpOnly=true backend (or DoT/DoH to the backend). The protocol is taken from the downstream server state and not the protocol that was actually used for the downstream connection.

Other information

I would think maybe something like this:

--- a/pdns/dnsdist-tcp.cc
+++ b/pdns/dnsdist-tcp.cc
@@ -256,7 +256,7 @@ static void handleResponseSent(std::shared_ptr<IncomingTCPConnectionState>& stat
     double udiff = ids.sentTime.udiff();
     vinfolog("Got answer from %s, relayed to %s (%s, %d bytes), took %f usec", ds->d_config.remote.toStringWithPort(), ids.origRemote.toStringWithPort(), (state->d_handler.isTLS() ? "DoT" : "TCP"), currentResponse.d_buffer.size(), udiff);
 
-    ::handleResponseSent(ids, udiff, state->d_ci.remote, ds->d_config.remote, static_cast<unsigned int>(currentResponse.d_buffer.size()), currentResponse.d_cleartextDH, ds->getProtocol());
+    ::handleResponseSent(ids, udiff, state->d_ci.remote, ds->d_config.remote, static_cast<unsigned int>(currentResponse.d_buffer.size()), currentResponse.d_cleartextDH, dnsdist::Protocol::DoTCP);
 
     updateTCPLatency(ds, udiff);
   }

but I am not sure if that works for DNSCrypt (or if DoT changes to not always map to TCP), and if this has any other fallout.

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

Successfully merging a pull request may close this issue.

2 participants