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.

Sign up for GitHub

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

Jump to bottom

draft-ietf-dnsop-dns-error-reporting #902

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open

draft-ietf-dnsop-dns-error-reporting #902

wants to merge 22 commits into from

Conversation

wtoorop
Copy link
Member

@wtoorop wtoorop commented Jun 20, 2023 •
edited by gthess
Loading

@wtoorop wtoorop changed the title Features/error reporting draft-ietf-dnsop-dns-error-reporting Jun 20, 2023
gthess added 13 commits July 20, 2024 19:40
@gthess
Merge branch 'master' into features/error-reporting-poc
a3f3da4
@gthess
Generated configparser and configlexer are not versioned in master an…
308847a 
…ymore
@gthess
- Remove NOERROR DNS Error Reporting; not part of final RFC.
ea17c0a 
- Use assigned IANA EDNS0 Option Code for Report-Channel.
@gthess
Fix buffer protection and agent domain validity
d509375
@gthess
Use DNS Error Reporting instead of the eder nickname
e9a4f31
@gthess
- Update documentation.
c472337
@gthess
- Fix typo.
d5d7a62
@gthess
- Bail out early if ede is not present.
7d9f2c9
@gthess
- Forget previous EDNS options from upstream; this is what was
56200cd 
  implicitly happening but not deterministacally.
@gthess
- Don't report LDNS_EDE_OTHER and bail early if there is no reporting
bb15329 
  agent.
@gthess
Merge branch 'master' into features/error-reporting-poc
1dc588d
@gthess
- Only do DNS error reporting when a client asked for something that
7de7fd5 
  went wrong.
@gthess
- Add an error reporting agent in the parent that should be ignored.
a79c06e
@gthess gthess marked this pull request as ready for review March 16, 2025 06:06
@gthess gthess requested a review from wcawijngaards March 16, 2025 06:06
@gthess
Copy link
Member

gthess commented Mar 16, 2025

I believe this is now ready for review.
There are no limits for error reporting.
Reports will be generated for client inited queries that gave an error during the mesh state.
This is mostly the validator at the moment. When EDEs for RPZ are introduced (blocked, censored, etc) it needs to be revisited (probably by excluding those codes from the report).
LDNS_EDE_OTHER is excluded from the reports because the code does not make sense by itself.
LDNS_EDE_STALE_ANSWER is not interfering with the report (it is generated when responding with stale data, not the same code path).

@gthess gthess self-assigned this Mar 19, 2025
@gthess gthess added this to the Future release milestone Mar 19, 2025
wcawijngaards
wcawijngaards approved these changes Mar 19, 2025
View reviewed changes
Copy link
Member

@wcawijngaards wcawijngaards left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks fine to include. The option set default off is a safe way to introduce the feature.

@gthess
Copy link
Member

gthess commented Mar 19, 2025

Thanks @wcawijngaards ! And nice catch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wcawijngaards wcawijngaards wcawijngaards approved these changes

Assignees

@gthess gthess

Labels
None yet
Projects
None yet
Milestone
Future release
Development

Successfully merging this pull request may close these issues.
3 participants
@wtoorop @gthess @wcawijngaards