Fix authtoken signature validation (#10)

* Transcode signature only if it isn't in ASN1 format already + add unit test

* Throw exception when openssl_verify returned -1 (error)

* Reverse formatting

* Use internal values for test

Author: Muzosh

src/util/AsnUtil.php

@@ -34,6 +34,23 @@ public function __construct()
         throw new BadFunctionCallException("Utility class");
     }
 
+    public static function isSignatureInAsn1Format(string $signature): bool
+    {
+        $sigByteArray = unpack('C*', $signature);
+
+        // ASN.1 format: 0x30 b1 0x02 b2 r 0x02 b3 s.
+        // Note: unpack() returns an array indexed from 1, not 0.
+        $b1 = $sigByteArray[2];
+        $b2 = $sigByteArray[4];
+        $b3 = $sigByteArray[6 + $b2];
+
+        return $sigByteArray[1] == 0x30 // Sequence tag
+            && $sigByteArray[3] == 0x02 // First integer tag
+            && $sigByteArray[5 + $b2] == 0x02 // Second integer tag
+            && count($sigByteArray) == 2 + $b1 // Length of contents
+            && count($sigByteArray) == 6 + $b2 + $b3; // Total length
+    }
+
     public static function transcodeSignatureToDER(string $p1363): string
     {
         // P1363 format: r followed by s.

src/validator/AuthTokenSignatureValidator.php

@@ -72,7 +72,7 @@ public function validate(string $algorithm, string $signature, $publicKey, strin
         $decodedSignature = base64_decode($signature);
 
         // Note that in case of ECDSA, some eID cards output raw R||S, so we need to trascode it to DER
-        if (in_array($algorithm, ["ES256", "ES384", "ES512"])) {
+        if (in_array($algorithm, ["ES256", "ES384", "ES512"]) && !AsnUtil::isSignatureInAsn1Format($decodedSignature)) {
             $decodedSignature = AsnUtil::transcodeSignatureToDER($decodedSignature);
         }
 
@@ -83,8 +83,8 @@ public function validate(string $algorithm, string $signature, $publicKey, strin
         $concatSignedFields = $originHash . $nonceHash;
 
         $result = openssl_verify($concatSignedFields, $decodedSignature, $publicKey, $hashAlgorithm);
-        if (!$result) {
-            throw new AuthTokenSignatureValidationException();
+        if ($result !== 1) {
+            throw new AuthTokenSignatureValidationException($result === -1 ? openssl_error_string() : "Signature is invalid");
         }
     }
 

tests/validator/AuthTokenSignatureValidatorTest.php

@@ -30,7 +30,9 @@
 use GuzzleHttp\Psr7\Uri;
 use web_eid\web_eid_authtoken_validation_php\authtoken\WebEidAuthToken;
 use web_eid\web_eid_authtoken_validation_php\testutil\AbstractTestWithValidator;
+use phpseclib3\Crypt\EC;
 use phpseclib3\File\X509;
+use web_eid\web_eid_authtoken_validation_php\util\AsnUtil;
 
 class AuthTokenSignatureValidatorTest extends TestCase
 {
@@ -63,4 +65,19 @@ public function testWhenValidRS256SignatureThenSucceeds(): void
 
         $signatureValidator->validate("RS256", $authToken->getSignature(), $x509Certificate->getPublicKey(), AbstractTestWithValidator::VALID_CHALLENGE_NONCE);
     }
+
+    public function testESSignatureDecoding(): void
+    {
+        $this->expectNotToPerformAssertions();
+        $signatureValidator = new AuthTokenSignatureValidator(new Uri("https://ria.ee"));
+
+        $authToken = new WebEidAuthToken(AbstractTestWithValidator::VALID_AUTH_TOKEN);
+        $x509Certificate = new X509();
+        $x509Certificate->loadX509($authToken->getUnverifiedCertificate());
+
+        $asn1Signature = base64_encode(AsnUtil::transcodeSignatureToDER(base64_decode($authToken->getSignature())));
+
+        $signatureValidator->validate("ES384", $authToken->getSignature(), $x509Certificate->getPublicKey(), AbstractTestWithValidator::VALID_CHALLENGE_NONCE);
+        $signatureValidator->validate("ES384", $asn1Signature, $x509Certificate->getPublicKey(), AbstractTestWithValidator::VALID_CHALLENGE_NONCE);
+    }
 }