-
-
Notifications
You must be signed in to change notification settings - Fork 971
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
LibDNS+LibWeb+Ladybird+RequestServer: Let there be DNS over TLS #2111
base: master
Are you sure you want to change the base?
Conversation
a38d166
to
73e401a
Compare
34f9268
to
884986a
Compare
ea0b8da
to
6920745
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment, otherwise lgtm. Beware Tim's "yeet Userland/" PR.
Ladybird/HelperProcess.cpp
Outdated
@@ -161,7 +161,19 @@ ErrorOr<NonnullRefPtr<Requests::RequestClient>> launch_request_server_process(Re | |||
arguments.append(server.value()); | |||
} | |||
|
|||
return launch_server_process<Requests::RequestClient>("RequestServer"sv, candidate_request_server_paths, move(arguments)); | |||
auto client = TRY(launch_server_process<Requests::RequestClient>("RequestServer"sv, candidate_request_server_paths, move(arguments))); | |||
WebView::Application::chrome_options().dns_settings.visit( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to do this via an explicit IPC message after process creation, or could it be added to the request server top-level arguments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a big proponent of not having to restart the application just to change your DNS settings.
We could add it to the command line parameters, but imo this is nicer (just one way to set DNS in RS)
54bac0a
to
1c8a3f3
Compare
This is a footgun with some massive bullets.
This just unifies the API for all three sockets (UDP, TCP and TLS)
This commit adds our own DNS resolver, with the aim of implementing DoT (and eventually DoH, maybe even DNSSEC etc.)
This can now happen due to the hostname not existing, as RS explicitly performs DNS resolution before setting up the response pipe.
In the future, we may want to parse /etc/hosts (or equivalent) into the cache; this commit only adds localhost to make the normal workflow work.
No description provided.