Skip to content

Commit 886b6dd

Browse files
sippejwclaude
andcommitted
Harden TCP listener: log Accept failures, bound ClientHello read
The wrapped TCP listeners (:443/:8443) were observed dying in production while :80 and the UDP/QUIC paths kept running. The Accept-error path that permanently stops the net/http server was silent, making the cause undiagnosable. Now: - Log the underlying Accept() error before propagating it. - Set a 10s read deadline around the synchronous ClientHello capture so a slow/silent client cannot wedge the Accept loop; clear it on success so the TLS handshake is unaffected. - Demote routine per-connection capture failures (non-TLS clients, port scanners) from Error to Debug. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 5faad5c commit 886b6dd

1 file changed

Lines changed: 25 additions & 1 deletion

File tree

modcaddy/listener/listener.go

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,21 @@ package listener
22

33
import (
44
"net"
5+
"time"
56

67
"github.com/caddyserver/caddy/v2"
78
"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
89
"github.com/refraction-networking/clienthellod/modcaddy/app"
910
"go.uber.org/zap"
1011
)
1112

13+
// clientHelloReadTimeout bounds how long the capture step may block waiting for
14+
// a client's ClientHello. HandleTCPConn reads it synchronously inside Accept(),
15+
// so without a deadline a client that connects but never sends a complete TLS
16+
// record would block the Accept loop indefinitely and stall every new
17+
// connection on this listener.
18+
const clientHelloReadTimeout = 10 * time.Second
19+
1220
func init() {
1321
caddy.RegisterModule(ListenerWrapper{})
1422
}
@@ -127,16 +135,32 @@ func (l *tlsListener) Accept() (net.Conn, error) {
127135
for {
128136
conn, err := l.Listener.Accept()
129137
if err != nil {
138+
// A non-temporary error returned here makes net/http's server stop
139+
// serving this listener permanently (the socket is closed and no
140+
// new connections are accepted). This path was previously silent,
141+
// which made an in-the-wild listener death impossible to diagnose.
142+
// Log it before propagating so the cause is always recorded.
143+
l.logger.Error("clienthellod listener: underlying Accept failed, listener will stop serving", zap.Error(err))
130144
return nil, err
131145
}
132146

147+
// Bound the synchronous ClientHello read so a slow or silent client
148+
// cannot wedge the Accept loop. Reset on success so the deadline does
149+
// not carry into the TLS handshake on the rewound connection.
150+
_ = conn.SetReadDeadline(time.Now().Add(clientHelloReadTimeout))
151+
133152
rewindConn, err := l.reservoir.TLSFingerprinter().HandleTCPConn(conn)
134153
if err != nil {
135-
l.logger.Error("internal error: TLSFingerprinter failed to handle TCP connection, closing connection", zap.Error(err))
154+
// Per-connection failure (non-TLS client, malformed ClientHello,
155+
// or read timeout). Drop just this connection and keep serving;
156+
// never propagate as an Accept error.
157+
l.logger.Debug("clienthellod listener: failed to handle TCP connection, closing it", zap.Error(err))
136158
conn.Close()
137159
continue
138160
}
139161

162+
_ = conn.SetReadDeadline(time.Time{})
163+
140164
return rewindConn, nil
141165
}
142166
}

0 commit comments

Comments
 (0)