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

Use SOCK_DNS extension on socket on OpenBSD #659

Merged
merged 3 commits into from Dec 17, 2023

Conversation

marty1885
Copy link
Contributor

@marty1885 marty1885 commented Dec 13, 2023

This patch added the SOCK_DNS flag when running on OpenBSD. Allowing a reduced set of pledge(2) promises. Before this patch. The "stdio rpath inet" promises must be used in order to resolve any records. After the patch. inet can be replaced with dns which only allows communication on destination port 53, instead of on all ports.

Side note: I checked the OpenBSD kernel source code. Even though the socket document says the DNS port (typically 53)., The OpenBSD 7.4 kernel only allows 53. Not sure why they say typically.

@bradh352
Copy link
Member

Interesting. That said, the manpage, https://man.openbsd.org/socket.2 doesn't say SOCK_DNS is limited to UDP. DNS will upgrade from UDP to TCP if it gets back a DNS response with the 'TC' bit set indicating the response was truncated due to UDP packet size limitations. Are you sure SOCK_DNS is really limited to UDP? It wouldn't make sense that it would be.

This patch added the SOCK_DNS flag when running on OpenBSD. Allowing a reduced set of pledge(2) promises. Before this patch. The "stdio rpath inet" promises must be used in order to resolve any records. After the patch. inet can be replaced with dns which only allows communication on destination port 53, instead of on all ports.
@marty1885
Copy link
Contributor Author

You are right. I forgot that DNS can run over TCP. Description edited. I also force pushed to update my commit message

@bradh352
Copy link
Member

Seems reasonable. I'll merge after the 1.24.0 release which has already been staged.

@bradh352 bradh352 merged commit e1c5994 into c-ares:main Dec 17, 2023
23 checks passed
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.

None yet

2 participants