From 2c1b66315620fda3311f9bdffd0867de1c71dc9e Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Tue, 7 Apr 2020 08:31:52 -0600 Subject: reverseproxy: Remove NTLM transport; refactor and improve docs --- modules/caddyhttp/reverseproxy/caddyfile.go | 26 +-- modules/caddyhttp/reverseproxy/httptransport.go | 126 +++++++++--- modules/caddyhttp/reverseproxy/ntlm.go | 244 ------------------------ modules/caddyhttp/reverseproxy/reverseproxy.go | 15 +- 4 files changed, 120 insertions(+), 291 deletions(-) delete mode 100644 modules/caddyhttp/reverseproxy/ntlm.go diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index cefb5b6..9636936 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -551,26 +551,20 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // verify transport configuration, and finally encode it if transport != nil { - // TODO: these two cases are identical, but I don't know how to reuse the code - switch ht := transport.(type) { - case *HTTPTransport: - if commonScheme == "https" && ht.TLS == nil { - ht.TLS = new(TLSConfig) - } - if ht.TLS != nil && commonScheme == "http" { - return d.Errf("upstream address scheme is HTTP but transport is configured for HTTP+TLS (HTTPS)") - } - - case *NTLMTransport: - if commonScheme == "https" && ht.TLS == nil { - ht.TLS = new(TLSConfig) + if te, ok := transport.(TLSTransport); ok { + if commonScheme == "https" && !te.TLSEnabled() { + err := te.EnableTLS(new(TLSConfig)) + if err != nil { + return err + } } - if ht.TLS != nil && commonScheme == "http" { + if commonScheme == "http" && te.TLSEnabled() { return d.Errf("upstream address scheme is HTTP but transport is configured for HTTP+TLS (HTTPS)") } + } else if commonScheme == "https" { + return d.Errf("upstreams are configured for HTTPS but transport module does not support TLS: %T", transport) } - - if !reflect.DeepEqual(transport, new(HTTPTransport)) { + if !reflect.DeepEqual(transport, reflect.New(reflect.TypeOf(transport).Elem()).Interface()) { h.TransportRaw = caddyconfig.JSONModuleObject(transport, "protocol", transportModuleName, nil) } } diff --git a/modules/caddyhttp/reverseproxy/httptransport.go b/modules/caddyhttp/reverseproxy/httptransport.go index fa73f9d..c0d9c8d 100644 --- a/modules/caddyhttp/reverseproxy/httptransport.go +++ b/modules/caddyhttp/reverseproxy/httptransport.go @@ -42,19 +42,50 @@ type HTTPTransport struct { // able to borrow/use at least some of these config fields; if so, // maybe move them into a type called CommonTransport and embed it? - TLS *TLSConfig `json:"tls,omitempty"` - KeepAlive *KeepAlive `json:"keep_alive,omitempty"` - Compression *bool `json:"compression,omitempty"` - MaxConnsPerHost int `json:"max_conns_per_host,omitempty"` - DialTimeout caddy.Duration `json:"dial_timeout,omitempty"` - FallbackDelay caddy.Duration `json:"dial_fallback_delay,omitempty"` + // Configures TLS to the upstream. Setting this to an empty struct + // is sufficient to enable TLS with reasonable defaults. + TLS *TLSConfig `json:"tls,omitempty"` + + // Configures HTTP Keep-Alive (enabled by default). Should only be + // necessary if rigorous testing has shown that tuning this helps + // improve performance. + KeepAlive *KeepAlive `json:"keep_alive,omitempty"` + + // Whether to enable compression to upstream. Default: true + Compression *bool `json:"compression,omitempty"` + + // Maximum number of connections per host. Default: 0 (no limit) + MaxConnsPerHost int `json:"max_conns_per_host,omitempty"` + + // How long to wait before timing out trying to connect to + // an upstream. + DialTimeout caddy.Duration `json:"dial_timeout,omitempty"` + + // How long to wait before spawning an RFC 6555 Fast Fallback + // connection. A negative value disables this. + FallbackDelay caddy.Duration `json:"dial_fallback_delay,omitempty"` + + // How long to wait for reading response headers from server. ResponseHeaderTimeout caddy.Duration `json:"response_header_timeout,omitempty"` + + // The length of time to wait for a server's first response + // headers after fully writing the request headers if the + // request has a header "Expect: 100-continue". ExpectContinueTimeout caddy.Duration `json:"expect_continue_timeout,omitempty"` - MaxResponseHeaderSize int64 `json:"max_response_header_size,omitempty"` - WriteBufferSize int `json:"write_buffer_size,omitempty"` - ReadBufferSize int `json:"read_buffer_size,omitempty"` - Versions []string `json:"versions,omitempty"` + // The maximum bytes to read from response headers. + MaxResponseHeaderSize int64 `json:"max_response_header_size,omitempty"` + + // The size of the write buffer in bytes. + WriteBufferSize int `json:"write_buffer_size,omitempty"` + + // The size of the read buffer in bytes. + ReadBufferSize int `json:"read_buffer_size,omitempty"` + + // The versions of HTTP to support. Default: ["1.1", "2"] + Versions []string `json:"versions,omitempty"` + + // The pre-configured underlying HTTP transport. Transport *http.Transport `json:"-"` } @@ -73,7 +104,7 @@ func (h *HTTPTransport) Provision(_ caddy.Context) error { h.Versions = []string{"1.1", "2"} } - rt, err := h.newTransport() + rt, err := h.NewTransport() if err != nil { return err } @@ -82,7 +113,9 @@ func (h *HTTPTransport) Provision(_ caddy.Context) error { return nil } -func (h *HTTPTransport) newTransport() (*http.Transport, error) { +// NewTransport builds a standard-lib-compatible +// http.Transport value from h. +func (h *HTTPTransport) NewTransport() (*http.Transport, error) { dialer := &net.Dialer{ Timeout: time.Duration(h.DialTimeout), FallbackDelay: time.Duration(h.FallbackDelay), @@ -148,14 +181,14 @@ func (h *HTTPTransport) newTransport() (*http.Transport, error) { // RoundTrip implements http.RoundTripper. func (h *HTTPTransport) RoundTrip(req *http.Request) (*http.Response, error) { - h.setScheme(req) + h.SetScheme(req) return h.Transport.RoundTrip(req) } -// setScheme ensures that the outbound request req +// SetScheme ensures that the outbound request req // has the scheme set in its URL; the underlying // http.Transport requires a scheme to be set. -func (h *HTTPTransport) setScheme(req *http.Request) { +func (h *HTTPTransport) SetScheme(req *http.Request) { if req.URL.Scheme == "" { req.URL.Scheme = "http" if h.TLS != nil { @@ -164,6 +197,17 @@ func (h *HTTPTransport) setScheme(req *http.Request) { } } +// TLSEnabled returns true if TLS is enabled. +func (h HTTPTransport) TLSEnabled() bool { + return h.TLS != nil +} + +// EnableTLS enables TLS on the transport. +func (h *HTTPTransport) EnableTLS(base *TLSConfig) error { + h.TLS = base + return nil +} + // Cleanup implements caddy.CleanerUpper and closes any idle connections. func (h HTTPTransport) Cleanup() error { if h.Transport == nil { @@ -173,18 +217,32 @@ func (h HTTPTransport) Cleanup() error { return nil } -// TLSConfig holds configuration related to the -// TLS configuration for the transport/client. +// TLSConfig holds configuration related to the TLS configuration for the +// transport/client. type TLSConfig struct { + // Optional list of base64-encoded DER-encoded CA certificates to trust. RootCAPool []string `json:"root_ca_pool,omitempty"` - // Added to the same pool as above, but brought in from files + + // List of PEM-encoded CA certificate files to add to the same trust + // store as RootCAPool (or root_ca_pool in the JSON). RootCAPEMFiles []string `json:"root_ca_pem_files,omitempty"` - // TODO: Should the client cert+key config use caddytls.CertificateLoader modules? - ClientCertificateFile string `json:"client_certificate_file,omitempty"` - ClientCertificateKeyFile string `json:"client_certificate_key_file,omitempty"` - InsecureSkipVerify bool `json:"insecure_skip_verify,omitempty"` - HandshakeTimeout caddy.Duration `json:"handshake_timeout,omitempty"` - ServerName string `json:"server_name,omitempty"` + + // PEM-encoded client certificate filename to present to servers. + ClientCertificateFile string `json:"client_certificate_file,omitempty"` + + // PEM-encoded key to use with the client certificate. + ClientCertificateKeyFile string `json:"client_certificate_key_file,omitempty"` + + // If true, TLS verification of server certificates will be disabled. + // This is insecure and may be removed in the future. Do not use this + // option except in testing or local development environments. + InsecureSkipVerify bool `json:"insecure_skip_verify,omitempty"` + + // The duration to allow a TLS handshake to a server. + HandshakeTimeout caddy.Duration `json:"handshake_timeout,omitempty"` + + // The server name (SNI) to use in TLS handshakes. + ServerName string `json:"server_name,omitempty"` } // MakeTLSClientConfig returns a tls.Config usable by a client to a backend. @@ -244,11 +302,20 @@ func (t TLSConfig) MakeTLSClientConfig() (*tls.Config, error) { // KeepAlive holds configuration pertaining to HTTP Keep-Alive. type KeepAlive struct { - Enabled *bool `json:"enabled,omitempty"` - ProbeInterval caddy.Duration `json:"probe_interval,omitempty"` - MaxIdleConns int `json:"max_idle_conns,omitempty"` - MaxIdleConnsPerHost int `json:"max_idle_conns_per_host,omitempty"` - IdleConnTimeout caddy.Duration `json:"idle_timeout,omitempty"` // how long should connections be kept alive when idle + // Whether HTTP Keep-Alive is enabled. Default: true + Enabled *bool `json:"enabled,omitempty"` + + // How often to probe for liveness. + ProbeInterval caddy.Duration `json:"probe_interval,omitempty"` + + // Maximum number of idle connections. + MaxIdleConns int `json:"max_idle_conns,omitempty"` + + // Maximum number of idle connections per upstream host. + MaxIdleConnsPerHost int `json:"max_idle_conns_per_host,omitempty"` + + // How long connections should be kept alive when idle. + IdleConnTimeout caddy.Duration `json:"idle_timeout,omitempty"` } // decodeBase64DERCert base64-decodes, then DER-decodes, certStr. @@ -278,4 +345,5 @@ var ( _ caddy.Provisioner = (*HTTPTransport)(nil) _ http.RoundTripper = (*HTTPTransport)(nil) _ caddy.CleanerUpper = (*HTTPTransport)(nil) + _ TLSTransport = (*HTTPTransport)(nil) ) diff --git a/modules/caddyhttp/reverseproxy/ntlm.go b/modules/caddyhttp/reverseproxy/ntlm.go deleted file mode 100644 index 270135a..0000000 --- a/modules/caddyhttp/reverseproxy/ntlm.go +++ /dev/null @@ -1,244 +0,0 @@ -// Copyright 2015 Matthew Holt and The Caddy Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package reverseproxy - -import ( - "context" - "fmt" - "net" - "net/http" - "strings" - "sync" - - "github.com/caddyserver/caddy/v2" - "github.com/caddyserver/caddy/v2/modules/caddyhttp" -) - -func init() { - caddy.RegisterModule(NTLMTransport{}) -} - -// NTLMTransport proxies HTTP with NTLM authentication. -// It basically wraps HTTPTransport so that it is compatible with -// NTLM's HTTP-hostile requirements. Specifically, it will use -// HTTPTransport's single, default *http.Transport for all requests -// (unless the client's connection is already mapped to a different -// transport) until a request comes in with an Authorization header -// that has "NTLM" or "Negotiate"; when that happens, NTLMTransport -// maps the client's connection (by its address, req.RemoteAddr) -// to a new transport that is used only by that downstream conn. -// When the upstream connection is closed, the mapping is deleted. -// This preserves NTLM authentication contexts by ensuring that -// client connections use the same upstream connection. It does -// hurt performance a bit, but that's NTLM for you. -// -// This transport also forces HTTP/1.1 and Keep-Alives in order -// for NTLM to succeed. -// -// It is basically the same thing as -// [nginx's paid ntlm directive](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#ntlm) -// (but is free in Caddy!). -type NTLMTransport struct { - *HTTPTransport - - transports map[string]*http.Transport - transportsMu *sync.RWMutex -} - -// CaddyModule returns the Caddy module information. -func (NTLMTransport) CaddyModule() caddy.ModuleInfo { - return caddy.ModuleInfo{ - ID: "http.reverse_proxy.transport.http_ntlm", - New: func() caddy.Module { - m := new(NTLMTransport) - m.HTTPTransport = new(HTTPTransport) - return m - }, - } -} - -// Provision sets up the transport module. -func (n *NTLMTransport) Provision(ctx caddy.Context) error { - n.transports = make(map[string]*http.Transport) - n.transportsMu = new(sync.RWMutex) - - if n.HTTPTransport == nil { - n.HTTPTransport = new(HTTPTransport) - } - - // NTLM requires HTTP/1.1 - n.HTTPTransport.Versions = []string{"1.1"} - - // NLTM requires keep-alive - if n.HTTPTransport.KeepAlive != nil { - enabled := true - n.HTTPTransport.KeepAlive.Enabled = &enabled - } - - // set up the underlying transport, since we - // rely on it for the heavy lifting - err := n.HTTPTransport.Provision(ctx) - if err != nil { - return err - } - - return nil -} - -// RoundTrip implements http.RoundTripper. It basically wraps -// the underlying HTTPTransport.Transport in a way that preserves -// NTLM context by mapping transports/connections. Note that this -// method does not call n.HTTPTransport.RoundTrip (our own method), -// but the underlying n.HTTPTransport.Transport.RoundTrip (standard -// library's method). -func (n *NTLMTransport) RoundTrip(req *http.Request) (*http.Response, error) { - n.HTTPTransport.setScheme(req) - - // when the upstream connection is closed, make sure - // we close the downstream connection with the client - // when this request is done; we only do this if - // using a bound transport - closeDownstreamIfClosedUpstream := func() { - n.transportsMu.Lock() - if _, ok := n.transports[req.RemoteAddr]; !ok { - req.Close = true - } - n.transportsMu.Unlock() - } - - // first, see if this downstream connection is - // already bound to a particular transport - // (transports are abstractions over connections - // to our upstream, and NTLM auth requires - // preserving authentication state for separate - // connections over multiple roundtrips, sigh) - n.transportsMu.Lock() - transport, ok := n.transports[req.RemoteAddr] - if ok { - n.transportsMu.Unlock() - defer closeDownstreamIfClosedUpstream() - return transport.RoundTrip(req) - } - - // otherwise, start by assuming we will use - // the default transport that carries all - // normal/non-NTLM-authenticated requests - transport = n.HTTPTransport.Transport - - // but if this request begins the NTLM authentication - // process, we need to pin it to a specific transport - if requestHasAuth(req) { - var err error - transport, err = n.newTransport() - if err != nil { - return nil, fmt.Errorf("making new transport for %s: %v", req.RemoteAddr, err) - } - n.transports[req.RemoteAddr] = transport - defer closeDownstreamIfClosedUpstream() - } - n.transportsMu.Unlock() - - // finally, do the roundtrip with the transport we selected - return transport.RoundTrip(req) -} - -// newTransport makes an NTLM-compatible transport. -func (n *NTLMTransport) newTransport() (*http.Transport, error) { - // start with a regular HTTP transport - transport, err := n.HTTPTransport.newTransport() - if err != nil { - return nil, err - } - - // we need to wrap upstream connections so we can - // clean up in two ways when that connection is - // closed: 1) destroy the transport that housed - // this connection, and 2) use that as a signal - // to close the connection to the downstream. - wrappedDialContext := transport.DialContext - - transport.DialContext = func(ctx context.Context, network, address string) (net.Conn, error) { - conn2, err := wrappedDialContext(ctx, network, address) - if err != nil { - return nil, err - } - req := ctx.Value(caddyhttp.OriginalRequestCtxKey).(http.Request) - conn := &unbinderConn{Conn: conn2, ntlm: n, clientAddr: req.RemoteAddr} - return conn, nil - } - - return transport, nil -} - -// Cleanup implements caddy.CleanerUpper and closes any idle connections. -func (n *NTLMTransport) Cleanup() error { - if err := n.HTTPTransport.Cleanup(); err != nil { - return err - } - - n.transportsMu.Lock() - for _, t := range n.transports { - t.CloseIdleConnections() - } - n.transports = make(map[string]*http.Transport) - n.transportsMu.Unlock() - - return nil -} - -// deleteTransportsForClient deletes (unmaps) transports that are -// associated with clientAddr (a req.RemoteAddr value). -func (n *NTLMTransport) deleteTransportsForClient(clientAddr string) { - n.transportsMu.Lock() - for key := range n.transports { - if key == clientAddr { - delete(n.transports, key) - } - } - n.transportsMu.Unlock() -} - -// requestHasAuth returns true if req has an Authorization -// header with values "NTLM" or "Negotiate". -func requestHasAuth(req *http.Request) bool { - for _, val := range req.Header["Authorization"] { - if strings.HasPrefix(val, "NTLM") || - strings.HasPrefix(val, "Negotiate") { - return true - } - } - return false -} - -// unbinderConn is used to wrap upstream connections -// so that we know when they are closed and can clean -// up after that. -type unbinderConn struct { - net.Conn - clientAddr string - ntlm *NTLMTransport -} - -func (uc *unbinderConn) Close() error { - uc.ntlm.deleteTransportsForClient(uc.clientAddr) - return uc.Conn.Close() -} - -// Interface guards -var ( - _ caddy.Provisioner = (*NTLMTransport)(nil) - _ http.RoundTripper = (*NTLMTransport)(nil) - _ caddy.CleanerUpper = (*NTLMTransport)(nil) -) diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 6d0d441..438533c 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -739,8 +739,19 @@ var hopHeaders = []string{ // DialError is an error that specifically occurs // in a call to Dial or DialContext. -type DialError struct { - error +type DialError struct{ error } + +// TLSTransport is implemented by transports +// that are capable of using TLS. +type TLSTransport interface { + // TLSEnabled returns true if the transport + // has TLS enabled, false otherwise. + TLSEnabled() bool + + // EnableTLS enables TLS within the transport + // if it is not already, using the provided + // value as a basis for the TLS config. + EnableTLS(base *TLSConfig) error } var bufPool = sync.Pool{ -- cgit v1.2.3