Skip to content

Commit

Permalink
Verify P-256 keys are on the curve (#77)
Browse files Browse the repository at this point in the history
I got some AI-assisted guidance on the implementation, but
cross-referenced several documents that specify and confirm the curve
parameters. The actual verification procedure is pretty straightforward
- do the systems-of-equations with basic algebra, and the apply the
modulus since that's how finite fields work.

Fixes #67
  • Loading branch information
Firehed authored Mar 10, 2024
1 parent 7584b95 commit ef12eb8
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 2 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,5 @@ jobs:
- name: Submit code coverage
if: ${{ always() }}
uses: codecov/codecov-action@v4
with:
token: ${{ secrets.CODECOV_TOKEN }}
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
},
"require": {
"php": "^8.1",
"ext-gmp": "*",
"ext-hash": "*",
"ext-openssl": "*",
"firehed/cbor": "^0.1.0"
Expand Down
35 changes: 33 additions & 2 deletions src/COSE/Curve.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@

namespace Firehed\WebAuthn\COSE;

use GMP;
use UnhandledMatchError;

use function gmp_init;

/**
* @link https://www.rfc-editor.org/rfc/rfc9053.html
* @see §7.1, table 18
Expand Down Expand Up @@ -33,9 +38,35 @@ enum Curve: int

public function getOid(): string
{
return match ($this) { // @phpstan-ignore-line default unhandled match is desired
return match ($this) {
self::P256 => '1.2.840.10045.3.1.7',
// TODO: add others as support increases
default => throw new UnhandledMatchError('Curve unsupported'),
};
}

// Curve parameters:
// https://www.secg.org/sec2-v2.pdf
public function getA(): GMP
{
return match ($this) {
self::P256 => gmp_init('0xFFFFFFFF 00000001 00000000 00000000 00000000 FFFFFFFF FFFFFFFF FFFFFFFC'),
default => throw new UnhandledMatchError('Curve unsupported'),
};
}

public function getB(): GMP
{
return match ($this) {
self::P256 => gmp_init('0x5AC635D8 AA3A93E7 B3EBBD55 769886BC 651D06B0 CC53B0F6 3BCE3C3E 27D2604B'),
default => throw new UnhandledMatchError('Curve unsupported'),
};
}

public function getP(): GMP
{
return match ($this) {
self::P256 => gmp_init('0xFFFFFFFF 00000001 00000000 00000000 00000000 FFFFFFFF FFFFFFFF FFFFFFFF'),
default => throw new UnhandledMatchError('Curve unsupported'),
};
}
}
33 changes: 33 additions & 0 deletions src/PublicKey/EllipticCurve.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,16 @@
use Firehed\WebAuthn\BinaryString;
use Firehed\WebAuthn\COSE;
use Firehed\WebAuthn\COSEKey;
use Firehed\WebAuthn\Errors\VerificationError;
use UnexpectedValueException;

use function gmp_pow;
use function gmp_add;
use function gmp_cmp;
use function gmp_import;
use function gmp_mod;
use function gmp_mul;

/**
* @internal
*
Expand Down Expand Up @@ -42,6 +50,9 @@ public function __construct(
if ($y->getLength() !== 32) {
throw new UnexpectedValueException('Y-coordinate not 32 bytes');
}
if (!$this->isOnCurve()) {
throw new VerificationError('5.8.5', 'Point not on curve');
}
}

/**
Expand Down Expand Up @@ -132,4 +143,26 @@ public function getPemFormatted(): string
$pem .= "-----END PUBLIC KEY-----";
return $pem;
}

private function isOnCurve(): bool
{
// The curve E: y^2 = x^3 + ax + b over Fp is defined by:
$a = $this->curve->getA();
$b = $this->curve->getB();
$p = $this->curve->getP();

$x = gmp_import($this->x->unwrap());
$y = gmp_import($this->y->unwrap());

// This is only tested with P256 (secp256r1) but SHOULD be the same for
// the other curves (none of which are supported yet)/
$x3 = gmp_pow($x, 3);
$ax = gmp_mul($a, $x);
$rhs = gmp_mod(gmp_add($x3, gmp_add($ax, $b)), $p);

$y2 = gmp_pow($y, 2);
$lhs = gmp_mod($y2, $p);

return 0 === gmp_cmp($lhs, $rhs); // Values match
}
}
1 change: 1 addition & 0 deletions tests/PublicKey/EllipticCurveTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

/**
* @covers Firehed\WebAuthn\PublicKey\EllipticCurve
* @covers Firehed\WebAuthn\COSE\Curve
*/
class EllipticCurveTest extends \PHPUnit\Framework\TestCase
{
Expand Down

0 comments on commit ef12eb8

Please sign in to comment.