diff --git a/CHANGELOG.md b/CHANGELOG.md index eabe8da0..232dacb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ All notable changes to `mcp/sdk` will be documented in this file. * Fix a `subscriptions/listen` stream that can carry nothing (the server announces none of the requested types, or the client asks for none) being held open for the full subscription lifetime: it is acknowledged with an empty `notifications` filter and closed gracefully right away, instead of pinning a PHP-FPM worker. Adds `NotificationFilter::isEmpty()`. * [BC Break] Answer `server/discover` with `DiscoverHandler`, which a handler added through `Builder::addRequestHandler()` can replace, and dispatch `RequestEvent`, `ResponseEvent` and `ErrorEvent` for it. A `StatelessProtocol` built without the `Builder` needs `DiscoverHandler` and a `MessageFactory` with `DiscoverRequest` to answer it. * Add a `binder` argument to `Builder::setRequestState()` binding the modern era's `requestState` to the request it was minted for, as the 2026-07-28 specification recommends: the binder's value is covered by the state's MAC, and a state echoed on a request that binds to another value is refused like a forged one. `OriginatingRequestBinder` binds the authenticated principal, the method, the tool, prompt or resource, and a digest of the arguments; a token without a subject fails closed, and a `subject` resolver identifies users by another claim. `RequestStateBinderInterface` takes custom bindings. Without a binder, nothing changes. +* Rotate the `requestState` signing key without failing the calls in flight: `Builder::setRequestState()` and `RequestStateCodec` take a list of keys, the first signing and every key verifying. The wire format is unchanged, and a single key works as before. ### Client diff --git a/docs/handlers/input-required.md b/docs/handlers/input-required.md index 83035b08..f5d29b14 100644 --- a/docs/handlers/input-required.md +++ b/docs/handlers/input-required.md @@ -142,6 +142,30 @@ it and its `AccessToken`. Bind only what stays the same across the rounds of one States minted before the binder was configured no longer verify, so turning it on fails the calls in flight for at most one TTL. +### Rotating the key + +Pass a list of keys instead of one, and the first key signs while every key verifies: + +```php +->setRequestState([$_ENV['MCP_REQUEST_STATE_KEY'], $_ENV['MCP_REQUEST_STATE_KEY_PREVIOUS']], ttl: 600) +``` + +Every entry must be a key: once the rotation is done, go back to a single key instead of +leaving an unset variable in the list. + +A state names no key, so the wire format does not change and a single key is a ring of one. +To rotate without failing the calls in flight: + +1. Add the new key **behind** the current one, `[$current, $new]`, and deploy it to every + process. They still sign with the current key but already accept the new one. + A single-process deployment can skip this step. +2. Move the new key to the front, `[$new, $current]`. States are signed with the new key from + now on, and the ones the old key signed still verify. +3. Wait one TTL after the **last** process switched to `[$new, $current]`: only then has every + state the old key signed expired. Remove it, `[$new]`. + +Each key needs at least 32 bytes. + ## Capabilities A server must not ask for input the client cannot provide. The SDK checks each ask against diff --git a/docs/run/server-builder.md b/docs/run/server-builder.md index d2667e45..4eae36f0 100644 --- a/docs/run/server-builder.md +++ b/docs/run/server-builder.md @@ -117,6 +117,7 @@ $server = Server::builder() // Signs the state a multi round-trip request carries through the client. // The same key must reach every process that might serve the retry. // The key needs at least 32 bytes, otherwise build() throws. TTL in seconds, default 600. + // A list of keys rotates them: the first signs, every key verifies. // Without a key, the server refuses every state the client sends back. // The optional binder ties each state to the principal and the call it was // minted for, so a client cannot echo it on another call or under another token. @@ -336,7 +337,7 @@ $server = Server::builder() | `setProtocolVersion()` | protocolVersion | Pin the handshake to one protocol revision | | `setModernVersions()` | versions | Narrow the revisions the modern (2026-07-28) leg answers for | | `withoutModernEra()` | - | Serve the handshake era only | -| `setRequestState()` | key, ttl?, binder? | Signing key, lifetime and optional binding to the originating request for multi round-trip request state | +| `setRequestState()` | key, ttl?, binder? | Signing key (or key ring, the first key signing), lifetime and optional binding to the originating request for multi round-trip request state | | `setInputRequiredLimits()` | maxRounds, roundTimeout | Bound the input-required shim on handshake-era connections | | `withoutInputRequiredShim()` | - | Do not fulfil an `InputRequiredResult` over a handshake-era connection | | `setCachePolicy()` | policy | Set the `ttlMs`/`cacheScope` hints on cacheable results | diff --git a/src/Server/Builder.php b/src/Server/Builder.php index 71a47f7c..db2b03c6 100644 --- a/src/Server/Builder.php +++ b/src/Server/Builder.php @@ -145,7 +145,8 @@ final class Builder /** @var list|class-string<\Mcp\Schema\JsonRpc\Notification>> */ private array $extensionMessages = []; - private ?string $requestStateKey = null; + /** @var string|list|null */ + private string|array|null $requestStateKey = null; private int $requestStateTtl = 600; @@ -381,6 +382,13 @@ public function setCachePolicy(CachePolicy $policy): self * The same key must reach every instance that might serve the retry, so a * per-process random value only works for a single-process deployment. * + * A list of keys is a key ring: the first key signs, every key verifies. + * To rotate across several instances, first add the new key behind the + * current one everywhere, `[$current, $new]`, so every instance accepts it + * before any signs with it. Then move it to the front, `[$new, $current]`, + * and remove the old key one TTL after the last instance stopped signing + * with it. See docs/handlers/input-required.md, "Rotating the key". + * * A binder ties every state the modern era mints to the request it was * minted for, e.g. {@see Stateless\OriginatingRequestBinder} to the * principal, method and arguments: a state echoed on a request that binds @@ -388,11 +396,11 @@ public function setCachePolicy(CachePolicy $policy): self * before a binder was configured no longer verify, so adding one fails the * multi round-trip calls in flight for at most the TTL. * - * @param string $key at least 32 bytes + * @param string|list $key the signing key or key ring, each key at least 32 bytes * @param int $ttl how long a minted state stays valid, in seconds * @param RequestStateBinderInterface|null $binder binds each state to its originating request */ - public function setRequestState(string $key, int $ttl = 600, ?RequestStateBinderInterface $binder = null): self + public function setRequestState(string|array $key, int $ttl = 600, ?RequestStateBinderInterface $binder = null): self { $this->requestStateKey = $key; $this->requestStateTtl = $ttl; diff --git a/src/Server/Stateless/RequestStateCodec.php b/src/Server/Stateless/RequestStateCodec.php index e0a9e9ed..6b81fab1 100644 --- a/src/Server/Stateless/RequestStateCodec.php +++ b/src/Server/Stateless/RequestStateCodec.php @@ -23,6 +23,10 @@ * {@see self::verify()} refuses anything whose MAC or expiry does not hold. * Signed, not encrypted: nothing secret belongs in the payload. * + * Takes one key or a key ring to rotate without failing the calls in flight: + * the first key signs, every key verifies. The value names no key, so the + * wire format does not change. + * * A codec {@see self::withBinding() bound} to a value also covers that value * with the MAC, without putting it on the wire: a state minted under one * binding fails verification under any other, and so does a state minted @@ -37,28 +41,50 @@ final class RequestStateCodec private const ALGORITHM = 'sha256'; + /** @var non-empty-list */ + private readonly array $keys; + + /** + * @param string|list $key the signing key, or a key ring whose first key signs and whose every key verifies + */ public function __construct( - private readonly string $key, + string|array $key, private readonly int $ttlSeconds = 600, private readonly ?string $binding = null, ) { - if (\strlen($this->key) < self::MINIMUM_KEY_BYTES) { - throw new InvalidArgumentException(\sprintf('The requestState signing key must be at least %d bytes, got %d.', self::MINIMUM_KEY_BYTES, \strlen($this->key))); + $keys = \is_string($key) ? [$key] : $key; + + if ([] === $keys || !array_is_list($keys)) { + throw new InvalidArgumentException('The requestState key ring must be a non-empty list of keys.'); + } + + foreach ($keys as $i => $candidate) { + $name = \is_string($key) ? 'The requestState signing key' : \sprintf('The requestState signing key at index %d', $i); + + if (!\is_string($candidate)) { + throw new InvalidArgumentException(\sprintf('%s must be a string, got %s.', $name, get_debug_type($candidate))); + } + + if (\strlen($candidate) < self::MINIMUM_KEY_BYTES) { + throw new InvalidArgumentException(\sprintf('%s must be at least %d bytes, got %d.', $name, self::MINIMUM_KEY_BYTES, \strlen($candidate))); + } } + $this->keys = $keys; + if ($this->ttlSeconds < 1) { throw new InvalidArgumentException(\sprintf('The requestState TTL must be at least one second, got %d.', $this->ttlSeconds)); } } /** - * A codec with the same key and TTL whose states only verify under the + * A codec with the same keys and TTL whose states only verify under the * same binding, e.g. the value a {@see RequestStateBinderInterface} * computed for the request at hand. */ public function withBinding(string $binding): self { - return new self($this->key, $this->ttlSeconds, $binding); + return new self($this->keys, $this->ttlSeconds, $binding); } /** @@ -71,14 +97,14 @@ public function mint(array $payload, ?int $now = null): string 'data' => $payload, ], \JSON_THROW_ON_ERROR)); - return $body.'.'.self::encode($this->sign($body)); + return $body.'.'.self::encode($this->sign($body, $this->keys[0])); } /** * @return array the payload that was sealed * - * @throws RequestStateException when the value is malformed, unsigned by this key, bound to - * something else than this codec, or expired + * @throws RequestStateException when the value is malformed, signed by none of the keys, + * bound to something else than this codec, or expired */ public function verify(string $state, ?int $now = null): array { @@ -90,11 +116,23 @@ public function verify(string $state, ?int $now = null): array [$body, $mac] = $parts; - $expected = $this->sign($body); $actual = self::decode($mac); - // Constant-time: a short-circuiting compare is a byte-at-a-time oracle. - if (null === $actual || !hash_equals($expected, $actual)) { + if (null === $actual) { + throw new RequestStateException('mac'); + } + + // Every key, no early exit: which key signed is nobody's business. A + // ring holds a handful of keys, so trying each costs next to nothing + // and spares the wire format a key id. + $verified = false; + + foreach ($this->keys as $key) { + // Constant-time: a short-circuiting compare is a byte-at-a-time oracle. + $verified = hash_equals($this->sign($body, $key), $actual) || $verified; + } + + if (!$verified) { throw new RequestStateException('mac'); } @@ -123,13 +161,13 @@ public function verify(string $state, ?int $now = null): array return $claims['data']; } - private function sign(string $body): string + private function sign(string $body, string $key): string { // The body is base64url, so it never contains the '.' separating it // from the binding: a bound and an unbound input can never collide. $input = null === $this->binding ? $body : $body.'.'.$this->binding; - return hash_hmac(self::ALGORITHM, $input, $this->key, true); + return hash_hmac(self::ALGORITHM, $input, $key, true); } private static function encode(string $raw): string diff --git a/tests/Unit/Server/Stateless/RequestStateCodecTest.php b/tests/Unit/Server/Stateless/RequestStateCodecTest.php index db6386dc..9cd205ea 100644 --- a/tests/Unit/Server/Stateless/RequestStateCodecTest.php +++ b/tests/Unit/Server/Stateless/RequestStateCodecTest.php @@ -14,6 +14,7 @@ use Mcp\Exception\InvalidArgumentException; use Mcp\Exception\RequestStateException; use Mcp\Server\Stateless\RequestStateCodec; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\TestDox; use PHPUnit\Framework\TestCase; @@ -21,6 +22,8 @@ class RequestStateCodecTest extends TestCase { private const KEY = 'a-key-that-is-long-enough-to-be-secure'; private const OTHER_KEY = 'a-different-key-that-is-also-long-enough'; + private const OLD_KEY = 'the-key-being-rotated-out-of-the-ring'; + private const NEW_KEY = 'the-key-being-rotated-into-the-ring-now'; #[TestDox('a sealed payload survives the round trip intact')] public function testRoundTrip(): void @@ -159,6 +162,85 @@ public function testBindingStaysOffTheWire(): void ); } + #[TestDox('rotating through a key ring never fails a state still within its TTL')] + public function testKeyRotation(): void + { + $old = new RequestStateCodec(self::OLD_KEY); + $mintedByOld = $old->mint(['round' => 1]); + + // Step 1, for more than one instance: every instance learns the new + // key before any signs with it. + $learning = new RequestStateCodec([self::OLD_KEY, self::NEW_KEY]); + $this->assertSame(['round' => 1], $learning->verify($mintedByOld)); + $mintedWhileLearning = $learning->mint(['round' => 2]); + $this->assertSame(['round' => 2], $old->verify($mintedWhileLearning), 'An instance not yet updated verifies it.'); + + // Step 2: the new key signs, the old one still verifies. + $rotated = new RequestStateCodec([self::NEW_KEY, self::OLD_KEY]); + $mintedByNew = $rotated->mint(['round' => 3]); + $this->assertSame(['round' => 1], $rotated->verify($mintedByOld)); + $this->assertSame(['round' => 3], $rotated->verify($mintedByNew)); + $this->assertSame(['round' => 3], $learning->verify($mintedByNew), 'An instance still on step 1 verifies it.'); + + // Step 3, one TTL later: the old key is gone, and so is what it signed. + $retired = new RequestStateCodec(self::NEW_KEY); + $this->assertSame(['round' => 3], $retired->verify($mintedByNew)); + + $this->expectExceptionObject(new RequestStateException('mac')); + $retired->verify($mintedByOld); + } + + #[TestDox('a state signed by none of the ring\'s keys is refused')] + public function testForeignKeyRefusedByARing(): void + { + $state = (new RequestStateCodec(self::OTHER_KEY))->mint(['round' => 1]); + + $this->expectExceptionObject(new RequestStateException('mac')); + (new RequestStateCodec([self::NEW_KEY, self::OLD_KEY, self::KEY]))->verify($state); + } + + #[TestDox('a bound codec keeps the key ring: a bound state the old key signed verifies after the rotation')] + public function testBindingSurvivesARotation(): void + { + $state = (new RequestStateCodec(self::OLD_KEY))->withBinding('tools/call|book')->mint(['round' => 1]); + $rotated = new RequestStateCodec([self::NEW_KEY, self::OLD_KEY]); + + $this->assertSame(['round' => 1], $rotated->withBinding('tools/call|book')->verify($state)); + + $this->expectExceptionObject(new RequestStateException('mac')); + $rotated->withBinding('tools/call|cancel')->verify($state); + } + + #[TestDox('a single key and a ring of that key mint the same wire value')] + public function testSingleKeyIsARingOfOne(): void + { + $single = new RequestStateCodec(self::KEY); + $ring = new RequestStateCodec([self::KEY]); + + $this->assertSame($single->mint(['round' => 1], now: 1_000), $ring->mint(['round' => 1], now: 1_000)); + $this->assertSame(['round' => 1], $ring->verify($single->mint(['round' => 1]))); + $this->assertSame(['round' => 1], $single->verify($ring->mint(['round' => 1]))); + } + + /** + * @return iterable + */ + public static function invalidRings(): iterable + { + yield 'empty' => [[]]; + yield 'short key behind a good one' => [[self::KEY, 'too-short']]; + yield 'not a list' => [['current' => self::KEY]]; + yield 'not a string' => [[self::KEY, 42]]; + } + + #[DataProvider('invalidRings')] + #[TestDox('an invalid key ring is refused at construction: $_dataName')] + public function testInvalidRingRefused(mixed $keys): void + { + $this->expectException(InvalidArgumentException::class); + new RequestStateCodec($keys); + } + #[TestDox('a key too short to be safe is refused at construction')] public function testShortKeyRefused(): void { diff --git a/tests/Unit/Server/Stateless/StatelessProtocolTest.php b/tests/Unit/Server/Stateless/StatelessProtocolTest.php index 8e837a4f..81ff7edd 100644 --- a/tests/Unit/Server/Stateless/StatelessProtocolTest.php +++ b/tests/Unit/Server/Stateless/StatelessProtocolTest.php @@ -586,8 +586,10 @@ public function testUndeclaredGatewayElicitationIsRefused(): void /** * A handler asking twice: the second round has to remember the first * round's answer, which is what the `requestState` is for. + * + * @param string|list|null $key the signing key or key ring, a fixed test key by default */ - private static function twoAskProtocol(bool $signed = true, ?RequestStateBinderInterface $binder = null): StatelessProtocol + private static function twoAskProtocol(bool $signed = true, ?RequestStateBinderInterface $binder = null, string|array|null $key = null): StatelessProtocol { $builder = Server::builder() ->setServerInfo('test-server', '1.0.0') @@ -611,7 +613,7 @@ static function (RequestContext $context): string { ); if ($signed) { - $builder->setRequestState(str_repeat('k', 32), binder: $binder); + $builder->setRequestState($key ?? str_repeat('k', 32), binder: $binder); } return $builder->buildStateless([ProtocolVersion::V2026_07_28]); @@ -827,6 +829,29 @@ public function testUnboundStateIsNotTiedToTheCall(): void $this->assertSame('morning/aisle', $answer['body']['result']['content'][0]['text']); } + #[TestDox('a server rotating its key still serves the retry of a call the old key signed')] + public function testRetryServedAcrossAKeyRotation(): void + { + $oldKey = str_repeat('o', 32); + $newKey = str_repeat('n', 32); + + $second = self::book(self::twoAskProtocol(key: $oldKey), [ + 'inputResponses' => ['when' => ['action' => 'accept', 'content' => ['v' => 'morning']]], + ]); + $retry = [ + 'inputResponses' => ['seat' => ['action' => 'accept', 'content' => ['v' => 'aisle']]], + 'requestState' => $second['body']['result']['requestState'], + ]; + + $rotated = self::book(self::twoAskProtocol(key: [$newKey, $oldKey]), $retry); + $this->assertSame(200, $rotated['status']); + $this->assertSame('morning/aisle', $rotated['body']['result']['content'][0]['text']); + + $retired = self::book(self::twoAskProtocol(key: [$newKey]), $retry); + $this->assertSame(400, $retired['status']); + $this->assertSame('The supplied requestState failed verification.', $retired['body']['error']['message']); + } + #[TestDox('carrying an answer to the next round without a signing key fails loudly')] public function testCarryingAnAnswerNeedsASigningKey(): void {