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

[API Proposal]: Should we design a DnsClient abstract class? #102105

Open
xljiulang opened this issue May 11, 2024 · 8 comments
Open

[API Proposal]: Should we design a DnsClient abstract class? #102105

xljiulang opened this issue May 11, 2024 · 8 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net
Milestone

Comments

@xljiulang
Copy link

xljiulang commented May 11, 2024

Background and motivation

Today, network components such as HttpClient directly rely on the Dns static class that is deeply bound to the operating system. This means that if you want to change the DNS resolution of the application, you can only change the Dns configuration of the operating system.

API Proposal

public abstract class DnsClient
{
    public static DnsClient System { get; }

    public abstract Task<IPHostEntry> GetHostEntryAsync(string hostNameOrAddress, AddressFamily family, CancellationToken cancellationToken = default);

    public abstract Task<IPHostEntry> GetHostEntryAsync(string hostNameOrAddress, CancellationToken cancellationToken);
}

API Usage

var httpHandler = new SocketsHttpHandler
{
    DnsClient = new DohDnsClient("https://xxx.com/dns-query")
};
var httpClient = new HttpClient(httpHandler);
var smtpClient = new SmtpClient
{
    DnsClient = new CustomDnsClient("8.8.8.8")
};
var socket = new Socket(SocketType.Stream, ProtocolType.Tcp)
{
    DnsClient = DnsClient.System
};

Alternative Designs

No response

Risks

No response

@xljiulang xljiulang added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label May 11, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label May 11, 2024
@wfurt
Copy link
Member

wfurt commented May 11, 2024

For HTTP, you can generally use the ConnectCallback @xljiulang. AFAIK This is only one place where DNS would be used and you can completely customize that.

On Socket there is also API that takes array of IPAddresses, so doing custom resolution and passing the result there would be trivial IMHO.

Now some improvements around name resolutions are being discussed. This is perhaps related to #19443. But that covers just the API not integration with rest of the .NET

I'm wondering if you can elaborate more on the use cases.

@xljiulang
Copy link
Author

xljiulang commented May 13, 2024

@wfurt
I am very pleased that many of these network components support passing in IPAddress directly or indirectly, but it obviously does not conflict with the need to design a DnsClient abstract class and apply it to these network components.
For SocketsHttpHandler, although it has a custom connection method of ConnectCallback, without DnsClient, I think many developers cannot handle the following connection method well:

  1. http/1.1 and http2 connection
  2. http3 connection
  3. WebProxy connection
  4. Socks4 and 5 connection
  5. https WebProxy connection

@xljiulang
Copy link
Author

I have implemented the HostResolver function of SocketsHttpHandler in a private project. In order to implement custom host resolution, I wrote a total of about 1,000 lines of cs code.

public static IHttpMessageHandlerBuilder AddHostResolver<THostResolver>(this IHttpMessageHandlerBuilder builder)
     where THostResolver : HostResolver
{
     builder.Services.TryAddTransient<THostResolver>();
     return builder.AddHostResolver(serviceProvider => serviceProvider.GetRequiredService<THostResolver>());
}

@antonfirsov
Copy link
Member

http/1.1 and http2 connection

This works fine with ConnectCallback since it's agnostic about the underlying HTTP2/HTTP3 protocol. The job of ConnectCallback is to create a Stream for the communication.

WebProxy connection
https WebProxy connection
Socks4 and 5 connection

Similarly, I don't see a problem with these. Proxy and tunnel connections are plumbed through ConnectCallback.

http3 connection

It would be very challanging to integrate msquic with a managed DnsResolver since the code that handles the resolution attempts has to live in msquic to be efficient, see #82404 (comment). It is doable if msquic implemented some sort of callback, but IMHO it is very far on their roadmap (for now the next step is microsoft/msquic#1181). #64449 might enable a sub-optimal implementation on user side.

In order to implement custom host resolution, I wrote a total of about 1,000 lines of cs code.

We plan to eventually address #19443 so users can get rid of that code, however currently we are uncertain if we can land that feature in .NET 9.0. Have you considered using DnsClient.NET?

@xljiulang
Copy link
Author

@antonfirsov
Thank you for your answer. The difficulty of my problem is not to implement a DnsClient with complete functions and protocols, but that even with DnsClient, developers still have to write a lot of cs code to integrate it into network components such as SocketsHttpHandler. For example, although it is enough to create a Socks4 and 5 connection as the function's return Stream, developers have to implement the complex connection process of the Socks4 and 5 connection from scratch. Of course, the same is true for other proxy connection methods. Think about what our original needs are? Why did it become a variety of complex connections.

@wfurt
Copy link
Member

wfurt commented May 13, 2024

There still may be some value in the proposal IMHO. But again back to my question, why do you need custom resolution to start with @xljiulang? I can see that the ConnectCallback may be more complicated if you want to cover all the cases above.

One thing developers sometimes struggle with are 3rd party libraries where something happens under the cover. Is that something we should also consider? This is also one case where the callback is problematic.

@xljiulang
Copy link
Author

Since the DNS protocol is insecure, its resolution results are contaminated in some specific areas. In this case, the application needs a more secure Doh protocol or other custom resolution implementation, and the application cannot modify the system. DNS configuration.
To be honest, I don't want to know how SocketsHttpHandler's ConnectCallback should work. I just want it to provide a DNS resolver property to set the resolver.

@antonfirsov
Copy link
Member

antonfirsov commented May 14, 2024

We can keep this as a tracking issue for exposing an easier API to customize DNS resolution than ConnectCallback assuming #19443 will be implemented. Given that the other issue has priority, I'm triaging this to Future, @wfurt let me know if you disagree.

To be honest, I don't want to know how SocketsHttpHandler's ConnectCallback should work. I just want it to provide a DNS resolver property to set the resolver

If this is blocking you, I think you should give it a try.

although it is enough to create a Socks4 and 5 connection as the function's return Stream, developers have to implement the complex connection process of the Socks4 and 5 connection from scratch

This is not true. ConnectCallback is a relatively thin abstraction for DNS resolution and TCP connection. SocketsHttpHandler will deal with the proxy/tunnel establishment for you. The callback could be as simple as:

 using SocketsHttpHandler handler = new()
 {
     ConnectCallback = async (ctx, ct) =>
     {
         var s = new Socket(SocketType.Stream, ProtocolType.Tcp) { NoDelay = true };
         try
         {
             IPAddress[] addresses = await MyCustomResolver.ResolveIPAddressesAsync(ctx.DnsEndPoint.Host, ct);
             await s.ConnectAsync(addresses, ctx.DnsEndPoint.Port, ct);
             return new NetworkStream(s, ownsSocket: true);
         }
         catch
         {
             s.Dispose();
             throw;
         }
     }
 };

@antonfirsov antonfirsov removed the untriaged New issue has not been triaged by the area owner label May 14, 2024
@antonfirsov antonfirsov added this to the Future milestone May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net
Projects
None yet
Development

No branches or pull requests

3 participants