diff --git a/internal/auth/jwtSession.go b/internal/auth/jwtSession.go index de7e985b..8fae61e1 100644 --- a/internal/auth/jwtSession.go +++ b/internal/auth/jwtSession.go @@ -25,15 +25,21 @@ type JWTSessionAuthenticator struct { var _ Authenticator = (*JWTSessionAuthenticator)(nil) func (ja *JWTSessionAuthenticator) Init() error { - if pubKey := os.Getenv("CROSS_LOGIN_JWT_HS512_KEY"); pubKey != "" { - bytes, err := base64.StdEncoding.DecodeString(pubKey) - if err != nil { - cclog.Warn("Could not decode cross login JWT HS512 key") - return err - } - ja.loginTokenKey = bytes + pubKey := os.Getenv("CROSS_LOGIN_JWT_HS512_KEY") + if pubKey == "" { + // Without a configured key the HMAC verification below would run against + // an empty key, which lets anyone forge a valid token. Refuse to register + // the authenticator in that case so JWT session login is simply disabled. + return errors.New("CROSS_LOGIN_JWT_HS512_KEY not set: JWT session login disabled") } + bytes, err := base64.StdEncoding.DecodeString(pubKey) + if err != nil { + cclog.Warn("Could not decode cross login JWT HS512 key") + return err + } + ja.loginTokenKey = bytes + cclog.Info("JWT Session authenticator successfully registered") return nil } @@ -60,6 +66,12 @@ func (ja *JWTSessionAuthenticator) Login( token, err := jwt.Parse(rawtoken, func(t *jwt.Token) (any, error) { if t.Method == jwt.SigningMethodHS256 || t.Method == jwt.SigningMethodHS512 { + // Defense in depth: an empty key would verify any HMAC signature. + // Init() already refuses to register without a key, so this should + // never trigger, but guard explicitly rather than trust the chain. + if len(ja.loginTokenKey) == 0 { + return nil, errors.New("HS login key not configured") + } return ja.loginTokenKey, nil } return nil, fmt.Errorf("unkown signing method for login token: %s (known: HS256, HS512, EdDSA)", t.Method.Alg()) diff --git a/internal/repository/jobQuery.go b/internal/repository/jobQuery.go index 5256bacb..0b162179 100644 --- a/internal/repository/jobQuery.go +++ b/internal/repository/jobQuery.go @@ -336,8 +336,18 @@ func buildTimeCondition(field string, cond *config.TimeRange, query sq.SelectBui } } +// validMetricName guards metric/footprint names that are interpolated into the +// json_extract() path of footprint queries. SQLite treats double-quoted strings +// as string literals, so an unescaped name (e.g. containing a `"`) would allow +// SQL injection. Legitimate metric names only use these characters. +var validMetricName = regexp.MustCompile(`^[a-zA-Z0-9_]+$`) + // buildFloatJSONCondition creates a filter on a numeric field within the footprint JSON column, using BETWEEN only if required. func buildFloatJSONCondition(jsonField string, cond *model.FloatRange, query sq.SelectBuilder) sq.SelectBuilder { + if !validMetricName.MatchString(jsonField) { + cclog.Warnf("buildFloatJSONCondition: rejecting invalid metric name %q", jsonField) + return query.Where("0 = 1") + } query = query.Where("JSON_VALID(footprint)") if cond.From > 0.0 && cond.To > 0.0 { return query.Where("JSON_EXTRACT(footprint, \"$."+jsonField+"\") BETWEEN ? AND ?", cond.From, cond.To) diff --git a/internal/repository/stats.go b/internal/repository/stats.go index 0bc85109..fb0bc075 100644 --- a/internal/repository/stats.go +++ b/internal/repository/stats.go @@ -909,6 +909,13 @@ func (r *JobRepository) jobsMetricStatisticsHistogram( filters []*model.JobFilter, bins *int, ) (*model.MetricHistoPoints, error) { + // The metric name is interpolated into the json_extract() path of the SQL + // below. SQLite parses double-quoted strings as literals, so reject anything + // that is not a plain metric identifier to prevent SQL injection. + if !validMetricName.MatchString(metric) { + return nil, fmt.Errorf("invalid metric name: %q", metric) + } + // Peak value defines the upper bound for binning: values are distributed across // bins from 0 to peak. First try to get peak from filtered cluster, otherwise // scan all clusters to find the maximum peak value. diff --git a/pkg/metricstore/lineprotocol.go b/pkg/metricstore/lineprotocol.go index ed42eead..c8b27126 100644 --- a/pkg/metricstore/lineprotocol.go +++ b/pkg/metricstore/lineprotocol.go @@ -22,6 +22,7 @@ import ( "bytes" "context" "fmt" + "strings" "sync" "sync/atomic" "time" @@ -196,6 +197,16 @@ var decodeStatePool = sync.Pool{ // When the checkpoint format is "wal" each successfully decoded sample is also // sent to WALMessages so the WAL staging goroutine can persist it durably // before the next binary snapshot. +// isValidPathComponent reports whether s is safe to use as a single filesystem +// path component (cluster or host name) when building checkpoint/WAL paths. It +// rejects path-traversal sequences and embedded separators. An empty string is +// allowed because a missing host tag is legitimate and does not escape the root. +func isValidPathComponent(s string) bool { + return !strings.Contains(s, "..") && + !strings.Contains(s, "/") && + !strings.Contains(s, "\\") +} + func DecodeLine(dec *lineprotocol.Decoder, ms *MemoryStore, clusterDefault string, @@ -282,6 +293,17 @@ func DecodeLine(dec *lineprotocol.Decoder, } } + // cluster and host are taken verbatim from attacker-controllable line + // protocol tags and are later used as filesystem path components for the + // checkpoint/WAL files (path.Join(RootDir, cluster, host)). Reject + // path-traversal sequences so a malicious tag cannot escape the + // checkpoint root. Skip the offending sample; the rest of the batch is + // still processed. + if !isValidPathComponent(cluster) || !isValidPathComponent(host) { + cclog.Warnf("[METRICSTORE]> rejecting metric with invalid cluster/host tag (cluster=%q host=%q)", cluster, host) + continue + } + // If the cluster or host changed, the lvl was set to nil if lvl == nil { st.selector = st.selector[:2]