Resolving issues from the OCSP repository and harmonizing code with Java library

example/src/Auth.php

@@ -88,14 +88,13 @@ public function getNonce()
 
     private function getPrincipalNameFromCertificate(X509 $userCertificate): string
     {
+        $givenName = CertificateData::getSubjectGivenName($userCertificate);
         $surname = CertificateData::getSubjectSurname($userCertificate);
-        $givenname = CertificateData::getSubjectGivenName($userCertificate);
-        if ($surname && $givenname) {
-            $principalName = $givenname . " " . $surname;
+        if ($givenName && $surname) {
+            return $givenName . " " . $surname;
         } else {
-            $principalName = CertificateData::getSubjectCN($userCertificate);
+            return CertificateData::getSubjectCN($userCertificate);
         }
-        return $principalName;
     }
 
     /**

example/tpl/welcome.phtml

@@ -1,2 +1,2 @@
-<h1 class="text-center mt-5">Hello <?php echo $auth_user;?></h1>
+<h1 class="text-center mt-5">Hello <?php echo htmlspecialchars($auth_user);?></h1>
 <p class="text-center"><a href="./logout">Logout</a></p>

src/authtoken/WebEidAuthToken.php

@@ -48,10 +48,6 @@ class WebEidAuthToken
      * @var string Format
      */
     private ?string $format = null;
-    /**
-     * @var string App version
-     */
-    private ?string $appVersion = null;
 
     public function __construct(string $authenticationTokenJSON)
     {
@@ -76,10 +72,6 @@ public function __construct(string $authenticationTokenJSON)
         if (isset($jsonDecoded['format'])) {
             $this->format = $this->filterString('format', $jsonDecoded['format']);
         }
-        // appVersion
-        if (isset($jsonDecoded['appVersion'])) {
-            $this->appVersion = $this->filterString('appVersion', $jsonDecoded['appVersion']);
-        }
     }
 
     public function getUnverifiedCertificate(): ?string
@@ -102,11 +94,6 @@ public function getFormat(): ?string
         return $this->format;
     }
 
-    public function getAppVersion(): ?string
-    {
-        return $this->appVersion;
-    }
-
     private function filterString(string $key, $data): string
     {
         $type = gettype($data);

src/certificate/CertificateData.php

@@ -80,15 +80,14 @@ public static function getSubjectCountryCode(X509 $certificate): ?string
     /**
      * Get specified subject field from x509 certificate
      *
-     * @return string
+     * @return ?string
      */
     private static function getField(X509 $certificate, string $fieldId): ?string
     {
         $result = $certificate->getSubjectDNProp($fieldId);
         if ($result) {
-            return join(" ", $result);
-        }
-        else {
+            return join(", ", $result);
+        } else {
             return null;
         }
     }

src/ocsp/OcspRequest.php

@@ -32,7 +32,7 @@
 
 class OcspRequest
 {
-    private array $ocspRequest = [];
+    private array $ocspRequest;
 
     public function __construct()
     {
@@ -41,8 +41,6 @@ public function __construct()
         $this->ocspRequest = [
             "tbsRequest" => [
                 "version" => "v1",
-                "requestList" => [],
-                "requestExtensions" => [],
             ],
         ];
     }
@@ -62,18 +60,15 @@ public function addNonceExtension(string $nonce): void
             "critical" => false,
             "extnValue" => ASN1::encodeDER($nonce, ['type' => ASN1::TYPE_OCTET_STRING]),
         ];
-        $this->ocspRequest["tbsRequest"][
-            "requestExtensions"
-        ][] = $nonceExtension;
+        $this->ocspRequest["tbsRequest"]["requestExtensions"][] = $nonceExtension;
     }
 
     /**
      * @copyright 2022 Petr Muzikant pmuzikant@email.cz
      */
-    public function getNonceExtension(): string
+    public function getNonceExtension(): ?string
     {
-        // TODO: the ?? '' is here only for v1.0 API compatibility. Remove this in version 1.2 and change the return type to ?string.
-        return AsnUtil::decodeNonceExtension($this->ocspRequest["tbsRequest"]["requestExtensions"]) ?? '';
+        return AsnUtil::decodeNonceExtension($this->ocspRequest["tbsRequest"]["requestExtensions"] ?? []);
     }
 
     public function getEncodeDer(): string

src/validator/AuthTokenSignatureValidator.php

@@ -54,19 +54,19 @@ public function __construct(Uri $siteOrigin)
 
     public function validate(string $algorithm, string $signature, $publicKey, string $currentChallengeNonce): void
     {
-        $this->requireNotEmpty($algorithm, "algorithm");
-        $this->requireNotEmpty($signature, "signature");
+        if (empty($currentChallengeNonce)) {
+            throw new ChallengeNullOrEmptyException();
+        }
 
         if (is_null($publicKey)) {
             throw new InvalidArgumentException("Public key is null");
         }
 
-        if (empty($currentChallengeNonce)) {
-            throw new ChallengeNullOrEmptyException();
-        }
+        $this->requireNotEmpty($algorithm, "algorithm");
+        $this->requireNotEmpty($signature, "signature");
 
         if (!in_array($algorithm, self::ALLOWED_SIGNATURE_ALGORITHMS)) {
-            throw new AuthTokenParseException("Invalid signature algorithm");
+            throw new AuthTokenParseException("Unsupported signature algorithm");
         }
 
         $decodedSignature = base64_decode($signature);

src/validator/ocsp/OcspRequestBuilder.php

@@ -26,6 +26,7 @@
 
 namespace web_eid\web_eid_authtoken_validation_php\validator\ocsp;
 
+use InvalidArgumentException;
 use web_eid\web_eid_authtoken_validation_php\ocsp\OcspRequest;
 use web_eid\web_eid_authtoken_validation_php\util\SecureRandom;
 
@@ -58,6 +59,9 @@ public function enableOcspNonce(bool $ocspNonceEnabled): OcspRequestBuilder
     public function build(): OcspRequest
     {
         $ocspRequest = new OcspRequest();
+        if (is_null($this->certificateId)) {
+            throw new InvalidArgumentException("Certificate Id must not be null");
+        }
         $ocspRequest->addCertificateId($this->certificateId);
 
         if ($this->ocspNonceEnabled) {

src/validator/ocsp/OcspServiceProvider.php

@@ -26,6 +26,7 @@
 
 namespace web_eid\web_eid_authtoken_validation_php\validator\ocsp;
 
+use InvalidArgumentException;
 use phpseclib3\File\X509;
 use web_eid\web_eid_authtoken_validation_php\validator\ocsp\service\AiaOcspService;
 use web_eid\web_eid_authtoken_validation_php\validator\ocsp\service\AiaOcspServiceConfiguration;
@@ -41,7 +42,8 @@ class OcspServiceProvider
     public function __construct(?DesignatedOcspServiceConfiguration $designatedOcspServiceConfiguration, AiaOcspServiceConfiguration $aiaOcspServiceConfiguration)
     {
         $this->designatedOcspService = !is_null($designatedOcspServiceConfiguration) ? new DesignatedOcspService($designatedOcspServiceConfiguration) : null;
-        $this->aiaOcspServiceConfiguration = $aiaOcspServiceConfiguration;
+        $this->aiaOcspServiceConfiguration = $aiaOcspServiceConfiguration ?? throw new InvalidArgumentException("AIA Ocsp Service Configuration must not be null");
+
     }
 
     /**

src/validator/ocsp/OcspUrl.php

@@ -28,6 +28,7 @@
 use phpseclib3\File\X509;
 use GuzzleHttp\Psr7\Uri;
 use Exception;
+use InvalidArgumentException;
 
 final class OcspUrl
 {
@@ -43,6 +44,9 @@ public function __construct()
      */
     public static function getOcspUri(X509 $certificate): ?Uri
     {
+        if (is_null($certificate)) {
+            throw new InvalidArgumentException("Certificate must not be null");
+        }
         $authorityInformationAccess = $certificate->getExtension("id-pe-authorityInfoAccess");
         if ($authorityInformationAccess) {
             foreach ($authorityInformationAccess as $accessDescription) {

src/validator/ocsp/service/AiaOcspService.php

@@ -35,6 +35,7 @@
 use DateTime;
 use web_eid\web_eid_authtoken_validation_php\validator\ocsp\OcspResponseValidator;
 use Exception;
+use InvalidArgumentException;
 
 /**
  * An OCSP service that uses the responders from the Certificates' Authority Information Access (AIA) extension.
@@ -48,6 +49,9 @@ class AiaOcspService implements OcspService
 
     public function __construct(AiaOcspServiceConfiguration $configuration, X509 $certificate)
     {
+        if (is_null($configuration)) {
+            throw new InvalidArgumentException("Configuration cannot be null");
+        }
         $this->url = self::getOcspAiaUrlFromCertificate($certificate);
         $this->trustedCACertificates = $configuration->getTrustedCACertificates();
         $this->supportsNonce = !in_array($this->url->jsonSerialize(), $configuration->getNonceDisabledOcspUrls()->getUrlsArray());

tests/authtoken/WebEidAuthTokenTest.php

@@ -48,7 +48,6 @@ public function testValidateAuthTokenParameters(): void
         $this->assertEquals("RS256", $authToken->getAlgorithm());
         $this->assertEquals("HBjNXIaUskXbfhzYQHvwjKDUWfNu4yxXZh", $authToken->getSignature());
         $this->assertEquals("web-eid:1.0", $authToken->getFormat());
-        $this->assertEquals("https://web-eid.eu/web-eid-app/releases/v2.0.0", $authToken->getAppVersion());
     }
 
     public function testWhenNotAuthToken(): void

tests/certificate/CertificateDataTest.php

@@ -49,25 +49,14 @@ public function testWhenOrganizationCertificateThenSubjectCNAndIdCodeAndCountryC
         $this->assertEquals("EE", CertificateData::getSubjectCountryCode($cert));
     }
 
-    public function testWhenOrganizationCertificateThenSubjectGivenNameAndSurnameAreNull(): void
-    {
-        $cert = Certificates::getOrganizationCert();
-        $this->assertEquals(null, CertificateData::getSubjectGivenName($cert));
-        $this->assertEquals(null, CertificateData::getSubjectSurname($cert));
-    }
-
-    public function testWhenOrganizationCertificateThenSucceeds(): void
+    public function testWhenOrganizationCertificateThenSubjectGivenNameAndSurnameAreEmptyAndSubjectCNSucceeds(): void
     {
         $cert = Certificates::getOrganizationCert();
+        $givenName = CertificateData::getSubjectGivenName($cert);
         $surname = CertificateData::getSubjectSurname($cert);
-        $givenname = CertificateData::getSubjectGivenName($cert);
-        if ($surname && $givenname) {
-            $principalName = $givenname . " " . $surname;
-        } else {
-            $principalName = CertificateData::getSubjectCN($cert);
-        }
-
+        $this->assertEmpty($givenName);
+        $this->assertEmpty($surname);
+        $principalName = CertificateData::getSubjectCN($cert);
         $this->assertEquals("Testijad.ee isikutuvastus", $principalName);
-
     }
 }

tests/ocsp/OcspRequestTest.php

@@ -39,8 +39,6 @@ private function getRequest(): array
         return [
             'tbsRequest' => [
                 'version' => 'v1',
-                'requestList' => [],
-                'requestExtensions' => [],
             ],
         ];
     }
@@ -101,4 +99,29 @@ public function testWhenGetNonceExtensionSuccess(): void
 
         $this->assertEquals("nonce", $request->getNonceExtension());
     }
+
+    public function testBinaryNonce(): void
+    {
+        // Create a nonce with a mixture of potentially problematic bytes,
+        // null bytes, control characters and high-byte values (above 0x7F),
+        // which are common sources of string decoding problems.
+        $nonce = "\0\1\2\3\4\5\6\7\x08\x09\x10\0"
+                       . "\x0A\x0D\x1B"
+                       . "\xE2\x82\xAC"
+                       . "\xFF";
+        $request = new OcspRequest();
+        $request->addNonceExtension($nonce);
+
+        $this->assertEquals($nonce, $request->getNonceExtension());
+    }
+
+    public function testOidNonce(): void
+    {
+        // Create a nonce that contains DER-encoded OID for SHA-256: 2.16.840.1.101.3.4.2.1.
+        $nonce = "\x06\x09\x60\x86\x48\x01\x65\x03\x04\x02\x01";
+        $request = new OcspRequest();
+        $request->addNonceExtension($nonce);
+
+        $this->assertEquals($nonce, $request->getNonceExtension());
+    }
 }

tests/validator/AuthTokenAlgorithmTest.php

@@ -35,7 +35,7 @@ public function testWhenAlgorithmNoneThenValidationFails(): void
         $authToken = $this->replaceTokenField(self::VALID_AUTH_TOKEN, "algorithm", "NONE");
 
         $this->expectException(AuthTokenParseException::class);
-        $this->expectExceptionMessage("Invalid signature algorithm");
+        $this->expectExceptionMessage("Unsupported signature algorithm");
         $this->validator->validate($authToken, self::VALID_AUTH_TOKEN);
     }
 
@@ -53,7 +53,7 @@ public function testWhenAlgorithmInvalidThenParsingFails(): void
         $authToken = $this->replaceTokenField(self::VALID_AUTH_TOKEN, "algorithm", "\\u0000\\t\\ninvalid");
 
         $this->expectException(AuthTokenParseException::class);
-        $this->expectExceptionMessage("Invalid signature algorithm");
+        $this->expectExceptionMessage("Unsupported signature algorithm");
         $this->validator->validate($authToken, self::VALID_AUTH_TOKEN);
     }
 }