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

Add new HTTPS and SVCB record types #1402

Merged
merged 5 commits into from Mar 15, 2021
Merged

Add new HTTPS and SVCB record types #1402

merged 5 commits into from Mar 15, 2021

Conversation

bluejekyll
Copy link
Member

@bluejekyll bluejekyll commented Mar 7, 2021

@briansmith @djc @sayrer @moonshiner

Ok, had some time today, through these record types in.

Some notes, currently this does not validate the SvcbParamValue against the SvcbParamKey type.
Currently this is not supported in the Zone file for the server, but that shouldn't be too hard, so I think I'll leave this in draft until I can do that.

This can be tested with:

$> cargo install --git https://github.com/bluejekyll/trust-dns.git --branch https_and_svcb --bin resolve -- trust-dns-util
$> resolve -t HTTPS crypto.cloudflare.com
crypto.cloudflare.com. 273 IN HTTPS 1 . alpn=h2, ipv4hint=162.159.135.79,162.159.136.79, echconfig="/gkAQwATY2xvdWRmbGFyZS1lc25pLmNvbQAgSiSc2sO3T91QP7ZBkmxa4DBEHReUB0M5UXn3iFQpvW8AIAAEAAEAAQAAAAA=" ipv6hint=2606:4700:7::a29f:874f,2606:4700:7::a29f:884f,

see: https://github.com/MikeBishop/dns-alt-svc

fixes: #1323

@codecov
Copy link

codecov bot commented Mar 7, 2021

Codecov Report

Merging #1402 (d14af60) into main (5f68b7d) will decrease coverage by 1.43%.
The diff coverage is 67.09%.

@@            Coverage Diff             @@
##             main    #1402      +/-   ##
==========================================
- Coverage   85.10%   83.67%   -1.43%     
==========================================
  Files         153      167      +14     
  Lines       15042    15872     +830     
==========================================
+ Hits        12801    13280     +479     
- Misses       2241     2592     +351     

Copy link
Collaborator

@djc djc left a comment

Choose a reason for hiding this comment

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

LGTM so far.

@bluejekyll
Copy link
Member Author

Ok, I fixed all the SvcParamValue encoders and decoders. I think everything is fully functional now. For completeness, I want to add the Zone file parsers too before this is ready to merge.

@bluejekyll bluejekyll force-pushed the https_and_svcb branch 2 times, most recently from 1b7ba9c to 178347c Compare March 12, 2021 03:23
@bluejekyll bluejekyll marked this pull request as ready for review March 12, 2021 03:23
@bluejekyll
Copy link
Member Author

Ok, @djc, I think this is ready to merge... we have full end-to-end support now for server, client, and resolver now.

@djc
Copy link
Collaborator

djc commented Mar 15, 2021

There's just too much code here for me to closely review. I presume most of it is based on copy/pasting from other places? It's probably fine, we'll iron out any bugs as it starts getting used I suppose.

@bluejekyll
Copy link
Member Author

Sorry, for the amount of code... I tried to do it in stages to build up the entire thing... but yeah, in the end I don't see a way to make this smaller than handling all of these cases.

@bluejekyll
Copy link
Member Author

If it helps, I think it's all perfect ;)

@bluejekyll
Copy link
Member Author

just rebased.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for SVCB/HTTPS record type
2 participants