fix: harden proxy edge cases and tunnel shutdown

README.md

@@ -118,6 +118,39 @@ $ xcaddy build --with github.com/imgk/caddy-trojan
 curl -X POST -H "Content-Type: application/json" -d '{"password": "test1234"}' http://localhost:2019/trojan/users/add
 ```
 
+## Rate Limiting / DoS Hardening
+
+Password validation applies a uniform 250ms delay on every connection (hit and
+miss alike, to keep validation timing-constant). This means an unauthenticated
+peer can hold a server goroutine + file descriptor for ~250ms per connection by
+sending 58 bytes after the TLS handshake. Caddy itself has no built-in
+per-IP connection limiting, so add one at the edge:
+
+1. Bound connection lifetime in the server block so stalled peers are dropped:
+```
+servers :443 {
+	listener_wrappers {
+		trojan
+	}
+	read_timeout 30s
+	write_timeout 30s
+	idle_timeout 1m
+}
+```
+2. For per-IP connection concurrency limits, build caddy with a limiting
+plugin, e.g. [mholt/caddy-limit](https://github.com/mholt/caddy-limit):
+```
+xcaddy build --with github.com/mholt/caddy-limit
+```
+```
+servers :443 {
+	...
+	conn_limit 20
+}
+```
+3. As a last line of defense, use a firewall / fail2ban at the host level to
+cap new connections per source IP on port 443.
+
 ## Docker
 
 ```

app/proxy.go

@@ -9,6 +9,7 @@ import (
 	"io"
 	"net"
 	"net/http"
+	"strings"
 	"sync"
 
 	"golang.org/x/net/proxy"
@@ -17,6 +18,7 @@ import (
 	"github.com/caddyserver/caddy/v2/caddyconfig"
 
 	"github.com/imgk/caddy-trojan/pkgs/domaintree"
+	"github.com/imgk/caddy-trojan/pkgs/rawconn"
 	"github.com/imgk/caddy-trojan/pkgs/trojan"
 	"github.com/imgk/caddy-trojan/pkgs/x"
 )
@@ -145,7 +147,7 @@ func (p *EnvProxy) Provision(ctx caddy.Context) error {
 	if p.ProxyRaw != nil {
 		mod, err := ctx.LoadModule(p, "ProxyRaw")
 		if err != nil {
-			return nil
+			return err
 		}
 		p.proxy = mod.(Proxy)
 		p.dialer = proxy.FromEnvironmentUsing(p.proxy)
@@ -210,7 +212,7 @@ func (p *SocksProxy) Provision(ctx caddy.Context) error {
 	if p.ProxyRaw != nil {
 		mod, err := ctx.LoadModule(p, "ProxyRaw")
 		if err != nil {
-			return nil
+			return err
 		}
 		p.proxy = mod.(Proxy)
 	} else {
@@ -291,7 +293,7 @@ func (p *HttpProxy) Provision(ctx caddy.Context) error {
 	if p.ProxyRaw != nil {
 		mod, err := ctx.LoadModule(p, "ProxyRaw")
 		if err != nil {
-			return nil
+			return err
 		}
 		p.proxy = mod.(Proxy)
 	} else {
@@ -310,15 +312,30 @@ func (p *HttpProxy) Close() error {
 	return p.proxy.Close()
 }
 
+// validateProxyTarget rejects target addresses that could break out of the
+// HTTP CONNECT request line (CR/LF header injection) or smuggle bytes into
+// the upstream proxy connection.
+func validateProxyTarget(addr string) error {
+	if strings.ContainsAny(addr, "\r\n\x00 \t") {
+		return errors.New("invalid target address: control characters are not allowed")
+	}
+	return nil
+}
+
 func (p *HttpProxy) Dial(network, addr string) (net.Conn, error) {
+	if err := validateProxyTarget(addr); err != nil {
+		return nil, err
+	}
+
 	conn, err := p.proxy.Dial("tcp", p.Server)
 	if err != nil {
 		return nil, err
 	}
 
 	req, err := http.NewRequest(http.MethodConnect, p.tcpURL, nil)
 	if err != nil {
-		return conn, fmt.Errorf("request error: %w", err)
+		_ = conn.Close()
+		return nil, fmt.Errorf("request error: %w", err)
 	}
 	req.URL.Opaque = addr
 	if p.basicAuth != "" {
@@ -327,15 +344,26 @@ func (p *HttpProxy) Dial(network, addr string) (net.Conn, error) {
 
 	err = req.WriteProxy(conn)
 	if err != nil {
-		return conn, fmt.Errorf("write request error: %w", err)
+		_ = conn.Close()
+		return nil, fmt.Errorf("write request error: %w", err)
 	}
 
-	resp, err := http.ReadResponse(bufio.NewReader(conn), nil)
+	br := bufio.NewReader(conn)
+	resp, err := http.ReadResponse(br, nil)
 	if err != nil {
-		return conn, fmt.Errorf("read response error: %w", err)
+		_ = conn.Close()
+		return nil, fmt.Errorf("read response error: %w", err)
 	}
 	if resp.StatusCode != http.StatusOK {
-		return conn, fmt.Errorf("server status code error: %v", resp.StatusCode)
+		_ = conn.Close()
+		return nil, fmt.Errorf("server status code error: %v", resp.StatusCode)
+	}
+	// bufio may have read past the response head into the tunnel payload;
+	// replay those bytes so the first tunneled packet is not lost.
+	if n := br.Buffered(); n > 0 {
+		if b, err := br.Peek(n); err == nil {
+			return rawconn.NewConn(conn, b), nil
+		}
 	}
 	return conn, nil
 }