#826 osctrl: defensive hardening round — login timing, rate-limit XFF bypass, JWT rotation, info-disclosure closes Pull request opened by alvarofraguas osctrl: defensive hardening round — login timing, rate-limit XFF bypass, JWT rotation, info-disclosure closes This is the first of a multi-PR series. Each PR after this one builds on its predecessor and will go up separately once you've had a chance to look at this one. This PR is intentionally the smallest of the series — pure defensive hardening, no new endpoints, no behaviour change for legitimate users. Eight discrete fixes, surfaced during the pentest passes I ran against the dev stack. Numbered below in the order I'd suggest reading the diff. 1. `pkg/users`: close username-enumeration timing leak

CheckLoginCredentials

returned in ~15 ms when the user didn't exist (DB miss → no bcrypt) vs ~300 ms when the user existed but the password was wrong (DB hit → bcrypt cost-12 compare). A trivial remote distinguisher for "valid username, wrong password" vs "unknown username." Fix: add a package-init

dummyHash

bcrypt-hashed at the same cost, and run

bcrypt.CompareHashAndPassword(dummyHash, …)

on the DB-miss path. Result is discarded; we still return

false

. The compare's sole purpose is to burn the same wall-clock as a real compare. Regression test in `pkg/users/users_test.go`:

TestCheckLoginCredentials_TimingEqualization

asserts ratio < 2.0×. Measured locally: 1.00×. Skipped under

-short

because bcrypt cost-12 burns ~300ms per iteration. A second regression test

TestCheckLoginCredentials_UnknownUserStillReturnsFalse

pins that the dummyHash compare doesn't accidentally turn an unknown user into a successful login — the compare result must be discarded. 2. `pkg/ratelimit`: close X-Forwarded-For rotation bypass

KeyByIP

keyed the token bucket on

X-Forwarded-For

(or

X-Real-IP

). When osctrl-api sits behind nginx that's the right key — but the same code path runs when osctrl-api is directly exposed (or when nginx isn't stripping client-supplied

X-Forwarded-For

). Result: an attacker rotates the header on each request and the bucket never fills. Fix: switch

KeyByIP

to use a new

utils.RemoteIP(r)

helper that returns the TCP peer address from

r.RemoteAddr

. The trusted-proxy case is handled at the proxy layer (nginx replaces XFF rather than appending), not in our rate-limiter. Regression tests:

TestKeyByIPIgnoresForwardingHeaders

covers 5 sub-cases (no headers, single XFF, chained XFF, X-Real-IP, both).

TestMiddlewareXFFRotationDoesNotBypass

is end-to-end: burst=2 then 5 rotated-XFF attempts all 429. 3. `osctrl-api`: require auth on

/queries/samples

+

/carves/samples

Both endpoints returned a sample-template library (

SELECT * FROM osquery_info LIMIT 5

-style) without any auth check. Two issues: • Pre-auth fingerprint: the template payload uniquely identifies the osctrl-api version (each release ships a different starter pack), letting a scanner version-pin without authenticating. • Carve target disclosure: the carve samples include file-path arguments (

'/etc/shadow'

,

'/var/log/secure'

) that name privileged read targets, useful for someone scoping a future CarveLevel-credential phishing campaign. Fix: move the route registrations from the pre-auth block to the authenticated block in

cmd/api/main.go

. The handler functions themselves were unchanged. 4. `osctrl-api`:

RootHandler

404s on misrouted GETs Go's

ServeMux

uses

/

as a wildcard catch-all for any GET request the mux doesn't have a more-specific pattern for. With

RootHandler

returning 200 unconditionally, typos like

GET /api/v1/totally-fake

silently succeeded — confusing for clients debugging an integration and a weak fingerprint signal for scanners ("endpoint X returned 200 → must exist"). Tighten the contract: respond 200 ONLY when

r.URL.Path == "/"

. Otherwise fall through to

http.NotFound

. 5. `deploy/nginx`: strip nginx version from Server header Without

server_tokens off

, nginx emits

Server: nginx/1.27.x

— a free fingerprint for vuln scanners (Shodan, nuclei) that match known CVEs by version.

server_tokens off

makes it just

Server: nginx

. Doesn't hide that this IS nginx (no portable way in OSS nginx) but stops version-keyed CVE matching. Applied to both

deploy/docker/conf/nginx/osctrl.conf

and the dev-stack

frontend-dev.conf

. 6. `osctrl-admin`: disable autoindex on

/static/

Go's

http.FileServer

autoindexed

/static/

—

GET /static/

returned an HTML directory listing. Useful recon for a scanner mapping the deployed JS / CSS / icon set. Fix: wrap

http.FileServer

in a

noDirListing

middleware that 404s any path ending in

/

. Static asset serving for real files (

/static/js/foo.js

, etc.) is unaffected. 7. `pkg/users`: stamp random

jti

on every JWT Foundation for the rotation fix in (8). `CreateToken`'s claims were deterministic:

Username

+

Issuer

+

ExpiresAt

(at 1s resolution). HMAC-SHA256 is deterministic for the same key + payload, so two

CreateToken

calls for the same user in the same second returned identical JWT strings. That silently broke any caller depending on token rotation as a revocation primitive. The auth middleware in

cmd/api/auth.go

compares every presented JWT against the stored

AdminUser.APIToken

(constant-time) — so logging in is supposed to invalidate the previous session by overwriting the stored token. But if the new "minted" token is bitwise identical to the old one,

UpdateToken

is a no-op, the stored value doesn't change, and any previously- issued copy keeps validating. Stamp a random 16-byte hex

jti

(RFC 7519 §4.1.7) on every issuance so claims are guaranteed distinct. Regression tests in `pkg/users/users_test.go`: •

TestCreateTokenIsNonDeterministic

— two

CreateToken

calls return different strings. •

TestCreateTokenStampsJTI

— the parsed claims carry distinct non-empty

jti

values. Both fail loudly if the

jti

claim is removed (verified locally). 8. `osctrl-api`: rotate JWT on every login

LoginHandler

had a 60s-freshness branch: if the user's stored

APIToken

had >60s of life left, login returned it as-is instead of minting fresh. The original intent was to avoid handing out a token that would fail mid-request. The side effect was that a second login from a different device got the SAME JWT — leaving the previous device's copy valid until natural expiry. Drop the reuse branch. Always

CreateToken

+

UpdateToken

on successful login. With the

jti

claim from (7), the new token is provably distinct, so

UpdateToken

actually rotates the stored value. The auth middleware's constant-time compare in

cmd/api/auth.go

then fails the old JWT 401 on its next request — even though the old JWT is still cryptographically valid against the secret. The DB-row

APIToken

check IS the revocation primitive; this commit just makes login exercise it correctly. Note: a server-side

/logout

endpoint that explicitly invalidates the stored token is part of the next PR in the series (OIDC support on osctrl-api). The richer logout flow there returns

idp_logout_url

for federated sessions, which makes more sense to introduce alongside OIDC than as a half-feature here. Verified •

go build ./...

clean. •

go vet ./...

clean. •

go test ./...

green across all packages on

pr/security-hardening

. • Live smoke on a dev stack against real Postgres + Redis + Keycloak: • Login timing:

bad-user

0.21s median vs

unknown-user

0.21s median, ratio 1.01×. • 15 burst requests with rotated

X-Forwarded-For

headers from the same TCP peer all returned 429 starting at request #11. •

GET /api/v1/queries/samples

and

GET /api/v1/carves/samples

unauthenticated → 401. •

GET /api/v1/totally-fake-path

→ 404. •

curl -I <http://host:8088/>

→

Server: nginx

with no version. •

GET /static/

on osctrl-admin → 404. • … jmpsec/osctrl