From c066e9da1716a94c1237184691ca728751bcddda Mon Sep 17 00:00:00 2001 From: Martin Dobrev Date: Wed, 5 Aug 2026 11:41:53 +0100 Subject: [PATCH 1/2] FEAT: accept TLS connections on the proxy listener A client talks to the proxy in the clear by default, which puts the Proxy-Authorization credentials and the host name of every CONNECT request on the local network. WithListenerTLS makes the HTTP frontend accept TLS instead, so the client speaks the proxy protocol inside a TLS connection: what a browser calls an HTTPS proxy. Most of this already worked, since Serve takes the listener from its caller and goproxy hijacks a tls.Conn as happily as any other. What was missing was a way to ask for it without knowing that, a way to configure it from a file, and somewhere for the certificate to live. The option copies the configuration it is given and applies TLS 1.2 as a floor when none is set, so a caller cannot accidentally offer 1.0. Serve and ListenAndServe wrap the listener themselves. The command grows tls_cert_file and tls_key_file, which are needed together or not at all, and holds the pair behind GetCertificate so that a renewal is picked up on USR2 without restarting: the listener is already up, the connections already established keep the certificate they started with, and a pair that cannot be read leaves the one in force alone rather than taking the listener down. This decrypts nothing. What a client sends through CONNECT stays opaque; it is the hop to the proxy that is protected, not the traffic inside it. The SOCKS frontend is untouched, since SOCKS5 has no TLS convention, and so is the health endpoint. --- README.md | 44 ++++++ cmd/microproxy/certs.go | 63 +++++++++ cmd/microproxy/certs_test.go | 156 +++++++++++++++++++++ cmd/microproxy/config.go | 15 +++ cmd/microproxy/main.go | 33 ++++- options.go | 41 +++++- server.go | 15 ++- tls_test.go | 254 +++++++++++++++++++++++++++++++++++ 8 files changed, 618 insertions(+), 3 deletions(-) create mode 100644 cmd/microproxy/certs.go create mode 100644 cmd/microproxy/certs_test.go create mode 100644 tls_test.go diff --git a/README.md b/README.md index 9d73025..613d22b 100644 --- a/README.md +++ b/README.md @@ -137,6 +137,7 @@ configuration file. Below is a list of supported options. * `bind_ip="ip"` -- specify which IP will be used for outgoing connections. * `add_headers=[["header1", "value1"], ["header2", "value2"]...]` -- adds the specified headers to outgoing HTTP requests, this option will not work for HTTPS connections. * `read_timeout`/`write_timeout` -- how long a single read or write on a proxied connection may take, e.g. `"15m"`. A negative value disables the deadline. Default: 15 minutes. +* `tls_cert_file`/`tls_key_file` -- the proxy's own certificate and key. Setting them makes the proxy listener accept TLS connections instead of plain ones. Both are needed, or neither. * `enable_health_check="on"` -- track whether the proxy is serving requests successfully and expose the result at `/health`. Needs `http_listen`. * `http_listen="ip:port"` -- ip address and port the `/health` endpoint is served on. This is not a proxy listener; nothing is proxied there. * `health_failure_limit=N` -- how many consecutive failures make the proxy unhealthy. Default: 5. @@ -191,6 +192,49 @@ computing the digest it would have produced. When no credentials are configured, SOCKS clients are accepted without authentication and the network ACLs are the only thing limiting them. +### Encrypting the connection to the proxy + +By default a client talks to the proxy in the clear, which puts the +`Proxy-Authorization` credentials and the host names of every `CONNECT` request +on the local network. Setting `tls_cert_file` and `tls_key_file` makes the proxy +listener accept TLS, so the client speaks the proxy protocol inside a TLS +connection — what a browser calls an HTTPS proxy. + +```toml +listen="0.0.0.0:3128" +tls_cert_file="/etc/microproxy/proxy.crt" +tls_key_file="/etc/microproxy/proxy.key" +``` + +The certificate is the proxy's own, for the name its clients reach it by. It is +re-read on `USR2` along with the configuration, so a renewal is picked up +without a restart, and a pair that can't be read leaves the one in force in +place. Connections already established keep the certificate they started with. + +This does not decrypt anything: what a client sends through `CONNECT` stays +opaque to the proxy. It protects the hop between the client and the proxy, not +the traffic inside it. + +The SOCKS frontend is unaffected — SOCKS5 has no TLS convention — and so is the +`/health` endpoint, which stays plain HTTP on `http_listen`. + +Note that not every client can be configured to use an HTTPS proxy: browsers +generally can, through a PAC file or a command-line flag, while system-wide +proxy settings on some platforms only accept a plain one. Check before turning +it on for an existing deployment. + +In Go, the same thing is `WithListenerTLS`: + +```go +srv, err := microproxy.New(cfg, microproxy.WithListenerTLS(&tls.Config{ + GetCertificate: myCertSource.GetCertificate, +})) +``` + +`Serve` and `ListenAndServe` then wrap the listener themselves, so do not pass +an already wrapped one as well. TLS 1.2 is applied as a floor when the +configuration does not set one. + ### Health `enable_health_check="on"` makes the proxy count how its work goes: a run of diff --git a/cmd/microproxy/certs.go b/cmd/microproxy/certs.go new file mode 100644 index 0000000..4b42924 --- /dev/null +++ b/cmd/microproxy/certs.go @@ -0,0 +1,63 @@ +package main + +import ( + "crypto/tls" + "fmt" + "sync" +) + +// certificateReloader holds the proxy's own certificate and can replace it +// without restarting, which is what a 90-day certificate needs. +// +// tls.Config asks for the certificate on every handshake, so a reload takes +// effect on the next connection and the ones already established are left +// alone. +type certificateReloader struct { + certFile string + keyFile string + + mu sync.RWMutex + certificate *tls.Certificate +} + +// newCertificateReloader loads the pair once, so that a bad path or an +// unreadable key is reported at startup rather than on the first client. +func newCertificateReloader(certFile, keyFile string) (*certificateReloader, error) { + reloader := &certificateReloader{certFile: certFile, keyFile: keyFile} + + if err := reloader.reload(); err != nil { + return nil, err + } + + return reloader, nil +} + +// reload re-reads the pair from disk. A pair that can't be used leaves the one +// in force untouched, so a half-written file during renewal does not take the +// listener down. +func (r *certificateReloader) reload() error { + certificate, err := tls.LoadX509KeyPair(r.certFile, r.keyFile) + if err != nil { + return fmt.Errorf("couldn't load the certificate %v and key %v: %w", r.certFile, r.keyFile, err) + } + + r.mu.Lock() + defer r.mu.Unlock() + + r.certificate = &certificate + + return nil +} + +// tlsConfig serves the certificate in force at the time of each handshake. +func (r *certificateReloader) tlsConfig() *tls.Config { + return &tls.Config{ + MinVersion: tls.VersionTLS12, + GetCertificate: func(*tls.ClientHelloInfo) (*tls.Certificate, error) { + r.mu.RLock() + defer r.mu.RUnlock() + + return r.certificate, nil + }, + } +} diff --git a/cmd/microproxy/certs_test.go b/cmd/microproxy/certs_test.go new file mode 100644 index 0000000..20cfb9d --- /dev/null +++ b/cmd/microproxy/certs_test.go @@ -0,0 +1,156 @@ +package main + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "os" + "path/filepath" + "testing" + "time" +) + +// writeKeyPair puts a self-signed certificate and its key at the given paths, +// with serial as the way to tell one from another. +func writeKeyPair(t *testing.T, certPath, keyPath string, serial int64) { + t.Helper() + + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + + template := x509.Certificate{ + SerialNumber: big.NewInt(serial), + Subject: pkix.Name{CommonName: "microproxy"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + } + + der, err := x509.CreateCertificate(rand.Reader, &template, &template, &key.PublicKey, key) + if err != nil { + t.Fatal(err) + } + + certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) + if err := os.WriteFile(certPath, certPEM, 0o600); err != nil { + t.Fatal(err) + } + + keyDER, err := x509.MarshalECPrivateKey(key) + if err != nil { + t.Fatal(err) + } + + keyPEM := pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyDER}) + if err := os.WriteFile(keyPath, keyPEM, 0o600); err != nil { + t.Fatal(err) + } +} + +func serialOf(t *testing.T, reloader *certificateReloader) int64 { + t.Helper() + + certificate, err := reloader.tlsConfig().GetCertificate(nil) + if err != nil { + t.Fatal(err) + } + + leaf, err := x509.ParseCertificate(certificate.Certificate[0]) + if err != nil { + t.Fatal(err) + } + + return leaf.SerialNumber.Int64() +} + +// A renewed certificate has to be picked up without restarting the proxy. +func TestCertificateReloader(t *testing.T) { + dir := t.TempDir() + certPath := filepath.Join(dir, "proxy.crt") + keyPath := filepath.Join(dir, "proxy.key") + + writeKeyPair(t, certPath, keyPath, 1) + + reloader, err := newCertificateReloader(certPath, keyPath) + if err != nil { + t.Fatal(err) + } + + if serial := serialOf(t, reloader); serial != 1 { + t.Fatalf("expected the first certificate, got serial %v", serial) + } + + // renewal replaces the files underneath the running proxy + writeKeyPair(t, certPath, keyPath, 2) + + if serial := serialOf(t, reloader); serial != 1 { + t.Error("expected the certificate in force to be unchanged until a reload") + } + + if err := reloader.reload(); err != nil { + t.Fatal(err) + } + + if serial := serialOf(t, reloader); serial != 2 { + t.Errorf("expected the renewed certificate, got serial %v", serial) + } +} + +// A half-written or broken pair must not take the listener down. +func TestCertificateReloaderKeepsTheCurrentPairOnError(t *testing.T) { + dir := t.TempDir() + certPath := filepath.Join(dir, "proxy.crt") + keyPath := filepath.Join(dir, "proxy.key") + + writeKeyPair(t, certPath, keyPath, 1) + + reloader, err := newCertificateReloader(certPath, keyPath) + if err != nil { + t.Fatal(err) + } + + if err := os.WriteFile(certPath, []byte("half a certificate"), 0o600); err != nil { + t.Fatal(err) + } + + if err := reloader.reload(); err == nil { + t.Error("expected a broken pair to be reported") + } + + if serial := serialOf(t, reloader); serial != 1 { + t.Errorf("expected the working certificate to still be served, got serial %v", serial) + } +} + +// A pair that cannot be read at all is a startup error, not a surprise on the +// first client. +func TestCertificateReloaderReportsAMissingPair(t *testing.T) { + dir := t.TempDir() + + if _, err := newCertificateReloader( + filepath.Join(dir, "absent.crt"), filepath.Join(dir, "absent.key")); err == nil { + t.Error("expected a missing certificate to be reported at startup") + } +} + +// The two settings only make sense together. +func TestConfigRejectsAHalfConfiguredCertificate(t *testing.T) { + for name, contents := range map[string]string{ + "cert without key": "tls_cert_file = \"/tmp/proxy.crt\"\n", + "key without cert": "tls_key_file = \"/tmp/proxy.key\"\n", + } { + path := filepath.Join(t.TempDir(), "microproxy.toml") + if err := os.WriteFile(path, []byte(contents), 0o600); err != nil { + t.Fatal(err) + } + + if _, err := loadConfig(path); err == nil { + t.Errorf("%v: expected a configuration error", name) + } + } +} diff --git a/cmd/microproxy/config.go b/cmd/microproxy/config.go index 1e85920..efb9e8b 100644 --- a/cmd/microproxy/config.go +++ b/cmd/microproxy/config.go @@ -28,6 +28,17 @@ type fileConfig struct { // HTTPListen is where the /health endpoint is served. The proxy itself // does not listen there. HTTPListen string `toml:"http_listen"` + + // TLSCertFile and TLSKeyFile turn the proxy listener into a TLS one, so + // that clients speak the proxy protocol inside a TLS connection. They are + // the proxy's own certificate, for the name its clients reach it by. + TLSCertFile string `toml:"tls_cert_file"` + TLSKeyFile string `toml:"tls_key_file"` +} + +// tlsEnabled reports whether the proxy listener should speak TLS. +func (c *fileConfig) tlsEnabled() bool { + return c.TLSCertFile != "" || c.TLSKeyFile != "" } // loadConfig reads and validates a configuration file. It reports an error @@ -66,6 +77,10 @@ func (c *fileConfig) validate() error { return fmt.Errorf("'enable_health_check' needs 'http_listen' to serve the endpoint on") } + if c.tlsEnabled() && (c.TLSCertFile == "" || c.TLSKeyFile == "") { + return fmt.Errorf("'tls_cert_file' and 'tls_key_file' have to be given together") + } + return c.Config.Validate() } diff --git a/cmd/microproxy/main.go b/cmd/microproxy/main.go index 3c38e41..867cfe4 100644 --- a/cmd/microproxy/main.go +++ b/cmd/microproxy/main.go @@ -62,6 +62,10 @@ type proxy struct { // healthServer is the separate listener the /health endpoint is served // on, when the configuration asks for one. healthServer *http.Server + + // certificates holds the proxy's own certificate when the listener speaks + // TLS, so that a renewed one can be picked up without a restart. + certificates *certificateReloader } func run(conf *fileConfig, path string, verbose, insecure bool) error { @@ -94,12 +98,28 @@ func run(conf *fileConfig, path string, verbose, insecure bool) error { } options = append(options, microproxy.WithCredentials(credentials)) + var certificates *certificateReloader + + if conf.tlsEnabled() { + if certificates, err = newCertificateReloader(conf.TLSCertFile, conf.TLSKeyFile); err != nil { + return err + } + + options = append(options, microproxy.WithListenerTLS(certificates.tlsConfig())) + } + server, err := microproxy.New(conf.Config, options...) if err != nil { return err } - command := &proxy{server: server, activity: activity, access: access, path: path} + command := &proxy{ + server: server, + activity: activity, + access: access, + path: path, + certificates: certificates, + } command.handleSignals() activity.Printf("starting proxy\n") @@ -240,5 +260,16 @@ func (p *proxy) reload() { return } + // The certificate is re-read from the paths the running proxy started + // with: the listener is already up, so a reload can't move it to a + // different file, and a renewal replaces the contents in place anyway. + if p.certificates != nil { + if err := p.certificates.reload(); err != nil { + p.activity.Printf("ERROR: couldn't reload the certificate, keeping the current one: %v\n", err) + } else { + p.activity.Printf("certificate reloaded\n") + } + } + p.activity.Printf("configuration reloaded\n") } diff --git a/options.go b/options.go index bbdecd9..764a3b6 100644 --- a/options.go +++ b/options.go @@ -68,8 +68,47 @@ func WithDialer(dialer ContextDialer) Option { } } +// WithListenerTLS makes the HTTP frontend accept TLS connections: clients +// connect to the proxy over TLS and speak the proxy protocol inside it, which +// is what a browser calls an "HTTPS proxy". +// +// This keeps the Proxy-Authorization credentials and the host names of CONNECT +// requests off the local network, which a plain proxy sends in the clear. It +// does not decrypt anything a client tunnels: what goes through CONNECT is +// still opaque to the proxy. +// +// The certificate is the proxy's own, for the name its clients reach it by. Use +// tls.Config.GetCertificate to serve one that can be replaced without a +// restart. A MinVersion of TLS 1.2 is applied when config does not set one, and +// config is copied, so later changes to it are not picked up. +// +// This is the server side. WithTLSClientConfig is the unrelated client side, +// used when this proxy talks to an https:// upstream. +// +// The SOCKS frontend is unaffected: SOCKS5 has no TLS convention and no client +// speaks one. +func WithListenerTLS(config *tls.Config) Option { + return func(s *Server) error { + if config == nil { + s.listenerTLS = nil + + return nil + } + + listenerTLS := config.Clone() + if listenerTLS.MinVersion == 0 { + listenerTLS.MinVersion = tls.VersionTLS12 + } + + s.listenerTLS = listenerTLS + + return nil + } +} + // WithTLSClientConfig is the TLS configuration used when the proxy itself -// speaks TLS, which is to an https:// upstream proxy. +// speaks TLS as a client, which is to an https:// upstream proxy. See +// WithListenerTLS for the server side. func WithTLSClientConfig(config *tls.Config) Option { return func(s *Server) error { s.tlsConfig = config diff --git a/server.go b/server.go index 968e3e1..f76aafb 100644 --- a/server.go +++ b/server.go @@ -50,6 +50,11 @@ type Server struct { verbose bool health *Health + // listenerTLS, when set, makes the HTTP frontend accept TLS connections + // rather than plain ones. It is the server side, and has nothing to do + // with tlsConfig, which is how this proxy talks to an https:// upstream. + listenerTLS *tls.Config + envRoutes routeCache // localAddr is where outgoing connections are made from. It comes from @@ -185,13 +190,21 @@ func (s *Server) Handler() http.Handler { } // Serve accepts connections on l until it is closed or Shutdown is called. +// +// When WithListenerTLS is in force, l is wrapped so that clients speak the +// proxy protocol inside a TLS connection. Do not pass an already wrapped +// listener as well. func (s *Server) Serve(l net.Listener) error { + if s.listenerTLS != nil { + l = tls.NewListener(l, s.listenerTLS) + } + server, err := s.startServing(l) if err != nil { return err } - s.log.Printf("starting proxy on %v\n", l.Addr()) + s.log.Printf("starting proxy on %v (tls: %v)\n", l.Addr(), s.listenerTLS != nil) return server.Serve(l) } diff --git a/tls_test.go b/tls_test.go new file mode 100644 index 0000000..ff243d9 --- /dev/null +++ b/tls_test.go @@ -0,0 +1,254 @@ +package microproxy + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/base64" + "math/big" + "net" + "net/http" + "net/http/httptest" + "net/url" + "testing" + "time" +) + +// selfSigned issues a certificate for 127.0.0.1 that a test client can pin, +// standing in for the proxy's own certificate. +func selfSigned(t *testing.T, commonName string) (tls.Certificate, *x509.CertPool) { + t.Helper() + + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + + template := x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: commonName, Organization: []string{"microproxy test"}}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + IPAddresses: []net.IP{net.ParseIP("127.0.0.1"), net.IPv6loopback}, + IsCA: true, + BasicConstraintsValid: true, + } + + der, err := x509.CreateCertificate(rand.Reader, &template, &template, &key.PublicKey, key) + if err != nil { + t.Fatal(err) + } + + parsed, err := x509.ParseCertificate(der) + if err != nil { + t.Fatal(err) + } + + pool := x509.NewCertPool() + pool.AddCert(parsed) + + return tls.Certificate{Certificate: [][]byte{der}, PrivateKey: key, Leaf: parsed}, pool +} + +// newTLSProxy serves the proxy over TLS on an ephemeral port and returns a +// client that reaches the internet through it. +func newTLSProxy(t *testing.T, cfg Config, tlsConfig *tls.Config, pool *x509.CertPool, opts ...Option) (*Server, *http.Client) { + t.Helper() + + server, err := New(cfg, append(opts, WithListenerTLS(tlsConfig))...) + if err != nil { + t.Fatalf("couldn't create the proxy: %v", err) + } + + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + + served := make(chan error, 1) + go func() { served <- server.Serve(listener) }() + + t.Cleanup(func() { + _ = server.Close() + <-served + }) + + // Serve wraps the listener, so its address is where the client connects. + proxyURL, err := url.Parse("https://" + listener.Addr().String()) + if err != nil { + t.Fatal(err) + } + + client := &http.Client{Transport: &http.Transport{ + Proxy: http.ProxyURL(proxyURL), + TLSClientConfig: &tls.Config{ + RootCAs: pool, + MinVersion: tls.VersionTLS12, + // the target servers in these tests are self-signed too + InsecureSkipVerify: true, //nolint:gosec // the proxy's own certificate is pinned through RootCAs + }, + }} + + return server, client +} + +// A client has to be able to speak the proxy protocol inside a TLS connection, +// for a plain target and for a tunnelled one. +func TestListenerTLS(t *testing.T) { + plain := httptest.NewServer(constantHandler("plain-ok")) + defer plain.Close() + + secure := httptest.NewTLSServer(constantHandler("tls-ok")) + defer secure.Close() + + certificate, pool := selfSigned(t, "127.0.0.1") + + _, client := newTLSProxy(t, + Config{AllowedConnectPorts: []int{portOf(t, secure.URL)}}, + &tls.Config{Certificates: []tls.Certificate{certificate}}, //nolint:gosec // MinVersion is applied by WithListenerTLS + pool) + + for name, test := range map[string]struct{ url, expected string }{ + "plain target": {plain.URL, "plain-ok"}, + "tunnelled target": {secure.URL, "tls-ok"}, + } { + resp, err := client.Get(test.url) + if err != nil { + t.Errorf("%v: %v", name, err) + + continue + } + + if body := readBody(t, resp); body != test.expected { + t.Errorf("%v: expected %q, got %q", name, test.expected, body) + } + } +} + +// Encrypting the hop to the proxy must not stop it authenticating the client; +// that is the main reason to encrypt it. +func TestListenerTLSStillAuthenticates(t *testing.T) { + background := httptest.NewServer(constantHandler("hello")) + defer background.Close() + + certificate, pool := selfSigned(t, "127.0.0.1") + + _, client := newTLSProxy(t, Config{}, + &tls.Config{Certificates: []tls.Certificate{certificate}}, //nolint:gosec // MinVersion is applied by WithListenerTLS + pool, + WithCredentials(testBasicUsers())) + + resp, err := client.Get(background.URL) + if err != nil { + t.Fatal(err) + } + resp.Body.Close() + + if resp.StatusCode != http.StatusProxyAuthRequired { + t.Fatalf("expected 407 without credentials, got %v", resp.Status) + } + + req, err := http.NewRequest(http.MethodGet, background.URL, http.NoBody) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Proxy-Authorization", + "Basic "+base64.StdEncoding.EncodeToString([]byte(user+":"+password))) + + resp, err = client.Do(req) + if err != nil { + t.Fatal(err) + } + + if body := readBody(t, resp); body != "hello" { + t.Errorf("expected 'hello', got %q", body) + } +} + +// A plaintext client talking to a TLS listener has to fail rather than be +// served, so that nobody is silently downgraded. +func TestListenerTLSRefusesPlaintext(t *testing.T) { + background := httptest.NewServer(constantHandler("hello")) + defer background.Close() + + certificate, _ := selfSigned(t, "127.0.0.1") + + server, err := New(Config{}, WithListenerTLS(&tls.Config{ //nolint:gosec // MinVersion is applied by WithListenerTLS + Certificates: []tls.Certificate{certificate}, + })) + if err != nil { + t.Fatal(err) + } + + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + + served := make(chan error, 1) + go func() { served <- server.Serve(listener) }() + + t.Cleanup(func() { + _ = server.Close() + <-served + }) + + proxyURL, err := url.Parse("http://" + listener.Addr().String()) + if err != nil { + t.Fatal(err) + } + + plaintext := &http.Client{Transport: &http.Transport{Proxy: http.ProxyURL(proxyURL)}} + + if _, err := plaintext.Get(background.URL); err == nil { + t.Error("expected a plaintext client to be refused by a TLS listener") + } +} + +// The option must not keep a handle on the caller's configuration, and has to +// insist on a floor for the protocol version. +func TestListenerTLSCopiesTheConfigAndSetsAFloor(t *testing.T) { + certificate, _ := selfSigned(t, "127.0.0.1") + + given := &tls.Config{Certificates: []tls.Certificate{certificate}} //nolint:gosec // that is what is being tested + + server, err := New(Config{}, WithListenerTLS(given)) + if err != nil { + t.Fatal(err) + } + + if server.listenerTLS.MinVersion != tls.VersionTLS12 { + t.Errorf("expected TLS 1.2 to be the floor, got %v", server.listenerTLS.MinVersion) + } + + if given.MinVersion != 0 { + t.Error("the caller's configuration was modified") + } + + // a floor the caller did set has to be respected + server, err = New(Config{}, WithListenerTLS(&tls.Config{ + Certificates: []tls.Certificate{certificate}, + MinVersion: tls.VersionTLS13, + })) + if err != nil { + t.Fatal(err) + } + + if server.listenerTLS.MinVersion != tls.VersionTLS13 { + t.Errorf("expected the configured floor to be kept, got %v", server.listenerTLS.MinVersion) + } +} + +// Without the option the listener stays plaintext, so nobody has to opt out. +func TestListenerIsPlaintextByDefault(t *testing.T) { + server := newTestServer(t, Config{}) + + if server.listenerTLS != nil { + t.Error("expected the listener to be plaintext by default") + } +} From 360d781764b3a0f95ebcc03572df1c3ad3a6e556 Mon Sep 17 00:00:00 2001 From: Martin Dobrev Date: Wed, 5 Aug 2026 11:56:12 +0100 Subject: [PATCH 2/2] FIX: verify the proxy's certificate in the TLS listener tests The client these tests proxy through set InsecureSkipVerify alongside RootCAs, which switches verification off entirely and leaves the pool doing nothing. The comment claiming the certificate was pinned was wrong, and the tests would have passed against any certificate at all: they showed that bytes flowed, not that TLS was done correctly. Verification is on now. It was off because one tls.Config covers both hops and the tunnelled target is self-signed too, so the fix is to trust the target's issuer as well rather than to trust nothing: httptest exposes it through Server.Certificate. A test is added for the property that was missing, where the client is given a pool that trusts a different issuer and has to refuse the connection with an unknown-authority error. With InsecureSkipVerify put back it fails, so it pins what it claims to. Reported by Copilot on #27. --- tls_test.go | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/tls_test.go b/tls_test.go index ff243d9..3580028 100644 --- a/tls_test.go +++ b/tls_test.go @@ -8,6 +8,7 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/base64" + "errors" "math/big" "net" "net/http" @@ -84,13 +85,14 @@ func newTLSProxy(t *testing.T, cfg Config, tlsConfig *tls.Config, pool *x509.Cer t.Fatal(err) } + // Verification stays on, so that the test fails if the proxy ever presents + // a certificate other than the one it was given. pool has to carry the + // targets' issuers too, since one tls.Config covers both hops. client := &http.Client{Transport: &http.Transport{ Proxy: http.ProxyURL(proxyURL), TLSClientConfig: &tls.Config{ RootCAs: pool, MinVersion: tls.VersionTLS12, - // the target servers in these tests are self-signed too - InsecureSkipVerify: true, //nolint:gosec // the proxy's own certificate is pinned through RootCAs }, }} @@ -108,6 +110,10 @@ func TestListenerTLS(t *testing.T) { certificate, pool := selfSigned(t, "127.0.0.1") + // the tunnelled target is self-signed too, and one tls.Config covers both + // hops, so its issuer joins the proxy's in the same pool + pool.AddCert(secure.Certificate()) + _, client := newTLSProxy(t, Config{AllowedConnectPorts: []int{portOf(t, secure.URL)}}, &tls.Config{Certificates: []tls.Certificate{certificate}}, //nolint:gosec // MinVersion is applied by WithListenerTLS @@ -244,6 +250,33 @@ func TestListenerTLSCopiesTheConfigAndSetsAFloor(t *testing.T) { } } +// The client in these tests has to be actually verifying the proxy, or they +// would pass against any certificate at all. A client that does not trust the +// issuer must fail to get through. +func TestListenerTLSCertificateIsVerified(t *testing.T) { + background := httptest.NewServer(constantHandler("hello")) + defer background.Close() + + certificate, _ := selfSigned(t, "127.0.0.1") + + // a pool that trusts something else entirely + _, otherPool := selfSigned(t, "127.0.0.1") + + _, client := newTLSProxy(t, Config{}, + &tls.Config{Certificates: []tls.Certificate{certificate}}, //nolint:gosec // MinVersion is applied by WithListenerTLS + otherPool) + + _, err := client.Get(background.URL) + if err == nil { + t.Fatal("expected the client to refuse a proxy certificate it does not trust") + } + + var unknownAuthority x509.UnknownAuthorityError + if !errors.As(err, &unknownAuthority) { + t.Errorf("expected an unknown-authority error, got %v", err) + } +} + // Without the option the listener stays plaintext, so nobody has to opt out. func TestListenerIsPlaintextByDefault(t *testing.T) { server := newTestServer(t, Config{})