Skip to content

Commit 6024cf4

Browse files
nficanocursoragent
andcommitted
fix: harden auth schemes (server-assigned principal, opaque jwt errors) (#67 #71 #72)
NoneAuth now always returns the configured default principal and ignores the untrusted PeerInfo principal, closing a per-principal isolation bypass. JwtAuth returns an opaque rejection reason and logs the underlying decode error server-side so trust material is not leaked to the peer. AuthRouter's reserved-scheme list is hoisted to a named constant. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 83f6138 commit 6024cf4

4 files changed

Lines changed: 29 additions & 5 deletions

File tree

src/Auth/AuthRouter.php

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@
1515
*/
1616
final class AuthRouter
1717
{
18+
/**
19+
* Schemes reserved by RFC §8.2 but not yet implemented; presenting one
20+
* surfaces UNIMPLEMENTED rather than a generic unknown-scheme rejection.
21+
*
22+
* @var list<string>
23+
*/
24+
private const array RESERVED_SCHEMES = ['mtls', 'oauth2'];
25+
1826
/** @var array<string, AuthScheme> */
1927
private array $schemes = [];
2028

@@ -35,7 +43,7 @@ public function verify(Auth $auth, PeerInfo $client): AuthResult
3543
{
3644
if (!isset($this->schemes[$auth->scheme])) {
3745
// mTLS and OAuth2 are reserved (RFC §8.2) but unimplemented in v0.1.
38-
if (\in_array($auth->scheme, ['mtls', 'oauth2'], true)) {
46+
if (\in_array($auth->scheme, self::RESERVED_SCHEMES, true)) {
3947
throw new UnimplementedException(
4048
'§8.2',
4149
\sprintf('auth scheme %s deferred to v0.2', $auth->scheme),

src/Auth/JwtAuth.php

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
use Arcp\Messages\Session\PeerInfo;
99
use Firebase\JWT\JWT;
1010
use Firebase\JWT\Key;
11+
use Psr\Log\LoggerInterface;
12+
use Psr\Log\NullLogger;
1113

1214
/**
1315
* `signed_jwt` verification (RFC §8.2). The runtime supplies the trust
@@ -16,10 +18,14 @@
1618
*/
1719
final readonly class JwtAuth implements AuthScheme
1820
{
21+
private LoggerInterface $logger;
22+
1923
public function __construct(
2024
private Key $key,
2125
private string $audience,
26+
?LoggerInterface $logger = null,
2227
) {
28+
$this->logger = $logger ?? new NullLogger();
2329
}
2430

2531
#[\Override]
@@ -40,7 +46,12 @@ public function verify(Auth $auth, PeerInfo $client): AuthResult
4046
try {
4147
$decoded = JWT::decode($auth->token, $this->key);
4248
} catch (\Throwable $e) {
43-
return AuthResult::reject('jwt verification failed: ' . $e->getMessage());
49+
// Keep the wire reason opaque: the underlying decode error can
50+
// leak key id, algorithm, or expiry details useful to an
51+
// attacker probing trust material. Log it server-side instead.
52+
$this->logger->info('jwt verification failed', ['error' => $e->getMessage()]);
53+
54+
return AuthResult::reject('jwt verification failed');
4455
}
4556
$claims = (array) $decoded;
4657
$aud = $claims['aud'] ?? null;

src/Auth/NoneAuth.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@ public function verify(Auth $auth, PeerInfo $client): AuthResult
2929
if ($auth->scheme !== 'none') {
3030
return AuthResult::reject('scheme mismatch');
3131
}
32-
return AuthResult::accept($client->principal ?? $this->defaultPrincipal);
32+
// Always use the configured default principal; do not trust the
33+
// principal supplied in the untrusted PeerInfo block (mirrors the
34+
// BearerAuth contract). Trusting it would let any anonymous peer
35+
// claim an arbitrary principal and bypass per-principal isolation.
36+
return AuthResult::accept($this->defaultPrincipal);
3337
}
3438
}

tests/Unit/AuthTest.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,12 @@ public function testNoneAccepts(): void
6767
self::assertSame('public', $result->principal);
6868
}
6969

70-
public function testNoneUsesProvidedPrincipal(): void
70+
public function testNoneIgnoresPeerSuppliedPrincipal(): void
7171
{
7272
$scheme = new NoneAuth('public');
7373
$result = $scheme->verify(Auth::none(), new PeerInfo('c', '0', principal: 'someone@x'));
74-
self::assertSame('someone@x', $result->principal);
74+
// The peer-supplied principal must not be trusted (#67).
75+
self::assertSame('public', $result->principal);
7576
}
7677

7778
public function testNoneRejectsWrongScheme(): void

0 commit comments

Comments
 (0)