Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
b09d23fa0d |
@@ -347,9 +347,9 @@ minute, failed logins included; beyond that it answers `429 Too Many Requests`
|
|||||||
without checking the password. A Prometheus server scraping every 15 seconds
|
without checking the password. A Prometheus server scraping every 15 seconds
|
||||||
sends 4 a minute. IPv6 addresses in one /64 count as one client. When the
|
sends 4 a minute. IPv6 addresses in one /64 count as one client. When the
|
||||||
request comes from a private or loopback address, such as a reverse proxy's,
|
request comes from a private or loopback address, such as a reverse proxy's,
|
||||||
the client address is taken from the `X-Real-IP` header the proxy sets, or else
|
the client address is taken from the `X-Real-IP` or `X-Forwarded-For` header
|
||||||
from `X-Forwarded-For`, as the last address in it that is not private or
|
the proxy sets; a proxy that sets neither makes all its clients share one
|
||||||
loopback. A proxy that sets neither makes all its clients share one allowance.
|
allowance.
|
||||||
|
|
||||||
**`DNSWATCHER_DNS_INTERVAL` and `DNSWATCHER_TLS_INTERVAL`** take a positive
|
**`DNSWATCHER_DNS_INTERVAL` and `DNSWATCHER_TLS_INTERVAL`** take a positive
|
||||||
duration: a number followed by a unit such as `s`, `m` or `h`, for example
|
duration: a number followed by a unit such as `s`, `m` or `h`, for example
|
||||||
@@ -625,8 +625,7 @@ repository's `Dockerfile` and runs it. The app needs:
|
|||||||
abandoned, and the number abandoned is logged at warn level rather
|
abandoned, and the number abandoned is logged at warn level rather
|
||||||
than dropped silently. Notifications generated after shutdown has
|
than dropped silently. Notifications generated after shutdown has
|
||||||
begun are refused and logged, so a late burst cannot extend the
|
begun are refused and logged, so a late burst cannot extend the
|
||||||
shutdown. A DNS lookup, port check or TLS check that shutdown cuts
|
shutdown.
|
||||||
short saves nothing and sends no notification.
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
@@ -21,10 +21,6 @@ nameserver IP address changes: https://git.eeqj.de/sneak/dnswatcher/issues/105
|
|||||||
|
|
||||||
- 2026-10-01: `DNSWATCHER_SENTRY_DSN` reports panics in HTTP handlers to Sentry,
|
- 2026-10-01: `DNSWATCHER_SENTRY_DSN` reports panics in HTTP handlers to Sentry,
|
||||||
and a DSN Sentry cannot parse stops startup (closes #107).
|
and a DSN Sentry cannot parse stops startup (closes #107).
|
||||||
- 2026-10-01: a port or TLS check that shutdown cuts short saves nothing and
|
|
||||||
sends no notification, as a cut-short DNS lookup already did (closes #185).
|
|
||||||
- 2026-10-01: the client address from `X-Forwarded-For` is the last entry that
|
|
||||||
is not a trusted proxy, not the first, which the client sets (closes #181).
|
|
||||||
- 2026-10-01: a nameserver that does not answer is saved as `error` with the
|
- 2026-10-01: a nameserver that does not answer is saved as `error` with the
|
||||||
reason, and NS failure and NS recovery are notified (closes #104).
|
reason, and NS failure and NS recovery are notified (closes #104).
|
||||||
- 2026-10-01: a `DNSWATCHER_DNS_INTERVAL` or `DNSWATCHER_TLS_INTERVAL` that is
|
- 2026-10-01: a `DNSWATCHER_DNS_INTERVAL` or `DNSWATCHER_TLS_INTERVAL` that is
|
||||||
|
|||||||
@@ -1,9 +1,6 @@
|
|||||||
package middleware
|
package middleware
|
||||||
|
|
||||||
import (
|
import "time"
|
||||||
"net/http"
|
|
||||||
"time"
|
|
||||||
)
|
|
||||||
|
|
||||||
// The /metrics rate limit, exported so the tests can count requests
|
// The /metrics rate limit, exported so the tests can count requests
|
||||||
// against it.
|
// against it.
|
||||||
@@ -11,9 +8,3 @@ const (
|
|||||||
MetricsRequestLimit = metricsRequestLimit
|
MetricsRequestLimit = metricsRequestLimit
|
||||||
MetricsRequestWindow time.Duration = metricsRequestWindow
|
MetricsRequestWindow time.Duration = metricsRequestWindow
|
||||||
)
|
)
|
||||||
|
|
||||||
// RealIP is realIP, exported so the tests can check which address it
|
|
||||||
// takes as the client's.
|
|
||||||
func RealIP(r *http.Request) string {
|
|
||||||
return realIP(r)
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -209,12 +209,6 @@ func isTrustedProxy(ip net.IP) bool {
|
|||||||
|
|
||||||
// realIP extracts the client's real IP address from the request.
|
// realIP extracts the client's real IP address from the request.
|
||||||
// Proxy headers are only trusted from RFC1918/loopback addresses.
|
// Proxy headers are only trusted from RFC1918/loopback addresses.
|
||||||
//
|
|
||||||
// Each proxy adds to the end of X-Forwarded-For the address it got the
|
|
||||||
// request from, so the client can write every entry before the one the
|
|
||||||
// first trusted proxy added. The client address is therefore the
|
|
||||||
// rightmost entry that is not a trusted proxy, or the leftmost entry
|
|
||||||
// when they all are.
|
|
||||||
func realIP(r *http.Request) string {
|
func realIP(r *http.Request) string {
|
||||||
addr := ipFromHostPort(r.RemoteAddr)
|
addr := ipFromHostPort(r.RemoteAddr)
|
||||||
remoteIP := net.ParseIP(addr)
|
remoteIP := net.ParseIP(addr)
|
||||||
@@ -229,26 +223,16 @@ func realIP(r *http.Request) string {
|
|||||||
return ip
|
return ip
|
||||||
}
|
}
|
||||||
|
|
||||||
// A proxy may add its entry as a header line of its own instead of
|
if xff := r.Header.Get("X-Forwarded-For"); xff != "" {
|
||||||
// appending to the line the client sent, so all lines form one list.
|
if parts := strings.SplitN(
|
||||||
entries := strings.Split(
|
xff, ",", 2, //nolint:mnd
|
||||||
strings.Join(r.Header.Values("X-Forwarded-For"), ","), ",",
|
); len(parts) > 0 {
|
||||||
)
|
if ip := strings.TrimSpace(parts[0]); ip != "" {
|
||||||
client := strings.TrimSpace(entries[0])
|
return ip
|
||||||
|
}
|
||||||
for i := len(entries) - 1; i > 0; i-- {
|
|
||||||
entry := strings.TrimSpace(entries[i])
|
|
||||||
if !isTrustedProxy(net.ParseIP(entry)) {
|
|
||||||
client = entry
|
|
||||||
|
|
||||||
break
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if client != "" {
|
|
||||||
return client
|
|
||||||
}
|
|
||||||
|
|
||||||
return addr
|
return addr
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -342,9 +342,9 @@ func TestDashboardRendersWithSecurityHeaders(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Addresses for the rate limit and realIP tests: a client connecting
|
// Addresses for the rate limit tests: a client connecting directly, a
|
||||||
// directly, a trusted proxy, and a client behind that proxy as the
|
// trusted proxy, and a client behind that proxy as its X-Real-IP
|
||||||
// proxy's X-Real-IP or X-Forwarded-For header names it.
|
// header names it.
|
||||||
const (
|
const (
|
||||||
directClient = "198.51.100.1:4000"
|
directClient = "198.51.100.1:4000"
|
||||||
trustedProxy = "10.0.0.1:4000"
|
trustedProxy = "10.0.0.1:4000"
|
||||||
@@ -482,84 +482,3 @@ func TestMetricsRateLimitKeysOnClientAddress(t *testing.T) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestRealIP checks which address realIP takes as the client's. Each
|
|
||||||
// element of forwardedFor is sent as an X-Forwarded-For header line of
|
|
||||||
// its own, and 198.51.100.9 is always an entry the client wrote itself.
|
|
||||||
func TestRealIP(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
tests := []struct {
|
|
||||||
name string
|
|
||||||
remoteAddr string
|
|
||||||
xRealIP string
|
|
||||||
forwardedFor []string
|
|
||||||
want string
|
|
||||||
}{
|
|
||||||
{
|
|
||||||
"untrusted peer, both headers ignored",
|
|
||||||
directClient, proxiedClient, []string{"198.51.100.9"},
|
|
||||||
"198.51.100.1",
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"X-Real-IP from a trusted proxy wins",
|
|
||||||
trustedProxy, proxiedClient, []string{"203.0.113.8"},
|
|
||||||
proxiedClient,
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"client's own entry, then the one the proxy added",
|
|
||||||
trustedProxy, "", []string{"198.51.100.9, 203.0.113.1"},
|
|
||||||
proxiedClient,
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"several trusted proxies",
|
|
||||||
trustedProxy, "",
|
|
||||||
[]string{"198.51.100.9, 203.0.113.1, 10.0.0.3, 10.0.0.2"},
|
|
||||||
proxiedClient,
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"proxy adds a header line of its own",
|
|
||||||
trustedProxy, "", []string{"198.51.100.9", proxiedClient},
|
|
||||||
proxiedClient,
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"every entry a trusted proxy",
|
|
||||||
trustedProxy, "", []string{"10.0.0.3, 10.0.0.2"},
|
|
||||||
"10.0.0.3",
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"empty where the client address belongs",
|
|
||||||
trustedProxy, "", []string{"203.0.113.1, , 10.0.0.2"},
|
|
||||||
"10.0.0.1",
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"no headers from a trusted proxy",
|
|
||||||
trustedProxy, "", nil,
|
|
||||||
"10.0.0.1",
|
|
||||||
},
|
|
||||||
}
|
|
||||||
|
|
||||||
for _, tt := range tests {
|
|
||||||
t.Run(tt.name, func(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
req := httptest.NewRequestWithContext(
|
|
||||||
t.Context(), http.MethodGet, "/", nil,
|
|
||||||
)
|
|
||||||
req.RemoteAddr = tt.remoteAddr
|
|
||||||
|
|
||||||
if tt.xRealIP != "" {
|
|
||||||
req.Header.Set("X-Real-IP", tt.xRealIP)
|
|
||||||
}
|
|
||||||
|
|
||||||
for _, line := range tt.forwardedFor {
|
|
||||||
req.Header.Add("X-Forwarded-For", line)
|
|
||||||
}
|
|
||||||
|
|
||||||
got := middleware.RealIP(req)
|
|
||||||
if got != tt.want {
|
|
||||||
t.Errorf("realIP = %q, want %q", got, tt.want)
|
|
||||||
}
|
|
||||||
})
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -106,8 +106,7 @@ func TestSentryUnsetDoesNothing(t *testing.T) {
|
|||||||
|
|
||||||
// TestSentryReportsHandlerPanic checks that with a valid DSN a panic in
|
// TestSentryReportsHandlerPanic checks that with a valid DSN a panic in
|
||||||
// a handler is reported to Sentry, still reaches chimw.Recoverer, and
|
// a handler is reported to Sentry, still reaches chimw.Recoverer, and
|
||||||
// has been sent by the time Shutdown returns, and that nothing else is
|
// has been sent by the time Shutdown returns.
|
||||||
// sent to Sentry.
|
|
||||||
func TestSentryReportsHandlerPanic(t *testing.T) {
|
func TestSentryReportsHandlerPanic(t *testing.T) {
|
||||||
standIn := newSentryStandIn(t)
|
standIn := newSentryStandIn(t)
|
||||||
|
|
||||||
@@ -138,12 +137,6 @@ func TestSentryReportsHandlerPanic(t *testing.T) {
|
|||||||
},
|
},
|
||||||
)
|
)
|
||||||
|
|
||||||
// An ordinary request first: with client reports on, Sentry would
|
|
||||||
// add a count of its dropped transaction to the panic report.
|
|
||||||
serve(srv, httptest.NewRequestWithContext(
|
|
||||||
t.Context(), http.MethodGet, "/.well-known/healthcheck", nil,
|
|
||||||
))
|
|
||||||
|
|
||||||
rec := serve(srv, httptest.NewRequestWithContext(
|
rec := serve(srv, httptest.NewRequestWithContext(
|
||||||
t.Context(), http.MethodGet, "/panic", nil,
|
t.Context(), http.MethodGet, "/panic", nil,
|
||||||
))
|
))
|
||||||
@@ -162,10 +155,6 @@ func TestSentryReportsHandlerPanic(t *testing.T) {
|
|||||||
if !standIn.received(panicMessage) {
|
if !standIn.received(panicMessage) {
|
||||||
t.Error("the panic had not been sent to Sentry when Shutdown returned")
|
t.Error("the panic had not been sent to Sentry when Shutdown returned")
|
||||||
}
|
}
|
||||||
|
|
||||||
if standIn.received("client_report") {
|
|
||||||
t.Error("Sentry was sent a client report, not only the panic")
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestSentryInvalidDSNStopsStartup(t *testing.T) {
|
func TestSentryInvalidDSNStopsStartup(t *testing.T) {
|
||||||
|
|||||||
@@ -216,9 +216,6 @@ func (s *Server) enableSentry() error {
|
|||||||
// the default one, Flush can return before sending a report made
|
// the default one, Flush can return before sending a report made
|
||||||
// just before it, such as one from the last request at shutdown.
|
// just before it, such as one from the last request at shutdown.
|
||||||
DisableTelemetryBuffer: true,
|
DisableTelemetryBuffer: true,
|
||||||
// Send panic reports only, not Sentry's counts of what it dropped,
|
|
||||||
// such as the transaction it starts for every request.
|
|
||||||
DisableClientReports: true,
|
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("invalid DNSWATCHER_SENTRY_DSN: %w", err)
|
return fmt.Errorf("invalid DNSWATCHER_SENTRY_DSN: %w", err)
|
||||||
|
|||||||
@@ -1,81 +0,0 @@
|
|||||||
package watcher_test
|
|
||||||
|
|
||||||
import (
|
|
||||||
"context"
|
|
||||||
"log/slog"
|
|
||||||
"reflect"
|
|
||||||
"testing"
|
|
||||||
|
|
||||||
"sneak.berlin/go/dnswatcher/internal/portcheck"
|
|
||||||
"sneak.berlin/go/dnswatcher/internal/resolver"
|
|
||||||
"sneak.berlin/go/dnswatcher/internal/state"
|
|
||||||
"sneak.berlin/go/dnswatcher/internal/tlscheck"
|
|
||||||
"sneak.berlin/go/dnswatcher/internal/watcher"
|
|
||||||
)
|
|
||||||
|
|
||||||
// TestCancelledCheckSavesNothing runs a check with its context already
|
|
||||||
// cancelled, which is how the rest of a check runs once shutdown cuts it
|
|
||||||
// short. The real resolver drops the DNS lookup without sending a query,
|
|
||||||
// and the real port and TLS checkers fail without connecting. The port
|
|
||||||
// and certificate state the last check saved must stay as it was, and
|
|
||||||
// nothing may be notified.
|
|
||||||
func TestCancelledCheckSavesNothing(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
cfg := defaultTestConfig(t)
|
|
||||||
cfg.Hostnames = []string{host}
|
|
||||||
|
|
||||||
// newTestWatcher's watcher has stand-in checkers. This one, on the
|
|
||||||
// same state and notifier, has the real ones.
|
|
||||||
_, deps := newTestWatcher(t, cfg)
|
|
||||||
w := watcher.NewForTest(
|
|
||||||
cfg,
|
|
||||||
deps.state,
|
|
||||||
resolver.NewFromLogger(slog.Default()),
|
|
||||||
portcheck.NewStandalone(),
|
|
||||||
tlscheck.NewStandalone(),
|
|
||||||
deps.notifier,
|
|
||||||
)
|
|
||||||
|
|
||||||
// The last check found host at a local address, with both ports
|
|
||||||
// open and a good certificate.
|
|
||||||
const localIP = "127.0.0.1"
|
|
||||||
|
|
||||||
deps.state.SetHostnameState(host, hostnameState(
|
|
||||||
map[string]map[string][]string{nsA: {"A": {localIP}}},
|
|
||||||
))
|
|
||||||
|
|
||||||
ports := map[string]*state.PortState{
|
|
||||||
localIP + ":80": {Open: true, Hostnames: []string{host}},
|
|
||||||
localIP + ":443": {Open: true, Hostnames: []string{host}},
|
|
||||||
}
|
|
||||||
for key, ps := range ports {
|
|
||||||
deps.state.SetPortState(key, ps)
|
|
||||||
}
|
|
||||||
|
|
||||||
certKey := localIP + ":443:" + host
|
|
||||||
cert := &state.CertificateState{CommonName: host, Status: "ok"}
|
|
||||||
deps.state.SetCertificateState(certKey, cert)
|
|
||||||
|
|
||||||
ctx, cancel := context.WithCancel(t.Context())
|
|
||||||
cancel()
|
|
||||||
|
|
||||||
w.RunOnce(ctx)
|
|
||||||
|
|
||||||
for key, want := range ports {
|
|
||||||
got, _ := deps.state.GetPortState(key)
|
|
||||||
if !reflect.DeepEqual(got, want) {
|
|
||||||
t.Errorf("port %s saved as %+v, want %+v", key, got, want)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
got, _ := deps.state.GetCertificateState(certKey)
|
|
||||||
if !reflect.DeepEqual(got, cert) {
|
|
||||||
t.Errorf("certificate saved as %+v, want %+v", got, cert)
|
|
||||||
}
|
|
||||||
|
|
||||||
notifications := deps.notifier.getNotifications()
|
|
||||||
if len(notifications) != 0 {
|
|
||||||
t.Errorf("sent %v, want no notifications", notifications)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
@@ -658,13 +658,6 @@ func (w *Watcher) checkSinglePort(
|
|||||||
hostnames []string,
|
hostnames []string,
|
||||||
) {
|
) {
|
||||||
result, err := w.portCheck.CheckPort(ctx, ip, port)
|
result, err := w.portCheck.CheckPort(ctx, ip, port)
|
||||||
|
|
||||||
// A check the context cut short says nothing about the port, so it
|
|
||||||
// is neither saved nor notified.
|
|
||||||
if ctx.Err() != nil {
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
w.log.Error(
|
w.log.Error(
|
||||||
"port check failed",
|
"port check failed",
|
||||||
@@ -740,13 +733,6 @@ func (w *Watcher) checkTLSCert(
|
|||||||
hostname string,
|
hostname string,
|
||||||
) {
|
) {
|
||||||
cert, err := w.tlsCheck.CheckCertificate(ctx, ip, hostname)
|
cert, err := w.tlsCheck.CheckCertificate(ctx, ip, hostname)
|
||||||
|
|
||||||
// A check the context cut short says nothing about the certificate,
|
|
||||||
// so it is neither saved nor notified.
|
|
||||||
if ctx.Err() != nil {
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
certKey := fmt.Sprintf("%s:%d:%s", ip, tlsPort, hostname)
|
certKey := fmt.Sprintf("%s:%d:%s", ip, tlsPort, hostname)
|
||||||
now := time.Now().UTC()
|
now := time.Now().UTC()
|
||||||
prev, hasPrev := w.state.GetCertificateState(certKey)
|
prev, hasPrev := w.state.GetCertificateState(certKey)
|
||||||
|
|||||||
Reference in New Issue
Block a user