-
Notifications
You must be signed in to change notification settings - Fork 7.6k
DNS Server : bug fix and prettifying #1011
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
Conversation
…bit the signification of several bytes in the response.
uhmm I do not really agree. here is what happens in the code:
Now this is all wrong! With the code you included, it did not work for me and nslookup reported malformed packet |
I beg to differ :)
So it seems that we agree on the first two, the header and the question. The thing is, you stop there, add a TTL and some data. Okay, but what does the spec say ? Straight to page 29 :
So, in the reply, if you just copy the question, then add the TTL, RDLength and RData, you just miss 3 fields : the name, the type and the class. You could copy the question again, but to have a complete message, you should, all things considered, copy the question twice instead of once like you do. My version, which is also the ESP8266 version, use the complete format, with a little trick named message compression described on page 30 (and in my code). I haven't seen anywhere your solution consisting of you just answering with the question padded with the TTL, RDLength and RData, so if you have any reference for that, I'd gladly read it. |
Hi me-no-dev & Loufylouf, I've checked out both the official master and Loufylouf's. On my devices (one smartphone and a windows desktop) the captive portal does not work when I'm on I feel it is safe to say that the captive portal / DNSServer does not fully work for all devices connected to the ESP, and it might be worth looking into this a bit more 😉 |
@BillyNate Can you be a bit more specific on the last part of your message ? Did my version work on all of your devices ? Or are you referring to the test of me-no-dev with my version ? |
I added your repository to my remotes, fetched and pulled your Does this make it more clear? |
Yeah thanks ! @me-no-dev I now get what you're doing ! I overlooked the Qcount = 0 part. The thing is, while browsing normally on my computer and wiresharking the DNS messages, all query responses carry the question as well ("dns && dns.count.queries == 0" filter returns nothing), so while I agree your solution should be working (after all, there's an ID to match an answer with a question), this does not seem to be the case at least for me (on Ubuntu and Android) and @BillyNate (windows desktop) . |
I think the best option here is to create a new answer and do not copy stuff over ;) that whole copy/paste thing sucks :D |
Well, in my code, that's exactly what is done for the answer. Since the format of the question doesn't change, I don't see the problem copy-pasting it. Well, except for the part where we modify the DNS header and then paste the whole buffer, that can be a bit unsettling at first. |
#1037 The compression is a nice touch, was wondering if the entire domain needed to be in response like that. |
…question data and to create the DNS answer from scratch.
I've added a commit that creates the DNS question structure and build the response from scratch instead of a dirty copy-paste |
@me-no-dev have you had a chance to look this over or document whatever issues you were having? |
* Retrieve some code from what has been done on the ESP8266. Clarify a bit the signification of several bytes in the response. * Add the type and class as members of the DNS class for an eventual future use. * Clarify the sense of a magic number present in DNS server. * A bit of aesthetics for the DNS server. * Add a structure for the DNS question, use it DNS server to store the question data and to create the DNS answer from scratch.
I had problem with the last version of the DNS server lib, there were lots of malformed packets on Wireshark and I could not get the library to work. So I checked what was done in the ESP8266 library, integrated some of it and prettified a bit the whole thing. Everything should be a little clearer for someone trying to integrate new functionalities. Tested it with success with WiFiManager.