#1139 Fixes for medium severity issues found in audit Pull request opened by javuto Problem A security review of revision

fc7ed508

reported 19 medium-severity findings. The high-severity ones were fixed in a separate PR. This PR addresses all 19, grouped by area. Changes Query results • [6] Untargeted nodes can inject results into known queries.

UpdateQueryStatus

now moves a node's assignment from

pending

to a final status in one conditional

UPDATE

, and returns

ErrNotPending

if there's nothing to move.

ProcessLogQueryResult

does this first, and runs result export, alert matching and counters only when it succeeds. Carve rows are only created for a node with a pending assignment for that carve query in its own environment (

HasPendingAssignment

). • [15] Retried query responses corrupt counters and final state. Covered by the same change: a retried or conflicting response finds nothing pending and changes nothing. Carves • [17] Empty session IDs authorize carve uploads.

GetBySession

rejects an empty session ID and requires a matching row. • [21] Duplicate blocks fake completion. A block is only accepted while the carve is

IN PROGRESS

, with

block_id

inside the declared block count, valid base64, and a decoded size no larger than

block_size

. A duplicate block is acknowledged but not stored again.

GetBlocks

writes each block once, even for duplicates stored before this change. • [11] Failed block writes are acknowledged. Blocks are stored before the node gets

success

. With the S3 carver, the upload happens before the database row is written. • [10] Concurrent workers lose completion counts.

completed_blocks

is computed from the distinct blocks stored instead of being incremented. Completion is claimed with one conditional

IN PROGRESS → COMPLETED

update, so exactly one worker finalizes the carve. Node keys • [7] Deleted or rotated keys stay valid in TLS caches. The node-key cache moves to Redis and is shared by osctrl-api and every osctrl-tls replica.

NodeManager.Update

(re-enrollment) and

ArchiveDelete

replace the old key with a "deleted" marker. Cache misses refill without overwriting an existing entry, so a concurrent reader can't put a revoked node back. Log ingestion • [19] Telemetry is acknowledged before it's stored. Log batches are processed inside the request. The DB and S3 sinks now return their errors, and a failure answers 500, so osquery keeps the batch and resends it. A batch where the database stores no row at all is reported as an outage. A row refused for its content is still logged and skipped, so one bad row can't block a node forever. • [24] Slow sinks cause an unbounded backlog. Batches being processed share a 256 MiB budget. When it's full, the handler answers 503 and increments the new

osctrl_tls_log_backpressure_total

metric. S3 logs • [8] Reads return only the first entry of each batch. Every entry of a stored batch is now a row, and search and severity filters apply per row. • [9] Batches overwrite each other within a millisecond. Object names get a random batch suffix:

{ts}-{batch}.json

. • [16] Every page read scans the whole history. Status and result keys add a UTC day folder:

{env}/{logType}/{uuid}/{YYYY-MM-DD}/{ts}-{batch}.json

. The reader lists day folders, walks them newest first, and stops when the page is full or the days are older than

since

. Keys written before this change are still read. Alerts • [18] A stalled SMTP relay blocks all alert delivery. SMTP has a dial timeout and one deadline for the whole exchange, including

QUIT

. • [22] Inactivity alerts are lost on queue overflow or delivery failure. The inactivity sweep delivers its alerts directly, eight at a time (

Worker.Deliver

), instead of through the lossy queue. The saved state only records transitions whose alerts were delivered, so a failure is retried on the next sweep. API • [13] Non-admins can read other operators' audit records. Non-admins are filtered by exact username (

PageFilter.Actor

). Before, a partial match meant

alice

also saw

alice-admin

, and

_

matched everyone. • [23] Anonymous login bodies use unbounded memory and audit storage. Login and MFA bodies are capped at 64 KiB. Usernames over 256 bytes and passwords over 1024 bytes are rejected before the credential lookup.

FailedLogin

truncates the stored username. A malformed login body now returns 400 instead of 500. osctrld rollouts • [20] Replayed rollback events can undo a resumed rollout. A new table,

node_osctrld_events

, records each

(node, kind, event_id)

once, with the time it was first seen. Recording it and updating the status row commit in one transaction. A replayed older event isn't counted again, doesn't become the node's latest event, and can't pause a resumed rollout. Events stored before the table existed aren't counted twice. Vulnerability matching • [14] Matching falls behind large fleets. • Each tick matches 500-node batches for up to 40 seconds. • An advisory change now records

vuln_worker_state.advisories_at

instead of resetting every node's

matched_at

. • Nodes due for matching are served oldest assessment first, so a new advisory generation continues where the last one stopped. Deployment • [12] Generated configs with credentials are world-readable. •

config-generate

writes

0600

files in a

0750

directory, and tightens the mode of a file it overwrites. •

provision.sh

creates the

osctrl

user first and sets the configs to

root:osctrl

with

0640

files in a

0750

directory, as the packages do. It now runs its later

config-verify

and

osctrl-cli --db

calls with

sudo

. Security / operational impact • Schema (

AutoMigrate

, additive): a new table

node_osctrld_events

and a new column

vuln_worker_state.advisories_at

. No unique index was added on carve blocks, because building one fails on tables that already hold duplicate rows. • Redis: osctrl-tls and osctrl-api now read the shared node cache from Redis on node-key lookups. Changes made with

osctrl-cli --db

don't touch Redis, so their old keys stay valid for up to the 10-minute cache lifetime. • Log latency and backpressure: log requests now wait for storage, so they take longer. Under load, nodes get 503s and retry; watch

osctrl_tls_log_backpressure_total

. • Log sinks: a batch that partly succeeded before failing can be stored twice on retry. Network sinks (Splunk, Kafka, Elastic, Graylog, Logstash, Kinesis, HTTP) still don't report their errors, so they stay best-effort. The query-result path still runs in the background. • S3 key layout: the new log keys use

{ts}-{batch}.json

, and status and result keys gain a day folder. Readers handle both the old and new layouts. Any tool outside osctrl that parses these keys needs updating. • Login: a malformed login body returns 400 instead of 500. • Not changed: the nginx

client_max_body_size 65m

on

/api/

. The app now enforces small caps on login and MFA itself. • Behavior changes: •

matched_at

is no longer cleared after advisory syncs, so it always shows a node's last assessment time. • The vuln worker state JSON includes

advisories_at

. Validation •

go test ./...

,

go vet ./...

and

make openapi-check

pass. •

go test -race

passes for

pkg/alerts

,

pkg/nodes

,

pkg/logging

and

cmd/tls/handlers

. • New regression tests: • Query results: unassigned and retried results; pending-assignment claims. • Carves: a block from another node; empty session ID; out-of-range, oversized, duplicate and post-completion blocks; completion claimed once; a failed S3 upload stores no block row. • Node keys: revocation on re-enroll and delete is shared across processes and survives a stale refill (fake Redis). • Logs: • a sink failure answers 500 and a full budget answers 503; • a database outage is reported while a single bad row is not; • keys from two batches in the same millisecond don't c… jmpsec/osctrl