Fix implicit tag collision

sop · Jan 1, 2021 · 4d461e0 · 4d461e0
1 parent cd265f1
commit 4d461e0
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 7 deletions.
diff --git a/lib/X509/Certificate/Extension/NameConstraints/GeneralSubtree.php b/lib/X509/Certificate/Extension/NameConstraints/GeneralSubtree.php
@@ -65,13 +65,22 @@ public static function fromASN1(Sequence $seq): self
         $base = GeneralName::fromASN1($seq->at(0)->asTagged());
         $min = 0;
         $max = null;
-        if ($seq->hasTagged(0)) {
-            $min = $seq->getTagged(0)->asImplicit(Element::TYPE_INTEGER)
-                ->asInteger()->intNumber();
-        }
-        if ($seq->hasTagged(1)) {
-            $max = $seq->getTagged(1)->asImplicit(Element::TYPE_INTEGER)
-                ->asInteger()->intNumber();
+        // GeneralName is a CHOICE, which may be tagged as otherName [0]
+        // or rfc822Name [1]. As minimum and maximum are also implicitly tagged,
+        // we have to iterate the remaining elements instead of just checking
+        // for tagged types.
+        for ($i = 1; $i < count($seq); ++$i) {
+            $el = $seq->at($i)->expectTagged();
+            switch ($el->tag()) {
+                case 0:
+                    $min = $el->asImplicit(Element::TYPE_INTEGER)
+                        ->asInteger()->intNumber();
+                    break;
+                case 1:
+                    $max = $el->asImplicit(Element::TYPE_INTEGER)
+                        ->asInteger()->intNumber();
+                    break;
+            }
         }
         return new self($base, $min, $max);
     }

diff --git a/test/unit/certificate/extension/name-constraints/GeneralSubtreeTest.php b/test/unit/certificate/extension/name-constraints/GeneralSubtreeTest.php
@@ -6,6 +6,7 @@
 use Sop\ASN1\Type\Constructed\Sequence;
 use Sop\X509\Certificate\Extension\NameConstraints\GeneralSubtree;
 use Sop\X509\GeneralName\GeneralName;
+use Sop\X509\GeneralName\RFC822Name;
 use Sop\X509\GeneralName\UniformResourceIdentifier;
 
 /**
@@ -116,4 +117,15 @@ public function testRecodedWithAll(GeneralSubtree $ref, GeneralSubtree $new)
     {
         $this->assertEquals($ref, $new);
     }
+
+    /**
+     * Test for GeneralName tag that collide with other GeneralSubtree tags.
+     */
+    public function testCollidingTag()
+    {
+        $subtree = new GeneralSubtree(new RFC822Name('test'));
+        $asn1 = $subtree->toASN1();
+        $result = GeneralSubtree::fromASN1($asn1);
+        $this->assertInstanceOf(GeneralSubtree::class, $result);
+    }
 }