0004-vault-atomic-update-user.patch makes the Vault IAM backend's UpdateUserAccount -- what admin update-user, and so every key rotation, runs -- one KV v2 write with cas set to the version it read, retried from a fresh read on a check-and-set conflict (up to 5 attempts). Upstream deleted the entry and created it again: in between, another instance on the same KV path whose IAM cache missed answered InvalidAccessKeyId, a failure between the two calls lost the account, and two updates at once lost one of them. Touches only auth/iam_vault.go and a new auth/iam_vault_update_test.go. tests/vault-update/proof.sh proves it on leil-rig against an OpenBao dev server, with two gateways on one KV path and gateway A's OpenBao calls held 3 s by a proxy so upstream's gap can be observed. On leil-test-1: patched 20 passed, 0 failed; 0001-0003 control 13 passed, 7 failed (entry missing 3.00 s, 28 InvalidAccessKeyId on the second gateway, concurrent rotation lost, account gone after a refused write). go test on the node: 35 of 35 packages ok. docs/vault-atomic-update.md has the problem, the change, the proof and its result; README and the multi-version pipeline doc point at it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
165 lines
13 KiB
Markdown
165 lines
13 KiB
Markdown
# Atomic Vault IAM update (`0004-vault-atomic-update-user.patch`)
|
||
|
||
`patches/v1.11.0/0004-vault-atomic-update-user.patch` applies after `0003`. It changes one call of VersityGW's Vault
|
||
IAM backend — `UpdateUserAccount`, which serves the admin API's `update-user` and so every key rotation Norn makes —
|
||
from delete-then-create into one check-and-set write of the account's KV v2 entry. Nothing else changes: the
|
||
internal IAM, the other Vault calls, the admin API and the S3 side are as upstream has them.
|
||
|
||
| | |
|
||
|---|---|
|
||
| Touches | `auth/iam_vault.go`, `auth/iam_vault_update_test.go` (new) |
|
||
| Base | upstream `v1.11.0` (`e42ae7ab`) + `0001` + `0002` + `0003` |
|
||
| Unit tests | `go test ./auth/ -run Vault` (table-driven, against a fake KV v2 + AppRole server) |
|
||
| Proof test | [`tests/vault-update/proof.sh`](../tests/vault-update/proof.sh), on leil-rig, below |
|
||
|
||
## The problem
|
||
|
||
Upstream v1.11.0's `VaultIAMService.UpdateUserAccount` reads the account (`GetUserAccount`), deletes its KV entry —
|
||
metadata and every version, `KvV2DeleteMetadataAndAllVersions` — and creates it again with `cas: 0`
|
||
(`CreateAccount`). Between the delete and the create the account does not exist, and nothing ties the three calls
|
||
together:
|
||
|
||
- **The account goes missing during a rotation.** Several gateway instances of one namespace share a KV path, and
|
||
each keeps a 120 s IAM cache that never caches a missing account. An instance whose cache misses inside the gap —
|
||
its entry for that key expired, or it had never looked the key up — finds no entry and answers
|
||
`InvalidAccessKeyId`, to every request in the gap. The gap lasts one OpenBao round trip of the writing instance
|
||
plus OpenBao's write: milliseconds on one host, more with OpenBao across the network or behind a Raft cluster.
|
||
- **A failure in the middle loses the account for good.** If the create fails after the delete succeeded — OpenBao
|
||
unreachable or sealed, a token that may no longer write, the gateway stopping — the update reports an error and
|
||
the account is gone, on every instance once its cache entry expires.
|
||
- **Two updates at once lose one of them.** Each deletes whatever is there and writes its own copy of what it read;
|
||
both report success.
|
||
|
||
## The change
|
||
|
||
| | |
|
||
|---|---|
|
||
| **One write** | `UpdateUserAccount` reads the entry with its KV version, applies the change, and writes it back with one `KvV2Write` carrying `cas` = that version. The account exists throughout; a write that fails leaves it exactly as it was. |
|
||
| **Conflicts retry** | If another writer changed the entry in between, OpenBao refuses the write (`400`, "check-and-set parameter did not match the current version") and the update starts again from a fresh read, up to 5 attempts (`vaultUpdateAttempts`). After that it fails with the last conflict and the account keeps the other writer's state. |
|
||
| **No such user** | An account that does not exist answers `ErrNoSuchUser` ("user not found", which the admin API maps to its user-not-found error), as before. So does one deleted between the read and the write: the conflict sends the update back to the read, which finds nothing — an update never re-creates a deleted account. |
|
||
| **403** | A `403` on the read or the write logs in again with the AppRole and retries that call once, as the backend's other calls do. |
|
||
| **Shared read** | The read moved into `readAccountEntry`, which `GetUserAccount` now calls too; its behaviour is unchanged. `CreateAccount`, `DeleteUserAccount` and `ListUserAccounts` are untouched. |
|
||
|
||
### What else changes with it
|
||
|
||
- **The gateways' policy** needs `read` and `update` on `<mount>/data/<path>/*` for an update, and no longer
|
||
`delete` on `<mount>/metadata/<path>/*` (which `delete-user` still needs). leil-cluster's proposed policy covers
|
||
both, and is what the proof runs with.
|
||
- **KV keeps history now.** Each update adds a version; the previous ones, earlier secrets included, stay readable at
|
||
`?version=N` until the mount's `max_versions` (OpenBao's default: 10) pushes them out. Upstream's delete wiped them
|
||
at every update. Only a token that can read the current secret can read them, but if retired secrets should not
|
||
linger, set `max_versions` low on the IAM mount (`bao write s3gw-iam/config max_versions=2`, or per entry) — the
|
||
version number keeps counting up, so check-and-set is unaffected.
|
||
- **Updating the root account's key** now answers "user not found", as the internal backend does. Upstream answered
|
||
"user exists" (its re-create refused root's key) after deleting a KV path that never held anything.
|
||
|
||
### What it does not change
|
||
|
||
- **Other instances still learn of a rotation through their cache.** The writing instance drops its own cache entry;
|
||
every other instance keeps accepting the old secret, and refusing the new one, until its entry expires — up to the
|
||
IAM cache time (120 s). The proof measures it (R5). Norn has to give a rotation that long before it retires the old
|
||
secret.
|
||
- **Concurrent creates** were already safe (`cas: 0`: the second gets `ErrUserExists`), and **deletes** still remove
|
||
the entry and its history at once.
|
||
- **ABA**: if an account is deleted and created again between an update's read and its write, and the new entry is
|
||
at the version the update read (both at version 1), the write lands on the new account. That takes a delete and a
|
||
create of the same key inside one update; nothing in Norn's flows does that.
|
||
|
||
## Proving the patch is in (on leil-rig)
|
||
|
||
`--version` reads `1.11.0` with or without the patch, so again the proof is behaviour. `tests/vault-update/proof.sh`
|
||
starts its own OpenBao dev server (`bao server -dev`, in memory, `127.0.0.1:18200`) with the installed `bao` binary —
|
||
not the rig's OpenBao service, whose data it never touches — and gives it the gateways' layout: a KV v2 mount
|
||
`s3gw-iam`, accounts at `proof/0x0/accounts`, AppRole login with leil-cluster's proposed policy. It then runs two
|
||
gateways from the given binary on that one KV path, each with the default 120 s IAM cache:
|
||
|
||
| Gateway | Listens on | Reaches OpenBao |
|
||
|---|---|---|
|
||
| A | `127.0.0.1:7391` | through `vaultproxy.py`, which holds every request 3 s (`PROOF_DELAY`), as from a distant OpenBao, and can refuse KV writes |
|
||
| B | `127.0.0.1:7392` | directly |
|
||
|
||
The proxy is what makes upstream's gap observable on one host: at 3 s per call, the delete reaches OpenBao 6 s into
|
||
the update and the create 9 s in, so the gap is 3 s wide instead of a millisecond or two. The patched build makes the
|
||
same calls minus the delete, so the delay costs it nothing but time. The script times the rotation so that the gap
|
||
spans the moment B's cache entry expires. A KV sampler (`watch.py kv`, every 5 ms) and an S3 sampler on B
|
||
(`watch.py s3`, `ListBuckets` signed with the old secret every 50 ms) record what happens. Everything is under
|
||
`/var/tmp/s3gw-vault-update` and is removed at exit (`KEEP=1` keeps it); nothing installed is touched, and nothing
|
||
needs root.
|
||
|
||
One account, created through A's admin API on leil-cluster's reserved test band: `vu-rot`, role `user`, UID 74992,
|
||
GID 74993.
|
||
|
||
Take the lease, build, copy the binary and both test directories (the proof signs with
|
||
`tests/owner-scope/s3req.py`) to the test node, run it with the node's `bao`:
|
||
|
||
```bash
|
||
RIG_LEASE_WHO="<you>: s3-gateway 0004 proof" ./bin/rig-lease.sh claim --why "s3-gateway 0004 proof on leil-test-1" --minutes 60
|
||
VGW_SRC_DIR=build-src VGW_TAG=v1.11.0 bash ci/run.sh
|
||
cd ../leil-rig && source bin/rig-lib.sh
|
||
on leil-test-1 'install -d -o leil -g leil /var/tmp/s3gw-0004'
|
||
rig_scp ../s3-gateway/build-src/dist/versitygw_linux_amd64_v1/versitygw leil-test-1 /var/tmp/s3gw-0004/versitygw
|
||
tar -czf /tmp/vault-update.tgz -C ../s3-gateway/tests owner-scope vault-update
|
||
rig_scp /tmp/vault-update.tgz leil-test-1 /var/tmp/s3gw-0004/vault-update.tgz
|
||
on leil-test-1 'cd /var/tmp/s3gw-0004 && chmod 0755 versitygw && tar -xzf vault-update.tgz && runuser -u leil -- bash vault-update/proof.sh $PWD/versitygw /usr/bin/bao'
|
||
```
|
||
|
||
It needs the ports free on loopback: 7391–7392 for the gateways, 18200 for OpenBao's API, 18201 for the dev server's
|
||
cluster listener and 18202 for the proxy. `/usr/bin/bao` belongs to the rig's `openbao.service`; the proof only runs it
|
||
in `-dev` mode and never talks to that service.
|
||
|
||
It takes about four minutes, most of it waiting out B's cache. Every line must read `PASS` and the last
|
||
`N passed, 0 failed`; the exit status says the same. `PROOF_PORT` / `PROOF_BAO_PORT` move the ports,
|
||
`PROOF_DELAY` the proxy's hold, `PROOF_CACHE_TTL` both gateways' cache time (shorter only for a quick try: the
|
||
rotation is lined up with B's expiry, and with a short cache it cannot be). What it covers, in order:
|
||
|
||
| Checks | Shows |
|
||
|---|---|
|
||
| S1–S4 | OpenBao dev server, mount, AppRole and policy; `create-user vu-rot` through A; the key signs on B (which caches it from then on) and on A |
|
||
| R1–R2 | `update-user --secret <new>` through A, timed so upstream's gap would span B's cache expiry; A takes the new secret at once |
|
||
| R3 | the KV entry was there at every 5 ms sample through the rotation |
|
||
| R4 | B, signing with the old secret through its cache expiry and past it, never answered `InvalidAccessKeyId` |
|
||
| R5 | B took the old secret until its cache entry expired, and refused it (`SignatureDoesNotMatch`) from then on — the propagation time other instances have |
|
||
| R6–R7 | B takes the new secret after its cache time; the entry still has role `user`, UID 74992, GID 74993, project 0 |
|
||
| R8 | one write: the entry's KV version went up by one and its creation time is the same |
|
||
| C1–C4 | `update-user --project-id 41` through A while, between A's read and its write, `update-user --secret <newer>` goes through B: both succeed and the entry has both changes, UID and GID intact (A's write hit the conflict and was retried); `admin list-users` on B agrees |
|
||
| F1–F4 | with the proxy refusing A's KV writes, `update-user --secret <another>` through A fails, and the entry, its version and its secret are as they were: both gateways still sign with the current secret |
|
||
|
||
Release the lease and clean up: `on leil-test-1 'rm -rf /var/tmp/s3gw-0004'`, then `./bin/rig-lease.sh release`.
|
||
|
||
### Result on leil-rig, 2026-11-11: **PASSED**
|
||
|
||
Run on `leil-test-1` (AlmaLinux 9.8, kernel 5.14.0-687.53.1, Python 3.9.25) as `leil`, under the lease, against a dev server of the node's
|
||
OpenBao 2.6.2 (`dd9c19c3`), from `ci/run.sh` builds of this repository's series:
|
||
|
||
| | |
|
||
|---|---|
|
||
| Build | `VGW_TAG=v1.11.0 bash ci/run.sh` (fresh clone of `v1.11.0`, 0001–0004 by `git am --3way`, GoReleaser); `--version` 1.11.0 |
|
||
| `versitygw` (linux amd64) sha256 | `035be817fac9659f9bbbe25307e0eedca16ab4a5eb67ee739c1aa995bc755aed` |
|
||
| `versitygw_1.11.0_linux_amd64.rpm` sha256 | `cbedf61c60ad207172b4adbe458f2ec73141c6f6fd72e737c501c022f0decde0` (built locally; not published, not pinned) |
|
||
| `proof.sh` | **20 passed, 0 failed** |
|
||
| Unit tests on the node (go1.27.3 linux/amd64) | `go test` of every package except `plugins/`: 35 of 35 ok, among them `auth` (with the new Vault cases) and `s3api/...` |
|
||
|
||
The patched rotation took 6.2 s with OpenBao 3 s away from A — a read and a write. The KV entry was present in all
|
||
16164 samples over 128 s, at version 1 then 2. B answered its 2415 requests with the old secret `200` (2247) or
|
||
`SignatureDoesNotMatch` (168), never `InvalidAccessKeyId`, and switched 0.04–0.10 s after its cache entry expired.
|
||
In the concurrent update A's first write hit the conflict and its retry landed: version 4 holds B's secret and A's
|
||
project.
|
||
|
||
**Control: 0001–0003 only** (same script, same node; `versitygw` sha256 `3ac9be149f1bc35d…`, built by `ci/run.sh`
|
||
from the same repository state with `PATCH_DIR` holding 0001–0003): **13 passed, 7 failed.**
|
||
|
||
- R3: the KV entry was missing (`404`) in 396 of 16134 samples, for 3.00 s — the delete-to-create gap, one proxy
|
||
delay wide. The rotation took 9.2 s: a read, a delete and a create.
|
||
- R4: B answered `InvalidAccessKeyId` 28 times, every request from its cache expiry until the create landed 1.70 s
|
||
later (R5's switch to `SignatureDoesNotMatch` at +1.70 s).
|
||
- R8: the entry came back at version 1 with a new creation time: deleted and made again, its history gone.
|
||
- C3: B's rotation was lost — A's delete removed it and A's create wrote A's stale copy (secret from before, project
|
||
41). Both `update-user` calls reported success (C1, C2).
|
||
- F2–F4: with A's create refused, the account was gone from OpenBao and both gateways answered `InvalidAccessKeyId`
|
||
to its current secret. The failed `update-user` (F1) had already deleted it.
|
||
|
||
The control passed R1–R2, R5–R7, C1–C2, C4 and F1 like the patched build: upstream also keeps UserID and GroupID
|
||
through a rotation, and every instance takes the new secret within its cache time once the account exists again.
|
||
|
||
Run this test again before pinning any build that carries 0004, in leil-rig or leil-cluster.
|