Adjust relying party verification

src/AppIdTrait.php

@@ -25,4 +25,12 @@ public function getApplicationParameter(): string
  {
  return hash('sha256', $this->appId, true);
  }
+
+ /**
+ * @return string The raw SHA-256 hash of the Relying Party ID
+ */
+ public function getRpIdHash(): string
+ {
+ return hash('sha256', $this->appId, true);
+ }
 }

src/SecurityException.php

@@ -12,6 +12,7 @@ class SecurityException extends Exception
  const CHALLENGE_MISMATCH = 3;
  const KEY_HANDLE_UNRECOGNIZED = 4;
  const NO_TRUSTED_CA = 5;
+ const WRONG_RELYING_PARTY = 6;
 
  const MESSAGES = [
  self::SIGNATURE_INVALID => 'Signature verification failed',
@@ -23,6 +24,7 @@ class SecurityException extends Exception
  self::CHALLENGE_MISMATCH => 'Response challenge does not match request',
  self::KEY_HANDLE_UNRECOGNIZED => 'Key handle has not been registered',
  self::NO_TRUSTED_CA => 'The attestation certificate was not signed by any trusted Certificate Authority',
+ self::WRONG_RELYING_PARTY => 'Relying party invalid for this server',
  ];
 
  public function __construct(int $code)

src/Server.php

@@ -196,15 +196,7 @@ public function register(RegistrationResponseInterface $response): RegistrationI
  }
  $this->validateChallenge($response->getChallengeProvider(), $this->registerRequest);
  // Check the Application Parameter
- // Note: this is a bit delicate at the moment, since different
- // protocols have different rules around the handling of Relying Party
- // verification. Expect this to be revised.
- if (!hash_equals(
- $this->registerRequest->getApplicationParameter(),
- $response->getRpIdHash()
- )) {
- throw new SE(SE::SIGNATURE_INVALID);
- }
+ $this->validateRelyingParty($response->getRpIdHash());
 
  if ($this->verifyCA) {
  $this->verifyAttestationCertAgainstTrustedCAs($response);
@@ -380,6 +372,15 @@ private function generateChallenge(): string
  return toBase64Web(\random_bytes(16));
  }
 
+ private function validateRelyingParty(string $rpIdHash): void
+ {
+ // Note: this is a bit delicate at the moment, since different
+ // protocols have different rules around the handling of Relying Party
+ // verification. Expect this to be revised.
+ if (!hash_equals($this->getRpIdHash(), $rpIdHash)) {
+ throw new SE(SE::WRONG_RELYING_PARTY);
+ }
+ }
  /**
  * Compares the Challenge value from a known source against the
  * user-provided value. A mismatch will throw a SE. Future

tests/AppIdTraitTest.php

@@ -50,4 +50,21 @@ public function testGetApplicationParameter()
  'getApplicationParamter should return the raw SHA256 hash of the application id'
  );
  }
+
+ /**
+ * @covers ::getRpIdHash
+ */
+ public function testGetRpIdHash()
+ {
+ $obj = new class {
+ use AppIdTrait;
+ };
+ $appId = 'https://u2f.example.com';
+ $obj->setAppId($appId);
+ $this->assertSame(
+ hash('sha256', $appId, true),
+ $obj->getRpIdHash(),
+ 'getRpIdHash should return the raw SHA256 hash of the application id'
+ );
+ }
 }

tests/ClientDataTest.php

@@ -10,7 +10,6 @@
  */
 class ClientDataTest extends \PHPUnit\Framework\TestCase
 {
-
  /**
  * @covers ::fromJson
  */

tests/ServerTest.php

@@ -12,7 +12,7 @@
  */
 class ServerTest extends \PHPUnit\Framework\TestCase
 {
- const APP_ID = 'https://u2f.example.com';
+ const APP_ID = 'https://u2f.ericstern.com';
 
  const ENCODED_KEY_HANDLE =
  'JUnVTStPn-V2-bCu0RlvPbukBpHTD5Mi1ZGglDOcN0vD45rnTD0BXdkRt78huTwJ7tVax'.
@@ -326,14 +326,19 @@ public function testRegisterWorksWithCAList()
  */
  public function testRegisterThrowsWithChangedApplicationParameter()
  {
- $request = (new RegisterRequest())
- ->setAppId('https://not.my.u2f.example.com')
- ->setChallenge('PfsWR1Umy2V5Al1Bam2tG0yfPLeJElfwRzzAzkYPgzo');
+ $request = $this->getDefaultRegisterRequest();
 
- $response = $this->getDefaultRegisterResponse();
+ $challengeProvider = $this->createMock(ChallengeProvider::class);
+ $challengeProvider->method('getChallenge')
+ ->willReturn($request->getChallenge());
+ $response = $this->createMock(RegistrationResponseInterface::class);
+ $response->method('getChallengeProvider')
+ ->willReturn($challengeProvider);
+ $response->method('getRpIdHash')
+ ->willReturn(hash('sha256', 'https://some.otherdomain.com', true));
 
  $this->expectException(SecurityException::class);
- $this->expectExceptionCode(SecurityException::SIGNATURE_INVALID);
+ $this->expectExceptionCode(SecurityException::WRONG_RELYING_PARTY);
  $this->server
  ->setRegisterRequest($request)
  ->register($response);
@@ -349,7 +354,7 @@ public function testRegisterThrowsWithChangedChallengeParameter()
  $data = $this->readJsonFile('register_response.json');
  $cli = fromBase64Web($data['clientData']);
  $obj = json_decode($cli, true);
- $obj['origin'] = 'https://not.my.u2f.example.com';
+ $obj['cid_pubkey'] = 'nonsense';
  $cli = toBase64Web($this->safeEncode($obj));
  $data['clientData'] = $cli;
  $response = RegisterResponse::fromJson($this->safeEncode($data));