Merge pull request #462 from terrafrost/asn1-handle-indef-lengths

ASN1: rewrite _decode_der

* terrafrost/asn1-handle-indef-lengths:
  ASN1: one more unit test change
  ASN1: another unit test update
  ASN1: unit test adjustments
  ASN1: cs adjustments to unit test
  ASN1: add unit tests for indefinite length decoding
  ASN1: CS adjustment (rm whitespace at eol)
  ASN1: rewrite _decode_der

phpseclib/File/ASN1.php

@@ -272,7 +272,8 @@ class File_ASN1
         }
 
         $this->encoded = $encoded;
-        return $this->_decode_ber($encoded);
+        // encapsulate in an array for BC with the old decodeBER
+        return array($this->_decode_ber($encoded));
     }
 
     /**
@@ -287,11 +288,8 @@ class File_ASN1
      * @return Array
      * @access private
      */
-    function _decode_ber(&$encoded, $start = 0)
+    function _decode_ber($encoded, $start = 0)
     {
-        $decoded = array();
-
-        while ( strlen($encoded) ) {
         $current = array('start' => $start);
 
         $type = ord($this->_string_shift($encoded));
@@ -317,16 +315,13 @@ class File_ASN1
         if ( $length == 0x80 ) { // indefinite length
             // "[A sender shall] use the indefinite form (see 8.1.3.6) if the encoding is constructed and is not all
             //  immediately available." -- paragraph 8.1.3.2.c
-                //if ( !$constructed ) {
-                //    return false;
-                //}
             $length = strlen($encoded);
         } elseif ( $length & 0x80 ) { // definite length, long form
             // technically, the long form of the length can be represented by up to 126 octets (bytes), but we'll only
             // support it up to four.
             $length&= 0x7F;
             $temp = $this->_string_shift($encoded, $length);
-                // tags of indefinite length don't really have a header length; this length includes the tag
+            // tags of indefinte length don't really have a header length; this length includes the tag
             $current+= array('headerlength' => $length + 2);
             $start+= $length;
             extract(unpack('Nlength', substr(str_pad($temp, 4, chr(0), STR_PAD_LEFT), -4)));
@@ -334,12 +329,10 @@ class File_ASN1
             $current+= array('headerlength' => 2);
         }
 
-            // End-of-content, see paragraphs 8.1.1.3, 8.1.3.2, 8.1.3.6, 8.1.5, and (for an example) 8.6.4.2
-            if (!$type && !$length) {
-                return $decoded;
-            }
         $content = $this->_string_shift($encoded, $length);
 
+        // at this point $length can be overwritten. it's only accurate for definite length things as is
+
         /* Class is UNIVERSAL, APPLICATION, PRIVATE, or CONTEXT-SPECIFIC. The UNIVERSAL class is restricted to the ASN.1
            built-in types. It defines an application-independent data type that must be distinguishable from all other
            data types. The other three classes are user defined. The APPLICATION class distinguishes data types that
@@ -354,14 +347,24 @@ class File_ASN1
             case FILE_ASN1_CLASS_APPLICATION:
             case FILE_ASN1_CLASS_PRIVATE:
             case FILE_ASN1_CLASS_CONTEXT_SPECIFIC:
-                    $decoded[] = array(
+                $newcontent = $this->_decode_ber($content, $start);
+                $length = $newcontent['length'];
+                if (substr($content, $length, 2) == "\0\0") {
+                    $length+= 2;
+                }
+
+                $start+= $length;
+
+                return array(
                     'type'     => $class,
                     'constant' => $tag,
-                        'content'  => $constructed ? $this->_decode_ber($content, $start) : $content,
+                    // the array encapsulation is for BC with the old format
-                        'length'   => $length + $start - $current['start']
+                    'content'  => array($newcontent),
+                    // the only time when $content['headerlength'] isn't defined is when the length is indefinite.
+                    // the absence of $content['headerlength'] is how we know if something is indefinite or not.
+                    // technically, it could be defined to be 2 and then another indicator could be used but whatever.
+                    'length'   => $start - $current['start']
                 ) + $current;
-                    $start+= $length;
-                    continue 2;
         }
 
         $current+= array('type' => $tag);
@@ -409,16 +412,21 @@ class File_ASN1
                 if (!$constructed) {
                     $current['content'] = $content;
                 } else {
-                        $temp = $this->_decode_ber($content, $start);
+                    $current['content'] = '';
-                        $length-= strlen($content);
+                    $length = 0;
-                        for ($i = 0, $size = count($temp); $i < $size; $i++) {
+                    while (substr($content, 0, 2) != "\0\0") {
+                        $temp = $this->_decode_ber($content, $length + $start);
+                        $this->_string_shift($content, $temp['length']);
                         // all subtags should be octet strings
-                            //if ($temp[$i]['type'] != FILE_ASN1_TYPE_OCTET_STRING) {
+                        //if ($temp['type'] != FILE_ASN1_TYPE_OCTET_STRING) {
                         //    return false;
                         //}
-                            $current['content'].= $temp[$i]['content'];
+                        $current['content'].= $temp['content'];
+                        $length+= $temp['length'];
+                    }
+                    if (substr($content, 0, 2) == "\0\0") {
+                        $length+= 2; // +2 for the EOC
                     }
-                        // $length =
                 }
                 break;
             case FILE_ASN1_TYPE_NULL:
@@ -429,7 +437,20 @@ class File_ASN1
                 break;
             case FILE_ASN1_TYPE_SEQUENCE:
             case FILE_ASN1_TYPE_SET:
-                    $current['content'] = $this->_decode_ber($content, $start);
+                $offset = 0;
+                $current['content'] = array();
+                while (strlen($content)) {
+                    // if indefinite length construction was used and we have an end-of-content string next
+                    // see paragraphs 8.1.1.3, 8.1.3.2, 8.1.3.6, 8.1.5, and (for an example) 8.6.4.2
+                    if (!isset($current['headerlength']) && substr($content, 0, 2) == "\0\0") {
+                        $length = $offset + 2; // +2 for the EOC
+                        break 2;
+                    }
+                    $temp = $this->_decode_ber($content, $start + $offset);
+                    $this->_string_shift($content, $temp['length']);
+                    $current['content'][] = $temp;
+                    $offset+= $temp['length'];
+                }
                 break;
             case FILE_ASN1_TYPE_OBJECT_IDENTIFIER:
                 $temp = ord($this->_string_shift($content));
@@ -484,18 +505,16 @@ class File_ASN1
             case FILE_ASN1_TYPE_GENERALIZED_TIME:
                 $current['content'] = $this->_decodeTime($content, $tag);
             default:
-
         }
 
         $start+= $length;
-            $decoded[] = $current + array('length' => $start - $current['start']);
-        }
 
-        return $decoded;
+        // ie. length is the length of the full TLV encoding - it's not just the length of the value
+        return $current + array('length' => $start - $current['start']);
     }
 
     /**
-     * ASN.1 Decode
+     * ASN.1 Map
      *
      * Provides an ASN.1 semantic mapping ($mapping) from a parsed BER-encoding to a human readable format.
      *
@@ -805,16 +824,6 @@ class File_ASN1
         return $this->_encode_der($source, $mapping, null, $special);
     }
 
-    /**
-     * ASN.1 Encode (Helper function)
-     *
-     * @param String $source
-     * @param Array $mapping
-     * @param Integer $idx
-     * @param Array $special
-     * @return String
-     * @access private
-     */
     /**
      * ASN.1 Encode (Helper function)
      *

tests/Unit/File/ASN1Test.php

@@ -7,7 +7,7 @@
 
 require_once 'File/ASN1.php';
 
-class Unit_File_ASN1_DevTest extends PhpseclibTestCase
+class Unit_File_ASN1Test extends PhpseclibTestCase
 {
     /**
     * on older versions of File_ASN1 this would yield a PHP Warning
@@ -233,4 +233,39 @@ class Unit_File_ASN1_DevTest extends PhpseclibTestCase
 
         $this->assertInternalType('array', $result);
     }
+
+    /**
+    * older versions of File_ASN1 didn't handle indefinite length tags very well
+    */
+    public function testIndefiniteLength()
+    {
+        $asn1 = new File_ASN1();
+        $decoded = $asn1->decodeBER(file_get_contents(dirname(__FILE__) . '/ASN1/FE.pdf.p7m'));
+        $this->assertEquals(count($decoded[0]['content'][1]['content'][0]['content']), 5); // older versions would have returned 3
+    }
+
+    public function testDefiniteLength()
+    {
+        // the following base64-encoded string is the X.509 cert from <http://phpseclib.sourceforge.net/x509/decoder.php>
+        $str = 'MIIDITCCAoqgAwIBAgIQT52W2WawmStUwpV8tBV9TTANBgkqhkiG9w0BAQUFADBM' .
+               'MQswCQYDVQQGEwJaQTElMCMGA1UEChMcVGhhd3RlIENvbnN1bHRpbmcgKFB0eSkg' .
+               'THRkLjEWMBQGA1UEAxMNVGhhd3RlIFNHQyBDQTAeFw0xMTEwMjYwMDAwMDBaFw0x' .
+               'MzA5MzAyMzU5NTlaMGgxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlh' .
+               'MRYwFAYDVQQHFA1Nb3VudGFpbiBWaWV3MRMwEQYDVQQKFApHb29nbGUgSW5jMRcw' .
+               'FQYDVQQDFA53d3cuZ29vZ2xlLmNvbTCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkC' .
+               'gYEA3rcmQ6aZhc04pxUJuc8PycNVjIjujI0oJyRLKl6g2Bb6YRhLz21ggNM1QDJy' .
+               'wI8S2OVOj7my9tkVXlqGMaO6hqpryNlxjMzNJxMenUJdOPanrO/6YvMYgdQkRn8B' .
+               'd3zGKokUmbuYOR2oGfs5AER9G5RqeC1prcB6LPrQ2iASmNMCAwEAAaOB5zCB5DAM' .
+               'BgNVHRMBAf8EAjAAMDYGA1UdHwQvMC0wK6ApoCeGJWh0dHA6Ly9jcmwudGhhd3Rl' .
+               'LmNvbS9UaGF3dGVTR0NDQS5jcmwwKAYDVR0lBCEwHwYIKwYBBQUHAwEGCCsGAQUF' .
+               'BwMCBglghkgBhvhCBAEwcgYIKwYBBQUHAQEEZjBkMCIGCCsGAQUFBzABhhZodHRw' .
+               'Oi8vb2NzcC50aGF3dGUuY29tMD4GCCsGAQUFBzAChjJodHRwOi8vd3d3LnRoYXd0' .
+               'ZS5jb20vcmVwb3NpdG9yeS9UaGF3dGVfU0dDX0NBLmNydDANBgkqhkiG9w0BAQUF' .
+               'AAOBgQAhrNWuyjSJWsKrUtKyNGadeqvu5nzVfsJcKLt0AMkQH0IT/GmKHiSgAgDp' .
+               'ulvKGQSy068Bsn5fFNum21K5mvMSf3yinDtvmX3qUA12IxL/92ZzKbeVCq3Yi7Le' .
+               'IOkKcGQRCMha8X2e7GmlpdWC1ycenlbN0nbVeSv3JUMcafC4+Q==';
+        $asn1 = new File_ASN1();
+        $decoded = $asn1->decodeBER(base64_decode($str));
+        $this->assertEquals(count($decoded[0]['content']), 3);
+    }
 }