Skip to content

Fix multipart integrity and reduce encrypted read overhead - #156

Open
ServerSideHannes wants to merge 12 commits into
mainfrom
fix/integrity-and-streaming-performance
Open

ServerSideHannes wants to merge 12 commits into
mainfrom
fix/integrity-and-streaming-performance

Conversation

@ServerSideHannes

@ServerSideHannes ServerSideHannes commented Sep 6, 2026 •

Copy link
Copy Markdown
Owner

Multipart replacements could reuse GCM nonces or overwrite accepted bytes before signature validation. Completion could report success for the wrong upload, and stale or unavailable sidecars could select the wrong read path. This change binds publication and reads to an object generation and keeps each verified part attempt immutable.

  • Stage and validate parts before publishing state, freeze the upload key atomically, assemble in client order, retain state on failed completion, and renew completion leases. Compatible whole-object copies preserve native ciphertext copying; later writers cannot change their key.
  • Publish required manifests before ciphertext, bind completion receipts to bucket/key/upload ID, and use consistent client ETags through GET/HEAD/LIST/conditions. Preserve user metadata and pass atomic write preconditions to the backend.
  • Verify buffered and streaming payloads, including AWS's published chunk-signature vectors; reject malformed/truncated bodies and unsupported trailer modes.
  • Share object resolution and contiguous frame reads, coalesce listing lookups, pool credential-isolated S3 clients, and tie cleanup/metrics to stream lifetime. Preallocated frame buffers remove the fragmentation found in load testing; large legacy seals authenticate through bounded spooling.

Validation: the CI unit selection passed 779 tests locally (including 110 mock integration tests); the separate full unit-directory run passed 670 tests including its slow case. 27 real MinIO/HTTP compatibility tests passed (including the optional Redis scenario), 11 additional copy/concurrency tests passed, and all nine native-copy tests passed, including 1,280 MiB objects and concurrent copies. Ruff lint and format checks pass. Tests cover hash tampering below/at/above 8 MiB, rejected replacement, uncertain state publication, retry after failed completion, key selection, out-of-order assembly, resource cleanup, range recovery, and real backend preconditions.

A local ten-sample 32 MiB GET comparison against d45732d measured median latency of 317.12 → 289.26 ms and sampled RSS of 179.23 → 175.06 MiB. A separate 50-GET run stayed below 176 MiB sampled RSS. These are local measurements, not production forecasts; staging adds storage/copy work to ordinary multipart writes.

Deployment: this is a new write format. Drain legacy active uploads and switch the fleet together; old readers cannot read v3 generations. Persistent Redis is required to resume active uploads across restarts. Configure orphan-attempt lifecycle expiry longer than the supported upload/retry window. Generation manifests are retained because copies and versions may reference them. Unsupported checksum-trailer formats fail explicitly. No deployment or bucket-policy changes are included.

See docs/GENERATION_FORMAT.md for the commit protocol, migration requirements, cleanup policy, limitations and benchmark results; docs/CODE_REVIEW.md retains the original findings.

Also included (rebased onto main at f5e279c):

  • Dependency bumps from chore(deps): bump h2 from 4.3.0 to 4.4.1 in the security group across 1 directory #158 (h2 4.4.1, hpack 4.2.0) and chore(deps): bump the all-dependencies group across 1 directory with 4 updates #159 (uvicorn 0.53.0, ruff 0.16.8, boto3-stubs 1.43.98, fakeredis 2.38.0).
  • minio/minio and quay.io/minio/minio no longer allow anonymous pulls, which broke every integration shard and the daily Helm Install Test. Test fixtures and the Helm install workflow now use the pgsty/minio fork.
  • The cluster e2e/ suite is no longer tracked.
  • Review fixes:
    • DeleteBucket through the proxy removes the hidden .s3proxy-internal/ metadata and retries, but only when no client-visible object and no in-progress upload remains. Previously an emptied bucket still returned BucketNotEmpty.
    • STREAMING-UNSIGNED-PAYLOAD-TRAILER is accepted, and its CRC32, SHA1 or SHA256 trailer is checked against the decoded body before anything is published. This is what boto3 sends by default over HTTPS, so without it PutObject and UploadPart failed; verified with stock boto3 over TLS. CRC32C/CRC64NVME and the signed trailer mode are still rejected explicitly.
    • Legacy large-seal reads keep their temporary spool encrypted with AES-CTR under a random key held only in memory, so decrypted data is never written to disk.

Final CI status: all 11 pull-request checks passed for commit 58488e1; see the latest run for 56b4143, and a manually dispatched Helm Install Test also passed on this branch (run).

Bumps the security group with 1 update in the / directory: [h2](https://github.com/python-hyper/h2).

Updates `h2` from 4.3.0 to 4.4.1
- [Changelog](https://github.com/python-hyper/h2/blob/master/CHANGELOG.rst)
- [Commits](python-hyper/h2@v4.3.0...v4.4.1)

---
updated-dependencies:
- dependency-name: h2
  dependency-version: 4.4.1
  dependency-type: indirect
  dependency-group: security
...

Signed-off-by: dependabot[bot] <support@github.com>
…4 updates

Updates the requirements on [uvicorn[standard]](https://github.com/Kludex/uvicorn), [ruff](https://github.com/astral-sh/ruff), [boto3-stubs[s3]](https://github.com/youtype/mypy_boto3_builder) and [fakeredis](https://github.com/cunla/fakeredis-py) to permit the latest version.

Updates `uvicorn[standard]` to 0.53.0
- [Release notes](https://github.com/Kludex/uvicorn/releases)
- [Changelog](https://github.com/Kludex/uvicorn/blob/main/docs/release-notes.md)
- [Commits](Kludex/uvicorn@0.52.4...0.53.0)

Updates `ruff` from 0.16.5 to 0.16.8
- [Release notes](https://github.com/astral-sh/ruff/releases)
- [Changelog](https://github.com/astral-sh/ruff/blob/main/CHANGELOG.md)
- [Commits](astral-sh/ruff@0.16.5...0.16.8)

Updates `boto3-stubs[s3]` to 1.43.98
- [Release notes](https://github.com/youtype/mypy_boto3_builder/releases)
- [Commits](https://github.com/youtype/mypy_boto3_builder/commits)

Updates `fakeredis` from 2.37.1 to 2.38.0
- [Release notes](https://github.com/cunla/fakeredis-py/releases)
- [Commits](cunla/fakeredis-py@v2.37.1...v2.38.0)

---
updated-dependencies:
- dependency-name: uvicorn[standard]
  dependency-version: 0.53.0
  dependency-type: direct:production
  dependency-group: all-dependencies
- dependency-name: ruff
  dependency-version: 0.16.8
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: all-dependencies
- dependency-name: boto3-stubs[s3]
  dependency-version: 1.43.98
  dependency-type: direct:production
  dependency-group: all-dependencies
- dependency-name: fakeredis
  dependency-version: 2.38.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: all-dependencies
...

Signed-off-by: dependabot[bot] <support@github.com>
@ServerSideHannes
ServerSideHannes force-pushed the fix/integrity-and-streaming-performance branch from 2c68c39 to 58488e1 Compare September 28, 2026 12:27
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