Merge pull request #69 from liamdennehy/optimise-object-creation

fromSequence() preferred over fromDER()
liamdennehy · Jun 2, 2020 · 984f983 · 984f983
2 parents 4e95664 + 8f59712
commit 984f983
Show file tree

Hide file tree

Showing 20 changed files with 2,782 additions and 189 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -8,9 +8,9 @@ install:
   - composer config -g github-oauth.github.com "$GITHUB_API_KEY"
   - cat composer.json
   - composer install
-  - bash -c tools/get-tls.sh
+  # - bash -c tools/get-tls.sh
   - scripts/showtests.sh
 cache:
   directories:
     - $HOME/.composer/cache/files
-    - $HOME/data/tl
+    # - $HOME/data/tl
diff --git a/data/eu-lotl.xml b/data/eu-lotl.xml
diff --git a/data/journal/c-276-1/8e508f03b132500c3403db66e9dd39cd78f4657c840958a77d34e7bd621468e7.crt b/data/journal/c-276-1/8e508f03b132500c3403db66e9dd39cd78f4657c840958a77d34e7bd621468e7.crt
@@ -0,0 +1,38 @@
+-----BEGIN CERTIFICATE-----
+MIIGlTCCBH2gAwIBAgIDMg6dMA0GCSqGSIb3DQEBCwUAME4xCzAJBgNVBAYTAkxV
+MRYwFAYDVQQKDA1MdXhUcnVzdCBTLkEuMScwJQYDVQQDDB5MdXhUcnVzdCBHbG9i
+YWwgUXVhbGlmaWVkIENBIDMwHhcNMTgwMjIwMTI1MjIxWhcNMjEwMjIwMTI1MjIx
+WjCCAQ0xKzApBgkqhkiG9w0BCQEWHGFkcmlhbi5jcm9pdG9ydUBlYy5ldXJvcGEu
+ZXUxCzAJBgNVBAYTAlJPMQswCQYDVQQHEwJCRTEcMBoGA1UEChMTRXVyb3BlYW4g
+Q29tbWlzc2lvbjEVMBMGA1UECxMMMDk0OS4zODMuMzQyMSMwIQYDVQQDExpDb25z
+dGFudGluLUFkcmlhbiBDcm9pdG9ydTERMA8GA1UEBBMIQ3JvaXRvcnUxGjAYBgNV
+BCoTEUNvbnN0YW50aW4tQWRyaWFuMR0wGwYDVQQFExQxMDMwNDM4NzU0MDEwNjEw
+MTc0MDEcMBoGA1UEDBMTUHJvZmVzc2lvbmFsIFBlcnNvbjCCASIwDQYJKoZIhvcN
+AQEBBQADggEPADCCAQoCggEBAKUebuK3GcmFVx01U3T3ex4VNteTsj9Cl0uaRROF
+8k7t9F48AG0RL1Pu1qwGGcmEz3mP0oV4m/TqkQt0YIkN1DS8aJHrBgWUxoO4F7NU
+c9EtlOIN0QgT/GXdYm4zUojyMPkW7FcW5OiaJV/LtSBVOw5/gRmaGH5/gdxE37lH
+q9V9rFKC2erUSbWXQB5uXsxMyzx1tJgV81WHpkn4OCIxFSjZDiYuZdpUZaE56zh8
+vZS5L22EHwCdUfxVTo5yeK7AHA2nKtXEn80eDb96cAuWuAoOiiJtjU6lx96iJQ99
+QLi2qhX+Tu+9YxlS5hPpxjBGhWkhgWZaMFE5cmdbeUGRRWUCAwEAAaOCAbkwggG1
+MB8GA1UdIwQYMBaAFGOPwosDsauO2FNHlh2ZqH32rKh1MIGBBggrBgEFBQcBAwR1
+MHMwCAYGBACORgEBMAgGBgQAjkYBBDBIBgYEAI5GAQUwPjA8FjZodHRwczovL3d3
+dy5sdXh0cnVzdC5sdS91cGxvYWQvZGF0YS9yZXBvc2l0b3J5L1BEUy5wZGYTAkVO
+MBMGBgQAjkYBBjAJBgcEAI5GAQYBMGYGCCsGAQUFBwEBBFowWDAnBggrBgEFBQcw
+AYYbaHR0cDovL3FjYS5vY3NwLmx1eHRydXN0Lmx1MC0GCCsGAQUFBzAChiFodHRw
+Oi8vY2EubHV4dHJ1c3QubHUvTFRHUUNBMy5jcnQwTgYDVR0gBEcwRTA4BggrgSsB
+AQoDGjAsMCoGCCsGAQUFBwIBFh5odHRwczovL3JlcG9zaXRvcnkubHV4dHJ1c3Qu
+bHUwCQYHBACL7EABAjAzBgNVHR8ELDAqMCigJqAkhiJodHRwOi8vY3JsLmx1eHRy
+dXN0Lmx1L0xUR1FDQTMuY3JsMBEGA1UdDgQKBAhGofvti7EAqjAOBgNVHQ8BAf8E
+BAMCBkAwDQYJKoZIhvcNAQELBQADggIBAHRVNf1uz9NJu6F3eBbwqlqA1RugR+z9
+wKuGno/8cF8eCv+iCjf8c3TrWjuJKBoyBlMxEwU8MmBElEMAaSNdWDBQWS8vD0ki
+bHLjz0WfwHJgKZBUhW9W8250PF3yU/Z9kh/dDBLbDvWoqYnzQbBd+4BnnLUH2iQp
+GF4Cqw0hmclgr0Ld4QJ1h1HuXhjJKt3iQAlmrE75dzZEkmxQzuzY+cQxxu/xRkLU
+TpScdKDtCiylKW6JBHCFpZaqlGY7vIy1n//LYzd3nWmyRYF26/J6rnkdd/P78BzK
+72xXoMa0klF4dZfHtMCQw/jbFAM4xB0245pqZeYZYo9fdd2XCqEVSp9DcNTTOI+B
+wXnuM+xFmr5PTuZ7LPQ/FtJIzJT4KqZRovQ/IgddwJk/sxq/Y73ZIg5qMisBGqQv
+AwohjYyQ8MC8NI8zlDPLj5+lTcX5ATmxzgpi0XoUsA9sv4tvVEgj4EBUmNi0UWPM
+YIvsIehx43f9YmW/kjA8Q0ado6DWZh1H9ama/wvq+dZoBEW8zg62RX///CWUtUCj
+8c6kauFFyCF9ZVgRP4Ec3h/aWP8SPN79pBUHXEpceh3sbl3ReOIS/ZtddB2aCwWt
+Tv9zSdnM0Bb3C4wfDsvc4lsMIRBkB8B+cl60jE1tHCJq8+VDKMKTukJCA0ny/13L
+n+tTFU0qoOgr
+-----END CERTIFICATE-----
diff --git a/data/tl-61c0487109be27255c19cff26d8f56bea621e7f381a7b4cbe7fb4750bd477bf9.xml b/data/tl-61c0487109be27255c19cff26d8f56bea621e7f381a7b4cbe7fb4750bd477bf9.xml
diff --git a/src/AlgorithmIdentifier.php b/src/AlgorithmIdentifier.php
@@ -52,17 +52,21 @@ public function __construct($id, $parameters = null, $parametersIncluded = true)
 
     public static function fromDER($der)
     {
-        $obj = UnspecifiedType::fromDER($der)->asSequence();
-        if ($obj->has(1) && $obj->at(1)->tag() == 16) {
+        return self::fromSequence(UnspecifiedType::fromDER($der)->asSequence());
+    }
+
+    public static function fromSequence($sequence)
+    {
+        if ($sequence->has(1) && $sequence->at(1)->tag() == 16) {
             $parameters = [];
-            foreach ($obj->at(1)->asSequence()->elements() as $parameter) {
+            foreach ($sequence->at(1)->asSequence()->elements() as $parameter) {
                 $parameters[] = $parameter->toDER();
             }
         } else {
             $parameters = null;
         }
         $aid = new AlgorithmIdentifier(
-            $obj->at(0)->asObjectIdentifier()->oid(),
+            $sequence->at(0)->asObjectIdentifier()->oid(),
             $parameters
         );
         return $aid;

diff --git a/src/Certificate/X509Certificate.php b/src/Certificate/X509Certificate.php
@@ -51,17 +51,12 @@ public function __construct($candidate)
         $this->crtBinary = X509Certificate::emit($candidate);
         $crtASN1 = UnspecifiedType::fromDER($this->crtBinary)->asSequence();
         $tbsCertificate = $crtASN1->at(0)->asSequence();
-        $this->signatureAlgorithmIdentifier = AlgorithmIdentifier::fromDER($crtASN1->at(1)->asSequence()->toDER());
+        $this->signatureAlgorithmIdentifier = AlgorithmIdentifier::fromSequence($crtASN1->at(1)->asSequence());
         $signatureValue = $crtASN1->at(2)->asBitString()->string();
         $idx = 0;
         if ($tbsCertificate->hasTagged(0)) {
             $crtVersion = $tbsCertificate->getTagged(0)->asExplicit()->asInteger()->intNumber();
             $idx++;
-            // } else {
-        //   $version = 1;
-        //   throw new CertificateException("Only X.509 v3 certificates are supported: ".base64_encode($this->crtBinary), 1);
-        //   return null;
-        //
         }
         $this->serialNumber = gmp_strval($tbsCertificate->at($idx++)->asInteger()->number(), 16);
         $this->signature = $tbsCertificate->at($idx++)->asSequence();

diff --git a/src/DistinguishedName.php b/src/DistinguishedName.php
@@ -11,7 +11,7 @@
  */
 class DistinguishedName implements ASN1Interface
 {
-    private $binary;
+    private $sequence;
 
     public function __construct($dnSequence)
     {

diff --git a/src/Extension.php b/src/Extension.php
@@ -26,7 +26,11 @@ abstract class Extension
 {
     public static function fromBinary($extensionDER)
     {
-        $extension = UnspecifiedType::fromDER($extensionDER)->asSequence();
+        return self::fromSequence(UnspecifiedType::fromDER($extensionDER)->asSequence());
+    }
+
+    public static function fromSequence($extension)
+    {
         $idx = 0;
         $extensionOid = $extension->at($idx++)->asObjectIdentifier()->oid();
         if ($extension->at($idx)->isType(1)) {

diff --git a/src/Extensions.php b/src/Extensions.php
@@ -25,7 +25,7 @@ public function __construct($extensionsDER)
         $extensionsSequence = UnspecifiedType::fromDER($extensionsDER)->asSequence();
         foreach ($extensionsSequence->elements() as $extension) {
             $extension = $extension->asSequence();
-            $v3Extension = Extension::fromBinary($extension->toDER());
+            $v3Extension = Extension::fromSequence($extension);
             if ($v3Extension) {
                 if ($v3Extension->getType() == 'unknown') {
                     $extName = 'unknown-'.$v3Extension->getOID();

diff --git a/src/OCSP/CertID.php b/src/OCSP/CertID.php
@@ -36,9 +36,12 @@ public function __construct(
 
     public static function fromDER($der)
     {
-        $obj = UnspecifiedType::fromDER($der)->asSequence();
-        // var_dump($obj);
-        $signatureAlgorithm = AlgorithmIdentifier::fromDER($obj->at(0)->toDER());
+        return self::fromSequence(UnspecifiedType::fromDER($der)->asSequence());
+    }
+
+    public static function fromSequence($obj)
+    {
+        $signatureAlgorithm = AlgorithmIdentifier::fromSequence($obj->at(0)->asSequence());
         $issuerNameHash = $obj->at(1)->asOctetString()->string();
         $issuerKeyHash = $obj->at(2)->asOctetString()->string();
         $serialNumber = gmp_strval($obj->at(3)->asInteger()->number(), 16);

diff --git a/src/OCSP/OCSPRequest.php b/src/OCSP/OCSPRequest.php
@@ -59,16 +59,15 @@ public function __construct(
         }
     }
 
-    /**
-     * [fromDER description]
-     * @param  string $der [binary request data]
-     * @return [type]      [description]
-     */
     public static function fromDER($der)
+    {
+        return self::fromSequence(UnspecifiedType::fromDER($der)->asSequence());
+    }
+
+    public static function fromSequence($OCSPRequest)
     {
         $top = [];
-        $OCSPRequest = UnspecifiedType::fromDER($der)->asSequence();
-        $tbsRequest = TBSRequest::fromDER($OCSPRequest->at(0)->asSequence()->toDER());
+        $tbsRequest = TBSRequest::fromSequence($OCSPRequest->at(0)->asSequence());
         if ($OCSPRequest->hasTagged(0)) {
             throw new ParseException("Cannot support signed Requests", 1);
         }

diff --git a/src/OCSP/Request.php b/src/OCSP/Request.php
@@ -27,8 +27,12 @@ public function __construct(CertID $certId, Extensions $extensions = null)
 
     public static function fromDER($der)
     {
-        $asn1 = UnspecifiedType::fromDER($der)->asSequence();
-        $certId = CertID::fromDER($asn1->at(0)->toDER());
+        return self::fromSequence(UnspecifiedType::fromDER($der)->asSequence());
+    }
+
+    public static function fromSequence($request)
+    {
+        $certId = CertID::fromSequence($request->at(0)->asSequence());
         return new Request($certId);
     }
 

diff --git a/src/OCSP/TBSRequest.php b/src/OCSP/TBSRequest.php
@@ -30,7 +30,11 @@ public function __construct($requestList, $nonce = null, $version = 1)
 
     public static function fromDER($der)
     {
-        $tbsRequest = UnspecifiedType::fromDER($der)->asSequence();
+        return self::fromSequence(UnspecifiedType::fromDER($der)->asSequence());
+    }
+
+    public static function fromSequence($tbsRequest)
+    {
         $idx = 0;
         if ($tbsRequest->hasTagged(0)) {
             if ($version !== 1) {
@@ -53,7 +57,7 @@ public static function fromDER($der)
         }
         $requestList = $tbsRequest->at($idx)->asSequence();
         foreach ($requestList->elements() as $request) {
-            $request = Request::fromDER($request->toDER());
+            $request = Request::fromSequence($request->asSequence());
             $requests[] = $request;
         }
         if ($tbsRequest->hasTagged(2)) {

diff --git a/tests/CertificateParseTest.php b/tests/CertificateParseTest.php
@@ -16,7 +16,7 @@ class CertificateParseTest extends TestCase
     const eucrtfile = 'European-Commission.crt';
     const euissuercrtfile = 'qvbecag2.crt';
     const euIssuercertId = 'd90b40132306d1094608b1b9a2f6a9e23b45fe121fef514a1c9df70a815ad95c';
-    const lotlSignerHash = 'd2064fdd70f6982dcc516b86d9d5c56aea939417c624b2e478c0b29de54f8474';
+    const lotlSignerHash = '8e508f03b132500c3403db66e9dd39cd78f4657c840958a77d34e7bd621468e7';
     const eucrtPublicKeyPEM =
         "-----BEGIN PUBLIC KEY-----\n".
         "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA6OaxkP4aEj/JK+Aw13o5\n".
@@ -50,7 +50,7 @@ class CertificateParseTest extends TestCase
 
     public function setUp()
     {
-        Helper::getHTTP(TLTest::testTLURI, 'tl');
+        // Helper::getHTTP(TLTest::testTLURI, 'tl');
         $this->testTime = new \DateTime('@1569225604');
         $this->eucrtSubject = [
           [
@@ -659,14 +659,14 @@ public function testQCIssuer()
     {
         $this->getTestCerts();
         $dataDir = __DIR__.'/../data/';
-        $signingCertPEM = file_get_contents($dataDir.'/journal/c-276-1/'.self::lotlSignerHash.'.crt');
+        $signingCertPEM = file_get_contents(__DIR__.'/../'.LOTLRootTest::lotlSigningCertPath);
         $signingCert = new X509Certificate($signingCertPEM);
         $lotl = new TrustedList(file_get_contents($dataDir.'/eu-lotl.xml'));
         // $eucrt = new X509Certificate($this->eucrt);
         $euissuercrt = new X509Certificate($this->euissuercrt);
         // $euissuercrt->setTSPService($tspServiceAttributes);)
         $lotl->verifyTSL($signingCert);
-        $testTLXML = file_get_contents($dataDir.TLTest::testTLXMLFileName);
+        $testTLXML = file_get_contents(__DIR__.'/../'.TLTest::testTLXMLFileName);
         $lotl->addTrustedListXML(TLTest::testTLName, $testTLXML);
         $issuerTSPService = ($lotl->getTSPServices(true)[TSPServicesTest::EUTSPServiceName]);
         $euissuercrt->setTSPService($issuerTSPService);

diff --git a/tests/Helper.php b/tests/Helper.php
@@ -8,6 +8,8 @@ class Helper
 {
     public static function getHTTP($URI, $type)
     {
+        throw new \Exception("Don't download", 1);
+
         $datadir = __DIR__ . '/../data/';
         $uriId = hash('sha256', $URI);
         switch ($type) {

diff --git a/tests/LOTLRootTest.php b/tests/LOTLRootTest.php
@@ -11,7 +11,6 @@
 use eIDASCertificate\CertificateException;
 use eIDASCertificate\TrustedListException;
 use DateTime;
-use eIDASCertificate\tests\Helper;
 
 class LOTLRootTest extends TestCase
 {
@@ -105,19 +104,17 @@ class LOTLRootTest extends TestCase
         ['lang' => 'sv','uri' => 'https://ec.europa.eu/tools/lotl/eu-lotl-legalnotice.html#sv'],
         ['lang' => 'hr','uri' => 'https://ec.europa.eu/tools/lotl/eu-lotl-legalnotice.html#hr'],
       ],
-      'sequenceNumber' => 250,
+      'sequenceNumber' => 266,
       'sourceURI' => 'https://ec.europa.eu/tools/lotl/eu-lotl.xml',
-      'issued' => '1570186800',
-      'nextUpdate' => '1585958400',
-      'fileHash' => '56bbdeb154d25bfc735bda4f958fead0b578712f79037227f87d2ad7bcf7880d',
+      'issued' => '1590487200',
+      'nextUpdate' => '1606348800',
+      'fileHash' => 'b3030a0d729e6bfefc18d4c7d0a3f0bce90528057ebed63e06d140dd2d1100d9',
       'signature' => [
-        'signerThumbprint' => 'd2064fdd70f6982dcc516b86d9d5c56aea939417c624b2e478c0b29de54f8474'
+        'signerThumbprint' => '8e508f03b132500c3403db66e9dd39cd78f4657c840958a77d34e7bd621468e7'
       ]
     ];
     const lotlSigningCertPath =
-      '/journal/c-276-1/d2064fdd70f6982dcc516b86d9d5c56aea939417c624b2e478c0b29de54f8474.crt';
-    const lotlHash =
-      '56bbdeb154d25bfc735bda4f958fead0b578712f79037227f87d2ad7bcf7880d';
+      'data/journal/c-276-1/8e508f03b132500c3403db66e9dd39cd78f4657c840958a77d34e7bd621468e7.crt';
     private $lotlxml;
     private $lotl;
     private $datadir;
@@ -149,10 +146,6 @@ public function testParseLOTL()
             "EUlistofthelists",
             $this->lotl->getTSLType()->getType()
         );
-        $this->assertEquals(
-            self::lotlHash,
-            $this->lotl->getXMLHash()
-        );
         $this->assertInternalType("int", $this->lotl->getVersionID());
         $this->assertInternalType("int", $this->lotl->getSequenceNumber());
         $this->assertEquals(
@@ -186,7 +179,7 @@ public function testParseLOTL()
     public function testVerifyLOTLExplicitSigned()
     {
         $wrongCertHash = '9c1a3b646eaf132398ef319e41c8e7ed725b64d5772580ae125d59c0f6845630';
-        $rightCertHash = 'd2064fdd70f6982dcc516b86d9d5c56aea939417c624b2e478c0b29de54f8474';
+        $rightCertHash = '8e508f03b132500c3403db66e9dd39cd78f4657c840958a77d34e7bd621468e7';
         $certpaths = scandir($this->datadir.'/journal/c-276-1');
         while ($certpaths[0] == '.' || $certpaths[0] == '..') {
             array_shift($certpaths);
@@ -205,7 +198,7 @@ public function testVerifyLOTLExplicitSigned()
         } catch (SignatureException $e) {
             $this->assertEquals(
                 [
-                'signedBy' => 'd2064fdd70f6982dcc516b86d9d5c56aea939417c624b2e478c0b29de54f8474',
+                'signedBy' => '8e508f03b132500c3403db66e9dd39cd78f4657c840958a77d34e7bd621468e7',
                 'availableCerts' => [
                   '9c1a3b646eaf132398ef319e41c8e7ed725b64d5772580ae125d59c0f6845630'
                 ]
@@ -218,13 +211,16 @@ public function testVerifyLOTLExplicitSigned()
         $lotl = new TrustedList($this->lotlXML);
         $this->assertTrue($lotl->verifyTSL($rightCert));
         $this->assertEquals(
-            'd2064fdd70f6982dcc516b86d9d5c56aea939417c624b2e478c0b29de54f8474',
+            '8e508f03b132500c3403db66e9dd39cd78f4657c840958a77d34e7bd621468e7',
             $lotl->getSignedBy()->getIdentifier()
         );
 
         $lotlSignedByDN = $lotl->getSignedBy()->getSubjectDN();
         $this->assertEquals(
-            '/C=BE/CN=Patrick Kremer (Signature)/SN=Kremer/GN=Patrick Jean/serialNumber=72020329970',
+            '/emailAddress=adrian.croitoru@ec.europa.eu/C=RO/L=BE'.
+            '/O=European Commission/OU=0949.383.342'.
+            '/CN=Constantin-Adrian Croitoru/SN=Croitoru/GN=Constantin-Adrian'.
+            '/serialNumber=10304387540106101740/title=Professional Person',
             $lotlSignedByDN
         );
     }
@@ -283,17 +279,17 @@ public function testAddTLstoLOTL()
         );
 
         $pointedTLs = [];
-        $crtFileName = $this->datadir.'/journal/c-276-1/d2064fdd70f6982dcc516b86d9d5c56aea939417c624b2e478c0b29de54f8474.crt';
+        $crtFileName = $this->datadir.'/journal/c-276-1/8e508f03b132500c3403db66e9dd39cd78f4657c840958a77d34e7bd621468e7.crt';
         $crt = file_get_contents($crtFileName);
         $rightCert = new X509Certificate(file_get_contents($crtFileName));
         $this->assertTrue($lotl->verifyTSL($rightCert));
         $now = (new DateTime('now'))->format('U');
         $this->assertEquals(
-            'd2064fdd70f6982dcc516b86d9d5c56aea939417c624b2e478c0b29de54f8474',
+            '8e508f03b132500c3403db66e9dd39cd78f4657c840958a77d34e7bd621468e7',
             $lotl->getSignedByHash()
         );
         // TODO: Handle bad TL Admins and distributions
-        
+
         // foreach ($lotl->getTLPointerPaths() as $title => $tlPointer) {
         //     $localFile = $this->datadir.'/tl-'.$tlPointer['id'].'.xml';
         //     if (file_exists($localFile)) {