diff --git a/CHANGELOG.md b/CHANGELOG.md index 468c1e9d..4c37354c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ All notable changes to `mcp/sdk` will be documented in this file. +0.9.1 +----- + +* 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. + 0.9.0 ----- diff --git a/docs/handlers/input-required.md b/docs/handlers/input-required.md index 5139e936..52ccb1aa 100644 --- a/docs/handlers/input-required.md +++ b/docs/handlers/input-required.md @@ -98,6 +98,26 @@ The **same key must reach every process that might serve the retry**. A per-proc value works only for a single-process deployment. Nothing secret belongs in the payload — it is signed, not encrypted. +### 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) +``` + +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. After one TTL, every state the old key signed has 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 b1fb6a16..1dbd3bef 100644 --- a/docs/run/server-builder.md +++ b/docs/run/server-builder.md @@ -116,6 +116,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. ->setRequestState($_ENV['MCP_REQUEST_STATE_KEY'], ttl: 600) @@ -329,7 +330,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? | Signing key and lifetime for multi round-trip request state | +| `setRequestState()` | key, ttl? | Signing key (or key ring, the first key signing) and lifetime 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 13f1eb59..38ff1e13 100644 --- a/src/Server/Builder.php +++ b/src/Server/Builder.php @@ -141,7 +141,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; @@ -374,10 +375,14 @@ 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. * - * @param string $key at least 32 bytes - * @param int $ttl how long a minted state stays valid, in seconds + * A list of keys is a key ring: the first key signs, every key verifies. + * To rotate, put the new key first and keep the old one behind it until + * the states it signed have expired, one TTL later. + * + * @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 */ - public function setRequestState(string $key, int $ttl = 600): self + public function setRequestState(string|array $key, int $ttl = 600): self { $this->requestStateKey = $key; $this->requestStateTtl = $ttl; diff --git a/src/Server/Stateless/RequestStateCodec.php b/src/Server/Stateless/RequestStateCodec.php index 9dcf477b..7ef0af19 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 is the same either way. + * * @author Christopher Hertel */ final class RequestStateCodec @@ -32,14 +36,36 @@ 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, ) { - 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)); } @@ -55,13 +81,13 @@ 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(self::sign($body, $this->keys[0])); } /** * @return array the payload that was sealed * - * @throws RequestStateException when the value is malformed, unsigned by this key, or expired + * @throws RequestStateException when the value is malformed, signed by none of the keys, or expired */ public function verify(string $state, ?int $now = null): array { @@ -73,11 +99,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(self::sign($body, $key), $actual) || $verified; + } + + if (!$verified) { throw new RequestStateException('mac'); } @@ -106,9 +144,9 @@ public function verify(string $state, ?int $now = null): array return $claims['data']; } - private function sign(string $body): string + private static function sign(string $body, string $key): string { - return hash_hmac(self::ALGORITHM, $body, $this->key, true); + return hash_hmac(self::ALGORITHM, $body, $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 6b84b8f9..027a95cd 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 @@ -112,6 +115,73 @@ public function testFailureReasonsAreOpaque(): 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 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 5938667c..91e0538e 100644 --- a/tests/Unit/Server/Stateless/StatelessProtocolTest.php +++ b/tests/Unit/Server/Stateless/StatelessProtocolTest.php @@ -577,7 +577,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. */ - private static function twoAskProtocol(bool $signed = true): StatelessProtocol + /** + * @param string|list|null $key the signing key or key ring, a fixed test key by default + */ + private static function twoAskProtocol(bool $signed = true, string|array|null $key = null): StatelessProtocol { $builder = Server::builder() ->setServerInfo('test-server', '1.0.0') @@ -601,7 +604,7 @@ static function (RequestContext $context): string { ); if ($signed) { - $builder->setRequestState(str_repeat('k', 32)); + $builder->setRequestState($key ?? str_repeat('k', 32)); } return $builder->buildStateless([ProtocolVersion::V2026_07_28]); @@ -650,6 +653,29 @@ public function testNamedAsksCarryAcrossRounds(): void $this->assertSame('morning/aisle', $third['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 {