From 4c33716a6046cc77311b03d9ec574141c327b7d3 Mon Sep 17 00:00:00 2001
From: Nikolay <nik@victoriametrics.com>
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 0233aff8c..f310e7290 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 c68d6e8ef..01c633a5f 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 c3a887417..101e40175 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -17,6 +17,7 @@ The following tip changes can be tested by building VictoriaMetrics components f
 
 * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): do not register `vm_promscrape_config_*` metrics if `-promscrape.config` flag is not used. Previously those metrics were registered and never updated, which was confusing and could trigger false-positive alerts.
 * BUGFIX: [vmctl](https://docs.victoriametrics.com/vmctl.html): skip measurements with no fields when migrating data from influxdb. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3837).
+* 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.87.2](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.87.2)
 
diff --git a/lib/netutil/tcplistener.go b/lib/netutil/tcplistener.go
index 0a0fb40fd..e3288097c 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{