Skip to content

Security hardening: fix authz, client verification, and protocol issues from full-codebase review - #10

Merged
rthellend merged 36 commits into
mainfrom
security-fixes
Oct 6, 2026
Merged

rthellend merged 36 commits into
mainfrom
security-fixes

Conversation

@rthellend

@rthellend rthellend commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes the findings from a full-codebase security review. Every fix has a regression test, and nearly all were confirmed to fail on the old code.

Threat model: unauthenticated network attackers, authenticated users attacking other users, and a malicious or compromised metadata server, which clients must detect.

⚠️ Breaking changes

  • Wire protocol: clients and servers must be upgraded together.
    • Login signatures are domain-separated ("DistFS-Login-v1\x00" || challenge).
    • Sealed requests must get sealed, cluster-bound responses.
    • The node-join discovery proof covers the response body.
    • Storage tokens follow stricter mode rules and may carry creator_only.
  • Signed inode format: Size is now part of ManifestHash. Inodes signed before this change no longer verify and must be rewritten.
  • Quota groups: only administrators can create quota-enabled groups.
  • New server flag: --oidc-client-id (default distfs). ID tokens must have a matching aud, the discovered iss, and an exp.
  • CLI: registry-add uses a 128-bit verification code (previously 24 bits). The user displays it with distfs whoami --security-code.

Fixes

Critical: server authorization

  • S1: A double RUnlock on an expired session key let any user crash the leader.
  • S2: The login challenge was a signing oracle: a malicious server could get valid signatures over forged inode hashes.
  • S3: IssueToken minted delete/read tokens for any chunk after only a read check. Tokens are now limited to the inode's manifest. Delete tokens, and reads of uncommitted chunks, only work for chunks the requesting session created on that data node.
  • S4: CreateGroup could overwrite an existing group (full group takeover).
  • S5: Client-supplied chunk page IDs let users delete or GC other inodes' manifests.
  • S6: Listing a victim's chunk IDs in your own manifest let GC or replication pruning delete their data. New FSM chunk-ownership index.

High: client must not trust the server

  • C1–C2: Signature checks used keys fetched unverified from the server, and deferred registry verification didn't confirm the keys actually used. Registry attestations must now name the entity being verified.
  • C3: File keys and group seeds could be encrypted to server-substituted keys. Recipients are now verified; out-of-band ContactInfo is honored.
  • C4: Admin anchoring had a TOCTOU and a 24-bit code. The attestation is now bound to the confirmed 128-bit code.
  • C5: The server could return a different validly signed inode than the one requested.
  • C6: Unsealed or unbound responses were trusted. The server also fell back to plaintext and answered several sealed actions (world key, ValidateMetadata, audit, empty Raft results) unsealed.
  • C7: Downloaded chunks weren't checked against their content address, allowing rollback.
  • C8: Inode Size wasn't signed, allowing truncation or extension.
  • C9: Write results were cached and used as the root anchor without verification.

Medium

  • M1: OIDC tokens are now checked for issuer, audience and expiry.
  • M2: Other sessions' lease identifiers are redacted from GetInode.
  • M3: Exclusive leases require write access. Batch sub-commands can no longer override the session, user or timestamp.
  • M4: The group membership privacy claim is corrected in DISTFS-RAFT.md (docs only).
  • M5: Quota bypasses are closed: size-0 manifests, self-made unlimited quota groups, and client-set group usage, limits and GID.
  • M6: Account locks now apply to existing sessions.
  • M7: Replay protection is keyed on the authenticated request signature, not an unauthenticated prefix.
  • M8: Cluster join: the discovery proof is bound to the response body, and the bootstrap push is pinned to the authenticated TLS key.
  • M9: The web service worker never serves decrypted files as active content (CSP sandbox, nosniff, inert MIME allowlist, no navigations).

Low

  • L1: mTLS peers are matched on the full TLS key, not the 64-bit node ID.
  • L2: Unauthenticated login endpoints are bounded (body size, user ID length, pending challenges).
  • L3: FSM apply is deterministic: commands are stamped by the proposer, and lease checks use the command timestamp.

Known limitations / follow-ups

  • M10, deferred: server KEM key pinning. The key rotates every 24h, so pinning it breaks. Proposed follow-up: sign each epoch key with the cluster signing key, and pin that key in CLI and web configs. Impact is low today, because requests after login use the session key.
  • Inode metadata visibility: signed inode fields remain readable by any user, because hiding them would break client-side verification (e.g. ls -l on unreadable files).
  • Shared leases: these are still unchecked, because the server can't see anonymous group members. Lease expiry and the per-user cap limit the impact.
  • Still trust-on-first-use: the world public key and the server/cluster signing keys.

Testing

  • Unit suites pass: go test ./pkg/... ./cmd/... (metadata, data, crypto, client, CLI), plus the WASM build and WASM unit tests.
  • End-to-end passes: scripts/run-tests.sh --fast, all 18 Docker e2e tests.
  • New tests: pkg/metadata/security_test.go, pkg/data/security_test.go and pkg/client/security_test.go. Existing fixtures that relied on the insecure behavior were updated.

🤖 Generated with Claude Code

rthellend and others added 30 commits October 5, 2026 08:56
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An expired cached session key caused sessionKeyMu.RUnlock() to be called
twice, a fatal unrecoverable error that let any authenticated user crash
the metadata leader.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The client signed a raw server-chosen 32-byte challenge with its identity
key. A malicious server could set the challenge to an inode ManifestHash
and obtain a valid UserSig for forged metadata. Login signatures are now
over "DistFS-Login-v1\x00" || challenge, and the client rejects
challenges that are not exactly 32 bytes.

This is a wire protocol change: SERVER-API.md and DISTFS-RAFT.md updated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
IssueToken copied the client's mode verbatim and only required write
permission when the mode was exactly "W", and never checked requested
chunks against the inode manifest. Any user could mint delete tokens for
any chunk in the cluster, read arbitrary chunk ciphertext, and skip quota
reservation with "RW".

- Clients may only request R, W or D.
- R requires every chunk to be in the inode's manifest.
- W and D require write access; D needs explicit chunks that are not in
  the committed manifest, and a session.
- Data nodes record which session created each chunk and only honor a
  session-bound delete for chunks that session created. Cluster-issued
  (unbound) tokens used by GC and replication are unaffected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
executeCreateGroup never checked whether the group already existed, and
self-owned groups skip management authorization. Any user could resend
CreateGroup with a victim group's ID and GID to replace its owner, keys
and lockbox, making themselves a member and evicting real members.

Also removes a test step that silently relied on recreating a group.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Chunk pages are generated by the FSM as <inodeID>:p<i>, but create,
update and lease placeholders accepted client-supplied page IDs as-is.
A user could reference another inode's pages, then update to [] to
delete them (wiping the victim's manifest) or delete the inode to have
GC remove the victim's chunks.

New inodes and placeholders may not carry pages; updates may only
reference pages the inode already owns.

Also adds TODO S6 for the remaining manifest/GC ownership issue.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Chunk manifests are client-supplied. A user could list a victim's chunk
IDs (visible via GetInode) in their own file and delete it, making GC
delete the victim's data; listing extra node IDs made the replication
monitor prune real replicas.

The FSM now keeps a chunk_owners index (chunk ID -> inode ID). Create
and update may only add chunks that are unowned or already owned by the
same inode, and GC only enqueues chunks owned by the inode being
finalized. Replication only acts on manifests, which can no longer
contain foreign chunks. The index is rebuilt from existing inodes on
open and after snapshot restore.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The S3 restriction (R tokens only for manifest chunks) broke the FUSE
read-modify-write path and direct chunk reads, which legitimately read
chunks uploaded in the current session before they are committed.

An R request for chunks outside the manifest now requires write access
and a session, and yields a creator_only token that data nodes honor
only for chunks the bound session created. Creator records now last for
the session lifetime (1h).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Inode, group and registry-attestation signatures were checked against
keys fetched unverified from the server, while deferred registry
verification only recorded the signer ID: it was skipped when the ID was
already in the verified cache and otherwise re-fetched the user. A
malicious server could return substitute keys for the signature check
and the genuine record for the confirmation, forging any metadata.

- Signer, owner and verifier keys now come from getUser: pinned or
  verified keys from cache, or keys recorded for deferred verification.
- The verification queue records the exact user record used and
  confirms that record; it fails if it differs from the verified cache
  or if different keys were used for the same user.
- verifyInode/verifyUser/verifyGroup process their own queue when
  called outside an existing one.
- Registry attestations must name the user/group being verified.
- verifyGroup only promotes a group to the verified cache after the ID
  commitment check.
- getUserRaw/getGroupRaw reject responses for a different ID.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
provisionRecipient used recipient keys fetched unverified from the
server, and AddUserToGroup ignored the out-of-band verified ContactInfo
it was given. A malicious server could substitute its own key and learn
file keys (chmod/chown/setfacl) or group epoch seeds (group add).

- provisionRecipient resolves users and groups through the verified
  getters; recipients without verified keys are not provisioned.
- AddUserToGroup encrypts to the ContactInfo keys when provided, and
  rejects the operation if the server's keys for that user differ.
- Tests that added unverified users now pass contact info.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
registry-add showed a verification code computed from one server fetch
and then signed an attestation for keys from a second fetch, so a
malicious server could pass the out-of-band check and still get its own
keys attested. The code was also only 24 bits, cheap to grind.

- AnchorUserInRegistryWithCode refuses to sign unless the attested keys
  match the code the admin confirmed; registry-add uses it.
- The code is now 128 bits (8 groups of 4 hex digits).
- 'distfs whoami' prints the user's own security code, so the user can
  actually read it to the admin.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An inode's signature binds its content to its own ID, but the client
never checked that the server answered with the inode it asked for. A
malicious server could serve any validly signed inode (e.g. a file
written by a colluding user) at any path.

getInodeInternal now rejects a response whose ID differs from the
requested ID, and getInodes rejects unrequested or duplicate inodes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The client returned unsealed response bodies as trusted data even when
it expected a sealed reply, and only checked the cluster binding
signature when the server chose to include one. A network attacker
could forge metadata responses and a forking server could skip the
timeline binding. The server also fell back to plaintext when sealing
failed, and several sealed actions (world private key, ValidateMetadata,
admin audit, empty Raft results) were answered in plaintext.

Client:
- Successful responses to sealed requests must be sealed.
- Once the cluster signing key is known, sealed responses must carry a
  valid binding signature.
- The client verifies its own identity against the keys it holds rather
  than the registry.
- AnchorGroupInRegistry verifies the group's signature and ID
  commitment before attesting it.

Server:
- Never falls back to plaintext for sealed requests.
- Seals the world private key, ValidateMetadata, audit stream and
  empty Raft command responses.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Size bounds reads of the zero-padded chunk data but was not covered by
ManifestHash, so a malicious server could truncate files (e.g. cut the
tail of a script or authorized_keys) or extend them with NULs without
invalidating the owner's signature.

This changes the signed format: inodes signed before this change no
longer verify and must be rewritten.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
Chunk IDs are SHA-256 of the ciphertext, but the client never checked
downloaded data against the ID named in the signed manifest. Since file
keys persist across versions, a malicious storage node could serve an
older ciphertext for a chunk and it would decrypt and be cached.

Each replica's response is now checked against the chunk ID (a
mismatch counts as a failed replica), and cached chunks are re-checked.
Tests that served arbitrary bytes under fake IDs now use real hashes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
Inodes returned by the server after a write were trusted, cached in the
path cache and used to advance the root anchor without verification. A
malicious server could inject forged directory or file inodes that later
path resolutions would use, and an unverified root fetch could poison
the root anchor (e.g. with a huge version).

- Create/update results must have the written ID and version and pass
  verifyInode before use.
- The root anchor (owner, version, TOFU keys) only advances from a
  verified root inode, and the version never moves backwards.
- The conflict mock now echoes the written inode like the server does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
verifyJWT only checked the signature against the IdP's JWKS. An ID token
the IdP issued to any other application for the same user (e.g. on a
shared Google/Okta tenant) was accepted, letting that application fetch
the user's KeySync blob or register the identity with its own keys.

Tokens must now carry iss = the discovered issuer, aud = the configured
client ID (new --oidc-client-id flag, default "distfs", matching all
DistFS clients) and an exp, and use an asymmetric algorithm. The test
OIDC server now sets aud.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
…ssion

- Any user could take an exclusive lease on any inode (e.g. the root
  directory or another user's home) and renew it, blocking every
  mutation by other sessions. Exclusive leases now require write access
  and lease placeholders must be owned by the requester (and only name
  groups they belong to). Shared leases stay unchecked because the
  server cannot see anonymous group membership.
- Batch sub-commands could set their own sid/ts/uid. A writer could use
  another session's ID to bypass its exclusive lease, or a far-future
  timestamp to expire leases. For client batches the authenticated
  user, session and timestamp now always override sub-command values.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
GetInode/GetInodes returned every lease's session ID and nonce to any
user, which (before the batch session binding fix) let a writer
impersonate another session to bypass its lease. Leases held by other
sessions are now returned without session identifiers; the requester's
own leases are unchanged.

Signed inode fields cannot be withheld without breaking client-side
verification, and chunk IDs are no longer exploitable after the
capability and chunk ownership fixes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
DISTFS-RAFT §2.3 claimed lockbox recipient IDs hide membership without
the ClusterSecret, but they are keyed by the public GroupID, so anyone
who can read the group can test a known UserID for membership (as §4
already acknowledged). This cannot be fixed in code: the server needs
membership to enforce group permissions, and a member cannot hold a
group secret before decrypting their own entry. Document the HMAC as
pseudonymization and point to the AnonymousLockbox for anonymity.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
- Usage is charged by an inode's declared Size, which could be 0 for a
  manifest of any length. A manifest may no longer hold more chunks than
  the declared size needs.
- Any user could create a quota-enabled group (quota 0 = unlimited) and
  charge all their files to it. Only administrators can now create
  quota-enabled groups; otherwise usage falls back to the user.
- CreateGroup/UpdateGroup persisted client-supplied Usage, Quota and
  GID, letting a manager zero a group's usage or limits or change its
  GID without a uniqueness check. These are now server-managed.

Test fixtures that created files with chunks but no size, or quota
groups as a non-admin, are updated. Also fixes gofmt alignment in
server.go.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
authenticate cached the *User for the session token's whole lifetime
(1h), so an admin locking a compromised account had no effect on its
existing sessions. The session cache now only skips re-verifying the
token signature; the user record is re-read from the FSM, whose cache
is evicted when the lock changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
The replay cache key used the first 16 bytes of the sealed request,
which in symmetric (session) mode is the unused, unauthenticated KEM
ciphertext slot. Anyone who observed a sealed request could flip a byte
there and replay it within the 2-minute window, repeating e.g. quota
reservations or lease and group operations.

The key is now a hash of the request's ML-DSA signature, which is
authenticated and unique per signed message.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
During AdminClusterJoin the leader probed the new node over an
unverified TLS connection, and the node's HMAC proof covered only the
nonce. An on-path attacker could relay the probe, keep the valid proof
header and substitute its own TLS, signing and encryption keys, then
receive the bootstrap payload (ClusterSecret, FSM keyring) and the raft
secret header, and be added as a voter.

- The node's proof now covers a hash of the exact response body, so the
  reported keys are authenticated by the raft secret.
- The bootstrap push uses a connection pinned to the TLS key that was
  authenticated during discovery.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
The service worker streamed decrypted files from /distfs-media/ on the
application's origin with a Content-Type derived from the file
extension or content sniffing (text/html, image/svg+xml, ...), no CSP
and no nosniff, and answered navigations by borrowing any open tab. A
shared HTML or SVG file opened as a page could run script that reads and
exfiltrates every file the victim can access through the worker.

- Media is only served to subresource requests from our pages; top-level
  and framed navigations are refused.
- Only inert image/video/audio types are served inline; everything else
  is application/octet-stream.
- All worker responses carry CSP sandbox and nosniff, and download
  filenames are percent-encoded.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
verifyPeer accepted any mTLS peer whose 8-byte node ID prefix matched
an active node, never comparing the stored public key. A key with a
colliding 64-bit prefix (about 2^64 work) would be accepted as that node
on the Raft and cluster APIs.

When a node's TLS key is on record it must now match exactly, and nodes
added via AdminClusterJoin now record the TLS and signing keys that
were authenticated during discovery.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
/v1/auth/challenge decoded an unbounded body and kept the caller's
user_id in a cache with no size limit for two minutes, letting
unauthenticated clients make the server retain arbitrary amounts of
memory. /v1/login and /v1/user/register also read unbounded bodies.

The challenge body is capped at 4KB, user IDs at 256 bytes, pending
challenges at 100k, and login/registration bodies at 1MB.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
Most commands were proposed without a timestamp, so each node applied
them with its own time.Now(), and lease checks always used time.Now().
Lease expiry, finalize-vs-unlink decisions and similar outcomes could
differ between nodes and on log replay, silently diverging FSM state
(usage, GC). The timeline hash covers log bytes, not state, so this was
not detected.

LogCommand.Marshal now stamps the proposer's time, all proposal sites
use it, and lease checks use the command timestamp. Legacy entries
without a timestamp keep the previous fallback.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
rthellend and others added 6 commits October 5, 2026 12:15
They use the native-only test helpers, which broke the WASM unit test
build.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
Printing the security code from 'whoami' broke scripts that capture its
output as the user ID. The code is now shown with
'distfs whoami --security-code'.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
getInodes rejected a response containing the same inode twice, but the
client legitimately requests repeated IDs (e.g. hard links) and the
server answers each. Responses are still rejected if they contain an
inode that was not requested.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
All items are resolved. M10 (pinning the rotating server KEM key) is
deferred pending a protocol change: sign epoch keys with the cluster
signing key and pin that key in CLI and web configs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
Truncation updated Size but kept the full chunk manifest. Since the
quota fix the server rejects manifests with more chunks than the size
needs, so truncating a chunked file (e.g. O_TRUNC racing another
writer in TestFUSE_POSIXCompliance/ConcurrentWrites) failed with EIO.

setAttr now trims the manifest to the chunks the new size covers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5185JhuUeHz5tmgWAFGeA
@rthellend
rthellend merged commit a734739 into main Oct 6, 2026
3 checks passed
@rthellend
rthellend deleted the security-fixes branch October 6, 2026 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant