diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 72393fe..d3108f8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -184,3 +184,66 @@ jobs: run: make install WP_VERSION=latest DB_HOST=127.0.0.1 - run: make test-ms + + # Outside `make ci` because it needs a service container (Moto, an AWS + # emulator) that the other jobs and the no-Docker local path do not provide. + # The examples under examples/ stay unlinted -- they are single files a host + # copies out, not part of this plugin's own coding-standard surface. + examples: + name: Examples (Moto) + needs: static + runs-on: ubuntu-latest + env: + WP_SECRETS_TEST_AWS_ENDPOINT: http://127.0.0.1:5000 + services: + mysql: + image: mysql:8.0 + env: + MYSQL_ALLOW_EMPTY_PASSWORD: 'yes' + MYSQL_DATABASE: wordpress_test + ports: + - 3306:3306 + options: >- + --health-cmd="mysqladmin ping" + --health-interval=10s + --health-timeout=5s + --health-retries=5 + # Pinned by digest, resolved from motoserver/moto:latest on 2026-09-24. + moto: + image: motoserver/moto@sha256:91fd602a21f49cf9eb82fdf474015a3c131d40104c8297ea6a2ca920708ae32c + ports: + - 5000:5000 + steps: + - name: Check out + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Set up PHP + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 + with: + php-version: '8.3' + extensions: sodium, mysqli + coverage: none + tools: composer + + - name: Cache Composer packages + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: ~/.cache/composer + key: composer-${{ runner.os }}-php8.3-${{ hashFiles('composer.lock') }} + restore-keys: composer-${{ runner.os }}-php8.3- + + - name: Install dependencies and the WordPress test suite + run: make install WP_VERSION=latest DB_HOST=127.0.0.1 + + - name: Wait for Moto + run: | + for i in $(seq 1 30); do + if curl -sf http://127.0.0.1:5000/moto-api/ >/dev/null; then + exit 0 + fi + sleep 1 + done + echo "Moto never answered on http://127.0.0.1:5000/moto-api/" >&2 + exit 1 + + - run: make test-examples diff --git a/.gitignore b/.gitignore index e5c99b2..e6b37d4 100644 --- a/.gitignore +++ b/.gitignore @@ -23,5 +23,3 @@ site/.astro/ # Spacefast CLI link and state. Written wherever sf publish runs from; never commit it. .spacefast/ -# Private reviewer asks; drafted locally, never committed. -docs-review-asks.md diff --git a/CLAUDE.md b/CLAUDE.md index 4340969..e3fc70f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,16 +7,19 @@ those do not. - `docs/` is the source of truth for the documentation site. `site/` only renders it: the Astro project reads `../docs` directly, and nothing under `site/src/` is content. -- `docs/reference/` is generated by `bin/gen-reference.php` from source docblocks. Never edit - those files by hand. Change the docblock, run `make reference`, and commit the result; `make ci` - and a CI job fail when the committed copy is stale. +- `docs/reference/functions.md`, `classes.md`, `hooks.md`, and `wp-cli.md` are generated by + `bin/gen-reference.php` from source docblocks. Never edit those four by hand. Change the + docblock, run `make reference`, and commit the result; `make ci` and a CI job fail when the + committed copy is stale. The other files in `docs/reference/` are written by hand. - Spec pages under `docs/spec/` use exactly three sections, in this order: **As proposed**, **As built**, **Why**. "Why" stays empty when the code matches the proposal. - The make/core proposal is linked, never restated. One or two sentences plus the link. - Design decisions are ADRs under `docs/decisions/`, numbered `NNNN-slug.md`, with number, title, date, status, context, decision, and consequences. - Journal entries are `docs/journal/YYYY-MM-DD-slug.md` with a `date:` field in the frontmatter. - The sidebar sorts on it. Undated files in that directory are tracking documents, not entries. + The sidebar sorts on it. Tracking documents (`open-questions.md`, `proposal-questions.md`, + `test-coverage-gaps.md`) have no date in the filename but do carry a `date:` field: the date of + their last substantive change. Update it whenever you change one. `docs/journal/_drafts/` is never published. - Nothing about employers, customers, or internal channels goes into `docs/`. Attribute feedback to the proposal thread generically unless the name is already public there. diff --git a/Makefile b/Makefile index c254ba9..596395f 100644 --- a/Makefile +++ b/Makefile @@ -15,7 +15,7 @@ DB_PASS ?= DB_HOST ?= 127.0.0.1 .DEFAULT_GOAL := help -.PHONY: help install lint lint-fix compat analyse test test-ms coverage reference reference-check ci clean +.PHONY: help install lint lint-fix compat analyse test test-ms test-examples coverage reference reference-check ci clean help: ## Show this help. @grep -hE '^[a-zA-Z_-]+:.*?## ' $(MAKEFILE_LIST) \ @@ -45,6 +45,9 @@ test: ## Run the single-site suite. test-ms: ## Run the multisite suite. WP_MULTISITE=1 $(VENDOR_BIN)/phpunit -c phpunit-multisite.xml.dist +test-examples: ## Run the platform examples suite against emulators. Needs Moto (see examples/README.md); not part of make ci. + $(VENDOR_BIN)/phpunit -c phpunit-examples.xml.dist + coverage: ## Run the single-site suite with coverage. $(VENDOR_BIN)/phpunit --coverage-html coverage --coverage-text diff --git a/README.md b/README.md index f24cb39..b2e8969 100644 --- a/README.md +++ b/README.md @@ -46,6 +46,7 @@ target list. | `make compat` | PHPCompatibilityWP at `testVersion 7.4-` | | `make analyse` | phpstan | | `make test` / `make test-ms` | phpunit, single site / multisite | +| `make test-examples` | phpunit against `examples/*/tests`, needs Moto running (see `examples/README.md`); not part of `make ci` | | `make coverage` | phpunit with an HTML coverage report (see `docs/journal/test-coverage-gaps.md` re: wp-env) | | `make reference` / `make reference-check` | regenerate `docs/reference/` from docblocks / fail if it is stale | | `make ci` | all of the above | @@ -144,7 +145,9 @@ Nothing there is loaded by the plugin, and it's excluded from `make ci` so those never become this project's. Read its README before writing one: a key-management service (AWS KMS, Google Cloud KMS) is a `WP_Secrets_Keyring` and takes three methods, while a secret store (Secrets Manager, Parameter Store) is a `WP_Secrets_Provider` and takes eight. People routinely -pick the wrong one and pay for it in per-operation API calls. +pick the wrong one and pay for it in per-operation API calls. Start from +[`examples/aws-kms-keyring/`](examples/aws-kms-keyring/) — it is the smaller interface, and it is +what most hosts are actually after: key custody moves to the KMS and nothing else changes. ## Contributing diff --git a/cli/class-wp-cli-secret-command.php b/cli/class-wp-cli-secret-command.php index be0fe12..ad9d3bb 100644 --- a/cli/class-wp-cli-secret-command.php +++ b/cli/class-wp-cli-secret-command.php @@ -487,42 +487,100 @@ public function migrate_legacy( $args, $assoc_args ) { } /** - * Re-wraps the root key under a new WP_SECRETS_KEY after a site-key change. - * - * No secret is re-encrypted: rotation only changes what the root key is + * Re-wraps the root key under the active keyring. + * + * There are two cases, chosen with --from. `--from=config-previous` (the + * default) is a site key change: the root key, currently wrapped under + * WP_SECRETS_KEY_PREVIOUS, is re-wrapped under the current WP_SECRETS_KEY. + * `--from=config` is moving the root key onto a new keyring: a secrets.php + * drop-in has installed one, and the root key, currently wrapped under the + * config keyring's WP_SECRETS_KEY, is re-wrapped under that new keyring. No + * secret is ever re-encrypted: rotation only changes what the root key is * wrapped under, not the root key's own bytes. * * ## OPTIONS * + * [--from=] + * : Which keyring currently wraps the root key. + * --- + * default: config-previous + * options: + * - config-previous + * - config + * --- + * * [--yes] * : Skip the confirmation prompt. * + * ## EXAMPLES + * + * $ wp secret rotate --yes + * $ wp secret rotate --from=config --yes + * * @when after_wp_load * * @param array $args Positional arguments. * @param array $assoc_args Associative arguments. */ public function rotate( $args, $assoc_args ) { - if ( ! defined( 'WP_SECRETS_KEY_PREVIOUS' ) ) { - WP_CLI::error( 'WP_SECRETS_KEY_PREVIOUS is not defined. Move the current WP_SECRETS_KEY value to WP_SECRETS_KEY_PREVIOUS, set WP_SECRETS_KEY to a new value from `wp secret generate-key`, then run this again.' ); + $from = isset( $assoc_args['from'] ) ? $assoc_args['from'] : 'config-previous'; + + if ( ! in_array( $from, array( 'config-previous', 'config' ), true ) ) { + WP_CLI::error( sprintf( 'Unknown --from value "%s". Use config-previous or config.', $from ) ); return; } - WP_CLI::confirm( 'Rotate the site key? This re-wraps the root key under the new WP_SECRETS_KEY.', $assoc_args ); + $key_manager = _wp_secrets_get_key_manager(); + $new_keyring = $key_manager->get_keyring(); + + if ( 'config-previous' === $from ) { + if ( ! defined( 'WP_SECRETS_KEY_PREVIOUS' ) ) { + WP_CLI::error( 'WP_SECRETS_KEY_PREVIOUS is not defined. Move the current WP_SECRETS_KEY value to WP_SECRETS_KEY_PREVIOUS, set WP_SECRETS_KEY to a new value from `wp secret generate-key`, then run this again.' ); + + return; + } - $result = _wp_secrets_get_key_manager()->rotate_site_key( - new WP_Secrets_Config_Key_Provider( true ), - new WP_Secrets_Config_Key_Provider( false ) + if ( $new_keyring instanceof WP_Secrets_Config_Key_Provider + && defined( 'WP_SECRETS_KEY' ) + && WP_SECRETS_KEY === WP_SECRETS_KEY_PREVIOUS + ) { + WP_CLI::error( 'WP_SECRETS_KEY and WP_SECRETS_KEY_PREVIOUS hold the same value. There is nothing to rotate.' ); + + return; + } + + $old_keyring = new WP_Secrets_Config_Key_Provider( true ); + } else { + if ( $new_keyring instanceof WP_Secrets_Config_Key_Provider ) { + WP_CLI::error( 'The active keyring already reads WP_SECRETS_KEY. --from=config only applies after a secrets.php drop-in installs a different keyring.' ); + + return; + } + + $old_keyring = new WP_Secrets_Config_Key_Provider( false ); + } + + WP_CLI::confirm( + sprintf( + 'Rotate the root key from "%s" to "%s"? This re-wraps the root key; no secret is re-encrypted.', + $old_keyring->get_key_source(), + $new_keyring->get_key_source() + ), + $assoc_args ); + $result = $key_manager->rotate_site_key( $old_keyring, $new_keyring ); + if ( is_wp_error( $result ) ) { WP_CLI::error( $result->get_error_message() ); return; } - WP_CLI::success( 'Site key rotated. No secret needed to be re-encrypted.' ); + WP_CLI::success( + sprintf( 'Root key re-wrapped under: %s. No secret needed to be re-encrypted.', $new_keyring->get_key_source() ) + ); } /** diff --git a/docs/decisions/0002-plugin-before-core-patch.md b/docs/decisions/0002-plugin-before-core-patch.md index a1faee9..d0d5129 100644 --- a/docs/decisions/0002-plugin-before-core-patch.md +++ b/docs/decisions/0002-plugin-before-core-patch.md @@ -9,7 +9,7 @@ description: "Why the Secrets API ships as a feature plugin first, and what fini |---|---| | **Number** | 0002 | | **Date** | 2026-08-25 | -| **Status** | Accepted | +| **Status** | Accepted. Amended by [ADR 0008](0008-the-trac-ticket-replaces-thread-confirmation.md). | ## Context @@ -39,8 +39,9 @@ the presence of the symbol, so a slip to 7.3 cannot silently disable it. **The plugin is done when:** -- the public surface matches the proposal, with every addition beyond it recorded and confirmed - on the thread; +- the public surface matches the proposal, with every addition beyond it recorded and listed on + the Trac ticket ([ADR 0008](0008-the-trac-ticket-replaces-thread-confirmation.md); originally + "confirmed on the thread"); - `make ci` is green across the PHP 7.4 to 8.3 and single-site to multisite matrix; - at least one real platform provider has been built against `WP_Secrets_Provider` and the conformance suite, and what it turned up has been fixed; diff --git a/docs/decisions/0008-the-trac-ticket-replaces-thread-confirmation.md b/docs/decisions/0008-the-trac-ticket-replaces-thread-confirmation.md new file mode 100644 index 0000000..e7fe8ee --- /dev/null +++ b/docs/decisions/0008-the-trac-ticket-replaces-thread-confirmation.md @@ -0,0 +1,71 @@ +--- +title: "ADR 0008: The Trac ticket replaces thread confirmation" +description: "Additions beyond the proposal are reviewed on the Trac ticket rather than waiting for confirmation on the make/core thread. Before the ticket opens, the extension points get two more implementations and the CLI gets a real end-to-end test." +--- + +# ADR 0008: The Trac ticket replaces thread confirmation + +| | | +|---|---| +| **Number** | 0008 | +| **Date** | 2026-09-24 | +| **Status** | Accepted. Amends [ADR 0002](0002-plugin-before-core-patch.md). | + +## Context + +[ADR 0002](0002-plugin-before-core-patch.md) says the plugin is done when every addition beyond +the [proposal][proposal] has been "recorded and confirmed on the thread". Those additions are +`wp_retire_secret_version()`, `wp_list_secrets()`, the network functions, the provider and keyring +interfaces, and the rest listed on the [scope](../spec/scope.md) page. + +Since 0.1.0 was tagged on 4 September, the proposal, the plugin, and a documentation site +listing each addition with its rationale have been put in front of contributors on the make/core +thread, in Core Slack, and in core dev chat. The response has been consistent support and no +critical feedback. No public issue has been opened, and no one has objected to a name. + +That is silence rather than confirmation, and waiting longer will not change it. A make/core post +gets read, not reviewed. Committers review Trac tickets. 7.2 Beta 1 is scheduled for 20 to 22 +October, and a patch that opens in mid-October leaves no time for that review to change anything. + +## Decision + +Opening the Trac ticket replaces thread confirmation as the review step for additions beyond +the proposal. + +- The ticket description lists every addition by name, each with a link to the spec page that + explains it, so review can reject a name specifically rather than approve the patch as a whole. +- The rounds of iteration that were planned for the thread happen as patch revisions on the + ticket. The plugin follows every revision to its surface, so the plugin and the patch stay the + same code. +- ADR 0002's first "plugin is done" criterion now reads: the public surface matches the proposal, + and every addition beyond it is recorded and listed on the Trac ticket. + +Before the ticket opens, three pieces of work test the surface in ways silence cannot: + +1. **A KMS-backed keyring example.** The documentation recommends it as the first integration a + host should write, and none exists. It is the first real test of `WP_Secrets_Keyring`, and of + how an existing site moves onto a new keyring. +2. **A HashiCorp Vault provider example.** Vault numbers versions with integers rather than + keeping two slots. It is the first backend that does not already share the + `CURRENT`/`PREVIOUS` shape, which is the design most likely to be wrong in a way nobody has + pointed out. +3. **A WP-CLI smoke test against a real `wp` binary.** This is the only coverage gap marked as + needing an answer before the core patch. The WP-CLI surface is part of what 7.2 ships, and + three dispatch bugs have already got past a green suite. + +ADR 0002's third criterion, one real platform provider, is already met by the AWS Secrets Manager +example, which was verified against live AWS on 4 September. + +## Consequences + +- A name can still change after the ticket opens. A rename then costs a patch revision and a + plugin release rather than an edit to a proposal, which is still cheap before Beta 1. +- Silence could mean no one read the additions closely. Listing each addition in the ticket + description, rather than leaving reviewers to diff the patch against the proposal, is the + mitigation. +- The three examples and the smoke test come before the ticket. If one of them changes an + interface, the ticket opens with that change already made. +- There is less time for the ticket itself. Work on the examples is limited to what tests the + interfaces, and anything beyond that waits until after Beta 1. + +[proposal]: https://make.wordpress.org/core/2026/08/25/proposal-a-secrets-api-for-wordpress-7-2/ diff --git a/docs/decisions/0009-root-key-cached-for-the-request.md b/docs/decisions/0009-root-key-cached-for-the-request.md new file mode 100644 index 0000000..fba5e48 --- /dev/null +++ b/docs/decisions/0009-root-key-cached-for-the-request.md @@ -0,0 +1,55 @@ +--- +title: "ADR 0009: Root key cached for the request" +description: "WP_Secrets_Key_Manager unwraps the root key at most once per request, keyed on the stored wrapped value, so a remote keyring pays one round trip per request instead of one per secret." +--- + +# ADR 0009: Root key cached for the request + +| | | +|---|---| +| **Number** | 0009 | +| **Date** | 2026-09-24 | +| **Status** | Accepted. | + +## Context + +`WP_Secrets_Key_Manager::get_root_key()` unwrapped the root key on every call, and every master-key +derivation called it. A request reading ten secrets across site and network scope unwrapped the +root key ten times. For the default `WP_Secrets_Config_Key_Provider`, an in-process derivation, +that cost is trivial. For a keyring backed by a KMS or HSM, each unwrap is a network round trip +against a service billed per call, and `examples/README.md`'s "Start with a KMS keyring" section +already claimed the opposite: that a KMS "gets called once per request at most instead of once per +secret." That claim was aspirational, not built. + +Writing the KMS keyring example first, as [ADR 0008](0008-the-trac-ticket-replaces-thread-confirmation.md) +schedules, surfaced this before the claim shipped to reviewers. The option considered and rejected +was to leave caching to each keyring implementation: every host writing a `WP_Secrets_Keyring` +would then have to build its own request-scoped memoisation to be usable at any real secret count, +each a fresh chance to get the "memory only, never the object cache" rule wrong. + +## Decision + +`WP_Secrets_Key_Manager` caches one unwrapped root key for the life of the object, which +`_wp_secrets_get_key_manager()` makes the life of the request. The cache is keyed on the stored +wrapped value, not on time or call count: `get_root_key()` serves the cached bytes only while +`get_site_option( ROOT_KEY_OPTION )` still returns the exact value the cache was unwrapped from. A +rotation, a re-wrap, or a restore changes that stored value, so the next `get_root_key()` unwraps +again rather than serving stale bytes. Root-key generation and `rotate_site_key()` both prime the +cache with the value they just produced, at no extra unwrap cost. An unwrap error is never cached, +so a transient failure does not stick for the rest of the request. The cache never touches +`wp_cache_*`, a transient, or any option other than `ROOT_KEY_OPTION`. + +## Consequences + +- One unwrapped copy of the root key lives in the key manager object for the request, in memory + only. The class docblock says so plainly, since this is now load-bearing behavior a reviewer + needs to see without reading the method bodies. +- The memzero discipline for callers of `get_root_key()` is unchanged: they still receive a copy + and are still responsible for zeroing it. The key manager's own cached copy is not theirs to + zero, and PHP's copy-on-write semantics mean a caller zeroing their copy cannot corrupt the + cached one. +- A remote keyring now costs one call per request rather than one per secret, which is what + `examples/README.md` already claimed before this existed. +- The cache is per key-manager instance. `_wp_secrets_get_key_manager()` already builds exactly one + per request via a static local, so no new global or lifecycle concept is introduced. +- This lands in the Trac patch alongside the rest of the key manager; it is not a follow-up. diff --git a/docs/index.md b/docs/index.md index 754896e..968b647 100644 --- a/docs/index.md +++ b/docs/index.md @@ -63,9 +63,12 @@ directory holds everything longer than that. - [`0005-namespaces-are-not-access-control.md`](decisions/0005-namespaces-are-not-access-control.md) — namespacing groups secrets; it does not isolate them. - [`0006-record-format-v2-not-read-compatible.md`](decisions/0006-record-format-v2-not-read-compatible.md) — a future record format bumps `v` rather than widening v1. - [`0007-fail-closed-on-a-broken-drop-in.md`](decisions/0007-fail-closed-on-a-broken-drop-in.md) — one provider per request, and a broken drop-in never falls back to the default. +- [`0008-the-trac-ticket-replaces-thread-confirmation.md`](decisions/0008-the-trac-ticket-replaces-thread-confirmation.md) — additions are reviewed on the Trac ticket, after two more examples and a CLI smoke test. +- [`0009-root-key-cached-for-the-request.md`](decisions/0009-root-key-cached-for-the-request.md) — the key manager unwraps the root key once per request instead of once per secret. ### journal/ - [`2026-09-04-0-1-0-is-public.md`](journal/2026-09-04-0-1-0-is-public.md) — devlog: what 0.1.0 shipped, what it left out, and the road to 7.2. +- [`2026-09-24-a-kms-keyring.md`](journal/2026-09-24-a-kms-keyring.md) — devlog: the first real `WP_Secrets_Keyring`, the root-key cache and `rotate --from` it drove, and what it found. - [`open-questions.md`](journal/open-questions.md) — what is still deliberately undecided. - [`proposal-questions.md`](journal/proposal-questions.md) — the five questions the proposal asked, and the answers so far. - [`test-coverage-gaps.md`](journal/test-coverage-gaps.md) — paths the suite cannot reach and what was verified by hand. diff --git a/docs/journal/2026-09-24-a-kms-keyring.md b/docs/journal/2026-09-24-a-kms-keyring.md new file mode 100644 index 0000000..b4dd746 --- /dev/null +++ b/docs/journal/2026-09-24-a-kms-keyring.md @@ -0,0 +1,78 @@ +--- +title: "A KMS keyring" +description: "The first real WP_Secrets_Keyring implementation: what building examples/aws-kms-keyring/ found, fixed, and left out, and what it means for the Trac patch." +date: 2026-09-24 +--- + +# A KMS keyring + +[ADR 0008](../decisions/0008-the-trac-ticket-replaces-thread-confirmation.md) named this as one of +the two examples to build before the Trac ticket. This is the first one: a real +`WP_Secrets_Keyring`, backed by AWS KMS, instead of the config-derived default. + +## What I built + +[`examples/aws-kms-keyring/`](../../examples/aws-kms-keyring/README.md) — `AWS_KMS_Keyring`, one +file, SigV4 by hand, no SDK, the same shape as the AWS Secrets Manager example. `wrap()` is one +`Encrypt` call, `unwrap()` is one `Decrypt` call with the key ID pinned, and a `kms1:` prefix turns +the most likely adoption mistake into a specific error instead of an opaque AWS exception. + +Alongside it: `WP_Secrets_Keyring_Conformance`, the keyring equivalent of the provider conformance +suite, checking the properties `implements WP_Secrets_Keyring` cannot — non-determinism, fail-closed +on tampering, a round trip that returns exactly what went in. It runs against the shipped config +keyring, against `Mock_Keyring`, and against the KMS example over +[Moto](https://github.com/getmoto/moto), an AWS emulator, in `make test-examples`. The harness that +runs it — `phpunit-examples.xml.dist`, `tests/bootstrap-examples.php`, and the `examples` CI job — +is shared with the AWS Secrets Manager example, whose conformance run had, until now, only been +described in a README rather than actually run anywhere. + +`wp secret rotate` gained `--from=config-previous|config`. The command was hard-coded to one +site-key rotation shape; it now generalises to cover adopting a new keyring over an existing root +key, which is exactly what installing this drop-in on a live site needs. See +[`examples/aws-kms-keyring/tests/test-aws-kms-keyring.php`](../../examples/aws-kms-keyring/tests/test-aws-kms-keyring.php) +for the end-to-end proof: a full round trip with the keyring active, ten secret reads making +exactly one `Decrypt` call, and adoption via `rotate --from=config` leaving every existing secret +readable. + +## What it found + +The spec for this example opened with three things "already known" from reading the code before +writing any of it, and building the example was there to confirm them and drive the fix, not to +discover them fresh. All three held: + +- **`unwrap()` ran on every master-key derivation**, which meant every secret read, write, and + fingerprint was its own KMS round trip. Fixed in `src/`, not in the example: `WP_Secrets_Key_Manager` + now caches the unwrapped root key in memory for the rest of the request, keyed on the wrapped + value it came from, so a rotation mid-request is never served a stale key. See + [ADR 0009](../decisions/0009-root-key-cached-for-the-request.md). +- **Nothing moved an existing site onto a new keyring.** `rotate` assumed the old and new keyrings + were always both `WP_Secrets_Config_Key_Provider`. `--from=config` is the fix, in `cli/`, which + never lands in the Trac patch. +- **`wrap()`'s non-determinism was load-bearing but undocumented.** `rotate_site_key()` depends on + it — `update_site_option()` reports an unchanged value as a failure — and the interface docblock + said nothing about it. It does now, and the conformance suite checks it. + +What only building the example showed, rather than what was predicted going in: `Mock_Keyring`, +the test double the conformance suite and dozens of other tests lean on, was deterministic and +returned `false` on a failed decode, so it failed the keyring contract the new conformance suite +checks. This work made it non-deterministic with an integrity tag and `WP_Error` on every failure, and +it now passes the suite it stands in for. + +## What I left out + +Named as out of scope from the start, not discovered as a gap partway through: IAM role and +instance-metadata credentials (static credentials only, as with the Secrets Manager example), KMS +multi-region keys, and moving between two different KMS keys — KMS's own automatic key rotation +keeps the key ID stable, so that case needs no re-wrap at all. The examples suite still runs +single-site only; multisite coverage for examples is a later flight's concern. And the live-AWS +run — a fresh site, adoption with `rotate --from=config`, a count of KMS calls for a ten-secret +read — is verified by hand once, the way the Secrets Manager example was, and has not happened +yet. It is the one thing this entry cannot yet report as done. + +## What it means for the patch + +The root-key cache and the non-determinism sentence on `WP_Secrets_Keyring::wrap()` are both in +`src/`, so both go into the Trac patch. `cli/`'s `rotate --from` and everything under `examples/` +do not; they are plugin-and-repository-only, same as ever. Neither interface changed shape: three +methods on `WP_Secrets_Keyring` before this, three methods after. What changed is that one of them +now has a real implementation behind it instead of only the shipped default and a test double. diff --git a/docs/journal/open-questions.md b/docs/journal/open-questions.md index 926bb75..0ae3204 100644 --- a/docs/journal/open-questions.md +++ b/docs/journal/open-questions.md @@ -1,7 +1,7 @@ --- title: "Open questions" description: "What this implementation deliberately did not decide, with the conservative choice made in the meantime." -date: 2026-09-04 +date: 2026-09-24 --- # Open questions @@ -29,12 +29,21 @@ provider can be stronger than the default, never weaker. [ADR 0001](../decisions/0001-provider-as-outermost-extension-point.md) records the discussion, the decision, and what shipped in 0.1.0. -**What is still open:** nobody has written a real platform provider against `WP_Secrets_Provider` -yet. The interface is shaped by hosts describing what they need rather than by anyone building -against it, and the first real implementation will turn something up. That is what to ask for on -the thread: not "does this look right" but "build against it and tell us what broke", and run -the conformance suite in `tests/includes/class-wp-secrets-provider-conformance.php` to see what it -fails to catch. +**What has been built:** one real provider, `examples/aws-secrets-manager/`, verified against live +AWS for set, masked read, rotation, and `--slot=previous`, and its conformance suite is now +automated against Moto in `make test-examples` rather than only described in its README. Building +it turned up four defects, none of them in the interface: a provider global of the wrong type fell +through to the default provider instead of failing closed, `wp secret dropin` reported internals +rather than the provider, and three WP-CLI dispatch bugs surfaced on the first end-to-end run. The +two-slot version model mapped onto `AWSCURRENT`/`AWSPREVIOUS` with no emulation. A KMS keyring +example, `examples/aws-kms-keyring/`, now exists too, and building it changed `src/` once +(request-scoped root-key caching, [ADR 0009](../decisions/0009-root-key-cached-for-the-request.md)) +and the CLI once (`wp secret rotate --from` generalised to cover adoption, not only a site-key +change). + +**What is still open:** those are two providers and one keyring, written by the same hands as the +interfaces, and no host has built against `WP_Secrets_Provider` or `WP_Secrets_Keyring` +independently. --- diff --git a/docs/journal/proposal-questions.md b/docs/journal/proposal-questions.md index 940d354..0fc367f 100644 --- a/docs/journal/proposal-questions.md +++ b/docs/journal/proposal-questions.md @@ -1,7 +1,7 @@ --- title: "The five questions the proposal asked the community" description: "Where answers from the proposal's comment thread are recorded, so they land somewhere rather than being absorbed into an assumption." -date: 2026-09-04 +date: 2026-09-24 --- ## 🟢 The five questions the proposal asked the community @@ -17,15 +17,25 @@ absorbed into an assumption. [Host and platform providers](open-questions.md#host-and-platform-providers) for what is still open. 2. **Are two version slots (`CURRENT`/`PREVIOUS`) adequate, or is a different rotation pattern - necessary?** — no answers recorded yet. `'v' => 1` leaves room to change this, but see + necessary?** — no objections raised. The AWS Secrets Manager example is supporting evidence: + its `AWSCURRENT`/`AWSPREVIOUS` staging labels are the same two slots, so the model needed no + emulation there. `'v' => 1` leaves room to change this, but see [ADR 0006](../decisions/0006-record-format-v2-not-read-compatible.md) for what a format bump would mean. 3. **Does `wp_import_option_as_secret()` fit actual plugin migration workflows?** - — no answers recorded yet. -4. **Which WP-CLI commands most need this surface, and in what priority order?** — the command set - implemented here is a starting set, not a settled one. Track real answers rather than assuming. + — no objections raised, and no plugin outside this project has used it yet. +4. **Which WP-CLI commands most need this surface, and in what priority order?** — no objections + raised to the implemented set, and no requests for others. + +Questions 2 to 4, and the names this implementation added beyond the proposal, have been in front +of the community through the make/core thread, the docs site, Core Slack, and core dev chat. +The response has been support without critical feedback. That is silence rather than +confirmation, and it is recorded as such. 5. **For hosts running secret stores or key backends: what is missing from the drop-in surface?** — answered at length by two hosting platforms on the thread; see [ADR 0001](../decisions/0001-provider-as-outermost-extension-point.md) and - [Host and platform providers](open-questions.md#host-and-platform-providers). + [Host and platform providers](open-questions.md#host-and-platform-providers). The keyring side + of the drop-in surface now has a real implementation, `examples/aws-kms-keyring/`, and it + needed nothing added to the interface itself — only a docblock sentence on non-determinism, a + request-scoped cache in the key manager, and a `--from` flag on `wp secret rotate`. diff --git a/docs/journal/test-coverage-gaps.md b/docs/journal/test-coverage-gaps.md index c613fa7..4dc88f1 100644 --- a/docs/journal/test-coverage-gaps.md +++ b/docs/journal/test-coverage-gaps.md @@ -1,7 +1,7 @@ --- title: "Test coverage gaps" description: "Code paths the automated suite does not reach, why, and what was verified by hand instead." -date: 2026-09-04 +date: 2026-09-24 --- ## 🟢 `sodium_compat` is never exercised by the test suite @@ -89,7 +89,8 @@ wp-env can. Not built. Until it is, treat any change to a command's docblock syn name as untested, and run it by hand. Cheap interim discipline: `wp help secret ` shows the synopsis WP-CLI actually built. -If a flag is missing there, it is missing everywhere. +If a flag is missing there, it is missing everywhere. `--from` on `wp secret rotate` was checked +this way by hand when it was generalised; the output is recorded in the body of the commit that added `--from`. --- @@ -127,3 +128,15 @@ the same unexplained cause). What is not known is why the split falls exactly al Left as-is: no coverage threshold gates anything in `make ci`. Trustworthy numbers, if wanted, should come from the non-Docker path against a host PHP with a coverage driver installed normally, not via a `pecl install` into an already-running container. + + +--- + +## 🟢 Examples run against an emulator, not live AWS + +`make test-examples` proves `examples/aws-secrets-manager/` and `examples/aws-kms-keyring/` +against [Moto](https://github.com/getmoto/moto), which is what runs in CI and on every developer +machine. Moto does not verify SigV4 signatures or IAM permissions the way real AWS does, so a +signing bug that happens to produce a request Moto accepts anyway, or a policy missing a +permission the example actually needs, is invisible to this suite. The live run against real AWS +is the manual step named in `examples/aws-kms-keyring/README.md` and has not been run yet. diff --git a/docs/reference/ci.md b/docs/reference/ci.md index f4b498d..063159d 100644 --- a/docs/reference/ci.md +++ b/docs/reference/ci.md @@ -77,7 +77,9 @@ hosted pipeline. **github.com, hosted runners.** Static analysis gates a PHP 7.4 / 8.0 / 8.3 × WordPress latest / trunk matrix, plus a multisite job. `shivammathur/setup-php` provides the interpreter and asks for the `sodium` extension by name. The whole API is built on libsodium, so relying on -whatever the runner image happens to ship wasn't good enough. +whatever the runner image happens to ship wasn't good enough. The `examples` job is the only one +with a non-database service container: a pinned Moto instance the AWS Secrets Manager provider +conformance run and the AWS KMS keyring conformance and integration tests both run against. The workflow declares `permissions: contents: read`. Nothing in it writes to the repository, publishes anything, or needs a token beyond reading the code under test. @@ -102,6 +104,7 @@ person pasted. | `static` | 8.3 | — | lint + compat + analyse. Gates everything else. | | `test` | 7.4, 8.0, 8.3 | latest, trunk | Single site | | `test-multisite` | 8.3 | latest | Multisite suite | +| `examples` | 8.3 | latest | `make test-examples` against a Moto (AWS emulator) service container, pinned by digest. Not part of `make ci`. | | `reference-docs` | 8.3 | — | `bin/gen-reference.php --check`: the committed docs/reference/ matches the source. No Composer install. | The 7.4 leg is not optional. Core's floor is 7.4 and `src/` must run there; PHPCompatibilityWP diff --git a/docs/reference/classes.md b/docs/reference/classes.md index 061cdde..ab79353 100644 --- a/docs/reference/classes.md +++ b/docs/reference/classes.md @@ -1085,6 +1085,13 @@ Master keys are derived from the root key on demand and never stored: differs from the site path, so there is no collision between the two. Identical on every blog, so a network secret written on one blog reads on every other. +One unwrapped copy of the root key lives in this object for the rest of the +request, in memory only, never in the object cache. It is replaced whenever the +stored wrapped value changes -- a rotation, a re-wrap, a restore -- so it is never +stale. Callers of get_root_key() still receive a copy and must zero it themselves; +this object's own copy is not theirs to zero. The practical effect: a remote +keyring (a KMS or HSM call) is invoked once per request, not once per secret. + **Since:** 7.2.0 **Source:** [`src/wp-includes/class-wp-secrets-key-manager.php`](../../src/wp-includes/class-wp-secrets-key-manager.php) @@ -1191,6 +1198,9 @@ implementation is never handed a plaintext secret, only 32 bytes of key material and cannot turn encryption off. There is no method here that accepts a plaintext secret value at all. +An implementation should run WP_Secrets_Keyring_Conformance against itself before +shipping. + **Since:** 7.2.0 **Source:** [`src/wp-includes/interface-wp-secrets-keyring.php`](../../src/wp-includes/interface-wp-secrets-keyring.php) @@ -1229,6 +1239,11 @@ public function unwrap( $wrapped ) Wraps (encrypts) raw key material for storage. +Must not be deterministic: two calls with the same key material must return +different values. WP_Secrets_Key_Manager::rotate_site_key() stores the +re-wrapped value with update_site_option(), which reports an unchanged value +as a failure, and WP_Secrets_Keyring_Conformance checks this. + ```php public function wrap( $key_material ) ``` diff --git a/docs/reference/wp-cli.md b/docs/reference/wp-cli.md index 3df46e5..68485e8 100644 --- a/docs/reference/wp-cli.md +++ b/docs/reference/wp-cli.md @@ -192,19 +192,33 @@ wp network-secret retire [--yes] ### `wp network-secret rotate` -Re-wraps the root key under a new WP_SECRETS_KEY after a site-key change. - -No secret is re-encrypted: rotation only changes what the root key is +Re-wraps the root key under the active keyring. + +There are two cases, chosen with --from. `--from=config-previous` (the +default) is a site key change: the root key, currently wrapped under +WP_SECRETS_KEY_PREVIOUS, is re-wrapped under the current WP_SECRETS_KEY. +`--from=config` is moving the root key onto a new keyring: a secrets.php +drop-in has installed one, and the root key, currently wrapped under the +config keyring's WP_SECRETS_KEY, is re-wrapped under that new keyring. No +secret is ever re-encrypted: rotation only changes what the root key is wrapped under, not the root key's own bytes. ``` -wp network-secret rotate [--yes] +wp network-secret rotate [--from=] [--yes] ``` | Option | Description | |---|---| +| `[--from=]` | Which keyring currently wraps the root key. Default: `config-previous`. Options: `config-previous`, `config`. | | `[--yes]` | Skip the confirmation prompt. | +**Examples** + +``` + $ wp secret rotate --yes + $ wp secret rotate --from=config --yes +``` + **Runs:** `after_wp_load` **Source:** [`cli/class-wp-cli-secret-command.php`](../../cli/class-wp-cli-secret-command.php) @@ -417,19 +431,33 @@ wp secret retire [--yes] ### `wp secret rotate` -Re-wraps the root key under a new WP_SECRETS_KEY after a site-key change. +Re-wraps the root key under the active keyring. -No secret is re-encrypted: rotation only changes what the root key is +There are two cases, chosen with --from. `--from=config-previous` (the +default) is a site key change: the root key, currently wrapped under +WP_SECRETS_KEY_PREVIOUS, is re-wrapped under the current WP_SECRETS_KEY. +`--from=config` is moving the root key onto a new keyring: a secrets.php +drop-in has installed one, and the root key, currently wrapped under the +config keyring's WP_SECRETS_KEY, is re-wrapped under that new keyring. No +secret is ever re-encrypted: rotation only changes what the root key is wrapped under, not the root key's own bytes. ``` -wp secret rotate [--yes] +wp secret rotate [--from=] [--yes] ``` | Option | Description | |---|---| +| `[--from=]` | Which keyring currently wraps the root key. Default: `config-previous`. Options: `config-previous`, `config`. | | `[--yes]` | Skip the confirmation prompt. | +**Examples** + +``` + $ wp secret rotate --yes + $ wp secret rotate --from=config --yes +``` + **Runs:** `after_wp_load` **Source:** [`cli/class-wp-cli-secret-command.php`](../../cli/class-wp-cli-secret-command.php) diff --git a/docs/spec/envelope-encryption.md b/docs/spec/envelope-encryption.md index 806b603..8c7783f 100644 --- a/docs/spec/envelope-encryption.md +++ b/docs/spec/envelope-encryption.md @@ -28,7 +28,10 @@ The 0.1.0 code has four layers, not two. Exactly one value is ever stored wrappe wraps them with the keyring, and stores the result under the `_wp_secrets_root_key` site option via `add_site_option()`, handling the two-requests-race by re-reading the winner. The default keyring wraps with `sodium_crypto_aead_xchacha20poly1305_ietf_encrypt()` under the - fixed AAD `wp-secrets-root-key-v1`, storing `nonce . ciphertext` base64-encoded. + fixed AAD `wp-secrets-root-key-v1`, storing `nonce . ciphertext` base64-encoded. The key + manager keeps the unwrapped root key in memory for the rest of the request, so a remote + keyring is invoked once per request rather than once per secret; see + [providers-and-keyrings.md](providers-and-keyrings.md). 3. **Master key.** `WP_Secrets_Key_Manager::get_master_key()` derives a per-scope master key from the root key on demand with `sodium_crypto_kdf_derive_from_key()`. Master keys are never stored. See [network.md](network.md) for the subkey and context values. diff --git a/docs/spec/extension-points.md b/docs/spec/extension-points.md index 7feaf58..22fb2cc 100644 --- a/docs/spec/extension-points.md +++ b/docs/spec/extension-points.md @@ -120,10 +120,29 @@ material, never a secret value. In a real deployment a KMS or HSM sits behind th shipped default, `WP_Secrets_Config_Key_Provider`, wraps the root key with a key derived from `wp-config.php`, since that is the only thing guaranteed to exist on every WordPress install. +`wrap()` must be non-deterministic: two calls on the same 32 bytes must return two different +wrapped values. `WP_Secrets_Key_Manager::rotate_site_key()` stores the re-wrapped root key with +`update_site_option()`, which reports an unchanged value as a failure the same way `update_option()` +does, so a keyring that ever produced the same wrapped output twice would make rotation +indistinguishable from a storage error. `unwrap()` returns `WP_Error` for anything it did not +produce, garbage, a truncated value, or a single tampered byte, and never throws: a caller holding +`is_wp_error()` as its only failure signal must never receive a plausible-looking wrong key instead +of a clear failure. + `get_key_source()` returns a short human-readable string for Site Health, so an operator can see whether they are on the config-derived default or something they wired up themselves. It describes the key; it never contains the key material. +**The keyring conformance suite** mirrors the provider one. `WP_Secrets_Keyring_Conformance` in +`tests/includes/class-wp-secrets-keyring-conformance.php` is an abstract test case with a +`keyring()` method to implement. It checks that `wrap()` of 32 random bytes returns a non-empty +string that `unwrap()` returns to the same bytes; that two `wrap()` calls on the same bytes never +match; that `unwrap()` of garbage, of a truncated value, and of a value with one flipped byte each +returns `WP_Error`; and that `get_key_source()` is a non-empty string. It runs against the shipped +`WP_Secrets_Config_Key_Provider` and against `Mock_Keyring`, the same way the provider suite runs +against the shipped provider. `examples/aws-kms-keyring/` runs it against a real +`WP_Secrets_Keyring` implementation, AWS KMS, reached through Moto in `make test-examples`. + ```php // wp-content/secrets.php $GLOBALS['wp_secrets_keyring'] = new My_KMS_Keyring(); diff --git a/docs/spec/providers-and-keyrings.md b/docs/spec/providers-and-keyrings.md index c50c90e..825e15b 100644 --- a/docs/spec/providers-and-keyrings.md +++ b/docs/spec/providers-and-keyrings.md @@ -49,6 +49,16 @@ a plaintext" holds. derived from `wp-config.php`. Its constructor takes a boolean to read `WP_SECRETS_KEY_PREVIOUS` instead, used only during site-key rotation. See [envelope-encryption.md](envelope-encryption.md). +**Root-key caching.** `WP_Secrets_Key_Manager` keeps one unwrapped copy of the root key for the +rest of the request, in memory only, never in the object cache. The cache is keyed on the stored +wrapped value: `get_root_key()` serves the cached bytes only when the value currently in +`get_site_option()` still matches the one the cache was unwrapped from, so a rotation, a re-wrap, +or a restore that changes the stored value is always unwrapped fresh rather than served stale. An +unwrap error is never cached. Root-key generation and `rotate_site_key()` both prime the cache +with the value they just produced, at no extra cost. Callers of `get_root_key()` still receive a +copy and are responsible for zeroing it; the manager's own cached copy is not theirs to zero. See +`tests/phpunit/test-wp-secrets-key-manager.php` for the coverage. + **Drop-in loading.** `wp_secrets_api_load_dropin()` in `secrets-api.php` requires `wp-content/secrets.php` inside `try`/`catch ( \Throwable )`, then type-checks all three globals. A missing global is fine. A throw or a wrong type sets `$GLOBALS['wp_secrets_dropin_broken']`. @@ -96,4 +106,12 @@ security controls. A drop-in is fully trusted code and could already read every implementing the keyring. They exist so Site Health, a reviewer, and a future settings screen can see what a provider claims. +**One unwrap per request.** The proposal does not discuss how often a keyring gets called. +Without the cache, a remote keyring pays one round trip per secret read -- a master key derivation +for every `wp_get_secret()` call, each unwrapping the same root key again. Left uncached, every +remote keyring implementation would have to build its own memoisation to be usable at any real +secret count, each hand-rolled and each a chance to get the memory-only, never-in-the-object-cache +rule wrong. The fix belongs in the key manager, once, so it reaches core with the rest of the +patch rather than becoming a burden on every keyring author. `docs/decisions/0009-root-key-cached-for-the-request.md` records the decision. + [proposal]: https://make.wordpress.org/core/2026/08/25/proposal-a-secrets-api-for-wordpress-7-2/ diff --git a/docs/spec/rotation.md b/docs/spec/rotation.md index dce4d8d..1841080 100644 --- a/docs/spec/rotation.md +++ b/docs/spec/rotation.md @@ -33,16 +33,27 @@ old value. the requested state already holds. `wp secret retire [--yes]` in `cli/class-wp-cli-secret-command.php` wraps it. -**Rotating the site key.** `wp secret rotate [--yes]` in `cli/class-wp-cli-secret-command.php` -requires `WP_SECRETS_KEY_PREVIOUS` to be defined and calls -`WP_Secrets_Key_Manager::rotate_site_key()` in `src/wp-includes/class-wp-secrets-key-manager.php` -with `new WP_Secrets_Config_Key_Provider( true )` as the old keyring and -`new WP_Secrets_Config_Key_Provider( false )` as the new one. The method unwraps the stored root -key under the old keyring, wraps it under the new one, and updates the `_wp_secrets_root_key` -site option. The root key's bytes do not change, so every derived master key is unchanged and no -secret is re-encrypted. `wp secret generate-key` prints a base64 32-byte value for the new -`WP_SECRETS_KEY`; it never writes `wp-config.php`. There is no public API function for site-key -rotation; the method is reached through the CLI. +**Rotating the site key.** `wp secret rotate [--from=] [--yes]` in +`cli/class-wp-cli-secret-command.php` calls `WP_Secrets_Key_Manager::rotate_site_key()` in +`src/wp-includes/class-wp-secrets-key-manager.php`. The new keyring is always whatever keyring is +currently active: a `secrets.php` drop-in's, if one is installed, otherwise +`WP_Secrets_Config_Key_Provider( false )`. `--from` names the old keyring, the one that currently +wraps the stored root key: + +- `config-previous` (the default) requires `WP_SECRETS_KEY_PREVIOUS` to be defined and unwraps + with it, via `new WP_Secrets_Config_Key_Provider( true )`. This is a site-key change: the same + keyring, a new key. +- `config` unwraps with the current `WP_SECRETS_KEY`, via `new WP_Secrets_Config_Key_Provider( false )`. + This is adoption: the root key has not moved, but a new keyring, such as a KMS-backed drop-in, + has just been installed over it. + +Either way, the command refuses with an error rather than a silent no-op if the old and new +keyrings resolve to the same configuration. The method unwraps the stored root key under the old +keyring, wraps it under the new one, and updates the `_wp_secrets_root_key` site option. The root +key's bytes do not change, so every derived master key is unchanged and no secret is re-encrypted. +`wp secret generate-key` prints a base64 32-byte value for the new `WP_SECRETS_KEY`; it never +writes `wp-config.php`. There is no public API function for site-key rotation; the method is +reached through the CLI. **Flagging for rotation.** Every slot carries `needs_rotation`. `wp_import_option_as_secret()` sets it to `true`; ordinary writes set `false`. It surfaces in `wp_list_secrets()`, in the Site @@ -66,6 +77,8 @@ a second function would invite two paths to the same state. **Site-key rotation is CLI-only.** The proposal does not place it. Changing the wrapping key needs both the old and the new constant present in `wp-config.php` at once, which is an operator's -deployment step and not something a plugin should trigger from a request. +deployment step and not something a plugin should trigger from a request. Moving the root key onto +a newly installed keyring is the same kind of deployment step, so it is the same command with a +different `--from`, not a second one. [proposal]: https://make.wordpress.org/core/2026/08/25/proposal-a-secrets-api-for-wordpress-7-2/ diff --git a/examples/README.md b/examples/README.md index b50aab4..289926e 100644 --- a/examples/README.md +++ b/examples/README.md @@ -7,6 +7,13 @@ by the plugin; you copy one into a `wp-content/secrets.php` drop-in. They're exc This will probably become a submodule once there's more than one, which is why it sits at the top level instead of under `docs/`. +## Examples in this directory + +- [`aws-kms-keyring/`](aws-kms-keyring/README.md) — a `WP_Secrets_Keyring`. AWS KMS holds the + root key; secrets stay in WordPress's own options tables. +- [`aws-secrets-manager/`](aws-secrets-manager/README.md) — a `WP_Secrets_Provider`. AWS Secrets + Manager holds the secret itself; WordPress becomes a consumer rather than a custodian. + ## Which interface do you need? Worth getting right before you write anything. **A key-management service is not a secret store**, @@ -37,8 +44,10 @@ about as small as a useful integration gets: What you get is what most hosts are actually after: **key custody moves to the KMS and nothing else changes.** Secrets stay in the options tables. The libsodium envelope is untouched. Rotating -the site key still re-wraps one value, and the KMS gets called once per request at most instead of -once per secret. +the site key still re-wraps one value. `WP_Secrets_Key_Manager` unwraps the root key once per +request and keeps it in memory for the rest of that request, so a KMS is called once per request, +not once per secret. That was not true before the caching change described in +[ADR 0009](../docs/decisions/0009-root-key-cached-for-the-request.md). ## When you need a provider instead @@ -72,7 +81,24 @@ something absent succeeding, fingerprints staying stable for the same value, lis containing a plaintext, and a read-only declaration actually being honoured. See [`../docs/spec/extension-points.md`](../docs/spec/extension-points.md). +## Run the examples suite + +Both examples' conformance suites run against [Moto](https://github.com/getmoto/moto), an AWS +emulator, so they run without real credentials or cost: + +```sh +docker pull motoserver/moto:latest +docker run -d --name secrets-api-moto-kms -p 5051:5000 motoserver/moto:latest +curl -sf http://localhost:5051/moto-api/ # 200 once it is up +``` + +Then `make test-examples`, or, under wp-env, run from the repository root so `$(basename "$PWD")` +resolves to the plugin's directory name: +`npx @wordpress/env run --env-cwd="wp-content/plugins/$(basename "$PWD")" tests-cli vendor/bin/phpunit -c phpunit-examples.xml.dist`. +This is outside `make ci`: it needs Moto running, a service container the other CI environments +do not provide. + ## Dependencies -Each binding has its own `composer.json`. The plugin's dependency tree stays clean, `make ci` -never installs an SDK, and `examples/*/vendor/` is git-ignored. +The examples have no Composer dependencies — that is the point of hand-rolling SigV4 instead of +pulling in an SDK. `examples/*/vendor/` stays git-ignored for any example that ever adds one. diff --git a/examples/aws-kms-keyring/README.md b/examples/aws-kms-keyring/README.md new file mode 100644 index 0000000..e89bca0 --- /dev/null +++ b/examples/aws-kms-keyring/README.md @@ -0,0 +1,197 @@ +# AWS KMS keyring + +A `wp-content/secrets.php` drop-in that moves custody of the site's **root key** to an AWS KMS +customer master key. WordPress keeps its own envelope: this only changes what wraps the root key +everything else derives from. `wp secret dropin` still reports `Encryption boundary: WordPress`, +and `Protected by: WordPress (libsodium), key source: AWS KMS key in `. + +**No Composer, no AWS SDK.** One SigV4 signature and `wp_remote_post()`, in a single file you can +read end to end. A drop-in that drags in a 100 MB SDK is a drop-in nobody audits. + +## Where the credentials go + +**Not `.wp-env.json`** — that file is committed. Use `.wp-env.override.json`, which wp-env merges +on top and which this repo git-ignores: + +```jsonc +// .wp-env.override.json (repo root, git-ignored) +{ + "config": { + "WP_SECRETS_KMS_KEY_ID": "1234abcd-12ab-34cd-56ef-1234567890ab", + "WP_SECRETS_AWS_REGION": "us-east-1", + "WP_SECRETS_AWS_KEY": "AKIAIOSFODNN7EXAMPLE", + "WP_SECRETS_AWS_SECRET": "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", + "WP_SECRETS_AWS_ENDPOINT": "" + } +} +``` + +`WP_SECRETS_AWS_ENDPOINT` is optional — leave it unset (or empty) to reach real AWS. It exists +for pointing the keyring at an emulator such as Moto during development; see "Run it against an +emulator" in `../aws-secrets-manager/README.md` for the general pattern. + +Anything under `config` becomes a PHP constant in `wp-config.php`. Then: + +```sh +npx @wordpress/env start # re-reads the config and rewrites wp-config.php +``` + +On a real site these are ordinary `wp-config.php` constants — or better, an IAM role, in which +case you would swap the signing block for instance-profile credentials (see "Known limits of this +example"). + +## Install the drop-in + +```sh +CID=$(docker ps --format '{{.Names}}' | grep -- '-cli-1' | grep -v tests) +docker cp examples/aws-kms-keyring/secrets.php "$CID":/var/www/html/wp-content/secrets.php +docker exec "$CID" wp secret dropin --verbose +``` + +Expected once the constants are set: + +``` +Drop-in active: yes +Provider: WP_Secrets_Libsodium_Provider +Protected by: WordPress (libsodium), key source: AWS KMS key 1234abcd-12ab-34cd-56ef-1234567890ab in us-east-1 +Encryption boundary: WordPress +Accepts writes: yes +Keyring class: AWS_KMS_Keyring +Store class: WP_Secrets_Option_Store +``` + +To take it back out — **and do this before running the test suite**: + +```sh +for c in $(docker ps --format '{{.Names}}' | grep -E 'cli-1|wordpress-1'); do + docker exec "$c" rm -f /var/www/html/wp-content/secrets.php +done +``` + +**The gotcha:** wp-env's dev and tests environments see the same `wp-content`, so an installed +drop-in is in front of PHPUnit too. A drop-in that cannot reach KMS will fail most of the suite, +which looks alarming and is not a code problem. Remove it, re-run, and it is green again. Removing +it from a single container is not enough — the loop above covers all four. + +## IAM permissions + +The smallest policy that runs everything above: + +``` +kms:Encrypt +kms:Decrypt +``` + +Scope the resource to the one key's ARN. `kms:Decrypt` is the sensitive one of the two: any +principal that holds it can unwrap the root key, and from there derive every master key on the +site. `kms:Encrypt` alone is enough to wrap a new root key but useless without `kms:Decrypt` to +read one back, which is why the two are worth reasoning about separately rather than as one grant. + +## Adopting an existing site + +A site that already has a root key — wrapped by the config keyring, using `WP_SECRETS_KEY` — moves +onto this keyring in three steps: + +1. **Install the drop-in** (above). From this point on, `unwrap()` is called with the existing + config-keyring-wrapped root key, and it is not a value AWS KMS produced. +2. **`wp secret rotate --from=config`.** This unwraps the root key with the config keyring and + re-wraps it under the now-active KMS keyring. No secret is re-encrypted — only what wraps the + root key changes. +3. **`wp secret health`.** Confirms nothing is left undecryptable. + +> **Between steps 1 and 2, every secret read fails closed.** `unwrap()` sees a value with no +> `kms1:` prefix and returns a `WP_Error` naming step 2 directly: *"The stored root key was not +> wrapped by AWS KMS (no kms1: prefix), so it was probably wrapped by the config keyring. Run `wp +> secret rotate --from=config` to move it onto this KMS key."* Do steps 1 and 2 in the same +> maintenance window — do not leave a site running with the drop-in installed but not yet rotated. + +What the failure looks like in the meantime: + +``` +$ wp secret get acme/api-key --reveal +Error: The stored root key was not wrapped by AWS KMS (no kms1: prefix), so it was probably +wrapped by the config keyring. Run `wp secret rotate --from=config` to move it onto this KMS key. +``` + +## How often KMS is called + +Once per request, at most. `WP_Secrets_Key_Manager` caches the unwrapped root key in memory for +the life of the request, keyed on the wrapped value it came from, so a re-wrap or rotation +replaces it rather than serving stale key material. See +[docs/decisions/0009-root-key-cached-for-the-request.md](../../docs/decisions/0009-root-key-cached-for-the-request.md). +Without that cache, every `wp_get_secret()` call in a request would be its own KMS round trip; +with it, ten reads make one `Decrypt` call, which `examples/aws-kms-keyring/tests/test-aws-kms-keyring.php` +proves directly. + +## Design points + +- **The encryption context is fixed, not per-site.** KMS authenticates it the way an AEAD cipher + authenticates AAD. Binding it to something like `home_url()` would make a domain change + unrecoverable, and there is exactly one root key per install, so there is nothing per-site to + bind it to. +- **`KeyId` is pinned on `Decrypt`.** Without it, KMS decrypts with whichever key the ciphertext + blob names, and a swapped blob under a key this IAM role can also use would otherwise succeed. +- **The `kms1:` prefix** turns the most likely adoption failure — a root key still wrapped by the + config keyring — into the specific, actionable error above instead of an opaque + `InvalidCiphertextException`. +- **Timeouts are short (3 s), the `AWS_KMS_Keyring::TIMEOUT` constant.** Every secret operation + waits on this call. A KMS outage that does not answer within it turns every read into a + `WP_Error` rather than hanging the request — fail closed, on purpose. +- **Install guard.** The drop-in installs only when all four constants are defined and non-empty + after `trim()`. A freshly-copied override file with the keys present but blank falls back to + WordPress's own keyring instead of installing one that fails every call. + +## Known limits of this example + +Stated because it is a demonstration, not a product: + +- **Static credentials.** Fine for a demo; use an IAM role or instance-metadata credentials in + production — this example does not implement either. +- **No KMS multi-region keys.** One key, one region. +- **No move between two different KMS keys.** Only adoption from the config keyring is covered. + KMS's own automatic key rotation keeps the key ID stable and decrypts old ciphertext under it, + so that case needs no re-wrap and no `rotate --from`. +- **A KMS outage is a `WP_Error` on every read**, by design (see "Design points" above) — this is + not a bug to work around, but it does mean the keyring has no offline fallback. + +## Prove it conforms + +```php +class Tests_AWS_KMS_Keyring_Conformance extends WP_Secrets_Keyring_Conformance { + protected function keyring() { + return new AWS_KMS_Keyring( getenv( 'KMS_KEY_ID' ), 'us-east-1', getenv( 'AWS_KEY' ), getenv( 'AWS_SECRET' ) ); + } +} +``` + +That checks the properties `implements WP_Secrets_Keyring` cannot: `wrap()` is non-deterministic, +`unwrap()` round-trips exactly the bytes that went in, and garbage, truncated, or tampered input +fails closed as `WP_Error` rather than returning a plausible-looking wrong key. It makes real API +calls, so point it at a throwaway AWS account. + +## Run it against an emulator + +`examples/aws-kms-keyring/tests/test-aws-kms-keyring-conformance.php` runs the conformance suite +above against [Moto](https://github.com/getmoto/moto) instead of real AWS, so it can run without +credentials or cost. Start it (shared with `../aws-secrets-manager/README.md`'s emulator, since +Moto serves both KMS and Secrets Manager from the same container): + +```sh +docker pull motoserver/moto:latest +docker run -d --name secrets-api-moto-kms -p 5051:5000 motoserver/moto:latest +curl -sf http://localhost:5051/moto-api/ # 200 once it is up +``` + +The fifth constructor argument, `$endpoint`, points the keyring at Moto instead of real AWS — this +is what `WP_SECRETS_AWS_ENDPOINT` sets when defined, and it is never set in production. +`phpunit-examples.xml.dist` already points `WP_SECRETS_TEST_AWS_ENDPOINT` at +`http://host.docker.internal:5051`, which is where the tests-cli container reaches a Moto +container published on the host. Then, run from the repository root so `$(basename "$PWD")` +resolves to the plugin's directory name: + +```sh +npx @wordpress/env run --env-cwd="wp-content/plugins/$(basename "$PWD")" tests-cli vendor/bin/phpunit -c phpunit-examples.xml.dist +``` + +or, without wp-env, `make test-examples`. Not part of `make ci`: it needs Moto running, and the +separate examples CI job runs it against a pinned Moto service container. diff --git a/examples/aws-kms-keyring/SPEC.md b/examples/aws-kms-keyring/SPEC.md new file mode 100644 index 0000000..6b8ee6e --- /dev/null +++ b/examples/aws-kms-keyring/SPEC.md @@ -0,0 +1,162 @@ +# Spec: AWS KMS keyring example + +Status: planned. Part of the pre-Trac work in +[ADR 0008](../../docs/decisions/0008-the-trac-ticket-replaces-thread-confirmation.md). + +## Why this example + +`examples/README.md` tells hosts to start with a KMS keyring: three methods, key custody moves to +the KMS, nothing else changes. No such example exists, so that advice has never been tested. This +is the first real implementation of `WP_Secrets_Keyring` other than the config keyring shipped in +`src/`. + +The interface is small, so the value is not in the three methods. It is in the three questions +around them, which nobody has had to answer yet: + +1. **How often does WordPress call `unwrap()`?** +2. **How does an existing site move onto a new keyring?** +3. **What does a keyring have to guarantee that the interface does not say?** + +## What is already known + +These came from reading the code while writing this spec. The example exists to confirm them and +drive the fix, not to discover them. + +- **`unwrap()` runs on every master-key derivation.** `WP_Secrets_Key_Manager::get_root_key()` + reads the option and calls `$this->keyring->unwrap()` every time, with no cache. + `get_master_key()` calls it for every secret read, every write, and every fingerprint. With the + config keyring that is a local libsodium call and costs nothing. With KMS it is one network round + trip per secret. `examples/README.md` says "the KMS gets called once per request at most", which + is not what the code does today. +- **There is no command that moves a site onto a new keyring.** `wp secret rotate` hard-codes + `new WP_Secrets_Config_Key_Provider( true )` as the old keyring and `( false )` as the new one. A + site that installs a KMS drop-in over an existing root key fails closed, because KMS cannot unwrap + a config-keyring blob, and nothing in the shipped tooling can re-wrap it. +- **`wrap()` must be non-deterministic.** `rotate_site_key()` stores the re-wrapped value with + `update_site_option()`, which returns false when the value is unchanged, and treats that as a + failure. Its own comment says this is safe only because `wrap()` draws a fresh nonce. The + interface docblock does not say so. KMS `Encrypt` is non-deterministic, so the example is fine, + but the requirement belongs in the contract. + +## Deliverables + +### 1. `examples/aws-kms-keyring/secrets.php` + +A single-file drop-in, following the conventions of the AWS Secrets Manager example: no Composer, +no SDK, SigV4 by hand, `wp_remote_post()`. The SigV4 signer is copied rather than shared, because +each example has to be one file a reviewer can read from top to bottom. + +`final class AWS_KMS_Keyring implements WP_Secrets_Keyring`: + +| Method | Behaviour | +|---|---| +| `wrap( $key_material )` | `TrentService.Encrypt` with `KeyId`, `Plaintext` (base64), and `EncryptionContext: { "wp-secrets": "root-key-v1" }`. Returns `'kms1:' . CiphertextBlob`. | +| `unwrap( $wrapped )` | Rejects anything without the `kms1:` prefix with `WP_SECRETS_ERROR_KEY_UNAVAILABLE` and a message that says the root key was wrapped by a different keyring and how to move it (see deliverable 3). Otherwise `Decrypt` with `KeyId` pinned, the same `EncryptionContext`, and the blob. Verifies the result is exactly 32 bytes. | +| `get_key_source()` | `AWS KMS key in `. The key ID is an identifier, not key material. | + +Design points, each explained in the file: + +- **The encryption context is fixed, not per-site.** KMS authenticates it the way the cipher + authenticates AAD. Binding it to `home_url()` would make a domain change unrecoverable. There is + one root key per install, so there is nothing per-site to bind. +- **`KeyId` is pinned on `Decrypt`.** Without it, KMS decrypts with whichever key the blob names, + and a swapped blob under a key the IAM role can also use would succeed. +- **The `kms1:` prefix** turns the most likely adoption failure, a config-keyring blob, into a + specific, actionable error instead of an opaque `InvalidCiphertextException`. +- **Timeouts are short (3 s).** Every secret operation waits on this call. A KMS outage turns + every read into a `WP_Error`, which is the fail-closed behaviour we want. The README says so. +- **Install guard.** Install only when `WP_SECRETS_KMS_KEY_ID`, `WP_SECRETS_AWS_REGION`, + `WP_SECRETS_AWS_KEY`, and `WP_SECRETS_AWS_SECRET` are all non-empty, for the reason recorded in + the AWS Secrets Manager example's guard. An optional `WP_SECRETS_AWS_ENDPOINT` points the + keyring at an emulator. + +Out of scope: IAM role and instance-metadata credentials (named in the README as what to use in +production), KMS multi-region keys, and moving between two different KMS keys. KMS automatic key +rotation keeps the key ID and decrypts old blobs, so that case needs no re-wrap. + +### 2. Root-key caching in the key manager (a change to `src/`) + +Fix the call volume in the key manager, not in the example. Every remote keyring would otherwise +have to rediscover the problem and cache key material in its own way. + +- `WP_Secrets_Key_Manager` keeps the unwrapped root key for the rest of the request, keyed on the + wrapped value it came from, so a re-wrap or rotation replaces it. Memory only. It must never go + near the object cache. +- `rotate_site_key()` and `generate_root_key()` update the cached value. +- Tests: unwrap is called once across N `wp_get_secret()` calls (a counting mock keyring); a + rotation mid-request is not served the stale key; a `WP_Error` from `unwrap()` is not cached, so + a transient KMS failure is not pinned for the rest of the request. +- The existing `wp_secrets_memzero()` discipline around `$root_key` stays: callers receive a copy + and zero their copy. What changes is that one copy lives for the request, and the class docblock + has to say so plainly rather than let the memzero calls imply otherwise. +- Correct `examples/README.md`'s "once per request at most", which becomes true with this change. + +Because this is in `src/`, it lands in the Trac patch. It is the example doing what ADR 0008 says +it is for. + +### 3. `wp secret rotate --from=` (a change to `cli/`) + +Generalise the command, not the example. `cli/` is never copied into core, so this costs nothing +on the patch. + +- The new keyring becomes whatever keyring is active: the drop-in's if one is set, otherwise + `WP_Secrets_Config_Key_Provider( false )`. On a site with no drop-in, that is exactly today's + behaviour. +- `--from=config-previous` (the default) keeps today's behaviour and today's check that + `WP_SECRETS_KEY_PREVIOUS` is defined. +- `--from=config` unwraps with the current `WP_SECRETS_KEY`. This is the adoption case: the + site key has not changed, but a new keyring has been installed. +- Refuses if the old and new keyrings resolve to the same configuration, with a message rather + than a no-op success. +- The KMS README walks through adoption: install the drop-in, run + `wp secret rotate --from=config`, then check `wp secret health`. Between the first two steps + every secret read fails closed. The README says so and says to do both steps in one maintenance + window. + +### 4. `WP_Secrets_Keyring_Conformance` (in `tests/includes/`) + +The keyring equivalent of the provider conformance suite. It is an abstract test case with a +`keyring()` method to implement, and it checks what `implements` cannot: + +- `wrap()` of 32 random bytes returns a non-empty string, and `unwrap()` of that string returns + the same bytes. +- Two `wrap()` calls on the same bytes return different strings. `rotate_site_key()` depends on + this, and the interface docblock gains a sentence saying so. +- `unwrap()` of garbage, of a truncated value, and of a value with one flipped byte each returns + `WP_Error`. It never throws and never returns a string. +- `get_key_source()` returns a non-empty string. + +It runs in `make test` against `WP_Secrets_Config_Key_Provider` and `Mock_Keyring`, the same way +the provider suite runs against the shipped provider. + +### 5. An examples test harness + +This is shared with the Vault example. Building it here, since this example comes first, also +closes the gap where the AWS Secrets Manager README describes a conformance run that nothing +performs. + +- Tests live in `examples//tests/`. `phpunit-examples.xml.dist` bootstraps through the main + `tests/bootstrap.php` and loads the example's `secrets.php` class file without its install block, + since tests construct the class directly. +- A `make test-examples` target. It stays out of `make ci`, because it needs service containers + that `make ci`'s environments do not provide. +- A CI job, `examples`, with a Moto server (`motoserver/moto`, Apache-2.0) as a service container. + Moto emulates both KMS and Secrets Manager, including `AWSCURRENT`/`AWSPREVIOUS`. The image is + pinned by digest, in keeping with the pin-by-SHA rule in `ci.yml`. +- The AWS Secrets Manager example gains the same optional `WP_SECRETS_AWS_ENDPOINT` constant so it + can run against Moto, plus a test class that runs `WP_Secrets_Provider_Conformance` against it. +- KMS tests: the keyring conformance suite; a full `wp_set_secret()`/`wp_get_secret()` round trip + with the KMS keyring installed as the active keyring; a config-keyring blob gives the specific + error; and adoption via the rotate path in deliverable 3 leaves every secret readable. + +Live AWS is verified by hand once, the way the Secrets Manager example was, and the result goes +in the commit message. + +## Done when + +- Deliverables 1 to 5 are merged, and `make ci` and the `examples` CI job are green. +- A manual run against live KMS covers: a fresh site, adopting an existing site with + `rotate --from=config`, and a count of KMS calls for a request that reads ten secrets, which + should be one. +- `docs/journal/test-coverage-gaps.md` and `docs/journal/open-questions.md` record what changed, + and the providers-and-keyrings spec page's "As built" section covers root-key caching. diff --git a/examples/aws-kms-keyring/secrets.php b/examples/aws-kms-keyring/secrets.php new file mode 100644 index 0000000..e10034c --- /dev/null +++ b/examples/aws-kms-keyring/secrets.php @@ -0,0 +1,325 @@ + 'root-key-v1' ); + + /** + * Seconds. Every secret operation waits on this call, so a KMS outage + * that does not answer within it turns every read into a WP_Error rather + * than hanging the request -- fail closed, on purpose. The README repeats + * this. + */ + const TIMEOUT = 3; + + /** Root key material is always exactly this many bytes. */ + const KEY_LENGTH = 32; + + /** @var string */ + private $key_id; + + /** @var string */ + private $region; + + /** @var string */ + private $access_key; + + /** @var string */ + private $secret_key; + + /** + * Emulator endpoint, e.g. Moto's http://host.docker.internal:5051. Empty in + * production: real KMS is always reached at its regional host. + * + * @var string + */ + private $endpoint; + + /** + * @param string $key_id KMS key id or ARN. + * @param string $region AWS region, e.g. 'us-east-1'. + * @param string $access_key Access key id. + * @param string $secret_key Secret access key. + * @param string $endpoint Emulator endpoint override, e.g. Moto. Never set + * in production; leave empty to reach real AWS. + */ + public function __construct( $key_id, $region, $access_key, $secret_key, $endpoint = '' ) { + $this->key_id = $key_id; + $this->region = $region; + $this->access_key = $access_key; + $this->secret_key = $secret_key; + $this->endpoint = $endpoint; + } + + // -- the keyring contract ---------------------------------------------- + + /** + * @param string $key_material Raw key material to protect. + * + * @return string|WP_Error + */ + public function wrap( $key_material ) { + if ( ! is_string( $key_material ) || '' === $key_material ) { + return new WP_Error( WP_SECRETS_ERROR_INVALID_VALUE, 'AWS_KMS_Keyring: key material must be a non-empty string.' ); + } + + $response = $this->call( + 'Encrypt', + array( + 'KeyId' => $this->key_id, + 'Plaintext' => base64_encode( $key_material ), + 'EncryptionContext' => self::ENCRYPTION_CONTEXT, + ) + ); + + if ( is_wp_error( $response ) ) { + return $response; + } + + if ( empty( $response['CiphertextBlob'] ) ) { + return new WP_Error( WP_SECRETS_ERROR_KEY_UNAVAILABLE, 'AWS KMS: Encrypt response had no CiphertextBlob.' ); + } + + // The blob is already base64 in the JSON response; stored as is, + // behind the prefix that marks it as ours. + return self::PREFIX . $response['CiphertextBlob']; + } + + /** + * @param string $wrapped An opaque value previously returned by wrap(). + * + * @return string|WP_Error + */ + public function unwrap( $wrapped ) { + if ( ! is_string( $wrapped ) || '' === $wrapped || 0 !== strpos( $wrapped, self::PREFIX ) ) { + // The most likely adoption failure -- a root key wrapped by the + // config keyring -- turned into a specific, actionable error + // instead of an opaque InvalidCiphertextException from KMS. + return new WP_Error( + WP_SECRETS_ERROR_KEY_UNAVAILABLE, + 'The stored root key was not wrapped by AWS KMS (no kms1: prefix), so it was probably wrapped by the config keyring. Run `wp secret rotate --from=config` to move it onto this KMS key.' + ); + } + + $blob = substr( $wrapped, strlen( self::PREFIX ) ); + + $response = $this->call( + 'Decrypt', + array( + // Pinned on Decrypt: without it, KMS decrypts with whichever + // key the blob names, and a swapped blob under a key this + // IAM role can also use would otherwise succeed. + 'KeyId' => $this->key_id, + 'CiphertextBlob' => $blob, + 'EncryptionContext' => self::ENCRYPTION_CONTEXT, + ) + ); + + if ( is_wp_error( $response ) ) { + return $response; + } + + $plaintext = isset( $response['Plaintext'] ) ? base64_decode( $response['Plaintext'], true ) : false; + + if ( false === $plaintext || self::KEY_LENGTH !== strlen( $plaintext ) ) { + return new WP_Error( WP_SECRETS_ERROR_KEY_UNAVAILABLE, 'AWS KMS: Decrypt did not return exactly 32 bytes of key material.' ); + } + + return $plaintext; + } + + /** + * @return string + */ + public function get_key_source() { + return sprintf( 'AWS KMS key %s in %s', $this->key_id, $this->region ); + } + + // -- internals ----------------------------------------------------------- + + /** + * Signs and sends one KMS API call. + * + * AWS Signature Version 4, by hand, copied from the Secrets Manager + * example rather than shared: each example has to be one file a reviewer + * can read from top to bottom. + * + * @param string $target API action, e.g. 'Encrypt'. + * @param array $payload Request body. + * + * @return array|WP_Error Decoded response, or WP_Error. + */ + private function call( $target, array $payload ) { + $service = 'kms'; + $host = "kms.{$this->region}.amazonaws.com"; + $url = "https://{$host}/"; + $body = wp_json_encode( $payload ); + $amz_date = gmdate( 'Ymd\THis\Z' ); + $datestamp = gmdate( 'Ymd' ); + $amz_target = "TrentService.{$target}"; + + /* + * An emulator (Moto) is reached at its own host and port instead of the + * real regional endpoint. The signed "host" header has to match exactly + * what wp_remote_post() actually sends -- derived from the URL, the same + * way WP_Http itself would -- or the emulator's own signature check fails. + */ + if ( '' !== $this->endpoint ) { + $url = rtrim( $this->endpoint, '/' ) . '/'; + $parsed = wp_parse_url( $url ); + $signed_host = isset( $parsed['host'] ) ? $parsed['host'] : $host; + + if ( isset( $parsed['port'] ) ) { + $signed_host .= ':' . $parsed['port']; + } + } else { + $signed_host = $host; + } + + $canonical_headers = "content-type:application/x-amz-json-1.1\n" + . "host:{$signed_host}\n" + . "x-amz-date:{$amz_date}\n" + . "x-amz-target:{$amz_target}\n"; + $signed_headers = 'content-type;host;x-amz-date;x-amz-target'; + + $canonical_request = "POST\n/\n\n{$canonical_headers}\n{$signed_headers}\n" . hash( 'sha256', $body ); + + $scope = "{$datestamp}/{$this->region}/{$service}/aws4_request"; + $string_to_sign = "AWS4-HMAC-SHA256\n{$amz_date}\n{$scope}\n" . hash( 'sha256', $canonical_request ); + + $k_date = hash_hmac( 'sha256', $datestamp, 'AWS4' . $this->secret_key, true ); + $k_region = hash_hmac( 'sha256', $this->region, $k_date, true ); + $k_service = hash_hmac( 'sha256', $service, $k_region, true ); + $k_signing = hash_hmac( 'sha256', 'aws4_request', $k_service, true ); + $signature = hash_hmac( 'sha256', $string_to_sign, $k_signing ); + + $response = wp_remote_post( + $url, + array( + // Fail closed: a stuck KMS call must not hang the request + // that is waiting on the root key. + 'timeout' => self::TIMEOUT, + 'headers' => array( + 'Content-Type' => 'application/x-amz-json-1.1', + 'X-Amz-Date' => $amz_date, + 'X-Amz-Target' => $amz_target, + 'Authorization' => "AWS4-HMAC-SHA256 Credential={$this->access_key}/{$scope}, " + . "SignedHeaders={$signed_headers}, Signature={$signature}", + ), + 'body' => $body, + ) + ); + + if ( is_wp_error( $response ) ) { + return new WP_Error( + WP_SECRETS_ERROR_KEY_UNAVAILABLE, + sprintf( 'AWS KMS unreachable: %s', $response->get_error_message() ) + ); + } + + $code = wp_remote_retrieve_response_code( $response ); + $parsed = json_decode( wp_remote_retrieve_body( $response ), true ); + + if ( 200 === $code ) { + return is_array( $parsed ) ? $parsed : array(); + } + + $aws_error = isset( $parsed['__type'] ) ? $parsed['__type'] : ''; + + /* + * AWS's JSON protocol is inconsistent about the case of this key, and + * reading only one spelling turns a precise error into a bare + * exception name. The request/response body is never echoed here -- + * only the __type and message fields, never the raw body, which could + * echo a plaintext on a malformed-request response. + */ + $detail = ''; + + foreach ( array( 'message', 'Message' ) as $key ) { + if ( ! empty( $parsed[ $key ] ) ) { + $detail = $parsed[ $key ]; + break; + } + } + + return new WP_Error( + WP_SECRETS_ERROR_KEY_UNAVAILABLE, + sprintf( 'AWS KMS error (HTTP %d): %s -- %s', $code, $aws_error, $detail ) + ); + } +} + +/* + * Install it, but only with all four settings actually filled in. + * + * Checked for emptiness rather than just defined(): a config file with the + * keys present but blank -- the state a freshly-copied override file is in -- + * would otherwise install a keyring that fails every single call. Falling + * back to WordPress's own keyring means an unpopulated config is just a + * normal site. + */ +if ( defined( 'WP_SECRETS_KMS_KEY_ID' ) && defined( 'WP_SECRETS_AWS_REGION' ) + && defined( 'WP_SECRETS_AWS_KEY' ) && defined( 'WP_SECRETS_AWS_SECRET' ) + && '' !== trim( (string) WP_SECRETS_KMS_KEY_ID ) + && '' !== trim( (string) WP_SECRETS_AWS_REGION ) + && '' !== trim( (string) WP_SECRETS_AWS_KEY ) + && '' !== trim( (string) WP_SECRETS_AWS_SECRET ) +) { + $GLOBALS['wp_secrets_keyring'] = new AWS_KMS_Keyring( + WP_SECRETS_KMS_KEY_ID, + WP_SECRETS_AWS_REGION, + WP_SECRETS_AWS_KEY, + WP_SECRETS_AWS_SECRET, + // For an emulator such as Moto during development. Never set in production. + defined( 'WP_SECRETS_AWS_ENDPOINT' ) ? (string) WP_SECRETS_AWS_ENDPOINT : '' + ); +} diff --git a/examples/aws-kms-keyring/tests/class-moto-kms-fixture.php b/examples/aws-kms-keyring/tests/class-moto-kms-fixture.php new file mode 100644 index 0000000..8bda17e --- /dev/null +++ b/examples/aws-kms-keyring/tests/class-moto-kms-fixture.php @@ -0,0 +1,147 @@ + 'wp-secrets examples test key' ) + ); + + if ( is_wp_error( $response ) || empty( $response['KeyMetadata']['KeyId'] ) ) { + $message = is_wp_error( $response ) ? $response->get_error_message() : 'no KeyMetadata.KeyId in the response'; + + // The body of a CreateKey call/response contains no secret, so it + // is safe to fail the test with it. + self::fail_test( sprintf( 'Moto_KMS_Fixture::create_key() failed: %s', $message ) ); + } + + return $response['KeyMetadata']['KeyId']; + } + + /** + * Fails the currently-running test with a message. Kept as a tiny wrapper + * so create_key() reads as "do the call, or fail the test", without a + * PHPUnit dependency spread through the rest of the class. + * + * @param string $message Failure message. + * + * @return never + */ + private static function fail_test( $message ) { + PHPUnit\Framework\Assert::fail( $message ); + } + + /** + * Signs and sends one KMS API call against Moto. Copied from + * AWS_KMS_Keyring::call(), fixed to the 'testing'/'testing' credentials + * Moto accepts for any request. + * + * @param string $target API action, e.g. 'CreateKey'. + * @param array $payload Request body. + * + * @return array|WP_Error Decoded response, or WP_Error. + */ + private static function call( $target, array $payload ) { + $access_key = 'testing'; + $secret_key = 'testing'; + $region = self::region(); + $service = 'kms'; + $url = rtrim( self::endpoint(), '/' ) . '/'; + $parsed = wp_parse_url( $url ); + $host = isset( $parsed['host'] ) ? $parsed['host'] : "kms.{$region}.amazonaws.com"; + + if ( isset( $parsed['port'] ) ) { + $host .= ':' . $parsed['port']; + } + + $body = wp_json_encode( $payload ); + $amz_date = gmdate( 'Ymd\THis\Z' ); + $datestamp = gmdate( 'Ymd' ); + $amz_target = "TrentService.{$target}"; + + $canonical_headers = "content-type:application/x-amz-json-1.1\n" + . "host:{$host}\n" + . "x-amz-date:{$amz_date}\n" + . "x-amz-target:{$amz_target}\n"; + $signed_headers = 'content-type;host;x-amz-date;x-amz-target'; + + $canonical_request = "POST\n/\n\n{$canonical_headers}\n{$signed_headers}\n" . hash( 'sha256', $body ); + + $scope = "{$datestamp}/{$region}/{$service}/aws4_request"; + $string_to_sign = "AWS4-HMAC-SHA256\n{$amz_date}\n{$scope}\n" . hash( 'sha256', $canonical_request ); + + $k_date = hash_hmac( 'sha256', $datestamp, 'AWS4' . $secret_key, true ); + $k_region = hash_hmac( 'sha256', $region, $k_date, true ); + $k_service = hash_hmac( 'sha256', $service, $k_region, true ); + $k_signing = hash_hmac( 'sha256', 'aws4_request', $k_service, true ); + $signature = hash_hmac( 'sha256', $string_to_sign, $k_signing ); + + $response = wp_remote_post( + $url, + array( + 'timeout' => 10, + 'headers' => array( + 'Content-Type' => 'application/x-amz-json-1.1', + 'X-Amz-Date' => $amz_date, + 'X-Amz-Target' => $amz_target, + 'Authorization' => "AWS4-HMAC-SHA256 Credential={$access_key}/{$scope}, " + . "SignedHeaders={$signed_headers}, Signature={$signature}", + ), + 'body' => $body, + ) + ); + + if ( is_wp_error( $response ) ) { + return new WP_Error( 'moto_kms_fixture_unreachable', $response->get_error_message() ); + } + + $code = wp_remote_retrieve_response_code( $response ); + $parsed = json_decode( wp_remote_retrieve_body( $response ), true ); + + if ( 200 === $code ) { + return is_array( $parsed ) ? $parsed : array(); + } + + return new WP_Error( + 'moto_kms_fixture_error', + sprintf( 'Moto KMS error (HTTP %d): %s', $code, wp_remote_retrieve_body( $response ) ) + ); + } +} diff --git a/examples/aws-kms-keyring/tests/test-aws-kms-keyring-conformance.php b/examples/aws-kms-keyring/tests/test-aws-kms-keyring-conformance.php new file mode 100644 index 0000000..705c6f3 --- /dev/null +++ b/examples/aws-kms-keyring/tests/test-aws-kms-keyring-conformance.php @@ -0,0 +1,47 @@ +assertArrayNotHasKey( 'wp_secrets_keyring', $GLOBALS ); + } +} diff --git a/examples/aws-kms-keyring/tests/test-aws-kms-keyring.php b/examples/aws-kms-keyring/tests/test-aws-kms-keyring.php new file mode 100644 index 0000000..cb7dc74 --- /dev/null +++ b/examples/aws-kms-keyring/tests/test-aws-kms-keyring.php @@ -0,0 +1,264 @@ +decrypt_calls = 0; + } + + public function tear_down() { + remove_action( 'http_api_debug', array( $this, 'count_decrypt_calls' ) ); + + parent::tear_down(); + } + + /** + * @param mixed $response Response or WP_Error. + * @param string $context Always 'response'. + * @param string $class HTTP transport class used. + * @param array $parsed_args Request args, including headers. + * @param string $url Request URL. + */ + public function count_decrypt_calls( $response, $context, $class, $parsed_args, $url ) { + unset( $response, $context, $class, $url ); + + if ( isset( $parsed_args['headers']['X-Amz-Target'] ) && 'TrentService.Decrypt' === $parsed_args['headers']['X-Amz-Target'] ) { + ++$this->decrypt_calls; + } + } + + /** + * @return AWS_KMS_Keyring + */ + private function keyring() { + return new AWS_KMS_Keyring( + self::$key_id, + Moto_KMS_Fixture::region(), + 'testing', + 'testing', + Moto_KMS_Fixture::endpoint() + ); + } + + /** + * Wraps 32 fresh random bytes under $keyring and stores the result as the + * site's root key, the way a site that has always used this keyring would + * already have one. + * + * @param WP_Secrets_Keyring $keyring + * + * @return string The 32 raw bytes that were wrapped. + */ + private function seed_root_key( WP_Secrets_Keyring $keyring ) { + $root = random_bytes( 32 ); + + update_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION, $keyring->wrap( $root ) ); + + return $root; + } + + /** + * A hand-built provider that never touches the static getters in + * secrets.php, so a test can write secrets before installing the KMS + * keyring without priming _wp_secrets_get_provider()'s cache to the + * pre-installation state. + * + * @return WP_Secrets_Libsodium_Provider + */ + private function provider_under_config_keyring() { + return new WP_Secrets_Libsodium_Provider( + new WP_Secrets_Option_Store(), + new WP_Secrets_Key_Manager( new WP_Secrets_Config_Key_Provider() ) + ); + } + + /** + * Collects every string WP_CLI recorded, across every kind of output the + * mock tracks, so a canary-leak assertion has one place to check. + * + * @return string + */ + private function all_wp_cli_output() { + return implode( + "\n", + array_merge( + WP_CLI::$log, + WP_CLI::$success, + WP_CLI::$warning, + WP_CLI::$errors, + array( wp_json_encode( WP_CLI::$formatted_items ) ) + ) + ); + } + + // -- install guard, wrap/unwrap contract, failure modes ------------------ + + public function test_loading_the_example_does_not_install_a_keyring_without_the_constants() { + $this->assertArrayNotHasKey( 'wp_secrets_keyring', $GLOBALS ); + } + + public function test_wrapped_values_carry_the_kms1_prefix_and_never_the_key_material() { + $material = random_bytes( 32 ); + $wrapped = $this->keyring()->wrap( $material ); + + $this->assertIsString( $wrapped ); + $this->assertStringStartsWith( 'kms1:', $wrapped ); + $this->assertStringNotContainsString( $material, $wrapped ); + $this->assertStringNotContainsString( base64_encode( $material ), $wrapped ); + } + + public function test_a_config_keyring_blob_is_refused_with_an_adoption_message() { + $config_wrapped = ( new WP_Secrets_Config_Key_Provider() )->wrap( random_bytes( 32 ) ); + + $result = $this->keyring()->unwrap( $config_wrapped ); + + $this->assertWPError( $result ); + $this->assertSame( WP_SECRETS_ERROR_KEY_UNAVAILABLE, $result->get_error_code() ); + $this->assertStringContainsString( 'rotate --from=config', $result->get_error_message() ); + } + + public function test_an_unreachable_kms_fails_closed_with_a_wp_error() { + $keyring = new AWS_KMS_Keyring( self::$key_id, Moto_KMS_Fixture::region(), 'testing', 'testing', 'http://127.0.0.1:9' ); + + $wrap_result = $keyring->wrap( random_bytes( 32 ) ); + $this->assertWPError( $wrap_result ); + $this->assertSame( WP_SECRETS_ERROR_KEY_UNAVAILABLE, $wrap_result->get_error_code() ); + + $unwrap_result = $keyring->unwrap( 'kms1:AAAA' ); + $this->assertWPError( $unwrap_result ); + $this->assertSame( WP_SECRETS_ERROR_KEY_UNAVAILABLE, $unwrap_result->get_error_code() ); + } + + public function test_get_key_source_names_the_key_and_region_but_not_the_credentials() { + $source = $this->keyring()->get_key_source(); + + $this->assertStringContainsString( self::$key_id, $source ); + $this->assertStringContainsString( 'us-east-1', $source ); + $this->assertStringNotContainsString( 'testing', $source ); + } + + // -- end-to-end, isolated-process tests ----------------------------------- + + /** + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function test_a_full_secret_round_trip_with_the_kms_keyring_active() { + $GLOBALS['wp_secrets_keyring'] = $this->keyring(); + + $set_result = wp_set_secret( 'kms/canary', 'UNIQUE-KMS-CANARY-4b1e' ); + $this->assertTrue( $set_result ); + + $secret = wp_get_secret( 'kms/canary' ); + $this->assertInstanceOf( WP_Secret::class, $secret ); + $this->assertSame( 'UNIQUE-KMS-CANARY-4b1e', $secret->reveal() ); + + $stored = get_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION ); + $this->assertStringStartsWith( 'kms1:', $stored ); + + $this->assertStringContainsString( 'AWS KMS key', wp_secrets_provider_label() ); + } + + /** + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function test_ten_secret_reads_make_one_kms_decrypt_call() { + $keyring = $this->keyring(); + $this->seed_root_key( $keyring ); + + $GLOBALS['wp_secrets_keyring'] = $keyring; + + add_action( 'http_api_debug', array( $this, 'count_decrypt_calls' ), 10, 5 ); + + $this->assertTrue( wp_set_secret( 'kms/ten-reads', 'UNIQUE-KMS-CANARY-4b1e' ) ); + + for ( $i = 0; $i < 10; $i++ ) { + $secret = wp_get_secret( 'kms/ten-reads' ); + $this->assertInstanceOf( WP_Secret::class, $secret ); + $this->assertSame( 'UNIQUE-KMS-CANARY-4b1e', $secret->reveal() ); + } + + $this->assertSame( 1, $this->decrypt_calls ); + } + + /** + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function test_adopting_an_existing_site_with_rotate_from_config_keeps_every_secret_readable() { + // Root key starts life wrapped by the config keyring, as an existing + // site's would. + define( 'WP_SECRETS_KEY', base64_encode( str_repeat( 'C', 32 ) ) ); + + $config_provider = $this->provider_under_config_keyring(); + + $names = array( 'kms/one', 'kms/two', 'kms/three' ); + + foreach ( $names as $name ) { + $this->assertTrue( $config_provider->set( $name, 'UNIQUE-KMS-CANARY-4b1e', false ) ); + } + + // Now the drop-in installs the KMS keyring. + $GLOBALS['wp_secrets_keyring'] = $this->keyring(); + + $blocked = wp_get_secret( $names[0] ); + $this->assertWPError( $blocked ); + $this->assertStringContainsString( 'rotate --from=config', $blocked->get_error_message() ); + + WP_CLI::reset(); + ( new WP_CLI_Secret_Command() )->rotate( array(), array( 'from' => 'config', 'yes' => true ) ); + + foreach ( $names as $name ) { + $secret = wp_get_secret( $name ); + $this->assertInstanceOf( WP_Secret::class, $secret ); + $this->assertSame( 'UNIQUE-KMS-CANARY-4b1e', $secret->reveal() ); + } + + $stored = get_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION ); + $this->assertStringStartsWith( 'kms1:', $stored ); + + ( new WP_CLI_Secret_Command() )->health( array(), array() ); + + foreach ( WP_CLI::$formatted_items as $formatted ) { + foreach ( $formatted['items'] as $item ) { + $this->assertNotSame( 'critical', $item['status'] ); + } + } + + $this->assertStringNotContainsString( 'UNIQUE-KMS-CANARY-4b1e', $this->all_wp_cli_output() ); + } +} diff --git a/examples/aws-secrets-manager/README.md b/examples/aws-secrets-manager/README.md index 31020d1..f49ff44 100644 --- a/examples/aws-secrets-manager/README.md +++ b/examples/aws-secrets-manager/README.md @@ -123,4 +123,31 @@ class Tests_AWS_Secrets_Manager_Provider extends WP_Secrets_Provider_Conformance That checks the properties `implements WP_Secrets_Provider` cannot: absence reported as `null` rather than an error, deleting something absent succeeding, fingerprints stable for the same value, and listings never containing a plaintext. It makes real API calls, so point it at a -throwaway AWS account. +throwaway AWS account. The repository itself now runs this class against Moto, an AWS emulator, in +`make test-examples` — see the next section. + +## Run it against an emulator + +`examples/aws-secrets-manager/tests/test-aws-secrets-manager-conformance.php` runs the conformance +suite above against [Moto](https://github.com/getmoto/moto) instead of real AWS, so it can run +without credentials or cost. Start it: + +```sh +docker pull motoserver/moto:latest +docker run -d --name secrets-api-moto-kms -p 5051:5000 motoserver/moto:latest +curl -sf http://localhost:5051/moto-api/ # 200 once it is up +``` + +The fourth constructor argument, `$endpoint`, points the provider at Moto instead of real AWS — +this is what `WP_SECRETS_AWS_ENDPOINT` sets when defined, and it is never set in production. +`phpunit-examples.xml.dist` already points `WP_SECRETS_TEST_AWS_ENDPOINT` at +`http://host.docker.internal:5051`, which is where the tests-cli container reaches a Moto +container published on the host. Then, run from the repository root so `$(basename "$PWD")` +resolves to the plugin's directory name: + +```sh +npx @wordpress/env run --env-cwd="wp-content/plugins/$(basename "$PWD")" tests-cli vendor/bin/phpunit -c phpunit-examples.xml.dist +``` + +or, without wp-env, `make test-examples`. Not part of `make ci`: it needs Moto running, and the +separate examples CI job runs it against a pinned Moto service container. diff --git a/examples/aws-secrets-manager/secrets.php b/examples/aws-secrets-manager/secrets.php index 4993fec..2c69998 100644 --- a/examples/aws-secrets-manager/secrets.php +++ b/examples/aws-secrets-manager/secrets.php @@ -51,6 +51,14 @@ final class AWS_Secrets_Manager_Provider implements WP_Secrets_Provider { /** @var string */ private $secret_key; + /** + * Emulator endpoint, e.g. Moto's http://host.docker.internal:5051. Empty in + * production: real Secrets Manager is always reached at its regional host. + * + * @var string + */ + private $endpoint; + /** * Request-scoped only. Never the persistent object cache: WP_Secret * deliberately cannot round-trip a plaintext through wp_cache_set(), and @@ -64,11 +72,14 @@ final class AWS_Secrets_Manager_Provider implements WP_Secrets_Provider { * @param string $region AWS region, e.g. 'us-east-1'. * @param string $access_key Access key id. * @param string $secret_key Secret access key. + * @param string $endpoint Emulator endpoint override, e.g. Moto. Never set + * in production; leave empty to reach real AWS. */ - public function __construct( $region, $access_key, $secret_key ) { + public function __construct( $region, $access_key, $secret_key, $endpoint = '' ) { $this->region = $region; $this->access_key = $access_key; $this->secret_key = $secret_key; + $this->endpoint = $endpoint; } // -- the provider contract ------------------------------------------------- @@ -364,13 +375,32 @@ private function wp_name( $aws_name, $network ) { private function call( $target, array $payload ) { $service = 'secretsmanager'; $host = "secretsmanager.{$this->region}.amazonaws.com"; + $url = "https://{$host}/"; $body = wp_json_encode( $payload ); $amz_date = gmdate( 'Ymd\THis\Z' ); $datestamp = gmdate( 'Ymd' ); $amz_target = "secretsmanager.{$target}"; + /* + * An emulator (Moto) is reached at its own host and port instead of the + * real regional endpoint. The signed "host" header has to match exactly + * what wp_remote_post() actually sends -- derived from the URL, the same + * way WP_Http itself would -- or the emulator's own signature check fails. + */ + if ( '' !== $this->endpoint ) { + $url = rtrim( $this->endpoint, '/' ) . '/'; + $parsed = wp_parse_url( $url ); + $signed_host = isset( $parsed['host'] ) ? $parsed['host'] : $host; + + if ( isset( $parsed['port'] ) ) { + $signed_host .= ':' . $parsed['port']; + } + } else { + $signed_host = $host; + } + $canonical_headers = "content-type:application/x-amz-json-1.1\n" - . "host:{$host}\n" + . "host:{$signed_host}\n" . "x-amz-date:{$amz_date}\n" . "x-amz-target:{$amz_target}\n"; $signed_headers = 'content-type;host;x-amz-date;x-amz-target'; @@ -387,7 +417,7 @@ private function call( $target, array $payload ) { $signature = hash_hmac( 'sha256', $string_to_sign, $k_signing ); $response = wp_remote_post( - "https://{$host}/", + $url, array( 'timeout' => 10, 'headers' => array( @@ -463,6 +493,8 @@ private function call( $target, array $payload ) { $GLOBALS['wp_secrets_provider'] = new AWS_Secrets_Manager_Provider( WP_SECRETS_AWS_REGION, WP_SECRETS_AWS_KEY, - WP_SECRETS_AWS_SECRET + WP_SECRETS_AWS_SECRET, + // For an emulator such as Moto during development. Never set in production. + defined( 'WP_SECRETS_AWS_ENDPOINT' ) ? (string) WP_SECRETS_AWS_ENDPOINT : '' ); } diff --git a/examples/aws-secrets-manager/tests/test-aws-secrets-manager-conformance.php b/examples/aws-secrets-manager/tests/test-aws-secrets-manager-conformance.php new file mode 100644 index 0000000..a05248e --- /dev/null +++ b/examples/aws-secrets-manager/tests/test-aws-secrets-manager-conformance.php @@ -0,0 +1,62 @@ +subject = 'conformance/s' . substr( md5( uniqid( '', true ) ), 0, 8 ); + } + + public function tear_down() { + $provider = $this->provider(); + + foreach ( array( $this->subject, 'conformance-a/one', 'conformance-b/two' ) as $name ) { + $provider->delete( $name ); + } + + parent::tear_down(); + } + + protected function provider() { + return new AWS_Secrets_Manager_Provider( 'us-east-1', 'testing', 'testing', $this->endpoint() ); + } + + protected function conformance_name() { + return $this->subject; + } + + /** + * @return string + */ + private function endpoint() { + $endpoint = getenv( 'WP_SECRETS_TEST_AWS_ENDPOINT' ); + + return false !== $endpoint && '' !== $endpoint ? $endpoint : 'http://host.docker.internal:5051'; + } + + /** + * The install block at the bottom of secrets.php is guarded on wp-config.php + * constants that tests/bootstrap-examples.php never defines, so requiring the + * file to get the class declaration must not also install a provider. + */ + public function test_loading_the_example_does_not_install_a_provider_without_the_constants() { + $this->assertArrayNotHasKey( 'wp_secrets_provider', $GLOBALS ); + } +} diff --git a/examples/vault-provider/SPEC.md b/examples/vault-provider/SPEC.md new file mode 100644 index 0000000..3939ff4 --- /dev/null +++ b/examples/vault-provider/SPEC.md @@ -0,0 +1,138 @@ +# Spec: HashiCorp Vault provider example + +Status: planned. Part of the pre-Trac work in +[ADR 0008](../../docs/decisions/0008-the-trac-ticket-replaces-thread-confirmation.md). Builds on +the examples test harness in [the KMS keyring spec](../aws-kms-keyring/SPEC.md#5-an-examples-test-harness), +so it comes second. + +## Why this example + +The AWS Secrets Manager example showed that the two-slot version model maps onto a backend +already built around two slots (`AWSCURRENT`/`AWSPREVIOUS`). That is agreement from a backend +that was going to agree. Vault's KV v2 engine numbers versions 1, 2, 3 and so on, keeps up to +`max_versions` of them, and can soft-delete or destroy any one of them. It is the first backend +where `WP_Secret_Version::CURRENT`/`PREVIOUS` is a translation rather than a match, which makes +it the test of the design most likely to be wrong in a way nobody has pointed out. + +It is also the first provider whose backend is self-hostable, so CI can run it against the real +server rather than an emulator. + +## The questions it has to answer + +1. **What is "previous" when the backend keeps ten versions?** It has to be exactly version N-1, + never "the newest surviving version below N". Otherwise retiring N-1 would promote N-2, and + `wp_retire_secret_version()`, meant to make a compromised credential unreachable, would bring + back an even older one. +2. **What happens to the versions the API cannot see?** KV v2 keeps up to 10 by default. The + shipped provider throws away the old previous value on every write, but Vault would keep N-2 + and older, still readable by anyone with a Vault token. The answer this spec adopts is to set + `max_versions: 2` on every secret the provider creates, which makes Vault a two-slot store. If + that turns out to be wrong in practice, it is a finding about the version model and goes on the + Trac ticket. +3. **Where does `needs_rotation` live** on a backend with no field for it? +4. **What does `list_secrets()` cost** on a remote backend, given the shape the interface requires? + +## Deliverable 1: `examples/vault-provider/secrets.php` + +A single-file drop-in: no Composer, no SDK. Vault's HTTP API needs only a token header, so this +is simpler than the AWS examples. + +`final class Vault_KV2_Provider implements WP_Secrets_Provider`, configured by +`WP_SECRETS_VAULT_ADDR`, `WP_SECRETS_VAULT_TOKEN`, `WP_SECRETS_VAULT_MOUNT` (default `secret`), +and an optional `WP_SECRETS_VAULT_NAMESPACE`, which is sent as `X-Vault-Namespace` for Vault +Enterprise and HCP. It is installed only when the address and token are non-empty, for the same +reason as the other examples. + +### Path mapping + +| Scope | Vault path under the mount | +|---|---| +| Site | `wp/site///` | +| Network | `wp/network//` | + +Site scope includes the blog ID because the shipped provider's site scope is per site: the option +store writes through `get_option()`, which reads the current blog's table. See also deliverable 3. +Names are `namespace/key` with one slash, and both segments match `[a-z0-9_-]`, so they map to +Vault paths unchanged. + +### Method mapping + +| Method | Vault calls | Notes | +|---|---|---| +| `get( CURRENT )` | `GET data/` | 404, or a current version that is soft-deleted or destroyed, is `null`. Value is `data.data.value`. | +| `get( PREVIOUS )` | `GET metadata/`, then `GET data/?version=N-1` | Strictly N-1. If N is 1, or N-1 is deleted or destroyed, the result is `null`, never an older version. | +| `set()` | on create: `POST metadata/` with `max_versions: 2`; then `POST data/` with `{ "data": { "value": … } }`; then `custom_metadata` if the flag changed | Created versus updated comes from whether metadata existed. Fires `wp_secret_changed` as the interface requires. | +| `delete()` | `DELETE metadata/` | Removes every version for good. Vault answers 204 whether or not the secret existed, which already matches "absent is success". | +| `retire_previous()` | `GET metadata/`, then `POST destroy/` with `{ "versions": [N-1] }` | Destroy, not soft delete: a soft-deleted version can be undeleted, and retire means gone. No previous version is a successful no-op. | +| `list_secrets()` | `LIST metadata/wp//` for namespaces, then `LIST` each one, then `GET metadata` per secret | `created` is `created_time`, `has_previous` applies the same N-1 rule as `get`, and `needs_rotation` comes from `custom_metadata`. The fingerprint is `''`, as in the AWS example. | +| `get_label()` | none | `HashiCorp Vault (, mount )` | +| `get_protection_boundary()` | none | `BOUNDARY_PROVIDER` | +| `is_writable()` | none | `true`. A token without write policy surfaces as a `WP_Error` on `set()`. It is not detected in advance, and the README says so. | + +**`needs_rotation`** is stored as `custom_metadata.needs_rotation = "1"`, which needs Vault 1.9 or +later. The flag is per secret rather than per version, so every `set()` writes it, and a set without +the flag clears it. If the flag write fails after the value write succeeded, and the caller asked +for the flag, `set()` returns `WP_Error`, because the interface says a provider "must not report it +as honored". If the caller did not ask for the flag, a failed clear is logged and ignored. The data +write and the metadata write are two requests, not a transaction, and the file says so. + +**Errors.** Vault's `errors[]` array goes into the `WP_Error` message, following the lesson in the +AWS example. A 403 and a sealed Vault (503) both become `WP_SECRETS_ERROR_STORE_UNAVAILABLE`, so +they read as unreachable, not absent. + +**Caching** is request-scoped only, the same rule and the same reasoning as the AWS example. + +**Fingerprints** still derive from the site master key, so a site whose values live entirely in +Vault still needs a working keyring and root key. That is inherited from the AWS example. It stays +as-is and is written down as a question: a provider reporting `BOUNDARY_PROVIDER` still depends on +local key material for one feature. + +## Deliverable 2: tests + +In `examples/vault-provider/tests/`, run by `make test-examples`: + +- `WP_Secrets_Provider_Conformance` against a real Vault dev server. +- Provider-specific tests: + - **Retiring does not resurrect.** Write v1, v2, v3, retire, then `PREVIOUS` is `null`. Write + v4, and `PREVIOUS` is v3. + - **Only two versions are kept.** After three writes, version 1 is gone from Vault itself, read + directly rather than through the provider. + - **`needs_rotation` round-trips,** set, cleared, and shown in `list_secrets()`. + - **Site scope is isolated per blog** on multisite. Run this under the multisite config. + - **A sealed or unreachable Vault reads as `WP_Error`,** never `null`. + +The CI `examples` job gains a Vault service container in dev mode (`hashicorp/vault`, pinned by +digest, root token passed through `VAULT_DEV_ROOT_TOKEN_ID`), where KV v2 is mounted at `secret/` +by default. Vault has been under the BSL since 1.15. The README notes that OpenBao implements the +same KV v2 API. CI tests Vault, the name hosts will search for, and one manual OpenBao run is +recorded in the commit message. + +## Deliverable 3: fix site-scope naming in the AWS Secrets Manager example + +`AWS_Secrets_Manager_Provider::aws_name()` maps site scope to `wp/` with no blog ID, so on +multisite every site reads and writes the same AWS secret for a given name. The shipped provider +keeps site scope per site, so the example is wrong, not the interface. Map site scope to +`wp/site//`, as Vault does. The README needs a note: existing single-site +deployments of the example move from `wp/` to `wp/site/1/`, so this is a rename on +AWS's side. Before 1.0 the example gets no compatibility read. + +This found its way into this spec because working out Vault's paths is what turned it up. It is a +separate commit. + +## Out of scope + +- Vault auth methods other than a static token. AppRole and Kubernetes auth are named in the + README as the production path. +- KV v1 and the dynamic-secret engines. Dynamic database credentials do not fit a stored-secret + API, and trying to make them fit is how an example turns into a product. +- Check-and-set (`cas`) on writes. Worth a sentence in the README as the answer to concurrent + writers, but not implemented. + +## Done when + +- Deliverables 1 to 3 are merged, and `make ci` and the `examples` CI job are green on single site + and multisite. +- Each of the four questions above has a written answer in the example's README. Any answer that + points at the interface rather than the example is added to the open questions and to the Trac + ticket description. +- `proposal-questions.md` question 2, on whether two slots are adequate, records what Vault showed. diff --git a/phpunit-examples.xml.dist b/phpunit-examples.xml.dist new file mode 100644 index 0000000..bbeb950 --- /dev/null +++ b/phpunit-examples.xml.dist @@ -0,0 +1,24 @@ + + + + + examples/*/tests + + + + + + + + diff --git a/src/wp-includes/class-wp-secrets-key-manager.php b/src/wp-includes/class-wp-secrets-key-manager.php index 6ac81d7..2528a83 100644 --- a/src/wp-includes/class-wp-secrets-key-manager.php +++ b/src/wp-includes/class-wp-secrets-key-manager.php @@ -26,6 +26,13 @@ * differs from the site path, so there is no collision between the two. Identical * on every blog, so a network secret written on one blog reads on every other. * + * One unwrapped copy of the root key lives in this object for the rest of the + * request, in memory only, never in the object cache. It is replaced whenever the + * stored wrapped value changes -- a rotation, a re-wrap, a restore -- so it is never + * stale. Callers of get_root_key() still receive a copy and must zero it themselves; + * this object's own copy is not theirs to zero. The practical effect: a remote + * keyring (a KMS or HSM call) is invoked once per request, not once per secret. + * * @since 7.2.0 */ final class WP_Secrets_Key_Manager { @@ -76,6 +83,26 @@ final class WP_Secrets_Key_Manager { */ private $keyring; + /** + * The unwrapped root key currently cached for this request, or null if nothing + * has been unwrapped yet. + * + * @since 7.2.0 + * @var string|null + */ + private $cached_root_key = null; + + /** + * The wrapped value $cached_root_key was unwrapped from, or null if nothing has + * been unwrapped yet. Used to detect that the stored wrapped value changed + * underneath this object (a rotation, a re-wrap, a restore) so the cache is not + * served stale. + * + * @since 7.2.0 + * @var string|null + */ + private $cached_wrapped = null; + /** * Constructor. * @@ -186,7 +213,18 @@ public function get_root_key() { ); } - return $this->keyring->unwrap( $wrapped ); + if ( null !== $this->cached_wrapped && $wrapped === $this->cached_wrapped ) { + return $this->cached_root_key; + } + + $root_key = $this->keyring->unwrap( $wrapped ); + + if ( is_string( $root_key ) ) { + $this->cached_wrapped = $wrapped; + $this->cached_root_key = $root_key; + } + + return $root_key; } /** @@ -213,7 +251,11 @@ public function rotate_site_key( WP_Secrets_Keyring $old_keyring, WP_Secrets_Key ); } - $root_key = $old_keyring->unwrap( $wrapped ); + if ( $old_keyring === $this->keyring && null !== $this->cached_wrapped && $wrapped === $this->cached_wrapped ) { + $root_key = $this->cached_root_key; + } else { + $root_key = $old_keyring->unwrap( $wrapped ); + } if ( is_wp_error( $root_key ) ) { return $root_key; @@ -221,9 +263,9 @@ public function rotate_site_key( WP_Secrets_Keyring $old_keyring, WP_Secrets_Key $rewrapped = $new_keyring->wrap( $root_key ); - wp_secrets_memzero( $root_key ); - if ( is_wp_error( $rewrapped ) ) { + wp_secrets_memzero( $root_key ); + return $rewrapped; } @@ -236,12 +278,19 @@ public function rotate_site_key( WP_Secrets_Keyring $old_keyring, WP_Secrets_Key * astronomically unlikely. */ if ( ! update_site_option( self::ROOT_KEY_OPTION, $rewrapped ) ) { + wp_secrets_memzero( $root_key ); + return new WP_Error( WP_SECRETS_ERROR_STORE_UNAVAILABLE, __( 'Could not store the re-wrapped root key.', 'default' ) ); } + $this->cached_wrapped = $rewrapped; + $this->cached_root_key = $root_key; + + wp_secrets_memzero( $root_key ); + return true; } @@ -266,6 +315,9 @@ private function generate_root_key() { } if ( add_site_option( self::ROOT_KEY_OPTION, $wrapped ) ) { + $this->cached_wrapped = $wrapped; + $this->cached_root_key = $candidate; + return $candidate; } @@ -281,6 +333,13 @@ private function generate_root_key() { ); } - return $this->keyring->unwrap( $existing ); + $root_key = $this->keyring->unwrap( $existing ); + + if ( is_string( $root_key ) ) { + $this->cached_wrapped = $existing; + $this->cached_root_key = $root_key; + } + + return $root_key; } } diff --git a/src/wp-includes/interface-wp-secrets-keyring.php b/src/wp-includes/interface-wp-secrets-keyring.php index cfc2030..3fe3c5b 100644 --- a/src/wp-includes/interface-wp-secrets-keyring.php +++ b/src/wp-includes/interface-wp-secrets-keyring.php @@ -15,6 +15,9 @@ * and cannot turn encryption off. There is no method here that * accepts a plaintext secret value at all. * + * An implementation should run WP_Secrets_Keyring_Conformance against itself before + * shipping. + * * @since 7.2.0 */ interface WP_Secrets_Keyring { @@ -22,6 +25,11 @@ interface WP_Secrets_Keyring { /** * Wraps (encrypts) raw key material for storage. * + * Must not be deterministic: two calls with the same key material must return + * different values. WP_Secrets_Key_Manager::rotate_site_key() stores the + * re-wrapped value with update_site_option(), which reports an unchanged value + * as a failure, and WP_Secrets_Keyring_Conformance checks this. + * * @since 7.2.0 * * @param string $key_material Raw key material to protect. diff --git a/tests/bootstrap-examples.php b/tests/bootstrap-examples.php new file mode 100644 index 0000000..5e75989 --- /dev/null +++ b/tests/bootstrap-examples.php @@ -0,0 +1,22 @@ +wrap_calls; + if ( $this->fail_wrap ) { return new WP_Error( WP_SECRETS_ERROR_KEY_UNAVAILABLE, 'Mock_Keyring: wrap() configured to fail.' ); } - return self::MARKER . base64_encode( $key_material ); + $nonce = random_bytes( 8 ); + $tag = hash( 'sha256', $nonce . $key_material, true ); + + return self::MARKER . base64_encode( $nonce . $key_material . $tag ); } public function unwrap( $wrapped ) { + ++$this->unwrap_calls; + if ( $this->fail_unwrap ) { return new WP_Error( WP_SECRETS_ERROR_KEY_UNAVAILABLE, 'Mock_Keyring: unwrap() configured to fail.' ); } @@ -28,7 +38,21 @@ public function unwrap( $wrapped ) { return new WP_Error( WP_SECRETS_ERROR_KEY_UNAVAILABLE, 'Mock_Keyring: not a value this keyring wrapped.' ); } - return base64_decode( substr( $wrapped, strlen( self::MARKER ) ), true ); + $decoded = base64_decode( substr( $wrapped, strlen( self::MARKER ) ), true ); + + if ( false === $decoded || strlen( $decoded ) < 41 ) { + return new WP_Error( WP_SECRETS_ERROR_KEY_UNAVAILABLE, 'Mock_Keyring: wrapped value is malformed.' ); + } + + $nonce = substr( $decoded, 0, 8 ); + $key_material = substr( $decoded, 8, -32 ); + $tag = substr( $decoded, -32 ); + + if ( ! hash_equals( hash( 'sha256', $nonce . $key_material, true ), $tag ) ) { + return new WP_Error( WP_SECRETS_ERROR_KEY_UNAVAILABLE, 'Mock_Keyring: integrity tag mismatch.' ); + } + + return $key_material; } public function get_key_source() { @@ -56,4 +80,18 @@ public function configure_fail_unwrap( $fail = true ) { return $this; } + + /** + * @return int Number of times wrap() has been called. + */ + public function wrap_call_count() { + return $this->wrap_calls; + } + + /** + * @return int Number of times unwrap() has been called. + */ + public function unwrap_call_count() { + return $this->unwrap_calls; + } } diff --git a/tests/includes/class-wp-secrets-keyring-conformance.php b/tests/includes/class-wp-secrets-keyring-conformance.php new file mode 100644 index 0000000..331911e --- /dev/null +++ b/tests/includes/class-wp-secrets-keyring-conformance.php @@ -0,0 +1,112 @@ +keyring(); + $key_material = random_bytes( 32 ); + + $wrapped = $keyring->wrap( $key_material ); + + $this->assertIsString( $wrapped ); + $this->assertNotSame( '', $wrapped ); + + $unwrapped = $keyring->unwrap( $wrapped ); + + $this->assertSame( $key_material, $unwrapped ); + } + + /** + * A deterministic wrap() leaks, via ciphertext comparison, whether two wrapped + * values protect the same key material -- something nothing outside the + * keyring is entitled to learn. A fresh nonce (or equivalent) per call is what + * WP_Secrets_Key_Manager relies on to keep that comparison unavailable. + */ + public function test_two_wraps_of_the_same_bytes_return_different_strings() { + $keyring = $this->keyring(); + $key_material = random_bytes( 32 ); + + $first = $keyring->wrap( $key_material ); + $second = $keyring->wrap( $key_material ); + + $this->assertNotSame( $first, $second ); + } + + /** + * Unwrap() never throws and never returns a plausible-looking string for input + * it did not produce -- WP_Secrets_Key_Manager treats anything other than + * WP_Error as usable key material, so a keyring that returns garbage bytes on + * garbage input hands a wrong root key downstream instead of failing. + */ + public function test_unwrap_of_garbage_is_a_wp_error() { + $keyring = $this->keyring(); + $garbage = 'garbage-' . bin2hex( random_bytes( 16 ) ); + + $this->assertWPError( $keyring->unwrap( $garbage ) ); + } + + /** + * A truncated wrapped value must fail closed rather than decode to a short, + * wrong key -- WP_Secrets_Key_Manager has no way to tell a merely-short key + * from a correctly-derived one except by trusting unwrap()'s success. + */ + public function test_unwrap_of_a_truncated_value_is_a_wp_error() { + $keyring = $this->keyring(); + $wrapped = $keyring->wrap( random_bytes( 32 ) ); + + $truncated = substr( $wrapped, 0, intdiv( strlen( $wrapped ), 2 ) ); + + $this->assertWPError( $keyring->unwrap( $truncated ) ); + } + + /** + * A single flipped bit anywhere in the wrapped value has to be caught -- + * that is the entire point of authenticated wrapping. Silently accepting it + * would let a corrupted or tampered wrapped root key through as genuine. + */ + public function test_unwrap_of_a_value_with_one_flipped_byte_is_a_wp_error() { + $keyring = $this->keyring(); + $wrapped = $keyring->wrap( random_bytes( 32 ) ); + + $flip_at = intdiv( strlen( $wrapped ), 2 ); + $flipped = $wrapped; + $flipped[ $flip_at ] = chr( ord( $wrapped[ $flip_at ] ) ^ 0x01 ); + + $this->assertWPError( $keyring->unwrap( $flipped ) ); + } + + /** + * Site Health renders get_key_source() directly; an empty or non-string value + * there is a blank line on a diagnostics page an operator is depending on. + */ + public function test_get_key_source_returns_a_non_empty_string() { + $source = $this->keyring()->get_key_source(); + + $this->assertIsString( $source ); + $this->assertNotSame( '', trim( $source ) ); + } +} diff --git a/tests/phpunit/test-secrets-config-keyring-conformance.php b/tests/phpunit/test-secrets-config-keyring-conformance.php new file mode 100644 index 0000000..51f486f --- /dev/null +++ b/tests/phpunit/test-secrets-config-keyring-conformance.php @@ -0,0 +1,17 @@ +set( 'myplugin/api-key', 'value' ); + + define( 'WP_SECRETS_KEY', 424242 ); + + $result = wp_get_secret( 'myplugin/api-key' ); + + $this->assertNotNull( $result ); + $this->assertWPError( $result ); + $this->assertSame( WP_SECRETS_ERROR_KEY_UNAVAILABLE, $result->get_error_code() ); + } + + /** + * Written under the ambient salt-fallback key, then read back after the stored + * wrapped root key has been corrupted -- simulating the option row being + * damaged after secrets already exist. get_master_key() must fail before + * decryption is ever attempted, since a usable root key was never obtained. + */ + public function test_a_corrupted_wrapped_root_key_is_wp_error_not_null() { wp_set_secret( 'myplugin/api-key', 'value' ); - define( 'WP_SECRETS_KEY', 424242 ); // Defined, but not a usable string. + update_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION, 'not-a-valid-wrapped-value' ); $result = wp_get_secret( 'myplugin/api-key' ); diff --git a/tests/phpunit/test-wp-cli-secret-command.php b/tests/phpunit/test-wp-cli-secret-command.php index 5034f99..51788eb 100644 --- a/tests/phpunit/test-wp-cli-secret-command.php +++ b/tests/phpunit/test-wp-cli-secret-command.php @@ -334,6 +334,155 @@ public function test_rotate_without_previous_key_constant_errors() { $this->command()->rotate( array(), array( 'yes' => true ) ); } + public function test_rotate_rejects_an_unknown_from_value() { + $this->expectException( Mock_WP_CLI_Exit_Exception::class ); + + try { + $this->command()->rotate( + array(), + array( + 'from' => 'vault', + 'yes' => true, + ) + ); + } finally { + $this->assertNotEmpty( WP_CLI::$errors ); + $this->assertStringContainsString( '--from', WP_CLI::$errors[0] ); + } + } + + public function test_rotate_from_config_refuses_when_the_active_keyring_is_the_config_keyring() { + $root = random_bytes( 32 ); + update_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION, ( new WP_Secrets_Config_Key_Provider() )->wrap( $root ) ); + + $before = get_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION ); + + $this->expectException( Mock_WP_CLI_Exit_Exception::class ); + + try { + $this->command()->rotate( + array(), + array( + 'from' => 'config', + 'yes' => true, + ) + ); + } finally { + $this->assertNotEmpty( WP_CLI::$errors ); + $this->assertStringContainsString( 'WP_SECRETS_KEY', WP_CLI::$errors[0] ); + $this->assertSame( $before, get_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION ) ); + } + } + + /** + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function test_rotate_from_config_previous_refuses_when_both_constants_are_identical() { + $same = base64_encode( str_repeat( 'A', 32 ) ); + define( 'WP_SECRETS_KEY_PREVIOUS', $same ); + define( 'WP_SECRETS_KEY', $same ); + + $root = random_bytes( 32 ); + update_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION, ( new WP_Secrets_Config_Key_Provider() )->wrap( $root ) ); + + $this->expectException( Mock_WP_CLI_Exit_Exception::class ); + + try { + $this->command()->rotate( array(), array( 'yes' => true ) ); + } finally { + $this->assertNotEmpty( WP_CLI::$errors ); + $this->assertStringContainsString( 'WP_SECRETS_KEY_PREVIOUS', WP_CLI::$errors[0] ); + $this->assertStringContainsString( 'WP_SECRETS_KEY', WP_CLI::$errors[0] ); + } + } + + /** + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function test_rotate_from_config_previous_rewraps_under_the_new_site_key() { + define( 'WP_SECRETS_KEY_PREVIOUS', base64_encode( str_repeat( 'A', 32 ) ) ); + define( 'WP_SECRETS_KEY', base64_encode( str_repeat( 'B', 32 ) ) ); + + $root = random_bytes( 32 ); + update_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION, ( new WP_Secrets_Config_Key_Provider( true ) )->wrap( $root ) ); + + $this->command()->rotate( array(), array( 'yes' => true ) ); + + $rewrapped = get_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION ); + + $this->assertSame( $root, ( new WP_Secrets_Config_Key_Provider( false ) )->unwrap( $rewrapped ) ); + $this->assertNotEmpty( WP_CLI::$success ); + } + + /** + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function test_rotate_from_config_moves_the_root_key_onto_the_dropin_keyring() { + $root = random_bytes( 32 ); + update_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION, ( new WP_Secrets_Config_Key_Provider() )->wrap( $root ) ); + + $provider = new WP_Secrets_Libsodium_Provider( + new WP_Secrets_Option_Store(), + new WP_Secrets_Key_Manager( new WP_Secrets_Config_Key_Provider() ) + ); + $provider->set( 'myplugin/api-key', 'value' ); + + $mock = new Mock_Keyring(); + + $GLOBALS['wp_secrets_keyring'] = $mock; + + $this->assertWPError( wp_get_secret( 'myplugin/api-key' ) ); + + $this->command()->rotate( + array(), + array( + 'from' => 'config', + 'yes' => true, + ) + ); + + $secret = wp_get_secret( 'myplugin/api-key' ); + $this->assertInstanceOf( 'WP_Secret', $secret ); + $this->assertSame( 'value', $secret->reveal() ); + + $stored = get_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION ); + $this->assertStringStartsWith( Mock_Keyring::MARKER, $stored ); + $this->assertSame( $root, $mock->unwrap( $stored ) ); + } + + /** + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function test_rotate_never_logs_key_material() { + $root = random_bytes( 32 ); + update_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION, ( new WP_Secrets_Config_Key_Provider() )->wrap( $root ) ); + + $provider = new WP_Secrets_Libsodium_Provider( + new WP_Secrets_Option_Store(), + new WP_Secrets_Key_Manager( new WP_Secrets_Config_Key_Provider() ) + ); + $provider->set( 'myplugin/api-key', 'value' ); + + $GLOBALS['wp_secrets_keyring'] = new Mock_Keyring(); + + $this->command()->rotate( + array(), + array( + 'from' => 'config', + 'yes' => true, + ) + ); + + $everything = implode( "\n", array_merge( WP_CLI::$log, WP_CLI::$success, WP_CLI::$warning, WP_CLI::$errors ) ); + + $this->assertStringNotContainsString( $root, $everything ); + $this->assertStringNotContainsString( base64_encode( $root ), $everything ); + } + // -- migrate-legacy ----------------------------------------------------- public function test_migrate_legacy_is_refused_for_network_scope() { diff --git a/tests/phpunit/test-wp-secrets-key-manager.php b/tests/phpunit/test-wp-secrets-key-manager.php index c5b3a58..84c330e 100644 --- a/tests/phpunit/test-wp-secrets-key-manager.php +++ b/tests/phpunit/test-wp-secrets-key-manager.php @@ -230,7 +230,120 @@ public function test_rotation_does_not_change_any_derived_master_key() { $this->assertSame( $master_before, $master_after ); // The old keyring alone is no longer sufficient: the stored root key is now - // wrapped under the new key. - $this->assertWPError( $manager_under_old_key->get_root_key() ); + // wrapped under the new key. Checked with a fresh manager instance, since + // $manager_under_old_key's own cache was correctly primed by the rotation + // it just performed and is not the thing under test here. + $fresh_manager_under_old_key = new WP_Secrets_Key_Manager( $old_keyring ); + $this->assertWPError( $fresh_manager_under_old_key->get_root_key() ); + } + + public function test_unwrap_is_called_once_across_repeated_master_key_derivations() { + $mock = new Mock_Keyring(); + $root = random_bytes( 32 ); + update_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION, $mock->wrap( $root ) ); + + $manager = new WP_Secrets_Key_Manager( $mock ); + + for ( $i = 0; $i < 5; $i++ ) { + $this->assertSame( 32, strlen( $manager->get_master_key( 'site', $i + 1 ) ) ); + $this->assertSame( 32, strlen( $manager->get_master_key( 'network' ) ) ); + } + + $this->assertSame( 1, $mock->unwrap_call_count() ); + } + + /** + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function test_unwrap_is_called_once_across_many_secret_reads() { + $mock = new Mock_Keyring(); + $root = random_bytes( 32 ); + update_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION, $mock->wrap( $root ) ); + + $GLOBALS['wp_secrets_keyring'] = $mock; + + $this->assertNotWPError( wp_set_secret( 'conformance/root-key-cache', 'the-value' ) ); + + for ( $i = 0; $i < 10; $i++ ) { + $secret = wp_get_secret( 'conformance/root-key-cache' ); + $this->assertInstanceOf( 'WP_Secret', $secret ); + $this->assertSame( 'the-value', $secret->reveal() ); + } + + $this->assertSame( 1, $mock->unwrap_call_count() ); + } + + public function test_rotate_site_key_updates_the_cache_without_another_unwrap() { + $mock = new Mock_Keyring(); + $second_mock = new Mock_Keyring(); + $root = random_bytes( 32 ); + update_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION, $mock->wrap( $root ) ); + + $manager = new WP_Secrets_Key_Manager( $mock ); + $root_key = $manager->get_root_key(); + + $this->assertNotWPError( $manager->rotate_site_key( $mock, $second_mock ) ); + + $this->assertSame( $root_key, $manager->get_root_key() ); + $this->assertSame( 1, $mock->unwrap_call_count() ); + $this->assertSame( 0, $second_mock->unwrap_call_count() ); + } + + public function test_a_changed_wrapped_value_is_unwrapped_again_rather_than_served_from_cache() { + $mock = new Mock_Keyring(); + $root = random_bytes( 32 ); + $other_root = random_bytes( 32 ); + update_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION, $mock->wrap( $root ) ); + + $manager = new WP_Secrets_Key_Manager( $mock ); + $this->assertSame( $root, $manager->get_root_key() ); + + update_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION, $mock->wrap( $other_root ) ); + + $this->assertSame( $other_root, $manager->get_root_key() ); + $this->assertSame( 2, $mock->unwrap_call_count() ); + } + + public function test_an_unwrap_error_is_not_cached() { + $mock = new Mock_Keyring(); + $root = random_bytes( 32 ); + update_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION, $mock->wrap( $root ) ); + + $manager = new WP_Secrets_Key_Manager( $mock ); + + $mock->configure_fail_unwrap( true ); + $this->assertWPError( $manager->get_root_key() ); + + $mock->configure_fail_unwrap( false ); + $this->assertSame( $root, $manager->get_root_key() ); + + $this->assertSame( $root, $manager->get_root_key() ); + $this->assertSame( 2, $mock->unwrap_call_count() ); + } + + public function test_generate_root_key_primes_the_cache() { + $mock = new Mock_Keyring(); + + $manager = new WP_Secrets_Key_Manager( $mock ); + $manager->get_root_key(); + $manager->get_master_key( 'site' ); + + $this->assertSame( 0, $mock->unwrap_call_count() ); + $this->assertSame( 1, $mock->wrap_call_count() ); + } + + public function test_the_returned_root_key_is_a_copy_the_caller_can_zero() { + $mock = new Mock_Keyring(); + $root = random_bytes( 32 ); + update_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION, $mock->wrap( $root ) ); + + $manager = new WP_Secrets_Key_Manager( $mock ); + + $copy = $manager->get_root_key(); + wp_secrets_memzero( $copy ); + + $this->assertSame( $root, $manager->get_root_key() ); + $this->assertSame( 1, $mock->unwrap_call_count() ); } } diff --git a/tests/smoke/SPEC.md b/tests/smoke/SPEC.md new file mode 100644 index 0000000..1e4a153 --- /dev/null +++ b/tests/smoke/SPEC.md @@ -0,0 +1,131 @@ +# Spec: WP-CLI smoke test + +Status: planned. Part of the pre-Trac work in +[ADR 0008](../../docs/decisions/0008-the-trac-ticket-replaces-thread-confirmation.md). + +## Why + +Every WP-CLI test in `tests/phpunit/` constructs the command class and calls its methods +directly. That tests the method bodies and nothing about how WP-CLI reaches them: flag +reservations, synopsis parsing, and mapping method names to subcommand names. Three bugs lived +there behind a green suite (see `docs/journal/test-coverage-gaps.md`). The worst was +`--version=previous` silently returning the current value. This is the only gap in that file +marked 🟡, meaning it needs an answer before the core patch. + +The same harness also closes two 🟢 gaps as a side effect: `set --stdin`, which PHPUnit cannot pipe +into safely, and drop-in file loading, which runs once per process before any test body. + +## Shape + +- **`tests/smoke/smoke.sh`.** Bash with no dependencies beyond `wp` and `php`. It uses small + `ok`/`not_ok` helpers that print TAP-style lines, and exits non-zero if any case fails. No bats: + one more tool to install is not worth it for about forty assertions. +- **`bin/smoke-install.sh`** provisions a throwaway install that the smoke test owns: + - Downloads a pinned `wp-cli.phar` and checks it against a committed SHA-256, following the + pin-everything rule in `ci.yml`. + - Runs `wp core download` into `.smoke/wordpress/` (git-ignored) and `wp config create` + against a separate `wordpress_smoke` database, using the same `DB_*` variables as + `make install`. It must not be `wordpress_test`, because the PHPUnit suite drops and recreates + that one. + - Defines `WP_SECRETS_KEY` before the first secret is ever written, so the rotation case has a + real site key to rotate from. + - Symlinks the plugin into place and activates it. +- **`make smoke`** runs the install, then the single-site pass, converts the install with + `wp core multisite-convert`, and runs the multisite pass. +- **`make ci` includes `smoke`.** The Makefile says `make ci` is the pipeline, and a CI job that + `make ci` does not run would break that. `bin/ci-local.sh` gets the same target. +- **The CI job `smoke`** runs after `static`, on PHP 7.4 (the floor, and where 7.4-only CLI + surprises would show up) and 8.3, with the same MySQL service as the test jobs. + +It uses its own install rather than wp-env's, because wp-env's dev and test environments share +`wp-content`. A drop-in the smoke test writes would sit in front of PHPUnit, which is exactly how +the AWS example once took down 85 tests. + +Each run namespaces its secrets as `smoke-/…`, and an `EXIT` trap removes any drop-in the +script wrote. The install is disposable, but a failed run should not leave a broken drop-in behind +for the next person debugging it. + +## Cases + +### A. Registration + +This catches bugs 2 and 3 from the gaps file. + +- For each of the 11 subcommands, under both `secret` and `network-secret`: + `wp cli has-command "secret "` exits 0. That covers `import-option`, `migrate-legacy`, and + `generate-key` by their hyphenated names. +- For each flag a subcommand declares, `wp help secret ` shows it: `--slot`, `--reveal`, + `--format`, `--field`, `--fields`, `--stdin`, `--porcelain`, `--yes`, `--namespace`, + `--dry-run`, `--name`, `--map`, and `--verbose`. The expected table is written out at the top of + the script, one row per subcommand, so a new flag without a row fails loudly rather than going + untested. +- `wp secret get --version=previous` is not accepted as the slot selector. This pins bug 1's cause, + not only its fix. + +### B. Behaviour and the exit-code contract + +`wp secret get` documents exit 0 when the secret is found, 1 when it is absent, and 2 on error. +Every case below checks the exit code as well as the output. + +- `set` with a positional value exits 0, and `set --stdin` from a pipe exits 0. `set --porcelain` + prints only what its docblock promises. +- `get` masks by default: the plaintext is not in stdout. `get --reveal` prints exactly the value. +- `set` A, then `set` B, then `get --slot=previous --reveal` prints A. This is bug 1, end to end. +- `get --format=json` parses as JSON with `php -r`. `list --format=json`, `--format=csv`, + `--fields`, and `--namespace` all filter as documented. +- After `retire --yes`, `get --slot=previous` exits 1. After `delete --yes`, `get` exits 1. +- `get` of a name that was never set exits 1, and `set` with no value exits non-zero. +- `generate-key` prints 44 characters that decode to exactly 32 bytes. +- `health --format=json` parses as JSON. `dropin` reports no drop-in. +- `import-option` moves a seeded option into a secret, and `get --reveal` matches it. +- `migrate-legacy --dry-run` exits 0 on an install with no prototype rows. + +### C. Rotation, end to end + +- Set a secret. Move the value of `WP_SECRETS_KEY` to `WP_SECRETS_KEY_PREVIOUS` with + `wp config set`, and set a new `WP_SECRETS_KEY` from `generate-key`. Then `rotate --yes` exits + 0, `get --reveal` still returns the value, and `health` reports no undecryptable secrets. +- `rotate` without `WP_SECRETS_KEY_PREVIOUS` exits non-zero with its explanatory message. +- Once the KMS spec's `--from` lands, add `rotate --from=config`, refused because the old and new + keyrings are the same configuration. + +### D. Drop-in loading + +This closes the 🟢 gap. + +Each case writes `wp-content/secrets.php`, runs the assertions, and removes the file. + +| Drop-in | Expect | +|---|---| +| Syntax error | `get` exits **2**, not 1, and `dropin` reports it broken | +| Throws on load | `get` exits 2, and `dropin` reports it broken | +| `$GLOBALS['wp_secrets_provider'] = new stdClass()` | `get` exits 2. This is the 4 September fail-closed fix, run through the real loader. | +| Sets nothing | `get` behaves exactly as with no drop-in | + +Exit 2 against exit 1 is ADR 0007's distinction between unreachable and absent. This is the first +test that checks it through the real `require` in `wp_secrets_api_load_dropin()` rather than by +setting globals. The known uncatchable case, a class that implements an interface but omits a +method, is recorded as a fatal: a non-zero exit with a PHP fatal in stderr. It is written down as +expected behaviour so a future PHP that makes it catchable shows up as a change. + +### E. Multisite pass + +- `network-secret set` and `get --reveal` round-trip. +- Site scope is per site: `set` with `--url=` is invisible from site 1. +- `network-secret` refusing on a single site is checked in the single-site pass. + +## Out of scope + +- Output formatting beyond what is needed to parse it. This is a dispatch test, not a snapshot + test. Byte-for-byte output assertions would break on every WP-CLI table change. +- Running the examples' providers through the CLI. Their own tests cover them. + +## Done when + +- `make smoke` passes locally through `bin/ci-local.sh`, and the CI `smoke` job passes on 7.4 + and 8.3. +- Reintroducing each of the three historical bugs makes the smoke test fail: restore the + `--version` flag, remove a `--format` description line, and drop the `@subcommand` tag. Each + one is checked by hand, and the result goes in the commit message. +- `docs/journal/test-coverage-gaps.md` drops the CLI dispatch entry and the `--stdin` entry, and + narrows the drop-in loading entry to the uncatchable-fatal case alone.