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

TXT query with no question causing exception in JSON stat API. #6924

Closed
austintrose opened this Issue Sep 4, 2018 · 4 comments

Comments

Projects
None yet
6 participants
@austintrose
Copy link

austintrose commented Sep 4, 2018

  • Program: Recursor
  • Issue type: Bug report

Short description

We have recently started receiving an odd DNS request that causes the PowerDNS Recursor JSON stat HTTP API to throw an exception.

Here are the bytes of the DNS query that we are receiving (application layer only). It is ostensibly coming from some Fortinet security product.

0000   a9 e9 00 00 00 00 00 00 00 00 00 01 14 73 65 63   ©é...........sec
0010   75 72 65 2d 64 6e 73 2d 76 65 72 73 69 6f 6e 2d   ure-dns-version-
0020   31 08 66 6f 72 74 69 6e 65 74 03 63 6f 6d 00 00   1.fortinet.com..
0030   10 00 01 00 00 00 00 00 35 34 46 42 36 34 30 51   ........54FB640Q
0040   6c 5a 66 78 41 42 52 6b 64 55 4e 6a 42 46 4e 46   lZfxABRkdUNjBFNF
0050   45 78 4e 6a 41 32 4d 54 4d 7a 4e 41 41 48 41 41   ExNjA2MTMzNAAHAA
0060   51 41 41 41 41 49 41 41 45 41 41 67 42 34         QAAAAIAAEAAgB4

Here is an image of the Wireshark details of the request, which may be easier to read.

While this query is in the ring buffer, calls to the JSON API can result in an internal server error:

curl -H 'X-API-Key: our-api-key' "http://127.0.0.1:8082/jsonstat?command=get-query-ring&name=queries"
Internal Server Error

Looking at the pdns.log file when this error occurs, we see the following:

HTTP ISE for "/jsonstat": STL Exception: Attempt to print an unset dnsname

For the record, the error is raised here in the source code.

Environment

  • Operating system: Ubuntu 14.04.5 LTS
  • Software version: 4.1.3
  • Software source: deb [arch=amd64] http://repo.powerdns.com/ubuntu trusty-rec-41 main

Steps to reproduce

I have been using the following Python script to reproduce the DNS query. You must sudo pip install pyip first, and run the script as superuser. There may be an easier way to reproduce with dig, but this has worked for me.

import udp
import socket

udp_packet = udp.Packet()
udp_packet.sport = 1024;
udp_packet.dport = 53;
udp_packet.data = '\xa9\xe9\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x14\x73\x65\x63\x75\x72\x65\x2d\x64\x6e\x73\x2d\x76\x65\x72\x73\x69\x6f\x6e\x2d\x31\x08\x66\x6f\x72\x74\x69\x6e\x65\x74\x03\x63\x6f\x6d\x00\x00\x10\x00\x01\x00\x00\x00\x00\x00\x35\x34\x46\x42\x36\x34\x30\x51\x6c\x5a\x66\x78\x41\x42\x52\x6b\x64\x55\x4e\x6a\x42\x46\x4e\x46\x45\x78\x4e\x6a\x41\x32\x4d\x54\x4d\x7a\x4e\x41\x41\x48\x41\x41\x51\x41\x41\x41\x41\x49\x41\x41\x45\x41\x41\x67\x42\x34'

packet = udp.assemble(udp_packet, 0)

sock = socket.socket(socket.AF_INET, socket.SOCK_RAW, socket.IPPROTO_UDP)
print sock.sendto(packet, ("127.0.0.1", 0))

The script will issue the DNS query in question to 127.0.0.0:53. Afterwards, you can make an HTTP query to the ring buffer JSON API:

curl -H 'X-API-Key: our-api-key' "http://127.0.0.1:8082/jsonstat?command=get-query-ring&name=queries"
Internal Server Error

And you will find the cause of the internal server error in pdns.log:

HTTP ISE for "/jsonstat": STL Exception: Attempt to print an unset dnsname

Expected behaviour

Ideally there would be no exception thrown. Perhaps the 'unset DNS name' gets represented as an empty string. Although that could cause some ambiguity.

Perhaps PowerDNS should respond to this query with Format Error flags, and not ultimately place the query in the ring buffer? See my comparison of the PowerDNS and Google DNS responses at the bottom of this issue.

Actual behaviour

HTTP ISE for "/jsonstat": STL Exception: Attempt to print an unset dnsname appears in pdns.log, and the HTTP request receives a 500 Internal Server Error.

Other information

We noticed that this exception was also being raised by our custom preresolve hook. In that case, the problem was this line:

local domain = dq.qname:toStringNoDot()

Which would raise the Attempt to print an unset dnsname exception. We now avoid the exception like so:

local domain = ''
if dq.qname:wirelength() > 0 then
    domain = dq.qname:toStringNoDot()
end

Finally

I was curious to see how PowerDNS actually responds to the query, so I compared the response from my local Recursor to the response from Google's DNS at 8.8.8.8.

My local Recursor gave a response with flags indicating a server failure, which you can see in this screenshot of the Wireshark details of the response

Google DNS gave a response with flags indicating a format error in the query, which you can see in this screenshot of the Wireshark details of the response.

@pieterlexis pieterlexis added auth defect rec and removed auth labels Sep 4, 2018

@phonedph1

This comment has been minimized.

Copy link
Contributor

phonedph1 commented Sep 4, 2018

There are probably a few places that could use toLogString instead which prints <empty> in these cases instead. I can probably poke around at that part this week while the smarter people come up with a better fix for handling these earlier on.

A quick check shows rec_control top-queries breaks as well for instance. Other ones like current-queries were previous patched to use toLogString though.

@phonedph1 phonedph1 referenced this issue Sep 4, 2018

Merged

rec: rec_control/ws toLogString #6925

3 of 7 tasks complete
@pieterlexis

This comment has been minimized.

Copy link
Member

pieterlexis commented Sep 5, 2018

I am surprised we have not dropped this query for being broken before adding it to the ringbuffer

@rgacogne rgacogne modified the milestones: rec-4.2.0, rec-4.1.x Sep 5, 2018

@ahupowerdns

This comment has been minimized.

Copy link
Member

ahupowerdns commented Sep 7, 2018

This is an outstanding bug report by the way. Thank you so much!

@clokep

This comment has been minimized.

Copy link
Contributor

clokep commented Nov 7, 2018

Thanks for fixing this so quickly! We just upgraded to 4.1.6 and can confirm this fixed our issue! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.