[codex] Batch D: crypto & webhook hardening (round-2 audit) - #22
Merged
Conversation
- Webhook v0 (legacy body-only, replayable - no timestamp binding) is now OFF by default; opt in for a migration window via BENMORE_WEBHOOK_LEGACY_V0=1. v1 (t.body) remains. Delivery already used the strict dial-time SSRF client. - Signed-URL signing key: replaced the sync.Once that permanently cached an EPHEMERAL key when serverSecret wasn't set yet (breaking signed-URL validation across instances/restart) with a mutex that returns an uncached ephemeral pre-init and derives+caches the real serverSecret-keyed key once available. - Argon2id KDF: warn (not reject - rejecting would make an existing key un-derivable and lose data) when params are below the OWASP-ish floor (time<2 / mem<19MiB). - Fixed the rotation doc comment that wrongly said sha256(newKeySource); it uses cryptoDeriveAppKey (versioned: SHA-256 or Argon2id). Note: the M-3 AAD column-move PREVENTION is intentionally left as-is on the generic read path - DecryptRowFields only has the result-set key (often an alias), so strict enforcement there would false-reject legit aliased reads and make data inaccessible; the threat needs DB-write access and enforcement remains in benmore_decrypt where the true column is known. Row-id AAD binding would need a v3 format + full re-encrypt migration (disproportionate to a DB-write threat). go build + vet + test -tags sqlite_fts5 ./... green; -race clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the round-2 crypto/webhook findings (discussion #13), favoring low-risk changes (crypto format changes that could make existing data un-decryptable are deliberately avoided).
BENMORE_WEBHOOK_LEGACY_V0=1for a migration window. v1 (t.body) remains. (Delivery already used the strict dial-time SSRF client.)sync.Oncethat permanently cached an ephemeral key whenserverSecretwasn't set yet (breaking signed-URL validation across instances/restart) with a mutex that returns an uncached ephemeral pre-init and derives+caches the real key onceserverSecretis available.cryptoDeriveAppKey, not baresha256).Intentionally not changed: M-3 AAD column-move prevention on the generic read path (
DecryptRowFieldsonly has the result-set key, often an alias → strict enforcement would false-reject legit reads and make data inaccessible; the threat needs DB-write access, andbenmore_decryptstill enforces where the true column is known). Row-id AAD binding would need a v3 format + full re-encrypt migration.Tests added;
go build+vet+test -tags sqlite_fts5 ./...green;-raceclean.