From a52f92c100a4e678092ad3d8ecc88211d19c6a6e Mon Sep 17 00:00:00 2001 From: William Gill Date: Wed, 11 Nov 2026 02:04:31 -0600 Subject: [PATCH] patches: add v1.11.0 atomic Vault IAM update patch and its leil-rig proof 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 --- README.md | 66 ++- docs/multi-version-pipeline.md | 16 +- docs/vault-atomic-update.md | 164 ++++++ .../0004-vault-atomic-update-user.patch | 488 ++++++++++++++++++ tests/vault-update/proof.sh | 244 +++++++++ tests/vault-update/vaultproxy.py | 73 +++ tests/vault-update/watch.py | 175 +++++++ 7 files changed, 1204 insertions(+), 22 deletions(-) create mode 100644 docs/vault-atomic-update.md create mode 100644 patches/v1.11.0/0004-vault-atomic-update-user.patch create mode 100644 tests/vault-update/proof.sh create mode 100644 tests/vault-update/vaultproxy.py create mode 100644 tests/vault-update/watch.py diff --git a/README.md b/README.md index b05d911..8beb0da 100644 --- a/README.md +++ b/README.md @@ -14,12 +14,18 @@ Artifact names are intentionally **unchanged** — the binary stays drop-in compatible with existing configs, systemd units, and the `versitygw` CLI. Only the embedded web UI is rebranded to "S3 Gateway". -The v1.11.0 series also carries one behavioural patch, -**owner-scoped bucket access**: a bucket's ACL and policy may name only -the owner's own keys, public access is read-only, and deleting a bucket -or changing its versioning, object lock or ownership controls is left -to the gateway's root and admins. See -[docs/owner-scoped-access.md](docs/owner-scoped-access.md). +The v1.11.0 series also carries two behavioural patches: + +- **Owner-scoped bucket access**: a bucket's ACL and policy may name + only the owner's own keys, public access is read-only, and deleting a + bucket or changing its versioning, object lock or ownership controls + is left to the gateway's root and admins. See + [docs/owner-scoped-access.md](docs/owner-scoped-access.md). +- **Atomic Vault IAM update**: with the Vault IAM backend, `update-user` + (a key rotation) is one check-and-set write of the account's KV entry + instead of a delete and a create, so the account never goes missing + and a failed update leaves it as it was. See + [docs/vault-atomic-update.md](docs/vault-atomic-update.md). ``` upstream tag (vX.Y.Z) this repo @@ -48,18 +54,22 @@ s3-gateway/ │ ├── v1.11.0/ # the supported series │ │ ├── 0001-webui-debrand.patch │ │ ├── 0002-webui-dark-theme.patch -│ │ └── 0003-owner-scoped-access.patch +│ │ ├── 0003-owner-scoped-access.patch +│ │ └── 0004-vault-atomic-update-user.patch │ └── archive/ # superseded; kept, not built │ ├── v1.10.0/ │ ├── v1.8.0/ │ ├── v1.7.0/ │ ├── v1.6.0/ │ └── v1.5.0/ -├── tests/owner-scope/ # leil-rig proof test for 0003 (proof.sh, s3req.py) +├── tests/ +│ ├── owner-scope/ # leil-rig proof test for 0003 (proof.sh, s3req.py) +│ └── vault-update/ # leil-rig proof test for 0004 (proof.sh, watch.py, vaultproxy.py) ├── .gitea/workflows/build.yml └── docs/ ├── multi-version-pipeline.md - └── owner-scoped-access.md # what 0003 does, and its proof test + ├── owner-scoped-access.md # what 0003 does, and its proof test + └── vault-atomic-update.md # what 0004 does, and its proof test ``` ## Build locally @@ -97,9 +107,9 @@ publish a build of a superseded release. Currently maintained: -| Upstream tag | Patch set | Contents | -|--------------|---------------|---------------------------------------------------------------------------------------------| -| `v1.11.0` | `v1.11.0/` | `0001-webui-debrand.patch`, `0002-webui-dark-theme.patch`, `0003-owner-scoped-access.patch` | +| Upstream tag | Patch set | Contents | +|--------------|---------------|--------------------------------------------------------------------------------------------------------------------------------| +| `v1.11.0` | `v1.11.0/` | `0001-webui-debrand.patch`, `0002-webui-dark-theme.patch`, `0003-owner-scoped-access.patch`, `0004-vault-atomic-update-user.patch` | **v1.11.0 is the only supported version.** It supersedes `v1.10.0` with 42 further upstream fixes, so the older series — `v1.10.0`, @@ -174,13 +184,37 @@ account's `UserID`. - An ACL holding the `all-users` `READ` grant can be written back unchanged (upstream answers `500`). -It is the series' only patch outside `webui/`, and it has unit tests: -after a rebase run `go test ./auth/ ./s3api/...` (on Linux — on Windows -upstream's standalone-IAM, posix and CLI tests fail with or without the -series) and +It and `0004` are the series' only patches outside `webui/`, and both +have unit tests: after a rebase run `go test ./auth/ ./s3api/...` (on +Linux — on Windows upstream's standalone-IAM, posix and CLI tests fail +with or without the series) and the leil-rig proof test in [docs/owner-scoped-access.md](docs/owner-scoped-access.md#proving-the-patch-is-in-on-leil-rig). +### What the atomic Vault update patch does + +`0004-vault-atomic-update-user.patch` changes one call in +`auth/iam_vault.go`, the Vault IAM backend's `UpdateUserAccount` — +what `versitygw admin update-user`, and so every key rotation, runs. +Upstream deletes the account's KV entry and creates it again; in +between the account does not exist, so another gateway instance on the +same KV path whose IAM cache misses answers `InvalidAccessKeyId`, and a +failure between the two calls loses the account for good. + +- **One write.** The entry is read with its KV version, changed, and + written back with `cas` set to that version. +- **Conflicts retry.** If another writer got there first, the update + starts again from a fresh read, up to 5 times. +- **Unchanged:** "user not found" for a missing account (also one + deleted mid-update, which is never re-created), the re-login on a + `403`, and every other Vault call. +- KV v2 now keeps the account's earlier versions, old secrets included, + up to the mount's `max_versions`. + +Its unit tests run with the ones above (`go test ./auth/`); its +leil-rig proof test is in +[docs/vault-atomic-update.md](docs/vault-atomic-update.md#proving-the-patch-is-in-on-leil-rig). + ## Adding / refreshing a version When upstream cuts a new release you want a build of, you rebase the diff --git a/docs/multi-version-pipeline.md b/docs/multi-version-pipeline.md index 2840a5d..1994693 100644 --- a/docs/multi-version-pipeline.md +++ b/docs/multi-version-pipeline.md @@ -111,19 +111,23 @@ VGW_REPO_URL="/path/to/local/versitygw" VGW_TAG=vX.Y.Z bash ci/run.sh ``` A clean `git am` plus a populated `src/dist/` means the web UI patches -are good. `0003-owner-scoped-access.patch` changes Go code in `auth/`, so -a clean apply is not enough for it: run its unit tests in the patched +are good. `0003-owner-scoped-access.patch` and +`0004-vault-atomic-update-user.patch` change Go code in `auth/`, so a +clean apply is not enough for them: run their unit tests in the patched tree, on Linux (on Windows several upstream packages fail regardless), ```bash go test ./auth/ ./s3api/... ``` -and its proof test on leil-rig — see -[owner-scoped-access.md](owner-scoped-access.md#proving-the-patch-is-in-on-leil-rig). +and each one's proof test on leil-rig — see +[owner-scoped-access.md](owner-scoped-access.md#proving-the-patch-is-in-on-leil-rig) +and +[vault-atomic-update.md](vault-atomic-update.md#proving-the-patch-is-in-on-leil-rig). If upstream has reworked `auth/access-control.go` or `auth/acl.go`, -re-read that document's rules against the new code rather than only -resolving the conflict. +re-read 0003's rules against the new code rather than only resolving +the conflict; likewise `auth/iam_vault.go` for 0004 — if upstream's +`UpdateUserAccount` already writes with check-and-set, drop 0004. `src/` may hold someone's local work tree; `ci/clone-source.sh` deletes it. Point `VGW_SRC_DIR` somewhere fresh (any `*-src/` name is diff --git a/docs/vault-atomic-update.md b/docs/vault-atomic-update.md new file mode 100644 index 0000000..2a7032f --- /dev/null +++ b/docs/vault-atomic-update.md @@ -0,0 +1,164 @@ +# 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 `/data//*` for an update, and no longer + `delete` on `/metadata//*` (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=": 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 ` 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 ` 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 ` 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. diff --git a/patches/v1.11.0/0004-vault-atomic-update-user.patch b/patches/v1.11.0/0004-vault-atomic-update-user.patch new file mode 100644 index 0000000..abe28fb --- /dev/null +++ b/patches/v1.11.0/0004-vault-atomic-update-user.patch @@ -0,0 +1,488 @@ +From 285a0f55757c668982cc5a3c3bf5e27b803fcc18 Mon Sep 17 00:00:00 2001 +From: s3-gateway CI +Date: Wed, 11 Nov 2026 01:30:01 -0600 +Subject: [PATCH 4/4] auth: update a Vault IAM account with one check-and-set + write + +UpdateUserAccount on the Vault backend read the account, deleted its +KV entry (metadata and every version) and created it again with cas=0. +Two things followed: + +- While a rotation ran the account did not exist. Another gateway + instance on the same KV path whose IAM cache missed in that gap -- an + expired entry, or a key it had not cached yet -- found no entry, + never caches a miss, and answered InvalidAccessKeyId. +- A failure between the delete and the create (OpenBao unreachable, a + token that could no longer write, the process stopping) lost the + account for good. + +The update is now one KV v2 write. The entry is read together with its +version, the change applied, and the result written back with cas set +to that version. The account exists throughout, and a write that fails +leaves it as it was. If another writer changed the entry in between, +OpenBao refuses the write with a check-and-set error and the update +starts again from a fresh read, up to five times; after that it fails +with the last conflict. An account deleted in between stays deleted: +the retry's read answers ErrNoSuchUser, as does an update of an account +that never existed. + +A 403 on the read or the write logs in again and retries once, as the +backend's other calls do. The read is shared with GetUserAccount +(readAccountEntry), whose behaviour is unchanged; CreateAccount, +DeleteUserAccount and ListUserAccounts are untouched. + +KV v2 now keeps an account's previous versions, earlier secrets +included, up to the mount's max_versions; the delete used to wipe them. +--- + auth/iam_vault.go | 103 ++++++++++-- + auth/iam_vault_update_test.go | 284 ++++++++++++++++++++++++++++++++++ + 2 files changed, 373 insertions(+), 14 deletions(-) + create mode 100644 auth/iam_vault_update_test.go + +diff --git a/auth/iam_vault.go b/auth/iam_vault.go +index c41747f..0b638c4 100644 +--- a/auth/iam_vault.go ++++ b/auth/iam_vault.go +@@ -240,31 +240,41 @@ func (vt *VaultIAMService) GetUserAccount(access string) (Account, error) { + if vt.rootAcc.Access == access { + return vt.rootAcc, nil + } ++ resp, err := vt.readAccountEntry(access) ++ if err != nil { ++ return Account{}, err ++ } ++ acc, err := parseVaultUserAccount(resp.Data.Data, access) ++ if err != nil { ++ return Account{}, err ++ } ++ return acc, nil ++} ++ ++// readAccountEntry reads the KV entry of access, data and metadata, logging ++// in again and retrying once on a 403. A missing entry is ErrNoSuchUser. ++func (vt *VaultIAMService) readAccountEntry(access string) (*vault.Response[schema.KvV2ReadResponse], error) { + resp, err := vt.client.Secrets.KvV2Read(context.Background(), + vt.secretStoragePath+"/"+access, vt.kvReqOpts...) + if err != nil { + reauthErr := vt.reAuthIfNeeded(err) + if reauthErr != nil { + if vault.IsErrorStatus(err, http.StatusNotFound) { +- return Account{}, ErrNoSuchUser ++ return nil, ErrNoSuchUser + } +- return Account{}, reauthErr ++ return nil, reauthErr + } + // retry once after re-auth + resp, err = vt.client.Secrets.KvV2Read(context.Background(), + vt.secretStoragePath+"/"+access, vt.kvReqOpts...) + if err != nil { + if vault.IsErrorStatus(err, http.StatusNotFound) { +- return Account{}, ErrNoSuchUser ++ return nil, ErrNoSuchUser + } +- return Account{}, err ++ return nil, err + } + } +- acc, err := parseVaultUserAccount(resp.Data.Data, access) +- if err != nil { +- return Account{}, err +- } +- return acc, nil ++ return resp, nil + } + + // ResolveAccounts returns the subset of accessKeyIDs that do not exist. +@@ -272,21 +282,71 @@ func (vt *VaultIAMService) ResolveAccounts(accessKeyIDs []string) ([]string, err + return resolveAccountsByLookup(accessKeyIDs, vt.GetUserAccount) + } + ++// vaultUpdateAttempts bounds how often UpdateUserAccount reads an account and ++// writes it back when another writer changes the entry in between. ++const vaultUpdateAttempts = 5 ++ ++// UpdateUserAccount changes an account with one check-and-set write of its KV ++// entry, at the version it was read at: the account exists throughout, and a ++// write that fails leaves it as it was. If another writer changed the entry ++// in between, the write is refused and the update starts again from a fresh ++// read, up to vaultUpdateAttempts times. + func (vt *VaultIAMService) UpdateUserAccount(access string, props MutableProps) error { +- acc, err := vt.GetUserAccount(access) ++ var err error ++ for range vaultUpdateAttempts { ++ err = vt.updateAccountEntry(access, props) ++ if !isVaultCASConflict(err) { ++ return err ++ } ++ } ++ return fmt.Errorf("update user %q: its entry changed during each of %d attempts: %w", ++ access, vaultUpdateAttempts, err) ++} ++ ++// updateAccountEntry reads the entry of access, applies props, and writes it ++// back with cas set to the version it read. ++func (vt *VaultIAMService) updateAccountEntry(access string, props MutableProps) error { ++ resp, err := vt.readAccountEntry(access) + if err != nil { + return err + } +- updateAcc(&acc, props) +- err = vt.DeleteUserAccount(access) ++ acc, err := parseVaultUserAccount(resp.Data.Data, access) + if err != nil { + return err + } +- err = vt.CreateAccount(acc) ++ version, err := parseVaultEntryVersion(resp.Data.Metadata) + if err != nil { + return err + } +- return nil ++ updateAcc(&acc, props) ++ ++ req := schema.KvV2WriteRequest{ ++ Data: map[string]any{ ++ access: acc, ++ }, ++ Options: map[string]any{ ++ "cas": version, ++ }, ++ } ++ _, err = vt.client.Secrets.KvV2Write(context.Background(), ++ vt.secretStoragePath+"/"+access, req, vt.kvReqOpts...) ++ if err == nil || isVaultCASConflict(err) { ++ return err ++ } ++ reauthErr := vt.reAuthIfNeeded(err) ++ if reauthErr != nil { ++ return reauthErr ++ } ++ // retry once after re-auth ++ _, err = vt.client.Secrets.KvV2Write(context.Background(), ++ vt.secretStoragePath+"/"+access, req, vt.kvReqOpts...) ++ return err ++} ++ ++// isVaultCASConflict reports a KV write refused because its cas option no ++// longer matched the entry's current version. ++func isVaultCASConflict(err error) bool { ++ return err != nil && strings.Contains(err.Error(), "check-and-set") + } + + func (vt *VaultIAMService) DeleteUserAccount(access string) error { +@@ -346,6 +406,21 @@ func (vt *VaultIAMService) Shutdown() error { + + var errInvalidUser error = errors.New("invalid user account entry in secrets engine") + ++var errInvalidVersion error = errors.New("invalid version of user account entry in secrets engine") ++ ++// parseVaultEntryVersion returns the KV v2 version a read found the entry at. ++func parseVaultEntryVersion(metadata map[string]any) (int64, error) { ++ versionJson, ok := metadata["version"].(json.Number) ++ if !ok { ++ return 0, errInvalidVersion ++ } ++ version, err := versionJson.Int64() ++ if err != nil || version < 1 { ++ return 0, errInvalidVersion ++ } ++ return version, nil ++} ++ + func parseVaultUserAccount(data map[string]any, access string) (acc Account, err error) { + usrAcc, ok := data[access].(map[string]any) + if !ok { +diff --git a/auth/iam_vault_update_test.go b/auth/iam_vault_update_test.go +new file mode 100644 +index 0000000..29e68b1 +--- /dev/null ++++ b/auth/iam_vault_update_test.go +@@ -0,0 +1,284 @@ ++// Copyright 2026 Versity Software ++// This file is licensed under the Apache License, Version 2.0 ++// (the "License"); you may not use this file except in compliance ++// with the License. You may obtain a copy of the License at ++// ++// http://www.apache.org/licenses/LICENSE-2.0 ++// ++// Unless required by applicable law or agreed to in writing, ++// software distributed under the License is distributed on an ++// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY ++// KIND, either express or implied. See the License for the ++// specific language governing permissions and limitations ++// under the License. ++ ++package auth ++ ++import ( ++ "encoding/json" ++ "fmt" ++ "net/http" ++ "net/http/httptest" ++ "strings" ++ "sync" ++ "testing" ++ ++ "github.com/stretchr/testify/assert" ++) ++ ++// fakeVaultKV is as much of Vault as VaultIAMService uses: AppRole login and ++// a KV v2 mount at kv-v2/ with versioned entries under users/, check-and-set ++// included. Requests carrying anything but the last login's token get 403. ++type fakeVaultKV struct { ++ mu sync.Mutex ++ entries map[string]fakeVaultEntry ++ token string ++ logins int ++ reads int ++ writes int ++ cas []int64 // the cas option of every data write that reached the check ++ deletes int ++ ++ // before, if set, runs at the start of every KV request, with the ++ // request's operation ("read", "write" or "delete") and its count so far. ++ before func(f *fakeVaultKV, op string, n int) ++} ++ ++type fakeVaultEntry struct { ++ acc Account ++ version int64 ++} ++ ++func (f *fakeVaultKV) put(acc Account) { ++ e := f.entries[acc.Access] ++ f.entries[acc.Access] = fakeVaultEntry{acc: acc, version: e.version + 1} ++} ++ ++func (f *fakeVaultKV) revoke() { f.token = "revoked" } ++ ++func (f *fakeVaultKV) ServeHTTP(w http.ResponseWriter, r *http.Request) { ++ f.mu.Lock() ++ defer f.mu.Unlock() ++ ++ reply := func(status int, body any) { ++ w.Header().Set("Content-Type", "application/json") ++ w.WriteHeader(status) ++ json.NewEncoder(w).Encode(body) ++ } ++ vaultErr := func(status int, msg ...string) { ++ reply(status, map[string]any{"errors": append([]string{}, msg...)}) ++ } ++ ++ if r.Method == http.MethodPost && r.URL.Path == "/v1/auth/approle/login" { ++ f.logins++ ++ f.token = fmt.Sprintf("token-%d", f.logins) ++ reply(http.StatusOK, map[string]any{"data": nil, "auth": map[string]any{"client_token": f.token}}) ++ return ++ } ++ ++ var op, access string ++ switch { ++ case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/v1/kv-v2/data/users/"): ++ op, access = "read", strings.TrimPrefix(r.URL.Path, "/v1/kv-v2/data/users/") ++ f.reads++ ++ case r.Method == http.MethodPost && strings.HasPrefix(r.URL.Path, "/v1/kv-v2/data/users/"): ++ op, access = "write", strings.TrimPrefix(r.URL.Path, "/v1/kv-v2/data/users/") ++ f.writes++ ++ case r.Method == http.MethodDelete && strings.HasPrefix(r.URL.Path, "/v1/kv-v2/metadata/users/"): ++ op, access = "delete", strings.TrimPrefix(r.URL.Path, "/v1/kv-v2/metadata/users/") ++ f.deletes++ ++ default: ++ vaultErr(http.StatusNotFound) ++ return ++ } ++ if f.before != nil { ++ n := map[string]int{"read": f.reads, "write": f.writes, "delete": f.deletes}[op] ++ f.before(f, op, n) ++ } ++ if r.Header.Get("X-Vault-Token") != f.token { ++ vaultErr(http.StatusForbidden, "permission denied") ++ return ++ } ++ ++ e, found := f.entries[access] ++ switch op { ++ case "read": ++ if !found { ++ vaultErr(http.StatusNotFound) ++ return ++ } ++ reply(http.StatusOK, map[string]any{"data": map[string]any{ ++ "data": map[string]any{access: e.acc}, ++ "metadata": map[string]any{"version": e.version}, ++ }}) ++ case "write": ++ var req struct { ++ Data map[string]Account `json:"data"` ++ Options struct { ++ CAS *int64 `json:"cas"` ++ } `json:"options"` ++ } ++ if err := json.NewDecoder(r.Body).Decode(&req); err != nil { ++ vaultErr(http.StatusBadRequest, err.Error()) ++ return ++ } ++ if req.Options.CAS == nil { ++ vaultErr(http.StatusBadRequest, "test fake: write without cas") ++ return ++ } ++ f.cas = append(f.cas, *req.Options.CAS) ++ if *req.Options.CAS != e.version { ++ vaultErr(http.StatusBadRequest, "check-and-set parameter did not match the current version") ++ return ++ } ++ f.put(req.Data[access]) ++ reply(http.StatusOK, map[string]any{"data": map[string]any{"version": e.version + 1}}) ++ case "delete": ++ delete(f.entries, access) ++ w.WriteHeader(http.StatusNoContent) ++ } ++} ++ ++func TestVaultIAMServiceUpdateUserAccount(t *testing.T) { ++ stored := Account{Access: "user1", Secret: "old-secret", Role: RoleUser, UserID: 74990, GroupID: 74991, ProjectID: 7} ++ newSecret := "new-secret" ++ rotated := stored ++ rotated.Secret = newSecret ++ otherWriter := func(f *fakeVaultKV) { // another instance changes the account's project ++ acc := f.entries["user1"].acc ++ acc.ProjectID++ ++ f.put(acc) ++ } ++ ++ tests := []struct { ++ name string ++ stored []Account ++ before func(f *fakeVaultKV, op string, n int) ++ wantErr error // compared with == ++ wantErrText string // or contained in the error ++ wantAcc *Account ++ wantVersion int64 ++ wantCAS []int64 ++ wantLogins int ++ }{ ++ { ++ name: "one write at the version read", ++ stored: []Account{stored}, ++ wantAcc: &rotated, ++ wantVersion: 2, ++ wantCAS: []int64{1}, ++ wantLogins: 1, ++ }, ++ { ++ name: "a conflict, then a retry from a fresh read", ++ stored: []Account{stored}, ++ before: func(f *fakeVaultKV, op string, n int) { ++ if op == "write" && n == 1 { ++ otherWriter(f) ++ } ++ }, ++ wantAcc: &Account{Access: "user1", Secret: newSecret, Role: RoleUser, UserID: 74990, GroupID: 74991, ProjectID: 8}, ++ wantVersion: 3, ++ wantCAS: []int64{1, 2}, ++ wantLogins: 1, ++ }, ++ { ++ name: "conflicts until the attempts run out", ++ stored: []Account{stored}, ++ before: func(f *fakeVaultKV, op string, n int) { ++ if op == "write" { ++ otherWriter(f) ++ } ++ }, ++ wantErrText: "check-and-set", ++ wantAcc: &Account{Access: "user1", Secret: "old-secret", Role: RoleUser, UserID: 74990, GroupID: 74991, ProjectID: 12}, ++ wantVersion: 6, ++ wantCAS: []int64{1, 2, 3, 4, 5}, // vaultUpdateAttempts ++ wantLogins: 1, ++ }, ++ { ++ name: "no such user", ++ wantErr: ErrNoSuchUser, ++ wantLogins: 1, ++ }, ++ { ++ name: "deleted between the read and the write", ++ stored: []Account{stored}, ++ before: func(f *fakeVaultKV, op string, n int) { ++ if op == "write" && n == 1 { ++ delete(f.entries, "user1") ++ } ++ }, ++ wantErr: ErrNoSuchUser, ++ wantCAS: []int64{1}, ++ wantLogins: 1, ++ }, ++ { ++ name: "a re-login after a 403 on the read", ++ stored: []Account{stored}, ++ before: func(f *fakeVaultKV, op string, n int) { ++ if op == "read" && n == 1 { ++ f.revoke() ++ } ++ }, ++ wantAcc: &rotated, ++ wantVersion: 2, ++ wantCAS: []int64{1}, ++ wantLogins: 2, ++ }, ++ { ++ name: "a re-login after a 403 on the write", ++ stored: []Account{stored}, ++ before: func(f *fakeVaultKV, op string, n int) { ++ if op == "write" && n == 1 { ++ f.revoke() ++ } ++ }, ++ wantAcc: &rotated, ++ wantVersion: 2, ++ wantCAS: []int64{1}, ++ wantLogins: 2, ++ }, ++ } ++ ++ for _, tt := range tests { ++ t.Run(tt.name, func(t *testing.T) { ++ fake := &fakeVaultKV{entries: map[string]fakeVaultEntry{}} ++ for _, acc := range tt.stored { ++ fake.put(acc) ++ } ++ srv := httptest.NewServer(fake) ++ defer srv.Close() ++ ++ iam, err := NewVaultIAMService(Account{Access: "root"}, srv.URL, "", "users", "", "", "", "", "", "role", "role-secret", "", "", "") ++ assert.NoError(t, err) ++ ++ fake.mu.Lock() ++ fake.before = tt.before ++ fake.mu.Unlock() ++ ++ err = iam.UpdateUserAccount("user1", MutableProps{Secret: &newSecret}) ++ switch { ++ case tt.wantErr != nil: ++ assert.Equal(t, tt.wantErr, err) ++ case tt.wantErrText != "": ++ assert.ErrorContains(t, err, tt.wantErrText) ++ default: ++ assert.NoError(t, err) ++ } ++ ++ fake.mu.Lock() ++ defer fake.mu.Unlock() ++ e, found := fake.entries["user1"] ++ if tt.wantAcc == nil { ++ assert.False(t, found, "the update made an account") ++ } else if assert.True(t, found, "the account is gone") { ++ assert.Equal(t, *tt.wantAcc, e.acc) ++ assert.Equal(t, tt.wantVersion, e.version) ++ } ++ assert.Equal(t, tt.wantCAS, fake.cas) ++ assert.Zero(t, fake.deletes, "the update deleted the entry") ++ assert.Equal(t, tt.wantLogins, fake.logins) ++ }) ++ } ++} +-- +2.49.0.windows.1 + diff --git a/tests/vault-update/proof.sh b/tests/vault-update/proof.sh new file mode 100644 index 0000000..2f94582 --- /dev/null +++ b/tests/vault-update/proof.sh @@ -0,0 +1,244 @@ +#!/bin/bash +# +# Proof test for patches/v1.11.0/0004-vault-atomic-update-user.patch. +# +# sudo bash proof.sh /path/to/versitygw [/path/to/bao] +# +# Starts a throwaway OpenBao dev server (in memory, 127.0.0.1:${PROOF_BAO_PORT:-18200}) and two +# gateways from the given binary on one KV path, as several instances of one namespace run: +# +# A 127.0.0.1:${PROOF_PORT:-7391} reaches OpenBao through vaultproxy.py, which holds every request +# ${PROOF_DELAY:-3} s, as from a distant OpenBao, and can refuse KV writes +# B next port reaches OpenBao directly +# +# Both use AppRole with the policy leil-cluster gives its gateways and the default 120 s IAM cache +# (PROOF_CACHE_TTL changes it, for a quick try only). Then it rotates an account through A's admin API +# while B signs requests with the old secret past its cache, updates the account from A and B at once, +# and has A's KV write refused. Everything lives under ${PROOF_DIR:-/var/tmp/s3gw-vault-update} and is +# removed at the end; nothing installed is touched. Prints one line per check and exits non-zero if +# any check fails. KEEP=1 leaves the directory (logs and samples) behind. About four minutes. +# +# The account, made by root through A's admin API, on leil-cluster's reserved test band: +# +# vu-rot uid 74992 gid 74993 user + +set -uo pipefail + +VGW=${1:?usage: proof.sh /path/to/versitygw [/path/to/bao]} +BAO=${2:-$(command -v bao)} +HERE=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +W=${PROOF_DIR:-/var/tmp/s3gw-vault-update} +PA=${PROOF_PORT:-7391} +PB=$((PA + 1)) +BAO_PORT=${PROOF_BAO_PORT:-18200} +PROXY_PORT=$((BAO_PORT + 2)) # the dev server's cluster listener takes BAO_PORT + 1 +TTL=${PROOF_CACHE_TTL:-120} +D=${PROOF_DELAY:-3} +BAO_URL=http://127.0.0.1:$BAO_PORT +KV=s3gw-iam +ACCOUNTS=proof/0x0/accounts +ENTRY=$KV/data/$ACCOUNTS/vu-rot +export S3_REGION=us-east-1 + +[ -x "$VGW" ] || { echo "FATAL: $VGW is not an executable" >&2; exit 2; } +[ -n "$BAO" ] && [ -x "$BAO" ] || { echo "FATAL: no bao binary (give its path as the second argument)" >&2; exit 2; } +for c in python3 curl; do + command -v $c >/dev/null || { echo "FATAL: $c is required" >&2; exit 2; } +done +listening() { (exec 3<>"/dev/tcp/127.0.0.1/$1") 2>/dev/null; } +for p in $PA $PB $BAO_PORT $((BAO_PORT + 1)) $PROXY_PORT; do + if listening $p; then + echo "FATAL: something already listens on 127.0.0.1:$p (set PROOF_PORT / PROOF_BAO_PORT)" >&2 + exit 2 + fi +done + +rm -rf "$W" && mkdir -p "$W/tmp" "$W/proxy" "$W/home" "$W/a-data" "$W/b-data" || exit 2 +chmod 0700 "$W" +secret() { python3 -c 'import secrets; print(secrets.token_hex(16))'; } +export BAO_TOKEN ROOT_KEY=vuroot ROOT_SECRET S1 S2 S3 S4 +BAO_TOKEN=$(secret) ROOT_SECRET=$(secret) S1=$(secret) S2=$(secret) S3=$(secret) S4=$(secret) + +PIDS=() +cleanup() { + kill "${PIDS[@]}" 2>/dev/null + wait "${PIDS[@]}" 2>/dev/null + [ "${KEEP:-0}" = 1 ] && echo "kept $W" || rm -rf "$W" +} +trap cleanup EXIT + +now() { date +%s.%N; } +calc() { awk "BEGIN { printf \"%.3f\", $* }"; } +sleep_until() { + local s + s=$(calc "$1 - $(now)") + awk "BEGIN { exit !($s > 0) }" && sleep "$s" +} +wait_port() { # wait_port + for _ in $(seq 1 100); do + listening "$1" && return 0 + kill -0 "$2" 2>/dev/null || { cat "$3"; echo "FATAL: $3: the process exited" >&2; exit 2; } + sleep 0.2 + done + echo "FATAL: nothing listens on $1" >&2 + exit 2 +} + +printf 'X-Vault-Token: %s\n' "$BAO_TOKEN" >"$W/tmp/bao.hdr" +bao_api() { # bao_api [json] + curl -sS -f -X "$1" -H "@$W/tmp/bao.hdr" ${3:+-d "$3"} "$BAO_URL/v1/$2" +} +jget() { python3 -c 'import json, sys +d = json.load(sys.stdin) +for k in sys.argv[1].split("."): d = d[k] +print(d)' "$1"; } + +PASS=0 FAIL=0 +result() { # result [detail] + if [ "$2" = 1 ]; then + PASS=$((PASS + 1)); printf 'PASS %-3s %s %s\n' "$1" "$3" "$4" + else + FAIL=$((FAIL + 1)); printf 'FAIL %-3s %s %s [%s]\n' "$1" "$3" "$4" "${5:-}" + fi +} +# check +check() { + local id=$1 want=$2 what=$3 gw=$4 key=$5 sec=$6 got port + shift 6 + [ "$gw" = A ] && port=$PA || port=$PB + got=$(S3_ENDPOINT=http://127.0.0.1:$port S3_KEY=$key S3_SECRET=${!sec} \ + python3 "$HERE/../owner-scope/s3req.py" "$@" 2>"$W/tmp/err") + [ "$got" = "$want" ]; result "$id" $((! $?)) "${got:-none}" "$what" "want $want: $(cat "$W/tmp/err")" +} +# check_cmd +check_cmd() { + local id=$1 want=$2 what=$3 got=ok + shift 3 + "$@" >"$W/tmp/err" 2>&1 || got=refused + [ "$got" = "$want" ]; result "$id" $((! $?)) "$got" "$what" "want $want: $(tr '\n' ' ' <"$W/tmp/err")" +} +# verdict -- the command's one line is the evidence, PASS or FAIL +verdict() { + local id=$1 what=$2 out ok=1 + shift 2 + out=$("$@" 2>&1) || ok=0 + if [ $ok = 1 ]; then + PASS=$((PASS + 1)); printf 'PASS %-3s %s: %s\n' "$id" "$what" "$out" + else + FAIL=$((FAIL + 1)); printf 'FAIL %-3s %s: %s\n' "$id" "$what" "$out" + fi +} +watch() { python3 "$HERE/watch.py" "$@"; } +admin() { # admin + local port + [ "$1" = A ] && port=$PA || port=$PB + shift + ADMIN_ACCESS_KEY_ID=$ROOT_KEY ADMIN_SECRET_ACCESS_KEY=$ROOT_SECRET \ + "$VGW" admin --region "$S3_REGION" --endpoint-url "http://127.0.0.1:$port" "$@" +} +listed() { admin B list-users | awk '$1 == "vu-rot" { print $2, $3, $4, $5 }' | grep -qx 'user 74992 74993 41'; } + +echo "--- setup" +HOME=$W/home BAO_DEV_ROOT_TOKEN_ID=$BAO_TOKEN VAULT_DEV_ROOT_TOKEN_ID=$BAO_TOKEN \ + "$BAO" server -dev -dev-listen-address="127.0.0.1:$BAO_PORT" >"$W/bao.log" 2>&1 & +PIDS+=($!) +wait_port "$BAO_PORT" "${PIDS[-1]}" "$W/bao.log" +for _ in $(seq 1 50); do curl -sf "$BAO_URL/v1/sys/health" >/dev/null && break; sleep 0.2; done +echo "openbao $("$BAO" version | head -1), dev server on 127.0.0.1:$BAO_PORT" +echo "gateway $("$VGW" --version 2>&1 | head -1), sha256 $(sha256sum "$VGW" | cut -c1-16)" + +POLICY="path \"$KV/data/$ACCOUNTS/*\" { capabilities = [\"create\", \"read\", \"update\"] } +path \"$KV/metadata/$ACCOUNTS/*\" { capabilities = [\"read\", \"delete\", \"list\"] } +path \"$KV/metadata/$ACCOUNTS\" { capabilities = [\"list\"] }" +setup_bao() { + bao_api POST "sys/mounts/$KV" '{"type": "kv", "options": {"version": "2"}}' && + bao_api POST sys/auth/approle '{"type": "approle"}' && + bao_api PUT sys/policies/acl/s3gw-proof "$(printf '%s' "$POLICY" | python3 -c 'import json, sys; print(json.dumps({"policy": sys.stdin.read()}))')" && + bao_api POST auth/approle/role/s3gw-proof '{"token_policies": ["s3gw-proof"], "token_ttl": "1h"}' && + ROLE_ID=$(bao_api GET auth/approle/role/s3gw-proof/role-id | jget data.role_id) && + SECRET_ID=$(bao_api POST auth/approle/role/s3gw-proof/secret-id | jget data.secret_id) +} +setup_bao >"$W/tmp/err" 2>&1 +[ -n "${SECRET_ID:-}" ]; result S1 $((! $?)) ok "KV v2 mount $KV, AppRole s3gw-proof with the gateways' policy" "$(tr '\n' ' ' <"$W/tmp/err")" +[ -n "${SECRET_ID:-}" ] || exit 1 + +python3 "$HERE/vaultproxy.py" "$PROXY_PORT" "$BAO_PORT" "$W/proxy" >"$W/proxy.log" 2>&1 & +PIDS+=($!) +wait_port "$PROXY_PORT" "${PIDS[-1]}" "$W/proxy.log" +gateway() { # gateway + ROOT_ACCESS_KEY_ID=$ROOT_KEY ROOT_SECRET_ACCESS_KEY=$ROOT_SECRET VGW_IAM_VAULT_ROLE_SECRET=$SECRET_ID \ + "$VGW" --region "$S3_REGION" --port "127.0.0.1:$2" \ + --iam-vault-endpoint-url "$3" --iam-vault-mount-path "$KV" --iam-vault-secret-storage-path "$ACCOUNTS" \ + --iam-vault-auth-method approle --iam-vault-role-id "$ROLE_ID" --iam-cache-ttl "$TTL" \ + posix "$W/$1-data" >"$W/$1.log" 2>&1 & + PIDS+=($!) + wait_port "$2" "${PIDS[-1]}" "$W/$1.log" +} +gateway a "$PA" "http://127.0.0.1:$PROXY_PORT" +gateway b "$PB" "$BAO_URL" +echo "gateway A on 127.0.0.1:$PA (OpenBao through the proxy), B on 127.0.0.1:$PB, IAM cache ${TTL} s" + +check_cmd S2 ok "create-user vu-rot (uid 74992, gid 74993) through A" \ + admin A create-user --access vu-rot --secret "$S1" --role user --user-id 74992 --group-id 74993 +read -r V0 CREATED0 < <(watch meta "$BAO_URL" "$KV/metadata/$ACCOUNTS/vu-rot") +t0=$(now) +check S3 200 "vu-rot signs on B with its secret (B caches it now)" B vu-rot S1 GET / +t1=$(now) +EXPIRY=$(calc "($t0 + $t1) / 2 + $TTL") +check S4 200 "vu-rot signs on A with its secret" A vu-rot S1 GET / + +echo "--- rotation: update-user through A while B signs with the old secret past its cache" +END=$(calc "$EXPIRY + 2 * $D + 3") +python3 "$HERE/watch.py" kv "$BAO_URL" "$ENTRY" "$END" "$W/kv-rotation.log" & +KVW=$! +S3_KEY=vu-rot S3_SECRET=$S1 python3 "$HERE/watch.py" s3 "http://127.0.0.1:$PB" "$END" "$W/b-rotation.log" & +S3W=$! +PIDS+=($KVW $S3W) +echo "$D" >"$W/proxy/delay" +# Upstream deletes the entry one delay into the update and makes it again a delay later: put +# that gap across B's cache expiry, so that B has to look the account up while it is open. +START=$(calc "$EXPIRY - 2.5 * $D") +awk "BEGIN { exit !($START < $(now)) }" && echo " (the cache time is too short to line the rotation up with B's expiry)" +sleep_until "$START" +t=$(now) +check_cmd R1 ok "update-user vu-rot --secret through A" admin A update-user --access vu-rot --secret "$S2" +echo " took $(calc "$(now) - $t") s with OpenBao $D s away from A; B's cache expires $(calc "$EXPIRY - $(now)") s from now" +check R2 200 "A takes the new secret at once (the update drops A's own cache entry)" A vu-rot S2 GET / +wait $KVW $S3W +verdict R3 "the KV entry never went missing" watch never-missing "$W/kv-rotation.log" +verdict R4 "B never answered InvalidAccessKeyId to the old secret" watch never-invalid "$W/b-rotation.log" +verdict R5 "B took the old secret until its cache expired, and not after" watch cache-expiry "$W/b-rotation.log" "$EXPIRY" +check R6 200 "B takes the new secret after its cache time" B vu-rot S2 GET / +verdict R7 "UserID and GroupID survive" watch expect "$BAO_URL" "$ENTRY" secret=S2 role=user userID=74992 groupID=74993 projectID=0 +read -r V1 CREATED1 < <(watch meta "$BAO_URL" "$KV/metadata/$ACCOUNTS/vu-rot") +[ "$V1" = $((V0 + 1)) ] && [ "$CREATED1" = "$CREATED0" ] +result R8 $((! $?)) "v$V0 -> v$V1" "one write: the entry's version went up by one and it kept its creation time" \ + "created $CREATED0, now $CREATED1: the entry was deleted and made again" + +echo "--- two updates at once: --project-id 41 through A, and B rotating the secret in between" +(admin A update-user --access vu-rot --project-id 41 >"$W/tmp/a-update" 2>&1; echo $? >"$W/tmp/a-update.rc") & +A_UPDATE=$! +sleep "$(calc "1.5 * $D")" +check_cmd C1 ok "update-user vu-rot --secret through B, between A's read and A's write" \ + admin B update-user --access vu-rot --secret "$S3" +wait $A_UPDATE +check_cmd C2 ok "A's update-user --project-id 41 finished" test "$(cat "$W/tmp/a-update.rc")" = 0 +verdict C3 "neither update was lost" watch expect "$BAO_URL" "$ENTRY" secret=S3 projectID=41 userID=74992 groupID=74993 +check_cmd C4 ok "admin list-users on B shows vu-rot as user, 74992, 74993, project 41" listed + +echo "--- a KV write that fails leaves the account as it was" +CUR=S3 +watch expect "$BAO_URL" "$ENTRY" secret=S3 >/dev/null || CUR=S2 +read -r V2 _ < <(watch meta "$BAO_URL" "$KV/metadata/$ACCOUNTS/vu-rot") +echo 0 >"$W/proxy/delay" +printf 'POST /v1/%s/data/' "$KV" >"$W/proxy/refuse" +check_cmd F1 refused "update-user vu-rot --secret through A, its KV write refused on the way" \ + admin A update-user --access vu-rot --secret "$S4" +rm -f "$W/proxy/refuse" +verdict F2 "the entry is unchanged" watch expect "$BAO_URL" "$ENTRY" version="$V2" secret=$CUR userID=74992 groupID=74993 +check F3 200 "A still signs vu-rot with its current secret (A caches nothing for it)" A vu-rot $CUR GET / +check F4 200 "so does B" B vu-rot $CUR GET / + +echo +echo "$PASS passed, $FAIL failed" +[ "$FAIL" = 0 ] diff --git a/tests/vault-update/vaultproxy.py b/tests/vault-update/vaultproxy.py new file mode 100644 index 0000000..6ff880a --- /dev/null +++ b/tests/vault-update/vaultproxy.py @@ -0,0 +1,73 @@ +#!/usr/bin/env python3 +"""A TCP proxy in front of a plain-HTTP OpenBao that holds requests, and can refuse some. + + vaultproxy.py LISTEN_PORT UPSTREAM_PORT CONTROL_DIR + +Everything a client sends is held for the number of seconds in CONTROL_DIR/delay +(none while the file is missing) before it goes on to 127.0.0.1:UPSTREAM_PORT, so +each Vault call of the gateway behind it takes that much longer, as against a +distant OpenBao. While CONTROL_DIR/refuse exists, a request that starts with its +contents (say "POST /v1/s3gw-iam/data/") never reaches OpenBao: the proxy answers +it 503 and closes the connection. Both files are read at every request, so a test +can change them while the proxy runs. Python standard library only. +""" + +import asyncio +import sys + +LISTEN, UPSTREAM, CONTROL = int(sys.argv[1]), int(sys.argv[2]), sys.argv[3] + + +def control(name, default): + try: + with open(f"{CONTROL}/{name}") as f: + return f.read().strip() + except FileNotFoundError: + return default + + +async def handle(client_r, client_w): + try: + server_r, server_w = await asyncio.open_connection("127.0.0.1", UPSTREAM) + except OSError: + client_w.close() + return + + async def upstream(): + try: + while data := await client_r.read(65536): + refuse = control("refuse", "") + if refuse and data.startswith(refuse.encode()): + body = b'{"errors":["refused by the proof proxy"]}' + client_w.write(b"HTTP/1.1 503 Service Unavailable\r\nContent-Type: application/json\r\n" + b"Connection: close\r\nContent-Length: %d\r\n\r\n%s" % (len(body), body)) + await client_w.drain() + break + delay = float(control("delay", "0") or 0) + if delay: + await asyncio.sleep(delay) + server_w.write(data) + await server_w.drain() + finally: + server_w.close() + client_w.close() + + async def downstream(): + try: + while data := await server_r.read(65536): + client_w.write(data) + await client_w.drain() + finally: + client_w.close() + + await asyncio.gather(upstream(), downstream(), return_exceptions=True) + + +async def main(): + server = await asyncio.start_server(handle, "127.0.0.1", LISTEN) + async with server: + await server.serve_forever() + + +if __name__ == "__main__": + asyncio.run(main()) diff --git a/tests/vault-update/watch.py b/tests/vault-update/watch.py new file mode 100644 index 0000000..d63bd04 --- /dev/null +++ b/tests/vault-update/watch.py @@ -0,0 +1,175 @@ +#!/usr/bin/env python3 +"""Samplers and record checks for proof.sh. + + watch.py kv BAO_URL PATH UNTIL OUT read a KV v2 entry until epoch UNTIL, + logging " " + watch.py s3 ENDPOINT UNTIL OUT sign ListBuckets with S3_KEY/S3_SECRET until + epoch UNTIL, logging " " + watch.py never-missing OUT the entry was there at every kv sample + watch.py never-invalid OUT no s3 request was answered InvalidAccessKeyId + watch.py cache-expiry OUT EXPIRY the key worked until epoch EXPIRY, not after + watch.py expect BAO_URL PATH FIELD=VALUE... the account in the entry has these fields + (secret=NAME compares with $NAME, unprinted) + watch.py meta BAO_URL PATH " ", or "missing" + +The KV commands authenticate with $BAO_TOKEN. The samplers run as fast as a round trip +allows, about 5 ms apart for the entry and 50 ms for the key. Every check prints one +line and exits 0 when it holds, 1 when it does not. Python standard library only; +s3req.py comes from ../owner-scope. +""" + +import http.client +import json +import os +import re +import sys +import time +import urllib.parse + +sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), "..", "owner-scope")) +from s3req import sign # noqa: E402 + + +def bao(url): + u = urllib.parse.urlsplit(url) + return http.client.HTTPConnection(u.hostname, u.port, timeout=10) + + +def bao_get(conn, path): + conn.request("GET", f"/v1/{path}", headers={"X-Vault-Token": os.environ["BAO_TOKEN"]}) + resp = conn.getresponse() + body = resp.read() + return resp.status, json.loads(body) if resp.status == 200 else None + + +def sample(connect, request, until, out, pause): + conn = None + with open(out, "w") as log: + while time.time() < until: + try: + conn = conn or connect() + status, detail = request(conn) + except (OSError, http.client.HTTPException) as e: + conn = None + status, detail = "err", type(e).__name__ + log.write(f"{time.time():.3f} {status} {detail}\n") + log.flush() + time.sleep(pause) + + +def kv(url, path, until, out): + def request(conn): + status, body = bao_get(conn, path) + return status, body["data"]["metadata"]["version"] if body else "-" + + sample(lambda: bao(url), request, until, out, 0.005) + + +def s3(endpoint, until, out): + u = urllib.parse.urlsplit(endpoint) + key, secret = os.environ["S3_KEY"], os.environ["S3_SECRET"] + + def request(conn): + headers = {} + sign("GET", u.netloc, "/", "", headers, b"", key, secret, os.environ.get("S3_REGION", "us-east-1")) + conn.request("GET", "/", headers=headers) + resp = conn.getresponse() + code = re.search(rb"(.*?)", resp.read()) + return resp.status, code.group(1).decode() if code else "-" + + sample(lambda: http.client.HTTPConnection(u.hostname, u.port, timeout=30), request, until, out, 0.05) + + +def read(out): + with open(out) as f: + return [(float(t), s, d) for t, s, d in (line.split() for line in f)] + + +def never_missing(out): + rows = read(out) + if not rows: + print("no samples") + return False + span = f"{len(rows)} samples over {rows[-1][0] - rows[0][0]:.0f} s" + missing = [t for t, s, _ in rows if s != "200"] + if missing: + statuses = "/".join(sorted({s for _, s, _ in rows if s != "200"})) + print(f"missing ({statuses}) in {len(missing)} of {span}, " + f"for {missing[-1] - missing[0]:.2f} s from {missing[0] - rows[0][0]:.2f} s in") + return False + versions = sorted({d for _, _, d in rows}, key=int) + print(f"present in all {span}, at version {' then '.join(versions)}") + return True + + +def never_invalid(out): + rows = read(out) + counts = {} + for _, s, d in rows: + counts[f"{s} {d}"] = counts.get(f"{s} {d}", 0) + 1 + print(f"{len(rows)} requests: " + ", ".join(f"{n} x {k}" for k, n in sorted(counts.items()))) + return bool(rows) and set(counts) <= {"200 -", "403 SignatureDoesNotMatch"} + + +def cache_expiry(out, expiry): + rows = [(t - expiry, s, d) for t, s, d in read(out)] + ok = [t for t, s, _ in rows if s == "200"] + refused = [t for t, s, d in rows if s == "403" and d == "SignatureDoesNotMatch"] + if not ok or not refused: + print(f"{len(ok)} accepted, {len(refused)} refused as SignatureDoesNotMatch") + return False + print(f"accepted until {ok[-1]:+.2f} s from the cache expiry, " + f"refused as SignatureDoesNotMatch from {refused[0]:+.2f} s") + return -1 <= ok[-1] <= 1 and refused[0] > ok[-1] + + +def expect(url, path, wants): + status, body = bao_get(bao(url), path) + if status != 200: + print(f"no entry (HTTP {status})") + return False + acc = next(iter(body["data"]["data"].values())) + got, held = [f"version {body['data']['metadata']['version']}:"], True + for want in wants: + field, value = want.split("=", 1) + if field == "secret": + same = acc["secret"] == os.environ[value] + got.append(f"secret {'is' if same else 'is not'} {value}") + elif field == "version": + same = str(body["data"]["metadata"]["version"]) == value + got[0] = f"version {body['data']['metadata']['version']}{'' if same else f' (want {value})'}:" + else: + same = str(acc.get(field)) == value + got.append(f"{field}={acc.get(field)}" + ("" if same else f" (want {value})")) + held = held and same + print(" ".join(got)) + return held + + +def meta(url, path): + status, body = bao_get(bao(url), path) + print(f"{body['data']['current_version']} {body['data']['created_time']}" if status == 200 else "missing") + + +def main(argv): + cmd, args = (argv[0], argv[1:]) if argv else ("", []) + if cmd == "kv": + kv(args[0], args[1], float(args[2]), args[3]) + elif cmd == "s3": + s3(args[0], float(args[1]), args[2]) + elif cmd == "never-missing": + sys.exit(0 if never_missing(args[0]) else 1) + elif cmd == "never-invalid": + sys.exit(0 if never_invalid(args[0]) else 1) + elif cmd == "cache-expiry": + sys.exit(0 if cache_expiry(args[0], float(args[1])) else 1) + elif cmd == "expect": + sys.exit(0 if expect(args[0], args[1], args[2:]) else 1) + elif cmd == "meta": + meta(args[0], args[1]) + else: + sys.exit(__doc__) + + +if __name__ == "__main__": + main(sys.argv[1:])