Add cargo-fuzz targets for the untrusted-input parsers and the hardening report
envelope_json, manifest_decode, item_replay, secret_key_parse, totp_parse, url_match, with seeds. fuzz/README.md records the runs (and the server/desktop targets), the two bugs fixed in the previous commits, dependency-policy results, CI changes and the deploy check. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
Claude Opus 5.5
+133
@@ -0,0 +1,133 @@
|
||||
# Fuzzing and hardening report
|
||||
|
||||
libFuzzer targets (via `cargo-fuzz`) for every parser that sees untrusted input: data from the server,
|
||||
the browser, the user, or the network. This file also records the October 2026 hardening pass across
|
||||
all repos: fuzzing, dependency policy, CI and the deploy check.
|
||||
|
||||
## Running
|
||||
|
||||
```sh
|
||||
rustup toolchain install nightly && cargo install cargo-fuzz --locked
|
||||
cd fuzz
|
||||
cargo +nightly fuzz list
|
||||
mkdir -p corpus/<target>
|
||||
cargo +nightly fuzz run <target> corpus/<target> seeds/<target> -- -max_total_time=600
|
||||
```
|
||||
|
||||
`seeds/` holds a few valid inputs per target and is committed. `corpus/`, `artifacts/` and `logs/` are
|
||||
gitignored. CI runs every target for 60 s from the seeds (the `fuzz` job in `.github/workflows/ci.yml`).
|
||||
A crash input lands in `artifacts/<target>/`. Reproduce it with `cargo +nightly fuzz run <target> <file>`.
|
||||
|
||||
The server and desktop targets live in their own repos (`kestrove-server/fuzz`,
|
||||
`kestrove-desktop/src-tauri/fuzz`) and run the same way.
|
||||
|
||||
## Targets and results (2026-10-16, Apple M-series, 1 job per target, ASan)
|
||||
|
||||
| Target | Code under test | Checks beyond "no panic" | Time | Execs | Coverage (edges) | Result |
|
||||
|---|---|---|---|---|---|---|
|
||||
| `envelope_json` | `kv_proto::Envelope` JSON decode | accepted ⇒ re-encode/decode round-trips | 8 min | 36.9 M | 2002 | clean |
|
||||
| `manifest_decode` | `kv_sync::manifest::ManifestBody::decode` (strict canonical CBOR) | accepted ⇒ input is exactly `encode()` | 8 min | 2.1 M | 711 | clean |
|
||||
| `item_replay` | item plaintext → `serde_json::Value` → `kv_model::Item`; `replay::check_pulled` / `check_successor` / `next_version`; `merge::merge3` | identical record never a replay; `next_version > seq` | 8 + 8 min | 1.5 M + 2.9 M | 2248 | **bug 2, fixed** |
|
||||
| `secret_key_parse` | `kv_crypto::secret_key::SecretKey::parse` | accepted ⇒ display form parses to the same key | 8 + 8 min | 19.7 M | 423 | **bug 1, fixed** |
|
||||
| `totp_parse` | `kv_model::totp::Totp::parse` (otpauth:// and bare base32), `code_at`, `remaining` | code has `digits` ASCII digits; 1 ≤ remaining ≤ period | 8 min | 3.0 M | 1688 | clean |
|
||||
| `url_match` | `kv_model::match_url` | an https item never matches an http page | 8 min | 2.7 M | 15307 | clean |
|
||||
| server `signature_headers` | `Signature-Input`, `Signature`, `Content-Digest` parsers (`kestrove-server/src/sigparse.rs`) | accepted ⇒ header == `kv=` + params; nonce valid; fixed component list | 10 min | 59.0 M | 696 | clean |
|
||||
| desktop `nmh_session` | whole `kestrove-nmh` connection into `Broker::serve`: framing, outer JSON, `nmh_hello` / `ping` / `pair_init` / `pair_confirm` / `open` / `msg` | every reply is a valid frame with a `t` field | 8 min | 2.5 M | 2082 | clean |
|
||||
| desktop `frame_read` | `framing::read_frame` (u32 length + payload, 1 MiB cap) | frame ≤ `MAX_FRAME`; re-framing round-trips | 8 min | 2.7 M | 63 | clean |
|
||||
|
||||
### Bug 1: `SecretKey::parse` panicked on non-ASCII input (fixed)
|
||||
|
||||
`parse` stripped separators, checked that the remainder was 27 **bytes**, then called `split_at(26)`. A
|
||||
multibyte character straddling byte 26 panicked (`end byte index 26 is not a char boundary`). Release
|
||||
builds use `panic = "abort"`, so a mistyped or pasted Secret Key containing, say, a Cyrillic `е` would
|
||||
have crashed the app through FFI instead of showing "invalid Secret Key".
|
||||
|
||||
Fix: reject non-ASCII before the length check (one line in `crates/kv-crypto/src/secret_key.rs`).
|
||||
Regression test: `secret_key::tests::non_ascii_is_rejected_without_panicking`, which uses the fuzzer's
|
||||
crash input. It fails without the fix. Commit: "kv-crypto: SecretKey::parse rejects non-ASCII input
|
||||
instead of panicking".
|
||||
|
||||
### Bug 2: `seq` + 1 overflowed at `u64::MAX` (fixed)
|
||||
|
||||
`replay::next_version` and the engine's two merge paths (`engine.rs`, "edit beats delete" and the
|
||||
three-way merge) computed `max(seq) + 1` on `seq` values read from decrypted plaintext. A version carrying
|
||||
`"seq": 18446744073709551615` panicked in debug and fuzz builds. In release, which has no overflow checks,
|
||||
it wrapped to `seq` 0, so the device's next save looked like a replay to every other device (an
|
||||
integrity alarm). The 8-minute random run didn't produce a 20-digit number. A targeted input, written while
|
||||
checking the target's `next_version` assertion, triggered it at once. Reaching it needs an authentic
|
||||
plaintext (one written by a device holding the vault key), so severity is low.
|
||||
|
||||
Fix: `saturating_add(1)` in the three places. Regression test:
|
||||
`replay::tests::next_version_saturates_at_u64_max`. The input is now a seed (`seeds/item_replay/seq_max`),
|
||||
so CI's smoke run covers it. Commit: "kv-sync: saturate seq increments instead of overflowing at
|
||||
u64::MAX".
|
||||
|
||||
`kv-ffi`'s `save_item` has the same pattern on `modified_at` (`p.modified_at + 1`, i64). In release it
|
||||
wraps to `i64::MIN` and `now.max(…)` absorbs it, so it's harmless there. It was left alone because the
|
||||
passkeys branch is changing `kv-ffi`. Worth a `saturating_add` when that merges.
|
||||
|
||||
### Notes
|
||||
|
||||
- The desktop's `msg` path only reaches `handle_request` with a valid AES-GCM session, so the fuzzer covers
|
||||
the session layer's rejection paths, not request handling. That is the expected boundary: an
|
||||
unauthenticated peer can't get past it.
|
||||
- To fuzz `parse_signature_input`, the pure header parsers moved from `kestrove-server/src/auth.rs` into
|
||||
`src/sigparse.rs` unchanged. The server is a bin-only crate, and the fuzz crate includes that file by path.
|
||||
|
||||
## Dependency policy
|
||||
|
||||
| Repo | Check | Result |
|
||||
|---|---|---|
|
||||
| kestrove-core | `cargo deny check` | passes. `allow-wildcard-paths` for the workspace's own path crates. Ignored: RUSTSEC-2025-0141 (bincode 1.x) and RUSTSEC-2024-0436 (paste), both *unmaintained* notices for build-time crates pulled in by uniffi 0.28's proc-macros and bindgen. **Not fixable here:** the fix is a uniffi upgrade, which regenerates the Swift and Kotlin bindings for both apps. |
|
||||
| kestrove-server | `cargo deny check` (new `deny.toml`) | passes, no ignores |
|
||||
| kestrove-desktop/src-tauri | `cargo deny check` (new `deny.toml`) | passes. Also allows BSL-1.0 (clipboard-win/error-code via arboard on Windows) and CDLA-Permissive-2.0 (webpki-roots). Ignores the two uniffi notices plus RUSTSEC-2024-0370 (proc-macro-error via gtk-rs glib-macros 0.18, Tauri's Linux stack, build-time). **Not fixable here:** it goes away when Tauri moves to gtk-rs ≥ 0.19 / gtk4. |
|
||||
| kestrove-browser | `npm audit --omit=dev` | 0 vulnerabilities. Dev-only: 1 moderate, esbuild ≤ 0.24.2 dev-server CORS issue (GHSA-67mh-4wv8-2f99). It only matters for `esbuild serve`, which the build script doesn't use, and none of it ships. |
|
||||
| kestrove-desktop | `npm audit --omit=dev` and `npm audit` | 0 vulnerabilities |
|
||||
| kestrove-android | Gradle dependency verification | new `gradle/verification-metadata.xml` pins sha256 for all 388 components used by `assembleDebug test lint`. The Linux and Windows `aapt2` jars were added by hand: the macOS hash Gradle recorded matched dl.google.com. The 106 release-runtime artifacts were checked against OSV.dev: 0 advisories. |
|
||||
|
||||
## CI (all repos)
|
||||
|
||||
All workflows pass `actionlint` (with shellcheck). Actions are pinned to commit SHAs with the version in a
|
||||
comment, `permissions: contents: read`, `persist-credentials: false`. Sibling repos are checked out side by
|
||||
side using `secrets.CORE_READ_TOKEN || github.token`. Add that secret (a fine-grained token with read
|
||||
access) if the repos are private.
|
||||
|
||||
- **server, desktop:** the old workflows were invalid YAML (an unquoted `${{ }}` inside a `{ }` flow
|
||||
mapping), so GitHub would have rejected them and never run anything.
|
||||
- **core:** adds a fuzz smoke job. Also fixed `scripts/build-android.sh`, which ran uniffi-bindgen on
|
||||
`libkv_ffi.dylib`, a file that doesn't exist on the Linux CI runner (`.so`).
|
||||
- **server:** adds fmt, cargo-deny, the fuzz smoke job and a `docker build` of the deploy image.
|
||||
- **desktop:** adds fmt and cargo-deny + `npm audit`. The code had never been through rustfmt, so this
|
||||
commit adds a `rustfmt.toml` (the same one as core and server) and a formatting-only commit.
|
||||
- **apple:** adds an `app` job: core → XCFramework, Safari JS, xcodegen, unsigned xcodebuild for iOS
|
||||
Simulator and macOS. Ran it locally (Xcode 27): both builds succeed. `swift test` failed once under heavy
|
||||
load and then passed 4 times in a row. The `SyncController` tests in `SyncTests.swift` rely on 100 ms
|
||||
debounces and fixed sleeps, so expect flakes on slow CI runners (follow-up).
|
||||
- **android:** gives the core checkout a token, adds Rust Android targets, `setup-gradle` (wrapper
|
||||
validation) and lint report upload.
|
||||
- **browser:** Node 24 (as used locally), plus `npm audit --omit=dev`.
|
||||
|
||||
## Deploy check (kestrove-server/deploy, Docker Desktop 29.4, DOMAIN=localhost)
|
||||
|
||||
Passed after these fixes:
|
||||
|
||||
1. **The image never built.** kv-proto inherits `edition`/deps from `kestrove-core/Cargo.toml`, which the
|
||||
Dockerfile didn't copy (`error inheriting edition from workspace root`). Now copied. Also removed the
|
||||
`--locked || cargo build` fallback that silently ignored the lockfile, and added
|
||||
`Dockerfile.dockerignore`: the build context is the parent folder, and every `target/` and
|
||||
`node_modules/` was being sent to the daemon.
|
||||
2. **`docker stop` SIGKILLed the server** (exit 137). It only listened for Ctrl-C, and as PID 1 it has no
|
||||
default SIGTERM action. It now shuts down on SIGTERM too and closes the pool, so the WAL is
|
||||
checkpointed (exit 0, single `kestrove.db` left).
|
||||
3. **backup.sh never handed restic its credentials.** It used `source` without `export`, so `RESTIC_*`
|
||||
never reached restic. Fixed with `set -a`. It now also runs `sqlite3` as the volume owner (uid 65532),
|
||||
so it can't leave root-owned `-wal`/`-shm` files. It runs `PRAGMA integrity_check` on the copy before
|
||||
uploading, and `KV_VOLUME` overrides the volume name.
|
||||
4. Smaller changes: `tmpfs: /tmp` for SQLite temp files under `read_only: true`, and Caddy drops `Via`.
|
||||
|
||||
Verified: `GET https://localhost/v1/server-info` → 200 over HTTP/2 via Caddy's internal CA, with HSTS,
|
||||
nosniff and no-referrer. `/` → 404. `http://` → 308 to https. TLS 1.2 is refused. `SETUP_TOKEN=…` is
|
||||
printed on first start. The container runs as uid/gid 65532 with `ReadonlyRootfs=true`, `CapDrop=[ALL]`
|
||||
and `no-new-privileges`. Data files are owned by 65532. backup.sh ran in a Linux container standing in for
|
||||
the VM host (real `sqlite3`, `docker volume inspect` and `restic` stubbed) while the server was live: the
|
||||
`.backup` copy passed `integrity_check` with all migrations present.
|
||||
Reference in New Issue
Block a user