dnsproxy icon indicating copy to clipboard operation
dnsproxy copied to clipboard

proxy: add support for HTTP-only listeners for DoH

Open gdm85 opened this issue 2 years ago • 8 comments

It is desirable to use the DoH endpoint even without TLS termination, for example in case of an external proxy (nginx) taking care of termination. This change allows to specify independent --http-port ports which use simple TCP listeners. It also contains a separate fix to print the arguments parsing error instead of exiting with exit code 1 and no output.

Fixes #146

gdm85 avatar Dec 22 '22 21:12 gdm85

Any update?

t-e-s-tweb avatar Aug 12 '23 01:08 t-e-s-tweb

Is h2c supported here?

I would like to see that. Nginx does not support it but Apache HTTP Server does that and would be something good for internal connections in load balancing.

Unsure if Caddy supports it too.

EchedelleLR avatar Sep 22 '23 08:09 EchedelleLR

@ameshkov

However, I'd prefer it to be made simpler. I don't think anyone will need to run both plain HTTP and HTTPS listeners at the same time. Internally, dnsproxy allows running DoH server with no TLS certificate. Could you please instead just expose this option via the command-line interface and not modify the internals?

  1. proxy/server_https.go p.TLSConfig.Clone() TLSConfig cannot to be nil when listenHTTP
  2. proxy/config.go validateListenAddrs TLSConfig cannot to be nil when HTTPSListenAddr is not nil

So exposing this option requires modifying the internals, not just on the command-line interface. If need be, I will try it.

leeeezp avatar Oct 31 '23 18:10 leeeezp

@leeeezp

Yes, let's modify the internals to allow plain HTTP server to run.

ameshkov avatar Nov 10 '23 10:11 ameshkov

@gdm85 I see you pushed new commits, are you actively working on this feature?

@ameshkov Looking at the code, it seems if we try to keep changes to internals to a minimum we end up with a lot of references to "https" when it could be http or https. For example, there's multiple log messages and function names that explicitly mention https in some form. If we want those to be accurate, I think we end up touching internals nonetheless. Another thing to take care of is handling HTTP/3 & plain HTTP. As in, you can't mix the two, but the code needs to be aware of it, meaning in validation it needs to check if the combination of those two options is specified. On the other hand, if we allow both HTTPS and HTTP to mix then we can avoid that issue. I understand that if it it was as simple as just specifying one option or the other then it would be much cleaner, but it might be we're introducing more complexity into the code, at least from what I could see so far. What do you think?

Javex avatar Jan 23 '24 10:01 Javex

@gdm85 I see you pushed new commits, are you actively working on this feature?

I rebased it, and then noticed the request for changes which I had unattended for more than a year (my bad). I was trying to figure out how to make the requested changes, and then I would post a reply.

gdm85 avatar Jan 23 '24 11:01 gdm85

On the other hand, if we allow both HTTPS and HTTP to mix then we can avoid that issue. I understand that if it it was as simple as just specifying one option or the other then it would be much cleaner, but it might be we're introducing more complexity into the code, at least from what I could see so far. What do you think?

I tend to agree; it might sound silly to run both, but there are use-cases where it might be handy, for example internal LAN traffic which can use DoH without TLS while at the same time having an external TLS-protected listener. I also prefer simpler setups for security reasons but - as far as the log lines can be made explicit about which endpoint was hit - I see no reason to limit the feature. I will wait for this discussion to continue before altering the PR in a more radical way.

gdm85 avatar Jan 23 '24 11:01 gdm85

Thanks for your work! I've already deployed some instances with http listener patch applied :)

for example internal LAN traffic which can use DoH without TLS

But I'll not agree with this statement, as DoH is explicitly requiring secure connection by its specification, currently no such client exists that will accept http doh endpoint (curl, browsers, ...) at all, AFAIK.

SalimTerryLi avatar Jan 23 '24 11:01 SalimTerryLi