diff --git a/cmd/tls/handlers/osctrld_limits.go b/cmd/tls/handlers/osctrld_limits.go index b856c5f1..8954607f 100644 --- a/cmd/tls/handlers/osctrld_limits.go +++ b/cmd/tls/handlers/osctrld_limits.go @@ -16,6 +16,10 @@ import ( // a spray from many addresses cannot flood audit_logs. const osctrldAuditPerHour = 60 +// osctrldUnknownEnv stands in for every environment in a path that does +// not resolve, so made-up UUIDs share one audit cap and one metric label. +const osctrldUnknownEnv = "" + // OsctrldLimits holds the per-process rate limiters of the osctrld routes. // They key on utils.GetIP: the TCP peer, or the forwarded client when the // peer is a configured trusted proxy (otherwise every client behind a proxy @@ -127,18 +131,16 @@ func (h *HandlersTLS) osctrldAudit(ip string, env environments.TLSEnvironment, r h.AuditLog.FailedOsctrld(ip, env.Name, reason, env.ID) } -// osctrldAuditEnv resolves the environment in the path for an audit entry, -// falling back to the (UUID-validated) path value with no ID. +// osctrldAuditEnv resolves the environment in the path for an audit entry. +// Any path that does not resolve maps to the osctrldUnknownEnv sentinel +// (ID 0), so it shares one audit cap, audit environment and metric label. func (h *HandlersTLS) osctrldAuditEnv(r *http.Request, envVar string) environments.TLSEnvironment { if h.EnvCache != nil { if env, err := h.EnvCache.GetByUUID(r.Context(), strings.ToLower(envVar)); err == nil { return env } } - if !utils.CheckUUID(envVar) { - envVar = "" - } - return environments.TLSEnvironment{UUID: envVar, Name: envVar} + return environments.TLSEnvironment{UUID: osctrldUnknownEnv, Name: osctrldUnknownEnv} } // osctrldSecret checks the secret of an osctrld request. The budget check, diff --git a/cmd/tls/handlers/osctrld_limits_test.go b/cmd/tls/handlers/osctrld_limits_test.go index 2567108a..0e98f0bb 100644 --- a/cmd/tls/handlers/osctrld_limits_test.go +++ b/cmd/tls/handlers/osctrld_limits_test.go @@ -3,6 +3,7 @@ package handlers import ( "bytes" "encoding/json" + "fmt" "net/http" "net/http/httptest" "strconv" @@ -14,6 +15,7 @@ import ( "github.com/jmpsec/osctrl/pkg/auditlog" "github.com/jmpsec/osctrl/pkg/config" "github.com/jmpsec/osctrl/pkg/environments" + "github.com/jmpsec/osctrl/pkg/ratelimit" "github.com/jmpsec/osctrl/pkg/utils" "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" @@ -368,3 +370,35 @@ func TestOsctrldBlockedAuditResolvesUpperCaseEnv(t *testing.T) { require.Equal(t, f.env.ID, row.EnvironmentID) require.Contains(t, row.Line, f.env.Name) } + +// Blocked rejections for environments that do not resolve all share one +// "" audit cap, audit environment and metric label, however many +// made-up UUIDs are used. Known environments keep their own. +func TestOsctrldUnknownEnvsShareAuditCap(t *testing.T) { + cfg := config.DefaultRateLimits() + cfg.OsctrldAuth.Burst = 1 + cfg.OsctrldAuth.MaxBuckets = 1 + f := newOsctrldFixture(t, cfg) + f.h.OsctrldLimits.authAudit = ratelimit.New(1, time.Hour, 2*time.Hour) + // Fill the map, then spend the shared overflow budget. + require.Equal(t, http.StatusForbidden, f.cert("198.51.100.1", f.env, "wrong")) + require.Equal(t, http.StatusForbidden, f.cert("198.51.100.2", f.env, "wrong")) + dropped := osctrldAuthAuditDropped.WithLabelValues(osctrldUnknownEnv) + before := metricValue(t, dropped) + for i := 0; i < 70; i++ { + unknown := environments.TLSEnvironment{UUID: fmt.Sprintf("%08d-1111-4111-8111-111111111111", i)} + require.Equal(t, http.StatusTooManyRequests, f.cert("203.0.113."+strconv.Itoa(i), unknown, "wrong")) + } + var rows []auditlog.AuditLog + require.NoError(t, f.db.Where("environment_id = ?", 0).Find(&rows).Error) + require.Len(t, rows, osctrldAuditPerHour) + for _, row := range rows { + require.Contains(t, row.Line, "env "+osctrldUnknownEnv+":") + } + require.Equal(t, before+70-osctrldAuditPerHour, metricValue(t, dropped)) + // The known environment's audits were not touched by the unknown cap. + var known int64 + require.NoError(t, f.db.Model(&auditlog.AuditLog{}).Where("environment_id = ?", f.env.ID).Count(&known).Error) + require.EqualValues(t, 2, known) + require.Equal(t, f.env.Name, f.h.osctrldAuditEnv(httptest.NewRequest(http.MethodPost, "/", nil), f.env.UUID).Name) +}