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

Implement support for SVCB and HTTPS records #452

Closed
wants to merge 1 commit into from
Closed

Implement support for SVCB and HTTPS records #452

wants to merge 1 commit into from

Conversation

ghedo
Copy link

@ghedo ghedo commented Apr 17, 2020

This implements the SVBC and HTTPS records as described in
https://tools.ietf.org/html/draft-ietf-dnsop-svcb-https-00

Given that the dnspython library is often used to test other
implementations, it is useful for it to implement this draft, even
though it's not finalized yet, to help with interop testing.

The wire and presentation formats for SVCB and HTTPS are identical,
so the base RD implementation is split into its own class, so that it
can be reused for all records that have the same format.


Note that this is a very early implementation of the draft. I'm publishing this code to allow others to test it (e.g. against their own implementations), but until more testing is done it's probably a good idea to keep the PR open.

@rthalley
Copy link
Owner

Thanks for submitting! I will leave this PR open so other early adopters can find it. Once it has an RFC and official code points, we can merge it.


return addrs

if key == "esniconfig":
Copy link
Collaborator

Choose a reason for hiding this comment

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

Needs update to latest draft version, the name has changed.

@codecov-commenter
Copy link

codecov-commenter commented Jun 25, 2020

Codecov Report

Merging #452 into master will increase coverage by 0.06%.
The diff coverage is 96.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #452      +/-   ##
==========================================
+ Coverage   95.35%   95.41%   +0.06%     
==========================================
  Files         102      105       +3     
  Lines        7427     7683     +256     
==========================================
+ Hits         7082     7331     +249     
- Misses        345      352       +7     
Impacted Files Coverage Δ
dns/rdtypes/svcbbase.py 96.17% <96.17%> (ø)
dns/rdatatype.py 100.00% <100.00%> (ø)
dns/rdtypes/IN/HTTPS.py 100.00% <100.00%> (ø)
dns/rdtypes/IN/SVCB.py 100.00% <100.00%> (ø)
dns/update.py 100.00% <0.00%> (ø)
dns/renderer.py 100.00% <0.00%> (ø)
dns/rdataclass.py 100.00% <0.00%> (ø)
dns/rdtypes/ANY/OPT.py 100.00% <0.00%> (ø)
dns/rdtypes/ANY/AFSDB.py 100.00% <0.00%> (ø)
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8cfbca6...fdbd28e. Read the comment docs.

@ghedo ghedo changed the title Implement support for SVCB and HTTPSSVC records Implement support for SVCB and HTTPS records Jun 25, 2020
@ghedo
Copy link
Author

ghedo commented Jun 25, 2020

Update with new names (HTTPSSVC -> HTTPS, esniconfig -> echconfig).

This implements the SVBC and HTTPS records as described in
https://tools.ietf.org/html/draft-ietf-dnsop-svcb-https-00

Given that the dnspython library is often used to test other
implementations, it is useful for it to implement this draft, even
though it's not finalized yet, to help with interop testing.

The wire and presentation formats for SVCB and HTTPS are identical,
so the base RD implementation is split into its own class, so that it
can be reused for all records that have the same format.
@ghedo ghedo mentioned this pull request Jun 30, 2020
@ghedo
Copy link
Author

ghedo commented Jun 30, 2020

FTR, SVCB and HTTPS now have official code-points:
https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-4

so I updated the PR to use those instead of the private-use ones.

@rthalley
Copy link
Owner

rthalley commented Jul 2, 2020

We did a massive overhaul of rdata wire processing today, making things much nicer. If you're ok with overhauling this PR to support the new scheme (look at from_wire_parser() in any rdata type), that's cool. If that's too much work, then I will update your patch once there is an RFC and then merge it.

@pspacek
Copy link
Collaborator

pspacek commented Aug 4, 2020

The wire format is now set in stone (as registered when allocating code point in https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml) so I think it would be handy to merge this before it becomes RFC, it would allow more people to experiment with SVCB/HTTPS records and also to write tests for other DNS software using dnspython.

@rthalley
Copy link
Owner

rthalley commented Aug 8, 2020

For a variety of reasons, including all the refactoring, I did my own implementation of these. Thank you for contributing though!

@rthalley rthalley closed this Aug 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants