Proper EDNS handling and cleaner NOERROR response in DNSSERVER

[Kolkman]

Description of Change

Bugs

The code solves two bugs:

• BUG 1: When trouble shooting a dnsserver with the 'dig' (one of the main DNS troubleshooting tools) the server doesn't answer because it doesn't handle EDNS (OPT records in the additional section).

  That has been fixed, the additional section in the query can safely be ignored.
  (mostly change around line 123)

• BUG 2: The code also returns an A record when the QUERY Type not 'A' (or 'ANY').

  This has been fixed too. For an A and ANY query type the server returns an A record.
  (mostly change around line 114)

Feature

The code continues to return a 'SERVFAIL' when no match with QNAME occurs. However, with the fix of bug 2 we created a proper 'No Data' response, (RFC2308 2.2) for queries that have a match for the name, but not for the type. The function DNSServer::replyWithNoAnsw implements that functionality.

The server now behaves somewhat more conform the DNS protocl specifications.

(most of the code changes)

Tests scenarios

I tested this code on an ESP32 and WireShark as protocol analyzer - there is no reason to suspect that it does not work on other platforms (to which I have no access).

Related links

• RFC2308 Section 2.2.

[CLAassistant]

All committers have signed the CLA.

[github-actions]

	Warnings
⚠️	Some issues found for the commit messages in this PR: the commit message "Proper EDNS handling and cleaner NOERROR response": summary looks empty type/action looks empty Please fix these commit messages - here are some basic tips: follow Conventional Commits style correct format of commit message should be: <type/action>(<scope/component>): <summary>, for example fix(esp32): Fixed startup timeout issue allowed types are: change,ci,docs,feat,fix,refactor,remove,revert,test sufficiently descriptive message summary should be between 10 to 72 characters and start with upper case letter avoid Jira references in commit messages (unavailable/irrelevant for our customers) TIP: Install pre-commit hooks and run this check when committing (uses the Conventional Precommit Linter).
⚠️	The source branch "DNSServer-EDNS" incorrect format: contains uppercase letters. This can cause troubles on case-insensitive file systems (macOS). Please rename your branch.

👋 Hello Kolkman, we appreciate your contribution to this project!

---

📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more.

---

🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project.

---

Click to see more instructions ...

This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Resolve all warnings (⚠️ ) before requesting a review from human reviewers - they will appreciate it.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...

We do welcome contributions in the form of bug reports, feature requests and pull requests.

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against cfa5ac5

[me-no-dev]

this version is updated with every new release. Please revert

[Kolkman]

Reverted the change with new commit

[me-no-dev]

what are MNAME and RNAME? How are their values chosen?

[Kolkman]

MNAME and RNAME are values defined in RFC1035 section 3.1.13.

Tne MNAME is the name of the primary nameserver the request came from - that value is only used in the context of dynamic DNS updates RFC 2136. In this contexts it is purely informational. I chose the 'ns' label as that is sort of customary for nameservers.

The RNAME is used to construct the email address of the administrator for the zone. The way things are coded now that is esp32@local. There nothing in the DNS protocol that uses that value, it is only for network administrator information siganling purpose.

[me-no-dev]

.local is used by MDNS. Is that the best value?

[Kolkman]

Excellent question, because it surfaces a dilemma that I had.

Here are the considerations:

• The MNAME and RNAME are purely informational still you do not want something to be used on the real Internet.
• But, suppose that somebody queries a DNSSERVER on an ESP and decides to complain to the admin who is indicated in the RNAME. Then you don't want that mail to end up with an existing person somewhere on the Internet.
• The domains in the MNAME and RNAME could be based on the queryname, but that would be problematic for wildcard resolution, where the administrator email might become esp32@captive.apple.com, so I figure that is a bad choice.
• One has to pick a value that is never going to appear in the real DNS. The IANA maintains a registry of those.
• I think invalid might be a good choice. RFC 6761 says that trying to resolve invalid must always expected to fail. However - folk seeing 'invalid' in their soa record during troubleshooting might misinterpret what is happening.
• .local demonstrates that something local is going on - which is the most likely place where ESP32s get deployed.

As said, there is nothing in the DNS (except for dynamic updates) that will use the M- and RNAME values, hence no interference with MDNS - the values are purely used in the context of local troubleshooting.