From ef12eb8991f6fdd963e6a76c93517e1ec069baf1 Mon Sep 17 00:00:00 2001
From: Eric Stern <eric@ericstern.com>
Date: Sat, 9 Mar 2024 17:06:30 -0800
Subject: [PATCH] Verify P-256 keys are on the curve (#77)

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
---
 .github/workflows/test.yml            |  2 ++
 composer.json                         |  1 +
 src/COSE/Curve.php                    | 35 +++++++++++++++++++++++++--
 src/PublicKey/EllipticCurve.php       | 33 +++++++++++++++++++++++++
 tests/PublicKey/EllipticCurveTest.php |  1 +
 5 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index e6c4625..a99f8bc 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -80,3 +80,5 @@ jobs:
       - name: Submit code coverage
         if: ${{ always() }}
         uses: codecov/codecov-action@v4
+        with:
+          token: ${{ secrets.CODECOV_TOKEN }}
diff --git a/composer.json b/composer.json
index e6de990..86240e9 100644
--- a/composer.json
+++ b/composer.json
@@ -29,6 +29,7 @@
     },
     "require": {
         "php": "^8.1",
+        "ext-gmp": "*",
         "ext-hash": "*",
         "ext-openssl": "*",
         "firehed/cbor": "^0.1.0"
diff --git a/src/COSE/Curve.php b/src/COSE/Curve.php
index 1e4bb98..7ccc2c6 100644
--- a/src/COSE/Curve.php
+++ b/src/COSE/Curve.php
@@ -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
@@ -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'),
         };
     }
 }
diff --git a/src/PublicKey/EllipticCurve.php b/src/PublicKey/EllipticCurve.php
index 4c853b8..1af49a8 100644
--- a/src/PublicKey/EllipticCurve.php
+++ b/src/PublicKey/EllipticCurve.php
@@ -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
  *
@@ -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');
+        }
     }
 
     /**
@@ -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
+    }
 }
diff --git a/tests/PublicKey/EllipticCurveTest.php b/tests/PublicKey/EllipticCurveTest.php
index f61b241..dc07313 100644
--- a/tests/PublicKey/EllipticCurveTest.php
+++ b/tests/PublicKey/EllipticCurveTest.php
@@ -9,6 +9,7 @@
 
 /**
  * @covers Firehed\WebAuthn\PublicKey\EllipticCurve
+ * @covers Firehed\WebAuthn\COSE\Curve
  */
 class EllipticCurveTest extends \PHPUnit\Framework\TestCase
 {