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

Make IPv4-mapped support optional and embedded support obligatory #362

Merged
merged 3 commits into from Apr 12, 2022
Merged

Make IPv4-mapped support optional and embedded support obligatory #362

merged 3 commits into from Apr 12, 2022

Conversation

DesWurstes
Copy link
Contributor

@DesWurstes DesWurstes commented Feb 22, 2022

Resolves #361

Looking forward for your comments @wtoorop

@wtoorop
Copy link
Contributor

wtoorop commented Feb 22, 2022

Resolves #361

Looking forward for your comments @wtoorop

👍 for the corrected and added test vector.

Not sure about the added text in the "ipv4hint" and "ipv6hint" section. I don't think you should just say "For security reasons" without saying what those reasons are. I suppose it would also be fine to just leave that text out. What do you think @DesWurstes ? Did you have a specific attack vector in mind?

@davidben
Copy link
Contributor

Agreed on "security reasons". But also, spelled out in the text or not, what is the motivation for requiring clients reject those values? I didn't think we did such things in, e.g., AAAA records. (@ericorth to confirm.)

@bemasc
Copy link
Collaborator

bemasc commented Feb 22, 2022

Yeah, I don't think we should be trying to identify which addresses are and are not eligible to be included in a hint. As much as possible, I think they should be just like A and AAAA records. Clients might indeed reject certain values, but that rejection should not depend on whether the address arrived in an A record or an ipv4hint.

@DesWurstes DesWurstes changed the title Forbid IPv6 in IPv4-mapped addresses Make IPv4-mapped support optional and embedded support obligatory Feb 22, 2022
@DesWurstes
Copy link
Contributor Author

DesWurstes commented Feb 22, 2022

Updated. Now only embedded-v4 support is obligatory as @wtoorop suggests (to increase compat with dnsop). Or would you prefer to make it "SHOULD" as well? In that case I should remove the test case and add a note under "ipv6hint"

Co-authored-by: Benjamin M. Schwartz <bemasc@uproxy.org>
@enygren
Copy link
Collaborator

enygren commented Feb 22, 2022

Do we want an operational consideration recommending against using them? Having IPv4-mapped addresses in AAAA records is a common operational mistake.

@DesWurstes
Copy link
Contributor Author

DesWurstes commented Feb 22, 2022

Then how about "Nameservers SHOULD NOT use IPv4-mapped addresses in ipv6hint as the functioning of such addresses is highly implementation-dependent." ? I hope someone can come up with a better suggestion.

@davidben
Copy link
Contributor

Is there something about SVCB that changes the considerations here? No strong objections, but it seems to me we can assume this covered by existing stuff.

@marka63
Copy link

marka63 commented Feb 23, 2022

Really there is nothing new here. Nameservers already have to deal with mapped addresses in AAAA records. If your acl / connection logic doesn't handle mapped address already it is broken.

@enygren
Copy link
Collaborator

enygren commented Feb 23, 2022

I'll mail v6ops for guidance.

@bemasc
Copy link
Collaborator

bemasc commented Feb 23, 2022

I don't think this is specific to v4-mapped v6, or even v6 generally. IPv4 and IPv6 both have various special ranges that probably should not appear in the DNS, but the DNS is nonetheless capable of conveying them. I suspect that v4-mapped v6 is just one of many weird IP ranges that are commonly published due to an operational mistake.

@DesWurstes
Copy link
Contributor Author

DesWurstes commented Feb 23, 2022

Then I would suggest the following under ipv6hint:

The same considerations apply as when interacting with AAAA: The behavior toward IPv4-mapped addresses in IPv6 syntax is implementation defined, which might result in some clients generating IPv4 packets.

and maybe?

Nevertheless IPv4-mapped addresses SHOULD NOT be used with ipv6hint instead of with ipv4hint.

Although these will be treated exactly the same way in practice AAAA is treated it's good practice to remind programmers.

@bemasc
Copy link
Collaborator

bemasc commented Apr 8, 2022

I think this change is good as-is. In very long documents such as this, adding more guidance that should be obvious can make it harder to find the guidance that is not so obvious.

@enygren enygren merged commit b7a7e55 into MikeBishop:main Apr 12, 2022
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.

Why is IPv6 in IPv4-mapped format allowed?
6 participants