TLS Client authentication

[MarinoMtz]

Hello I added support for TLS Authentication. For now I tested it with my own DoH server and open ssl certificates. It should also work for DoT and DoQ but need further tested.

I tried my best to follow the comments from @ameshkov in my previous pull request (doauth).

BR
Ivan

[ameshkov]

Please read all comments before making changes.

Briefly:

1. Instead of TLSClientConfig we should pass TLSClientCertificates down to the upstreams. Passing the whole config seems unnecessary.
2. There are some naming inconsistencies that should be resolved.
3. There are no unit tests at all for this feature.

How to write a unit test. The tests that we have in the upstream right now are not ideal since they depend on remote servers.

For this feature, we'd better write a normal unit test that does not have any external dependencies.

Let's start with a DOT server. Here's what needs to be done:

1. Generate a server cert-key pair (see createServerTLSConfig in proxy_test.go)
2. Generate a client cert-key pair.
3. Create a net.Listener that listens to 127.0.0.1:0 (this way it will use an unoccupied port)
4. Grab the port from listener.Addr(), you'll use it to create an upstream
5. Create a tls.Listener with the tls.Config from steps 1 and 2. This tls.Config should have the following fields set:
   • Certificates, ClientAuth and ClientCAs -- so that it was verifying the client cert. See the example here: https://gist.github.com/xjdrew/97be3811966c8300b724deabc10e38e2#file-server-go-L29
6. This tls.Listener should accept the incoming connection, read the DNS message, check that it's valid and return some response.
7. Create an Upstream with InsecureSkipVerify set to true and TLS client auth enabled.
8. Send a DNS message through it and verify the response.

One thing to note. Instead of setting InsecureSkipVerify on step 6 it might be better to have an option to pass RootCAs to the the upstream's TLS config. Currently, it can be done via upstream.RootCAs property, but tbh this is a part of the old code that is not used anymore and can be removed. Consider extending upstream.Options and allowing passing RootCAs via it.

[ameshkov]

Why? We can test this rather easily with a unit-test for all three

[codecov-commenter]

Codecov Report

Merging #172 (4126b9a) into master (5956b6d) will increase coverage by 0.06%.
The diff coverage is 72.72%.

@@            Coverage Diff             @@
##           master     #172      +/-   ##
==========================================
+ Coverage   68.67%   68.74%   +0.06%     
==========================================
  Files          40       40              
  Lines        2369     2377       +8     
==========================================
+ Hits         1627     1634       +7     
- Misses        539      540       +1     
  Partials      203      203              

Impacted Files	Coverage Δ
proxy/config.go	47.61% <ø> (ø)
upstream/parallel.go	59.40% <ø> (ø)
upstream/upstream.go	80.20% <ø> (ø)
upstream/bootstrap.go	85.57% <57.14%> (-2.06%)	⬇️
proxy/upstreams.go	84.72% <100.00%> (+0.21%)	⬆️
proxy/server_udp.go	58.57% <0.00%> (+2.85%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5956b6d...4126b9a. Read the comment docs.