Automattic / Automattic/wordpress-activitypub
Signature::get_key_pair() persists during a read and has no in-process memo
- Dominant language
- PHP
- Stars
- 579
- Forks
- 92
- Avg merge
- 13h 5m
- Merged PRs (30d)
- 85
Description
### Description
`Signature::get_key_pair()` (`includes/class-signature.php`) performs an `update_option()` as part of what is otherwise a read:
```php
$key_pair = \get_option( $option_key );
if ( $key_pair ) {
return $key_pair;
}
$key_pair = $legacy_callback ? $legacy_callback() : false;
if ( ! $key_pair ) {
$key_pair = self::generate_key_pair();
// ...
}
\update_option( $option_key, $key_pair );
return $key_pair;
```
The stored option row is the only cache. There is no static memo, object-cache entry or transient in `Signature`, `Application` or `Actors` — `Application::get_keypair()` and `Actors::get_keypair()` are thin pass-throughs, and `Application::get_public_key()` / `get_private_key()` each call `get_keypair()` separately, so a caller that needs both pays twice.
Two consequences:
**1. A read can create state, on whichever site happens to be current.** `Http::get()` builds `key_id` / `private_key` from `Application::get_private_key()` eagerly on every uncached request, so an ordinary outbound fetch can mint a keypair. On a multisite host, plugin code may run before the request has been switched to the site it is serving — in that window the row is written to the wrong site. Because generation is non-deterministic, that site ends up with a *different* keypair rather than a stray copy of the right one.
**2. If the write does not stick, generation repeats forever.** Since `get_option()` is the only memo, a persist that is filtered, blocked or simply fails means `generate_key_pair()` runs again on the very next call. That is an unconditional 2048-bit RSA keygen (`openssl_pkey_new`, `sha512`) — I measured ~280 ms per keypair, with the long tail RSA keygen normally has. Inbound signature verification resolves remote actors through several `Http::get()` calls per request, so this compounds quickly.
### Suggested change
- Add a static memo keyed on `$option_key`, so repeated calls within a request cost nothing regardless of whether the row exists.
- Separate "read the keypair" from "generate and persist one", so a caller that only wants to read cannot trigger a write as a side effect.
The memo alone is a clear win independent of the multisite angle.
Contributor guide
Research direction
Start in includes/class-signature.php, then trace the pass-through methods in Application and Actors and the eager key access in Http::get(). Verify repeated calls reuse the in-process result, read-only access does not persist or generate a keypair, and generation still persists one when required.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- backend, security
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100