8000 Support new SVCB parameters: ohttp, dohpath, tls-supported-groups by zeha · Pull Request #15550 · PowerDNS/pdns · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Support new SVCB parameters: ohttp, dohpath, tls-supported-groups #15550

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
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Support new SVCB parameters: ohttp, dohpath, tls-supported-groups #15550

wants to merge 1 commit into from

Conversation

zeha
Copy link
Collaborator
@zeha zeha commented May 11, 2025 •
edited
Loading

Short description

#15479

TODO:

  • write more unit tests
  • find a working implementation to compare against, dig 9.20.8 and drill 1.8.4 do not understand the new types yet
  • fix the (preexisting) clang-tidy issues in test-svc_records_cc.cc
  • remove quotes around dohpath data

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@zeha zeha requested a review from Habbie May 11, 2025 16:18
github-advanced-security[bot]
github-advanced-security bot found potential problems May 11, 2025
View reviewed changes
miodvallat
miodvallat reviewed May 13, 2025
View reviewed changes
regression-tests/zones/example.com
@@ -20223,6 +20223,8 @@ foo1.svcb IN A 192.0.2.2 ; Should show up in additional
bar.svcb IN SVCB 1 . alpn=h2
bar.svcb IN SVCB 3 . alpn=h3 port=1500
bar.svcb IN SVCB 4 . alpn=h3 port="1501"
bar.svcb IN SVCB 5 . port="8004" no-default-alpn dohpath=/dns-query{?dns} ohttp tls-supported-groups=29,23
Copy link
Contributor

Choose a reason for hiding this comment

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

Some regression tests due to these changes not reported on the tinydns equivalent zone.
In order to fix this:

  • update the zone file in modules/tinydns/data
  • run /usr/sbin/tinydns-data to update the matching binary file
  • in regression-tests.nobackend, run ./runtests tinydns-data-check and update the expected_result file with the new hash.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

2025-05-13T18:55:23.6565952Z May 13 18:55:23 [TinyDNSBackend] Failed to parse record content for bar.svcb.example.com with type SVCB. Erroring out!
2025-05-13T18:55:23.6566362Z May 13 18:55:23 Exception building answer packet for bar.svcb.example.com/SVCB (record is shorter than SVCB lengthfield implies) sending out servfail

looks like i fucked up the encoding in the tinydns data file

Copy link
Contributor

Choose a reason for hiding this comment

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

Have you used the generate-data.sh script in modules/tinydnsbackend to perform the update?

@klaus-nicat
Copy link
Contributor

Any chance to get this into 5.0 release? In our tests this PR worked.

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

@miodvallat miodvallat miodvallat left review comments

@Habbie Habbie Awaiting requested review from Habbie

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
auth
49B4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zeha @klaus-nicat @miodvallat
0