Avoid redundant ASN.1 re-decoding and re-encoding in OCSP validation

src/ocsp/OcspBasicResponse.php

@@ -37,6 +37,7 @@
 class OcspBasicResponse
 {
     private array $ocspBasicResponse = [];
+    private ?array $certificates = null;
 
     public function __construct(array $ocspBasicResponse)
     {
@@ -53,6 +54,10 @@ public function getResponses(): array
      */
     public function getCertificates(): array
     {
+        if ($this->certificates !== null) {
+            return $this->certificates;
+        }
+
         $certificatesArr = [];
         if (isset($this->ocspBasicResponse["certs"])) {
             foreach ($this->ocspBasicResponse["certs"] as $cert) {
@@ -69,7 +74,7 @@ public function getCertificates(): array
             unset($x509);
         }
 
-        return $certificatesArr;
+        return $this->certificates = $certificatesArr;
     }
 
     public function getSignature(): string

src/ocsp/OcspResponse.php

@@ -37,6 +37,7 @@ class OcspResponse
 {
     private array $ocspResponse = [];
     private string $revokeReason = "";
+    private ?OcspBasicResponse $basicResponse = null;
 
     public function __construct(string $encodedBER)
     {
@@ -59,6 +60,10 @@ public function getResponse(): array
 
     public function getBasicResponse(): OcspBasicResponse
     {
+        if ($this->basicResponse !== null) {
+            return $this->basicResponse;
+        }
+
         if (
             Ocsp::ID_PKIX_OCSP_BASIC_STRING !=
             $this->ocspResponse["responseBytes"]["responseType"]
@@ -75,7 +80,7 @@ public function getBasicResponse(): OcspBasicResponse
             );
         }
 
-        return new OcspBasicResponse(
+        return $this->basicResponse = new OcspBasicResponse(
             $this->ocspResponse["responseBytes"]["response"]
         );
     }
@@ -111,37 +116,6 @@ public function isRevoked()
         return null;
     }
 
-    public function validateSignature(): void
-    {
-        $basicResponse = $this->getBasicResponse();
-        $this->validateResponse($basicResponse);
-
-        $responderCert = $basicResponse->getCertificates()[0];
-        // get public key from responder certificate in order to verify signature on response
-        $publicKey = $responderCert
-            ->getPublicKey()
-            ->withHash($basicResponse->getSignatureAlgorithm());
-        // verify response data
-        $encodedTbsResponseData = $basicResponse->getEncodedResponseData();
-        $signature = $basicResponse->getSignature();
-
-        if (!$publicKey->verify($encodedTbsResponseData, $signature)) {
-            throw new OcspVerifyFailedException(
-                "OCSP response signature is not valid"
-            );
-        }
-    }
-
-    public function validateCertificateId(array $requestCertificateId): void
-    {
-        $basicResponse = $this->getBasicResponse();
-        if ($requestCertificateId != $basicResponse->getCertID()) {
-            throw new OcspVerifyFailedException(
-                "OCSP responded with certificate ID that differs from the requested ID"
-            );
-        }
-    }
-
     private function validateResponse(OcspBasicResponse $basicResponse): void
     {
         // Must be one response

src/ocsp/certificate/CertificateLoader.php

@@ -85,40 +85,6 @@ public function fromString(string $certString)
         return $this;
     }
 
-    public function getIssuerCertificateUrl(): string
-    {
-        if (!$this->certificate) {
-            throw new OcspCertificateException("Certificate not loaded");
-        }
-
-        $url = "";
-        $opts = $this->certificate->getExtension("id-pe-authorityInfoAccess");
-        foreach ($opts as $opt) {
-            if ($opt["accessMethod"] == "id-ad-caIssuers") {
-                $url = $opt["accessLocation"]["uniformResourceIdentifier"];
-                break;
-            }
-        }
-        return $url;
-    }
-
-    public function getOcspResponderUrl(): string
-    {
-        if (!$this->certificate) {
-            throw new OcspCertificateException("Certificate not loaded");
-        }
-
-        $url = "";
-        $opts = $this->certificate->getExtension("id-pe-authorityInfoAccess");
-        foreach ($opts as $opt) {
-            if ($opt["accessMethod"] == "id-ad-ocsp" || $opt["accessMethod"] == "id-pkix-ocsp") {
-                $url = $opt["accessLocation"]["uniformResourceIdentifier"];
-                break;
-            }
-        }
-        return $url;
-    }
-
     public function getCert(): X509
     {
         if (!$this->certificate) {

src/validator/certvalidators/SubjectCertificateNotRevokedValidator.php

@@ -123,12 +123,13 @@ private function verifyOcspResponse(OcspResponse $response, OcspService $ocspSer
         // that helps us to verify it. According to RFC 2560 this field is optional, but including it
         // is standard practice.
 
-        if (count($basicResponse->getCertificates()) < 1) {
+        $responderCertificates = $basicResponse->getCertificates();
+        if (count($responderCertificates) < 1) {
             throw new UserCertificateOCSPCheckFailedException("OCSP response must contain the responder certificate, but none was provided");
         }
 
         // The first certificate is the responder certificate, other certificates, if given, are the certificate's chain.
-        $responderCert = $basicResponse->getCertificates()[0];
+        $responderCert = $responderCertificates[0];
 
         OcspResponseValidator::validateResponseSignature($basicResponse, $responderCert);
 

src/validator/ocsp/OcspClientImpl.php

@@ -67,7 +67,7 @@ public function request(Uri $uri, string $encodedOcspRequest): OcspResponse
 
         $info = curl_getinfo($curl);
         if ($info["http_code"] !== 200) {
-            throw new UserCertificateOCSPCheckFailedException("OCSP request was not successful, response: " + $result);
+            throw new UserCertificateOCSPCheckFailedException("OCSP request was not successful, response: " . $result);
         }
 
         $response = new OcspResponse($result);

src/validator/ocsp/OcspResponseValidator.php

@@ -117,15 +117,15 @@ public static function validateCertificateStatusUpdateTime(OcspBasicResponse $ba
 
     public static function validateSubjectCertificateStatus(OcspResponse $response): void
     {
-        if (is_null($response->isRevoked())) {
+        $isRevoked = $response->isRevoked();
+
+        if (is_null($isRevoked)) {
             throw new UserCertificateRevokedException("Unknown status");
         }
-        if ($response->isRevoked() === false) {
+        if ($isRevoked === false) {
             return;
         }
-        if ($response->isRevoked() === true) {
-            throw ($response->getRevokeReason() == "") ? new UserCertificateRevokedException() : new UserCertificateRevokedException("Revocation reason: " . $response->getRevokeReason());
-        }
-        throw new UserCertificateRevokedException("Status is neither good, revoked nor unknown");
+
+        throw ($response->getRevokeReason() == "") ? new UserCertificateRevokedException() : new UserCertificateRevokedException("Revocation reason: " . $response->getRevokeReason());
     }
 }

tests/ocsp/OcspResponseTest.php

@@ -64,12 +64,6 @@ public function testWhenCertificateNotRevoked(): void
         $response = new OcspResponse(self::getOcspResponseBytesFromResources());
         $basicResponse = $response->getBasicResponse();
 
-        $mockCertificateID = $basicResponse->getResponses()[0]['certID'];
-        $mockCertificateID['hashAlgorithm']['algorithm'] = ASN1::getOID('id-sha1');
-
-        $response->validateCertificateId($mockCertificateID);
-        $response->validateSignature();
-
         $this->assertFalse($response->isRevoked());
         $this->assertEquals("successful", $response->getStatus());
         $this->assertEquals("2021-09-17 18:25:24", $basicResponse->getProducedAt()->format("Y-m-d H:i:s"));
@@ -105,21 +99,6 @@ public function testWhenTwoResponsesThenThrows(): void
         $response->isRevoked();
     }
 
-    public function testWhenCertificateIdsDoNotMatchThenThrows(): void
-    {
-        $response = new OcspResponse(self::getOcspResponseBytesFromResources());
-        $basicResponse = $response->getBasicResponse();
-
-        $mockCertificateID = $basicResponse->getResponses()[0]['certID'];
-        $mockCertificateID['issuerNameHash'] = "1234";
-        $mockCertificateID['hashAlgorithm']['algorithm'] = ASN1::getOID('id-sha1');
-
-        $this->expectException(OcspVerifyFailedException::class);
-        $this->expectExceptionMessage("OCSP responded with certificate ID that differs from the requested ID");
-
-        $response->validateCertificateId($mockCertificateID);
-    }
-
     public function testWhenResponseTypeNotBasicResponseThrows(): void
     {
 
@@ -160,10 +139,6 @@ public function testWhenNoCertificatesInResponseThrows(): void
 
         $response = new OcspResponse(self::getOcspResponseBytesFromResources());
 
-        $basicResponse = $response->getBasicResponse();
-        $mockCertificateID = $basicResponse->getResponses()[0]['certID'];
-        $mockCertificateID['hashAlgorithm']['algorithm'] = ASN1::getOID('id-sha1');
-
         $reflection = new ReflectionClass(get_class($response));
         $property = $reflection->getProperty('ocspResponse');
         $mockResponse = $property->getValue($response);
@@ -174,27 +149,6 @@ public function testWhenNoCertificatesInResponseThrows(): void
         $response->isRevoked();
     }
 
-    public function testWhenResponseSignatureIsNotValidThrows(): void
-    {
-        $this->expectException(OcspVerifyFailedException::class);
-        $this->expectExceptionMessage('OCSP response signature is not valid');
-
-        $response = new OcspResponse(self::getOcspResponseBytesFromResources());
-
-        $basicResponse = $response->getBasicResponse();
-        $mockCertificateID = $basicResponse->getResponses()[0]['certID'];
-        $mockCertificateID['hashAlgorithm']['algorithm'] = ASN1::getOID('id-sha1');
-
-        $reflection = new ReflectionClass(get_class($response));
-        $property = $reflection->getProperty('ocspResponse');
-        $mockResponse = $property->getValue($response);
-        $mockResponse['responseBytes']['response']['signature'] = "somesignature";
-
-        $property->setValue($response, $mockResponse);
-
-        $response->validateSignature();
-    }
-
     public function testWhenSignatureAlgorithmIsSha3(): void
     {
         $response = new OcspResponse(self::getOcspResponseBytesFromResources());

tests/ocsp/certificate/CertificateLoaderTest.php

@@ -26,7 +26,6 @@
 
 use PHPUnit\Framework\TestCase;
 use web_eid\web_eid_authtoken_validation_php\ocsp\exceptions\OcspCertificateException;
-use web_eid\web_eid_authtoken_validation_php\util\AsnUtil;
 
 class CertificateLoaderTest extends TestCase
 {
@@ -35,8 +34,6 @@ public function testWhenCertificateLoaderFromFileSuccess(): void
         $loader = (new CertificateLoader)->fromFile(__DIR__.'/../../_resources/revoked.crt');
 
         $this->assertEquals("318601422914101149693420017798940712227677", $loader->getCert()->getCurrentCert()['tbsCertificate']['serialNumber']);
-        $this->assertEquals("http://cert.int-x3.letsencrypt.org/", $loader->getIssuerCertificateUrl());
-        $this->assertEquals("http://ocsp.int-x3.letsencrypt.org", $loader->getOcspResponderUrl());
     }
 
     public function testWhenCertificateLoaderFromStringSuccess(): void
@@ -71,22 +68,6 @@ public function testWhenCertificateStringIsNotValidThrows(): void
         (new CertificateLoader)->fromString("certsource");
     }
 
-    public function testWhenCertificateIsNotLoadedOnIssuerCertificateUrlThrows(): void
-    {
-        $this->expectException(OcspCertificateException::class);
-        $this->expectExceptionMessage('Certificate not loaded');
-
-        (new CertificateLoader)->getIssuerCertificateUrl();
-    }
-
-    public function testWhenCertificateIsNotLoadedOnOcspResponderUrlThrows(): void
-    {
-        $this->expectException(OcspCertificateException::class);
-        $this->expectExceptionMessage('Certificate not loaded');
-
-        (new CertificateLoader)->getOcspResponderUrl();
-    }
-
     public function testWhenCertificateIsNotLoadedOnGetCertThrows(): void
     {
         $this->expectException(OcspCertificateException::class);