From 7c94cb393131c2119b2f6bfe779b99219a01442d Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Tue, 18 Oct 2022 21:05:56 +0000 Subject: [PATCH 1/7] Stop storing probe metrics. --- service/metrics/metrics.go | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/service/metrics/metrics.go b/service/metrics/metrics.go index e9a3905b..57265505 100644 --- a/service/metrics/metrics.go +++ b/service/metrics/metrics.go @@ -19,7 +19,6 @@ import ( "fmt" "io" "net" - "strconv" "time" onet "github.com/Jigsaw-Code/outline-ss-server/net" @@ -57,7 +56,6 @@ type shadowsocksMetrics struct { timeToCipherMs *prometheus.HistogramVec // TODO: Add time to first byte. - tcpProbes *prometheus.HistogramVec tcpOpenConnections *prometheus.CounterVec tcpClosedConnections *prometheus.CounterVec tcpConnectionDurationMs *prometheus.HistogramVec @@ -117,12 +115,6 @@ func newShadowsocksMetrics(ipCountryDB *geoip2.Reader) *shadowsocksMetrics { Name: "data_bytes", Help: "Bytes transferred by the proxy", }, []string{"dir", "proto", "location", "status", "access_key"}), - tcpProbes: prometheus.NewHistogramVec(prometheus.HistogramOpts{ - Namespace: "shadowsocks", - Name: "tcp_probes", - Buckets: []float64{0, 49, 50, 51, 73, 91}, - Help: "Histogram of number of bytes from client to proxy, for detecting possible probes", - }, []string{"location", "port", "status", "error"}), timeToCipherMs: prometheus.NewHistogramVec( prometheus.HistogramOpts{ Namespace: "shadowsocks", @@ -154,7 +146,7 @@ func newShadowsocksMetrics(ipCountryDB *geoip2.Reader) *shadowsocksMetrics { func NewPrometheusShadowsocksMetrics(ipCountryDB *geoip2.Reader, registerer prometheus.Registerer) ShadowsocksMetrics { m := newShadowsocksMetrics(ipCountryDB) // TODO: Is it possible to pass where to register the collectors? - registerer.MustRegister(m.buildInfo, m.accessKeys, m.ports, m.tcpOpenConnections, m.tcpProbes, m.tcpClosedConnections, m.tcpConnectionDurationMs, + registerer.MustRegister(m.buildInfo, m.accessKeys, m.ports, m.tcpOpenConnections, m.tcpClosedConnections, m.tcpConnectionDurationMs, m.dataBytes, m.timeToCipherMs, m.udpAddedNatEntries, m.udpRemovedNatEntries) return m } @@ -233,7 +225,7 @@ func (m *shadowsocksMetrics) AddClosedTCPConnection(clientLocation, accessKey, s } func (m *shadowsocksMetrics) AddTCPProbe(clientLocation, status, drainResult string, port int, data ProxyMetrics) { - m.tcpProbes.WithLabelValues(clientLocation, strconv.Itoa(port), status, drainResult).Observe(float64(data.ClientProxy)) + // We no longer track probe metrics, as it takes too much memory. } func (m *shadowsocksMetrics) AddUDPPacketFromClient(clientLocation, accessKey, status string, clientProxyBytes, proxyTargetBytes int, timeToCipher time.Duration) { From 2d821bc323b96e0009dcf6974944cd602cf18887 Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Tue, 18 Oct 2022 21:36:57 +0000 Subject: [PATCH 2/7] Don't create zero counters --- service/metrics/metrics.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/service/metrics/metrics.go b/service/metrics/metrics.go index 57265505..f4b13902 100644 --- a/service/metrics/metrics.go +++ b/service/metrics/metrics.go @@ -208,9 +208,9 @@ func isFound(accessKey string) string { } // addIfNonZero helps avoid the creation of series that are always zero. -func addIfNonZero(counter prometheus.Counter, value int64) { +func addIfNonZero(value int64, counterVec *prometheus.CounterVec, lvs ...string) { if value > 0 { - counter.Add(float64(value)) + counterVec.WithLabelValues(lvs...).Add(float64(value)) } } @@ -218,10 +218,10 @@ func (m *shadowsocksMetrics) AddClosedTCPConnection(clientLocation, accessKey, s m.tcpClosedConnections.WithLabelValues(clientLocation, status, accessKey).Inc() m.tcpConnectionDurationMs.WithLabelValues(status).Observe(duration.Seconds() * 1000) m.timeToCipherMs.WithLabelValues("tcp", isFound(accessKey)).Observe(timeToCipher.Seconds() * 1000) - addIfNonZero(m.dataBytes.WithLabelValues("c>p", "tcp", clientLocation, status, accessKey), data.ClientProxy) - addIfNonZero(m.dataBytes.WithLabelValues("p>t", "tcp", clientLocation, status, accessKey), data.ProxyTarget) - addIfNonZero(m.dataBytes.WithLabelValues("pp", "tcp", clientLocation, status, accessKey) + addIfNonZero(data.ProxyTarget, m.dataBytes, "p>t", "tcp", clientLocation, status, accessKey) + addIfNonZero(data.TargetProxy, m.dataBytes, "pp", "udp", clientLocation, status, accessKey), int64(clientProxyBytes)) - addIfNonZero(m.dataBytes.WithLabelValues("p>t", "udp", clientLocation, status, accessKey), int64(proxyTargetBytes)) + addIfNonZero(int64(clientProxyBytes), m.dataBytes, "c>p", "udp", clientLocation, status, accessKey) + addIfNonZero(int64(proxyTargetBytes), m.dataBytes, "p>t", "udp", clientLocation, status, accessKey) } func (m *shadowsocksMetrics) AddUDPPacketFromTarget(clientLocation, accessKey, status string, targetProxyBytes, proxyClientBytes int) { - addIfNonZero(m.dataBytes.WithLabelValues("p Date: Tue, 18 Oct 2022 22:20:10 +0000 Subject: [PATCH 3/7] Minimize shadowsocks_data_bytes --- service/metrics/metrics.go | 59 +++++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 17 deletions(-) diff --git a/service/metrics/metrics.go b/service/metrics/metrics.go index f4b13902..a7ff6388 100644 --- a/service/metrics/metrics.go +++ b/service/metrics/metrics.go @@ -49,22 +49,25 @@ type ShadowsocksMetrics interface { type shadowsocksMetrics struct { ipCountryDB *geoip2.Reader - buildInfo *prometheus.GaugeVec - accessKeys prometheus.Gauge - ports prometheus.Gauge - dataBytes *prometheus.CounterVec - timeToCipherMs *prometheus.HistogramVec + buildInfo *prometheus.GaugeVec + accessKeys prometheus.Gauge + ports prometheus.Gauge + dataBytes *prometheus.CounterVec + dataBytesPerLocation *prometheus.CounterVec + timeToCipherMs *prometheus.HistogramVec // TODO: Add time to first byte. tcpOpenConnections *prometheus.CounterVec tcpClosedConnections *prometheus.CounterVec tcpConnectionDurationMs *prometheus.HistogramVec - udpAddedNatEntries prometheus.Counter - udpRemovedNatEntries prometheus.Counter + udpPacketsFromClientPerLocation *prometheus.CounterVec + udpAddedNatEntries prometheus.Counter + udpRemovedNatEntries prometheus.Counter } func newShadowsocksMetrics(ipCountryDB *geoip2.Reader) *shadowsocksMetrics { + // Don't forget to pass the counters to the registerer.MustRegister call in NewPrometheusShadowsocksMetrics. return &shadowsocksMetrics{ ipCountryDB: ipCountryDB, buildInfo: prometheus.NewGaugeVec(prometheus.GaugeOpts{ @@ -114,7 +117,13 @@ func newShadowsocksMetrics(ipCountryDB *geoip2.Reader) *shadowsocksMetrics { Namespace: "shadowsocks", Name: "data_bytes", Help: "Bytes transferred by the proxy", - }, []string{"dir", "proto", "location", "status", "access_key"}), + }, []string{"dir", "proto", "access_key"}), + dataBytesPerLocation: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: "shadowsocks", + Name: "data_bytes_per_location", + Help: "Bytes transferred by the proxy", + }, []string{"dir", "proto", "location"}), timeToCipherMs: prometheus.NewHistogramVec( prometheus.HistogramOpts{ Namespace: "shadowsocks", @@ -122,6 +131,13 @@ func newShadowsocksMetrics(ipCountryDB *geoip2.Reader) *shadowsocksMetrics { Help: "Time needed to find the cipher", Buckets: []float64{0.1, 1, 10, 100, 1000}, }, []string{"proto", "found_key"}), + udpPacketsFromClientPerLocation: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: "shadowsocks", + Subsystem: "udp", + Name: "packets_from_client_per_location", + Help: "Packets received from the client", + }, []string{"location", "status"}), udpAddedNatEntries: prometheus.NewCounter( prometheus.CounterOpts{ Namespace: "shadowsocks", @@ -147,7 +163,7 @@ func NewPrometheusShadowsocksMetrics(ipCountryDB *geoip2.Reader, registerer prom m := newShadowsocksMetrics(ipCountryDB) // TODO: Is it possible to pass where to register the collectors? registerer.MustRegister(m.buildInfo, m.accessKeys, m.ports, m.tcpOpenConnections, m.tcpClosedConnections, m.tcpConnectionDurationMs, - m.dataBytes, m.timeToCipherMs, m.udpAddedNatEntries, m.udpRemovedNatEntries) + m.dataBytes, m.dataBytesPerLocation, m.timeToCipherMs, m.udpPacketsFromClientPerLocation, m.udpAddedNatEntries, m.udpRemovedNatEntries) return m } @@ -218,10 +234,14 @@ func (m *shadowsocksMetrics) AddClosedTCPConnection(clientLocation, accessKey, s m.tcpClosedConnections.WithLabelValues(clientLocation, status, accessKey).Inc() m.tcpConnectionDurationMs.WithLabelValues(status).Observe(duration.Seconds() * 1000) m.timeToCipherMs.WithLabelValues("tcp", isFound(accessKey)).Observe(timeToCipher.Seconds() * 1000) - addIfNonZero(data.ClientProxy, m.dataBytes, "c>p", "tcp", clientLocation, status, accessKey) - addIfNonZero(data.ProxyTarget, m.dataBytes, "p>t", "tcp", clientLocation, status, accessKey) - addIfNonZero(data.TargetProxy, m.dataBytes, "pp", "tcp", accessKey) + addIfNonZero(data.ClientProxy, m.dataBytesPerLocation, "c>p", "tcp", clientLocation) + addIfNonZero(data.ProxyTarget, m.dataBytes, "p>t", "tcp", accessKey) + addIfNonZero(data.ProxyTarget, m.dataBytesPerLocation, "p>t", "tcp", clientLocation) + addIfNonZero(data.TargetProxy, m.dataBytes, "pp", "udp", clientLocation, status, accessKey) - addIfNonZero(int64(proxyTargetBytes), m.dataBytes, "p>t", "udp", clientLocation, status, accessKey) + m.udpPacketsFromClientPerLocation.WithLabelValues(clientLocation, status).Inc() + addIfNonZero(int64(clientProxyBytes), m.dataBytes, "c>p", "udp", accessKey) + addIfNonZero(int64(clientProxyBytes), m.dataBytesPerLocation, "c>p", "udp", clientLocation) + addIfNonZero(int64(proxyTargetBytes), m.dataBytes, "p>t", "udp", accessKey) + addIfNonZero(int64(proxyTargetBytes), m.dataBytesPerLocation, "p>t", "udp", clientLocation) } func (m *shadowsocksMetrics) AddUDPPacketFromTarget(clientLocation, accessKey, status string, targetProxyBytes, proxyClientBytes int) { - addIfNonZero(int64(targetProxyBytes), m.dataBytes, "p Date: Tue, 18 Oct 2022 22:40:46 +0000 Subject: [PATCH 4/7] Update PROBES.md --- service/PROBES.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/service/PROBES.md b/service/PROBES.md index c34bf614..befbcbe7 100644 --- a/service/PROBES.md +++ b/service/PROBES.md @@ -29,7 +29,3 @@ This feature is on by default in Outline. Admins who are using outline-ss-serve Shadowsocks uses the same Key Derivation Function for both upstream and downstream flows, so in principle an attacker could record data sent from the server to the client, and use it in a "reflected replay" attack as simulated client->server data. The data would appear to be valid and authenticated to the server, but the connection would most likely fail when attempting to parse the destination address header, perhaps leading to a distinctive failure behavior. To avoid this class of attacks, outline-ss-server uses an [HMAC](https://en.wikipedia.org/wiki/HMAC) with a 32-bit tag to mark all server handshakes, and checks for the presence of this tag in all incoming handshakes. If the tag is present, the connection is a reflected replay, with a false positive probability of 1 in 4 billion. - -## Metrics - -Outline provides server operators with metrics on a variety of aspects of server activity, including any detected attacks. To observe attacks detected by your server, look at the `tcp_probes` histogram vector in Prometheus. The `status` field will be `"ERR_CIPHER"` (indicating invalid probe data), `"ERR_REPLAY_CLIENT"`, or `"ERR_REPLAY_SERVER"`, depending on the kind of attack your server observed. You can also see what country each probe appeared to originate from, and approximately how many bytes were sent before giving up. \ No newline at end of file From 8cbaa24cfd7804251bce08ae9f536f367450049c Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Tue, 18 Oct 2022 22:41:49 +0000 Subject: [PATCH 5/7] Remove string-owners --- .github/CODEOWNERS | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index fa84ab13..acf2e595 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,4 +1 @@ * @Jigsaw-Code/outline-networking-owners - -*.md @Jigsaw-Code/outline-strings-owners -LICENSE @Jigsaw-Code/outline-strings-owners From 355ef2e69441bbcbd03e42d02d9c6b7b60d94ae2 Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Tue, 18 Oct 2022 22:43:32 +0000 Subject: [PATCH 6/7] Update Help strings --- service/metrics/metrics.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/service/metrics/metrics.go b/service/metrics/metrics.go index a7ff6388..3c59b515 100644 --- a/service/metrics/metrics.go +++ b/service/metrics/metrics.go @@ -116,13 +116,13 @@ func newShadowsocksMetrics(ipCountryDB *geoip2.Reader) *shadowsocksMetrics { prometheus.CounterOpts{ Namespace: "shadowsocks", Name: "data_bytes", - Help: "Bytes transferred by the proxy", + Help: "Bytes transferred by the proxy, per access key", }, []string{"dir", "proto", "access_key"}), dataBytesPerLocation: prometheus.NewCounterVec( prometheus.CounterOpts{ Namespace: "shadowsocks", Name: "data_bytes_per_location", - Help: "Bytes transferred by the proxy", + Help: "Bytes transferred by the proxy, per location", }, []string{"dir", "proto", "location"}), timeToCipherMs: prometheus.NewHistogramVec( prometheus.HistogramOpts{ @@ -136,7 +136,7 @@ func newShadowsocksMetrics(ipCountryDB *geoip2.Reader) *shadowsocksMetrics { Namespace: "shadowsocks", Subsystem: "udp", Name: "packets_from_client_per_location", - Help: "Packets received from the client", + Help: "Packets received from the client, per location and status", }, []string{"location", "status"}), udpAddedNatEntries: prometheus.NewCounter( prometheus.CounterOpts{ From fad6981e6b291b568e86f3521b0c95afe2bb2298 Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Thu, 20 Oct 2022 03:42:14 +0000 Subject: [PATCH 7/7] Restore probe metrics --- service/metrics/metrics.go | 18 +++++++++++++----- service/metrics/metrics_test.go | 5 ++--- service/tcp.go | 2 +- service/tcp_test.go | 2 +- service/udp_test.go | 2 +- 5 files changed, 18 insertions(+), 11 deletions(-) diff --git a/service/metrics/metrics.go b/service/metrics/metrics.go index 3c59b515..bda7b0b7 100644 --- a/service/metrics/metrics.go +++ b/service/metrics/metrics.go @@ -19,6 +19,7 @@ import ( "fmt" "io" "net" + "strconv" "time" onet "github.com/Jigsaw-Code/outline-ss-server/net" @@ -37,7 +38,7 @@ type ShadowsocksMetrics interface { // TCP metrics AddOpenTCPConnection(clientLocation string) AddClosedTCPConnection(clientLocation, accessKey, status string, data ProxyMetrics, timeToCipher, duration time.Duration) - AddTCPProbe(clientLocation, status, drainResult string, port int, data ProxyMetrics) + AddTCPProbe(status, drainResult string, port int, data ProxyMetrics) // UDP metrics AddUDPPacketFromClient(clientLocation, accessKey, status string, clientProxyBytes, proxyTargetBytes int, timeToCipher time.Duration) @@ -57,6 +58,7 @@ type shadowsocksMetrics struct { timeToCipherMs *prometheus.HistogramVec // TODO: Add time to first byte. + tcpProbes *prometheus.HistogramVec tcpOpenConnections *prometheus.CounterVec tcpClosedConnections *prometheus.CounterVec tcpConnectionDurationMs *prometheus.HistogramVec @@ -85,6 +87,12 @@ func newShadowsocksMetrics(ipCountryDB *geoip2.Reader) *shadowsocksMetrics { Name: "ports", Help: "Count of open Shadowsocks ports", }), + tcpProbes: prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: "shadowsocks", + Name: "tcp_probes", + Buckets: []float64{0, 49, 50, 51, 73, 91}, + Help: "Histogram of number of bytes from client to proxy, for detecting possible probes", + }, []string{"port", "status", "error"}), tcpOpenConnections: prometheus.NewCounterVec(prometheus.CounterOpts{ Namespace: "shadowsocks", Subsystem: "tcp", @@ -162,7 +170,7 @@ func newShadowsocksMetrics(ipCountryDB *geoip2.Reader) *shadowsocksMetrics { func NewPrometheusShadowsocksMetrics(ipCountryDB *geoip2.Reader, registerer prometheus.Registerer) ShadowsocksMetrics { m := newShadowsocksMetrics(ipCountryDB) // TODO: Is it possible to pass where to register the collectors? - registerer.MustRegister(m.buildInfo, m.accessKeys, m.ports, m.tcpOpenConnections, m.tcpClosedConnections, m.tcpConnectionDurationMs, + registerer.MustRegister(m.buildInfo, m.accessKeys, m.ports, m.tcpProbes, m.tcpOpenConnections, m.tcpClosedConnections, m.tcpConnectionDurationMs, m.dataBytes, m.dataBytesPerLocation, m.timeToCipherMs, m.udpPacketsFromClientPerLocation, m.udpAddedNatEntries, m.udpRemovedNatEntries) return m } @@ -244,8 +252,8 @@ func (m *shadowsocksMetrics) AddClosedTCPConnection(clientLocation, accessKey, s addIfNonZero(data.ProxyClient, m.dataBytesPerLocation, "c