carlos / platform Public

worktree-store-virus-scanning

Opt-in virus scanning for the object-storage primitive, and the substrate the on-demand half will sit on. Default off, and inert until three separate things are true — an operator wires clamd, a member requests scanning for their app, and an operator grants it. Shipping this changes nothing on its own, which is what makes it safe to land ahead of the rollout.

Sixty commits, but two things to read: what it does, and the security property it took four attempts to get right.

What it does

An hourly reconciling walk, not a pipeline. The console never sees a PUT — a tenant's app writes to its bucket with its own credential — so the only honest shape is a sweep that lists, scans what it has not judged, and records the answer on the object itself.

The verdict lives on the object as S3 tags, so it survives the console, and the app can read its own verdict through s3:GetObjectTagging. It binds to etag@last-modified, not the ETag alone: for a single-part upload the ETag is the content MD5, and chosen-prefix MD5 collisions are practical, so an attacker controlling both uploads could hand a payload the ETag of the clean object it replaces. Last-modified moves on every overwrite and is not theirs to choose.

The audit answer this exists to produce is two sentences, and the design is shaped by needing both: object X was scanned at T against engine E and database D and found clean, and N objects in this store carry no verdict.

  • carlos store scan request | grant | enforce | status — a member asks, an operator grants; an operator cannot start it alone.
  • carlos system antivirus enable | disable — the deployment switch, a record rather than a unit-file edit, so turning scanning off does not need a restart.
  • ClamAV via a hand-rolled INSTREAM client. GuardDuty is deliberately not wired: 25 protected buckets per account per Region, not adjustable, and our unit of tenancy is a bucket.
  • Client-encrypted stores are refused, not silently swept — a clean verdict on ciphertext means nothing, and the CLI says so rather than claiming coverage it cannot have.

The security property, and why it is the interesting half

Everything rests on one asymmetry: the console can write the carlos-scan tag and the app cannot. If an app can tag its own object, it marks its own malware clean, the sweep skips it forever, and carlos store scan status reports it clean. That is worse than having no scanner, because no scanner reports nothing while this reports the attacker's answer.

The original design enforced that by keeping s3:PutObjectTagging out of the app's IAM policy. It was stated in capitals in the code, and a purpose-built test helper checked it. It was correct, enforced, and not the property. Four defects, all found against a real MinIO in one night, all fixed here:

| | | |---|---| | x-amz-tagging on PutObject set the tag anyway | the header names no action, so no search for an action name could find it | | an explicit Deny on s3:PutObjectTagging did not close it | only Deny s3:PutObject on s3:RequestObjectTagKeys did | | eight junk tags made an object permanently unmarkable | S3 caps at ten; carlos-scan=infected could never be written, so enforce never engaged | | a browser-direct POST-policy upload forges freely | no IAM condition key describes a form upload — nothing in a policy document reaches it |

So the property changed. A verdict is now a signature, not a tag. Every verdict carries an HMAC over the object, its version, the verdict and the moment it was judged, keyed by a deployment secret the console never gives to anybody. readSignedTag is the single trust boundary both Verdict and Status go through, and a tag that does not verify comes back as no verdict at all — the same answer as a bare object, producing the same behaviour: scan it. An app can still write the bytes. It cannot make them mean anything.

That is gateway-independent, which is the point: it closes the routes found, the ones not found, and the ones a future S3 implementation adds. The IAM denies remain, demoted to defence in depth, which is the job they were always suited to.

TestDeliveredContractMinio holds the live half against a real server on every push, including a subtest that performs the whole forgery — guessing the last-modified stamp and retrying until the forged version matches — and asserts both that the tagged upload succeeds and that the tag is not believed.

What an operator needs to know

CARLOS_SCAN_TAG_KEY is new and required. The console refuses to start with a clamd address configured and no key. 32 random bytes; it never leaves the console and is not sealed into any app's config. Losing it loses no data but invalidates every stored verdict, so the next sweep rescans everything. Rotation is two-step: set CARLOS_SCAN_TAG_KEY_PREVIOUS alongside the new key, let a sweep re-sign, then drop the previous — and the console refuses two identical keys, which is a rotation that rotated to nothing.

tito/carlos-infrastructure #6 carries the infrastructure half as a staged install/activate checklist.

Reviewing it

The security work is internal/objstore/scantag.go (new), clamav.go's readSignedTag/tag/resign, and aws.go's storePolicyJSON. Everything asserted about a real gateway is in internal/objstore/contract_test.go, run by .amadan/ci.d/30-minio-contract.

If you change the policy or the verification, watch that test fail before believing it passes. AGENTS.md now carries the rule this branch paid for: a rule stated as an absence can only be checked by looking for the absent thing, and cannot see a second door.

Scan-on-demand — the enqueue door, queue, drain and signed callback — is worktree-store-scan-on-demand, stacked on this.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PEkJh1ESqiA84quAHJV4K6

Commits 0
Discussions 0 0 open
Tasks 0/0 done

Discussions

  • No discussions on this branch yet.

Tasks

  • No tasks yet.

Activity

  • No activity yet.