Use BIT STRING capture API.

- Use BIT STRING capture API to help ensure data is the proper format. - Avoids the need to check if signatures were parsed as ASN.1. - Note about future support for arbitrary bit length ids.
digitalbazaar · Feb 3, 2017 · c1f8dc8 · c1f8dc8
1 parent d009763
commit c1f8dc8
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 30 deletions.
diff --git a/lib/x509.js b/lib/x509.js
@@ -265,7 +265,8 @@ var x509CertificateValidator = {
         tagClass: asn1.Class.UNIVERSAL,
         type: asn1.Type.BITSTRING,
         constructed: false,
-        capture: 'certIssuerUniqueId'
+        // TODO: support arbitrary bit length ids
+        captureBitStringValue: 'certIssuerUniqueId'
       }]
     }, {
       // subjectUniqueID (optional)
@@ -279,7 +280,8 @@ var x509CertificateValidator = {
         tagClass: asn1.Class.UNIVERSAL,
         type: asn1.Type.BITSTRING,
         constructed: false,
-        capture: 'certSubjectUniqueId'
+        // TODO: support arbitrary bit length ids
+        captureBitStringValue: 'certSubjectUniqueId'
       }]
     }, {
       // Extensions (optional)
@@ -315,7 +317,7 @@ var x509CertificateValidator = {
     tagClass: asn1.Class.UNIVERSAL,
     type: asn1.Type.BITSTRING,
     constructed: false,
-    capture: 'certSignature'
+    captureBitStringValue: 'certSignature'
   }]
 };
 
@@ -486,7 +488,7 @@ var certificationRequestValidator = {
     tagClass: asn1.Class.UNIVERSAL,
     type: asn1.Type.BITSTRING,
     constructed: false,
-    capture: 'csrSignature'
+    captureBitStringValue: 'csrSignature'
   }]
 };
 
@@ -955,6 +957,7 @@ pki.createCertificate = function() {
     cert.subject.attributes = attrs;
     delete cert.subject.uniqueId;
     if(uniqueId) {
+      // TODO: support arbitrary bit length ids
       cert.subject.uniqueId = uniqueId;
     }
     cert.subject.hash = null;
@@ -972,6 +975,7 @@ pki.createCertificate = function() {
     cert.issuer.attributes = attrs;
     delete cert.issuer.uniqueId;
     if(uniqueId) {
+      // TODO: support arbitrary bit length ids
       cert.issuer.uniqueId = uniqueId;
     }
     cert.issuer.hash = null;
@@ -1272,15 +1276,6 @@ pki.certificateFromAsn1 = function(obj, computeHash) {
     throw error;
   }
 
-  // ensure signature is not interpreted as an embedded ASN.1 object
-  if(typeof capture.certSignature !== 'string') {
-    var certSignature = '\x00';
-    for(var i = 0; i < capture.certSignature.length; ++i) {
-      certSignature += asn1.toDer(capture.certSignature[i]).getBytes();
-    }
-    capture.certSignature = certSignature;
-  }
-
   // get oid
   var oid = asn1.derToOid(capture.publicKeyOid);
   if(oid !== pki.oids.rsaEncryption) {
@@ -1299,10 +1294,7 @@ pki.certificateFromAsn1 = function(obj, computeHash) {
   cert.siginfo.algorithmOid = forge.asn1.derToOid(capture.certinfoSignatureOid);
   cert.siginfo.parameters = _readSignatureParameters(cert.siginfo.algorithmOid,
     capture.certinfoSignatureParams, false);
-  // skip "unused bits" in signature value BITSTRING
-  var signature = forge.util.createBuffer(capture.certSignature);
-  ++signature.read;
-  cert.signature = signature.getBytes();
+  cert.signature = capture.certSignature;
 
   var validity = [];
   if(capture.certValidity1UTCTime !== undefined) {
@@ -1654,15 +1646,6 @@ pki.certificationRequestFromAsn1 = function(obj, computeHash) {
     throw error;
   }
 
-  // ensure signature is not interpreted as an embedded ASN.1 object
-  if(typeof capture.csrSignature !== 'string') {
-    var csrSignature = '\x00';
-    for(var i = 0; i < capture.csrSignature.length; ++i) {
-      csrSignature += asn1.toDer(capture.csrSignature[i]).getBytes();
-    }
-    capture.csrSignature = csrSignature;
-  }
-
   // get oid
   var oid = asn1.derToOid(capture.publicKeyOid);
   if(oid !== pki.oids.rsaEncryption) {
@@ -1678,10 +1661,7 @@ pki.certificationRequestFromAsn1 = function(obj, computeHash) {
   csr.siginfo.algorithmOid = forge.asn1.derToOid(capture.csrSignatureOid);
   csr.siginfo.parameters = _readSignatureParameters(
     csr.siginfo.algorithmOid, capture.csrSignatureParams, false);
-  // skip "unused bits" in signature value BITSTRING
-  var signature = forge.util.createBuffer(capture.csrSignature);
-  ++signature.read;
-  csr.signature = signature.getBytes();
+  csr.signature = capture.csrSignature;
 
   // keep CertificationRequestInfo to preserve signature when exporting
   csr.certificationRequestInfo = capture.certificationRequestInfo;
@@ -2523,6 +2503,7 @@ pki.getTBSCertificate = function(cert) {
     tbs.value.push(
       asn1.create(asn1.Class.CONTEXT_SPECIFIC, 1, true, [
         asn1.create(asn1.Class.UNIVERSAL, asn1.Type.BITSTRING, false,
+          // TODO: support arbitrary bit length ids
           String.fromCharCode(0x00) +
           cert.issuer.uniqueId
         )
@@ -2534,6 +2515,7 @@ pki.getTBSCertificate = function(cert) {
     tbs.value.push(
       asn1.create(asn1.Class.CONTEXT_SPECIFIC, 2, true, [
         asn1.create(asn1.Class.UNIVERSAL, asn1.Type.BITSTRING, false,
+          // TODO: support arbitrary bit length ids
           String.fromCharCode(0x00) +
           cert.subject.uniqueId
         )

diff --git a/tests/unit/x509.js b/tests/unit/x509.js
@@ -1,4 +1,5 @@
 var ASSERT = require('assert');
+var ASN1 = require('../../lib/asn1');
 var MD = require('../../lib/md.all');
 var PKI = require('../../lib/pki');
 var UTIL = require('../../lib/util');
@@ -856,6 +857,44 @@ var UTIL = require('../../lib/util');
       var issuer = PKI.certificateFromPem(issuerPem);
       ASSERT.ok(issuer.verify(cert));
     });
+
+    it('should parse certificate with ASN.1 signature data', function() {
+      // signature has bytes that could look like ASN.1 data
+      // TODO: add more similar tests with possible ASN.1 data in BIT
+      // .. STRINGS that are just data.
+      var certPem =
+        '-----BEGIN CERTIFICATE-----\r\n' +
+        'MIIDsjCCApqgAwIBAgIUR+JnrOX42d+AXaXwLDWYsxmr+DwwDQYJKoZIhvcNAQEL\r\n' +
+        'BQAwgYgxJjAkBgNVBAMMHVRlc3QgSW50ZXJtZWRpYXRlIENlcnRpZmljYXRlMSEw\r\n' +
+        'HwYDVQQLDBhUZXN0IE9yZ2FuaXphdGlvbmFsIFVuaXQxGjAYBgNVBAoMEVRlc3Qg\r\n' +
+        'T3JnYW5pemF0aW9uMRIwEAYDVQQHDAlUZXN0IENpdHkxCzAJBgNVBAYTAlVTMB4X\r\n' +
+        'DTE3MDEzMDE1NTk0MloXDTIyMDEzMDE1NTk0MlowezEZMBcGA1UEAwwQVGVzdCBD\r\n' +
+        'ZXJ0aWZpY2F0ZTEhMB8GA1UECwwYVGVzdCBPcmdhbml6YXRpb25hbCBVbml0MRow\r\n' +
+        'GAYDVQQKDBFUZXN0IE9yZ2FuaXphdGlvbjESMBAGA1UEBwwJVGVzdCBDaXR5MQsw\r\n' +
+        'CQYDVQQGEwJVUzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAMEBoBE9\r\n' +
+        'Kprzj7LRbK0JHKSNI93eKFDgkRc1lQWVf2cVyU6bncBDdxrCFLG6vadpQzPQDCJs\r\n' +
+        'ePlzM4J2TueTtbNTgLj3EBwwB1Ju9kOidxn5hCASWR0EObLiHSq3zlM/ABZOEOtz\r\n' +
+        'hPTGWnp7jfoDeTgK/zNnCsqdT1w1nzsz1u8zZ+96dEul6RC313CxV5Xq+Qacky8f\r\n' +
+        '2tug0wgvmcYqfd6AIg0btIjhREsulJK0QqSjqmzkLiDBJRsOHzi9zAusYBkvqUMj\r\n' +
+        '9ae2j4adIHKNzzgRAif8Bu9yXZ0iK3im6BQhBiC+unwMRAjbHdPI73ASJFdqjHb0\r\n' +
+        'uAkno0WIORHnQvECAwEAAaMgMB4wDgYDVR0PAQH/BAQDAgbAMAwGA1UdEwEB/wQC\r\n' +
+        'MAAwDQYJKoZIhvcNAQELBQADggEBACsFnYH7ByzOFQo5zdOJp4NcmV6yDaTgJoEg\r\n' +
+        '71oPI0bgRkpde2rJT7E41fxxajIGbGgVnvIT2yo2QZNRTJjrnzIoVAfOsgWSp8jf\r\n' +
+        'L6HjyZwK5L6ziBfPYnCAzRC4mwjgR2EkEhbA/HDZCko5CfRR8WIKVmtGweIL/5I+\r\n' +
+        '9aUG7lUKbf3aGLnBMG6YzThNnMW1a4EZt7EZUlz4mZ3C7KH1lqdmeaZT+BdnZFL2\r\n' +
+        'Mjf0zXRaL1k1BpBrzPfmfWfE+gx7EAWF6E/iDu+g1PhX678vFEJiAQkINVwkjA1d\r\n' +
+        '/fpSWNjJEEVPrhWwmoK5+xfM5qC9una9BfFwaUOdYDH59BN6jHE=\r\n' +
+        '-----END CERTIFICATE-----\r\n';
+      // round trip pem -> cert -> asn1 -> der -> asn1 -> cert -> pem
+      var inCert = PKI.certificateFromPem(certPem);
+      var inAsn1 = PKI.certificateToAsn1(inCert);
+      var inDer = ASN1.toDer(inAsn1);
+      var outAsn1 = ASN1.fromDer(inDer);
+      var outCert = PKI.certificateFromAsn1(outAsn1);
+      var outPem = PKI.certificateToPem(outCert);
+      ASSERT.equal(certPem, outPem);
+    });
+
   });
 
   // TODO: add sha-512 and sha-256 fingerprint tests