From 93bc1b72e3cd566e6447ad7a1f832474aad5dfcc Mon Sep 17 00:00:00 2001 From: Mohammed Al Sahaf Date: Tue, 12 Nov 2019 01:33:38 +0300 Subject: core: Use port ranges to avoid OOM with bad inputs (#2859) * fix OOM issue caught by fuzzing * use ParsedAddress as the struct name for the result of ParseNetworkAddress * simplify code using the ParsedAddress type * minor cleanups --- modules/caddyhttp/caddyhttp.go | 33 +++++++++++---------- modules/caddyhttp/reverseproxy/healthchecks.go | 10 +++---- modules/caddyhttp/reverseproxy/hosts.go | 24 ++++++---------- modules/caddyhttp/server.go | 40 ++++++++++++++------------ 4 files changed, 53 insertions(+), 54 deletions(-) (limited to 'modules') diff --git a/modules/caddyhttp/caddyhttp.go b/modules/caddyhttp/caddyhttp.go index 99a64c3..36d8154 100644 --- a/modules/caddyhttp/caddyhttp.go +++ b/modules/caddyhttp/caddyhttp.go @@ -135,15 +135,18 @@ func (app *App) Validate() error { lnAddrs := make(map[string]string) for srvName, srv := range app.Servers { for _, addr := range srv.Listen { - netw, expanded, err := caddy.ParseNetworkAddress(addr) + listenAddr, err := caddy.ParseNetworkAddress(addr) if err != nil { return fmt.Errorf("invalid listener address '%s': %v", addr, err) } - for _, a := range expanded { - if sn, ok := lnAddrs[netw+a]; ok { - return fmt.Errorf("server %s: listener address repeated: %s (already claimed by server '%s')", srvName, a, sn) + // check that every address in the port range is unique to this server; + // we do not use <= here because PortRangeSize() adds 1 to EndPort for us + for i := uint(0); i < listenAddr.PortRangeSize(); i++ { + addr := caddy.JoinNetworkAddress(listenAddr.Network, listenAddr.Host, strconv.Itoa(int(listenAddr.StartPort+i))) + if sn, ok := lnAddrs[addr]; ok { + return fmt.Errorf("server %s: listener address repeated: %s (already claimed by server '%s')", srvName, addr, sn) } - lnAddrs[netw+a] = srvName + lnAddrs[addr] = srvName } } } @@ -176,14 +179,15 @@ func (app *App) Start() error { } for _, lnAddr := range srv.Listen { - network, addrs, err := caddy.ParseNetworkAddress(lnAddr) + listenAddr, err := caddy.ParseNetworkAddress(lnAddr) if err != nil { return fmt.Errorf("%s: parsing listen address '%s': %v", srvName, lnAddr, err) } - for _, addr := range addrs { - ln, err := caddy.Listen(network, addr) + for i := uint(0); i <= listenAddr.PortRangeSize(); i++ { + hostport := listenAddr.JoinHostPort(i) + ln, err := caddy.Listen(listenAddr.Network, hostport) if err != nil { - return fmt.Errorf("%s: listening on %s: %v", network, addr, err) + return fmt.Errorf("%s: listening on %s: %v", listenAddr.Network, hostport, err) } // enable HTTP/2 by default @@ -194,11 +198,10 @@ func (app *App) Start() error { } // enable TLS - _, port, _ := net.SplitHostPort(addr) - if len(srv.TLSConnPolicies) > 0 && port != strconv.Itoa(app.httpPort()) { + if len(srv.TLSConnPolicies) > 0 && int(i) != app.httpPort() { tlsCfg, err := srv.TLSConnPolicies.TLSConfig(app.ctx) if err != nil { - return fmt.Errorf("%s/%s: making TLS configuration: %v", network, addr, err) + return fmt.Errorf("%s/%s: making TLS configuration: %v", listenAddr.Network, hostport, err) } ln = tls.NewListener(ln, tlsCfg) @@ -206,15 +209,15 @@ func (app *App) Start() error { // TODO: HTTP/3 support is experimental for now if srv.ExperimentalHTTP3 { app.logger.Info("enabling experimental HTTP/3 listener", - zap.String("addr", addr), + zap.String("addr", hostport), ) - h3ln, err := caddy.ListenPacket("udp", addr) + h3ln, err := caddy.ListenPacket("udp", hostport) if err != nil { return fmt.Errorf("getting HTTP/3 UDP listener: %v", err) } h3srv := &http3.Server{ Server: &http.Server{ - Addr: addr, + Addr: hostport, Handler: srv, TLSConfig: tlsCfg, }, diff --git a/modules/caddyhttp/reverseproxy/healthchecks.go b/modules/caddyhttp/reverseproxy/healthchecks.go index 56e97bc..92b3547 100644 --- a/modules/caddyhttp/reverseproxy/healthchecks.go +++ b/modules/caddyhttp/reverseproxy/healthchecks.go @@ -102,7 +102,7 @@ func (h *Handler) doActiveHealthChecksForAllHosts() { host := value.(Host) go func(networkAddr string, host Host) { - network, addrs, err := caddy.ParseNetworkAddress(networkAddr) + addr, err := caddy.ParseNetworkAddress(networkAddr) if err != nil { h.HealthChecks.Active.logger.Error("bad network address", zap.String("address", networkAddr), @@ -110,20 +110,20 @@ func (h *Handler) doActiveHealthChecksForAllHosts() { ) return } - if len(addrs) != 1 { + if addr.PortRangeSize() != 1 { h.HealthChecks.Active.logger.Error("multiple addresses (upstream must map to only one address)", zap.String("address", networkAddr), ) return } - hostAddr := addrs[0] - if network == "unix" || network == "unixgram" || network == "unixpacket" { + hostAddr := addr.JoinHostPort(0) + if addr.Network == "unix" || addr.Network == "unixgram" || addr.Network == "unixpacket" { // this will be used as the Host portion of a http.Request URL, and // paths to socket files would produce an error when creating URL, // so use a fake Host value instead; unix sockets are usually local hostAddr = "localhost" } - err = h.doActiveHealthCheck(DialInfo{Network: network, Address: addrs[0]}, hostAddr, host) + err = h.doActiveHealthCheck(DialInfo{Network: addr.Network, Address: hostAddr}, hostAddr, host) if err != nil { h.HealthChecks.Active.logger.Error("active health check failed", zap.String("address", networkAddr), diff --git a/modules/caddyhttp/reverseproxy/hosts.go b/modules/caddyhttp/reverseproxy/hosts.go index a16bed0..8bad7c2 100644 --- a/modules/caddyhttp/reverseproxy/hosts.go +++ b/modules/caddyhttp/reverseproxy/hosts.go @@ -16,8 +16,7 @@ package reverseproxy import ( "fmt" - "net" - "strings" + "strconv" "sync/atomic" "github.com/caddyserver/caddy/v2" @@ -193,27 +192,20 @@ func (di DialInfo) String() string { // the given Replacer. Note that the returned value is not a pointer. func fillDialInfo(upstream *Upstream, repl caddy.Replacer) (DialInfo, error) { dial := repl.ReplaceAll(upstream.Dial, "") - netw, addrs, err := caddy.ParseNetworkAddress(dial) + addr, err := caddy.ParseNetworkAddress(dial) if err != nil { return DialInfo{}, fmt.Errorf("upstream %s: invalid dial address %s: %v", upstream.Dial, dial, err) } - if len(addrs) != 1 { + if numPorts := addr.PortRangeSize(); numPorts != 1 { return DialInfo{}, fmt.Errorf("upstream %s: dial address must represent precisely one socket: %s represents %d", - upstream.Dial, dial, len(addrs)) - } - var dialHost, dialPort string - if !strings.Contains(netw, "unix") { - dialHost, dialPort, err = net.SplitHostPort(addrs[0]) - if err != nil { - dialHost = addrs[0] // assume there was no port - } + upstream.Dial, dial, numPorts) } return DialInfo{ Upstream: upstream, - Network: netw, - Address: addrs[0], - Host: dialHost, - Port: dialPort, + Network: addr.Network, + Address: addr.JoinHostPort(0), + Host: addr.Host, + Port: strconv.Itoa(int(addr.StartPort)), }, nil } diff --git a/modules/caddyhttp/server.go b/modules/caddyhttp/server.go index e119c2d..17860ed 100644 --- a/modules/caddyhttp/server.go +++ b/modules/caddyhttp/server.go @@ -242,40 +242,44 @@ func (s *Server) enforcementHandler(w http.ResponseWriter, r *http.Request, next // listeners in s that use a port which is not otherPort. func (s *Server) listenersUseAnyPortOtherThan(otherPort int) bool { for _, lnAddr := range s.Listen { - _, addrs, err := caddy.ParseNetworkAddress(lnAddr) - if err == nil { - for _, a := range addrs { - _, port, err := net.SplitHostPort(a) - if err == nil && port != strconv.Itoa(otherPort) { - return true - } - } + laddrs, err := caddy.ParseNetworkAddress(lnAddr) + if err != nil { + continue + } + if uint(otherPort) > laddrs.EndPort || uint(otherPort) < laddrs.StartPort { + return true } } return false } +// hasListenerAddress returns true if s has a listener +// at the given address fullAddr. Currently, fullAddr +// must represent exactly one socket address (port +// ranges are not supported) func (s *Server) hasListenerAddress(fullAddr string) bool { - netw, addrs, err := caddy.ParseNetworkAddress(fullAddr) + laddrs, err := caddy.ParseNetworkAddress(fullAddr) if err != nil { return false } - if len(addrs) != 1 { - return false + if laddrs.PortRangeSize() != 1 { + return false // TODO: support port ranges } - addr := addrs[0] + for _, lnAddr := range s.Listen { - thisNetw, thisAddrs, err := caddy.ParseNetworkAddress(lnAddr) + thisAddrs, err := caddy.ParseNetworkAddress(lnAddr) if err != nil { continue } - if thisNetw != netw { + if thisAddrs.Network != laddrs.Network { continue } - for _, a := range thisAddrs { - if a == addr { - return true - } + + // host must be the same and port must fall within port range + if (thisAddrs.Host == laddrs.Host) && + (laddrs.StartPort <= thisAddrs.EndPort) && + (laddrs.StartPort >= thisAddrs.StartPort) { + return true } } return false -- cgit v1.2.3