From 7a3e16e77476a8b678be455728c0e47611359261 Mon Sep 17 00:00:00 2001 From: Nikolay Date: Tue, 7 Mar 2023 17:50:18 +0100 Subject: [PATCH] lib/netutil: fixes panic at proxy protocol (#3905) it may occur if non proxy protocol message received by tcp server. Listener Accept method must return only non-recoverable errors. https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3335 --- app/victoria-metrics/main.go | 3 ++- app/vmauth/main.go | 3 ++- docs/CHANGELOG.md | 1 + lib/netutil/tcplistener.go | 9 +++++++-- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/app/victoria-metrics/main.go b/app/victoria-metrics/main.go index 362aa8dd8d..484e31f5ff 100644 --- a/app/victoria-metrics/main.go +++ b/app/victoria-metrics/main.go @@ -26,7 +26,8 @@ import ( var ( httpListenAddr = flag.String("httpListenAddr", ":8428", "TCP address to listen for http connections. See also -httpListenAddr.useProxyProtocol") useProxyProtocol = flag.Bool("httpListenAddr.useProxyProtocol", false, "Whether to use proxy protocol for connections accepted at -httpListenAddr . "+ - "See https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt") + "See https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt."+ + "With enabled proxy protocol http server cannot serve regular /metrics endpoint. Use -pushmetrics.url for metrics pushing.") minScrapeInterval = flag.Duration("dedup.minScrapeInterval", 0, "Leave only the last sample in every time series per each discrete interval "+ "equal to -dedup.minScrapeInterval > 0. See https://docs.victoriametrics.com/#deduplication and https://docs.victoriametrics.com/#downsampling") dryRun = flag.Bool("dryRun", false, "Whether to check only -promscrape.config and then exit. "+ diff --git a/app/vmauth/main.go b/app/vmauth/main.go index f64f567667..5591080764 100644 --- a/app/vmauth/main.go +++ b/app/vmauth/main.go @@ -28,7 +28,8 @@ import ( var ( httpListenAddr = flag.String("httpListenAddr", ":8427", "TCP address to listen for http connections. See also -httpListenAddr.useProxyProtocol") useProxyProtocol = flag.Bool("httpListenAddr.useProxyProtocol", false, "Whether to use proxy protocol for connections accepted at -httpListenAddr . "+ - "See https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt") + "See https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt."+ + "With enabled proxy protocol http server cannot serve regular /metrics endpoint. Use -pushmetrics.url for metrics pushing.") maxIdleConnsPerBackend = flag.Int("maxIdleConnsPerBackend", 100, "The maximum number of idle connections vmauth can open per each backend host. "+ "See also -maxConcurrentRequests") responseTimeout = flag.Duration("responseTimeout", 5*time.Minute, "The timeout for receiving a response from backend") diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 0bfbe72bfc..9f4af04706 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -22,6 +22,7 @@ The following tip changes can be tested by building VictoriaMetrics components f * BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): stop showing `Please enter a valid Query and execute it` error message on the first load of vmui. * BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): support old format of URL params. This should make compatible copying URL between vmui of versions or using `Run in VMUI` button in datasource plugin. * BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): fix the display of the selected value for dropdowns on `Explore` page. +* BUGFIX: [vmauth](https://docs.victoriametrics.com/vmauth.html) fixes possible panic with enabled proxy-protocol. It was introduced at [v1.87.0](https://docs.victoriametrics.com/CHANGELOG.html#v1870) when implementing [this feature](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3335). ## [v1.88.1](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.88.1) diff --git a/lib/netutil/tcplistener.go b/lib/netutil/tcplistener.go index 0a0fb40fdc..e3288097c2 100644 --- a/lib/netutil/tcplistener.go +++ b/lib/netutil/tcplistener.go @@ -78,6 +78,8 @@ type TCPListener struct { connMetrics } +var proxyProtocolReadErrorLogger = logger.WithThrottler("proxyProtocolReadError", 5*time.Second) + // Accept accepts connections from the addr passed to NewTCPListener. func (ln *TCPListener) Accept() (net.Conn, error) { for { @@ -94,10 +96,13 @@ func (ln *TCPListener) Accept() (net.Conn, error) { return nil, err } if ln.useProxyProtocol { - conn, err = newProxyProtocolConn(conn) + pConn, err := newProxyProtocolConn(conn) if err != nil { - return nil, err + proxyProtocolReadErrorLogger.Errorf("cannot read proxy proto conn for TCP addr %q: %s", ln.Addr(), err) + _ = conn.Close() + continue } + conn = pConn } ln.conns.Inc() sc := &statConn{