Support PEM BER data with unparsed trailing data.

.github/workflows/main.yml

@@ -8,7 +8,7 @@ jobs:
     timeout-minutes: 10
     strategy:
       matrix:
-        node-version: [6.x, 8.x, 10.x, 12.x, 14.x]
+        node-version: [6.x, 8.x, 10.x, 12.x, 14.x, 16.x]
     steps:
     - uses: actions/checkout@v2
     - name: Use Node.js ${{ matrix.node-version }}

CHANGELOG.md

@@ -1,6 +1,36 @@
 Forge ChangeLog
 ===============
 
+## 1.4.0 - 2022-xx-xx
+
+### Added
+- [asn1] Add `fromPemBer()` call that is more permissive than `fromDer()` and
+  allows trailing data.
+  - [RFC 7468](https://www.rfc-editor.org/rfc/rfc7468) PEM data is BER encoded.
+    The RFC recommends to prefer DER over BER encoding throughout and is
+    described in Appendix B.
+  - PKCS#7 PEM data with trailing zeros appears in the wild. This may be
+    intentional, but unneeded, padding. In any case, it should be accepted.
+  - Recent `node-forge` releases made `fromDer` more strict to by default throw
+    an error when not all data is decoded. `fromDer` is used in many places
+    even for BER data, but in this case an alternate is needed to allow for at
+    least this trailing data.
+  - The API is named `fromPemBer` rather than `fromBer` since it is currently
+    intended to handle only the subset of PEM BER that is DER data followed by
+    possible unparsed bytes.
+  - **NOTE**: This API may not handle all PEM BER data. If other data in wild
+    is found that needs better support please file an issue with an example.
+
+### Fixes
+- Calls to `asn1.fromDer` that occurred on data from `pem.decode()`, or
+  similar, now use `asn1.fromPemBer` to be more permissive in allowing possible
+  trailing or padding bytes.
+
+### Changed
+- [asn1] `fromDer` error message changed to reflect also using the API with BER
+  data. Changed from `'Unparsed DER bytes remain after ASN.1 parsing.'` to
+  `'Unparsed bytes remain after ASN.1 parsing.'`.
+
 ## 1.3.1 - 2022-03-29
 
 ### Fixes

lib/asn1.js

@@ -457,14 +457,55 @@ asn1.fromDer = function(bytes, options) {
   var byteCount = bytes.length();
   var value = _fromDer(bytes, bytes.length(), 0, options);
   if(options.parseAllBytes && bytes.length() !== 0) {
-    var error = new Error('Unparsed DER bytes remain after ASN.1 parsing.');
+    var error = new Error('Unparsed bytes remain after ASN.1 parsing.');
     error.byteCount = byteCount;
     error.remaining = bytes.length();
     throw error;
   }
   return value;
 };
 
+/**
+ * Parses an asn1 object from a byte buffer in PEM BER format as DER.
+ *
+ * RFC 7468 specifies PEM data is BER encoded. This call will decode data using
+ * `fromDer` but allow trailing data. This will allow a subset of BER data that
+ * appears in the wild with trailing data. Note that RFC 7468 recommends
+ * to prefer DER over BER encoding as mentioned throughout and described in
+ * Appendix B.
+ *
+ * @reference https://www.rfc-editor.org/rfc/rfc7468
+ *
+ * @param bytes the byte buffer to parse from.
+ * @param [options] object with options or boolean strict flag
+ *          [strict] true to be strict when checking value lengths, false to
+ *            allow truncated values (default: true).
+ *          [parseAllBytes] true to ensure all bytes are parsed
+ *            (default: false)
+ *          [decodeBitStrings] true to attempt to decode the content of
+ *            BIT STRINGs (not OCTET STRINGs) using strict mode. Note that
+ *            without schema support to understand the data context this can
+ *            erroneously decode values that happen to be valid ASN.1. This
+ *            flag will be deprecated or removed as soon as schema support is
+ *            available. (default: true)
+ *
+ * @throws Will throw an error for various malformed input conditions.
+ *
+ * @return the parsed asn1 object.
+ */
+asn1.fromPemBer = function(bytes, options) {
+  if(options === undefined) {
+    options = {
+      parseAllBytes: false
+    };
+  }
+  if(!('parseAllBytes' in options)) {
+    options.parseAllBytes = false;
+  }
+
+  return asn1.fromDer(bytes, options);
+};
+
 /**
  * Internal function to parse an asn1 object from a byte buffer in DER format.
  *

lib/pbe.js

@@ -421,8 +421,8 @@ pki.encryptedPrivateKeyFromPem = function(pem) {
       'PEM is encrypted.');
   }
 
-  // convert DER to ASN.1 object
-  return asn1.fromDer(msg.body);
+  // convert BER to ASN.1 object
+  return asn1.fromPemBer(msg.body);
 };
 
 /**
@@ -616,7 +616,7 @@ pki.decryptRsaPrivateKey = function(pem, password) {
     rval = pki.decryptPrivateKeyInfo(asn1.fromDer(rval), password);
   } else {
     // decryption already performed above
-    rval = asn1.fromDer(rval);
+    rval = asn1.fromPemBer(rval);
   }
 
   if(rval !== null) {

lib/pkcs12.js

@@ -517,7 +517,8 @@ function _decodePkcs7Data(data) {
  * @param {String} password Password to decrypt with (optional).
  */
 function _decodeAuthenticatedSafe(pfx, authSafe, strict, password) {
-  authSafe = asn1.fromDer(authSafe, strict);  /* actually it's BER encoded */
+  // actually it's BER encoded
+  authSafe = asn1.fromDer(authSafe, strict);
 
   if(authSafe.tagClass !== asn1.Class.UNIVERSAL ||
      authSafe.type !== asn1.Type.SEQUENCE ||

lib/pkcs7.js

@@ -53,8 +53,8 @@ p7.messageFromPem = function(pem) {
     throw new Error('Could not convert PKCS#7 message from PEM; PEM is encrypted.');
   }
 
-  // convert DER to ASN.1 object
-  var obj = asn1.fromDer(msg.body);
+  // convert BER to ASN.1 object
+  var obj = asn1.fromPemBer(msg.body);
 
   return p7.messageFromAsn1(obj);
 };

lib/pki.js

@@ -61,8 +61,8 @@ pki.privateKeyFromPem = function(pem) {
     throw new Error('Could not convert private key from PEM; PEM is encrypted.');
   }
 
-  // convert DER to ASN.1 object
-  var obj = asn1.fromDer(msg.body);
+  // convert BER to ASN.1 object
+  var obj = asn1.fromPemBer(msg.body);
 
   return pki.privateKeyFromAsn1(obj);
 };

lib/tls.js

@@ -2903,7 +2903,7 @@ tls.createCertificate = function(c) {
 
         var der = forge.util.createBuffer(msg.body);
         if(asn1 === null) {
-          asn1 = forge.asn1.fromDer(der.bytes(), false);
+          asn1 = forge.asn1.fromPemBer(der.bytes(), {strict: false});
         }
 
         // certificate entry is itself a vector with 3 length bytes

lib/x509.js

@@ -816,8 +816,8 @@ pki.certificateFromPem = function(pem, computeHash, strict) {
       'Could not convert certificate from PEM; PEM is encrypted.');
   }
 
-  // convert DER to ASN.1 object
-  var obj = asn1.fromDer(msg.body, strict);
+  // convert BER to ASN.1 object
+  var obj = asn1.fromPemBer(msg.body, {strict: strict});
 
   return pki.certificateFromAsn1(obj, computeHash);
 };
@@ -859,8 +859,8 @@ pki.publicKeyFromPem = function(pem) {
     throw new Error('Could not convert public key from PEM; PEM is encrypted.');
   }
 
-  // convert DER to ASN.1 object
-  var obj = asn1.fromDer(msg.body);
+  // convert BER to ASN.1 object
+  var obj = asn1.fromPemBer(msg.body);
 
   return pki.publicKeyFromAsn1(obj);
 };
@@ -977,8 +977,8 @@ pki.certificationRequestFromPem = function(pem, computeHash, strict) {
       'PEM is encrypted.');
   }
 
-  // convert DER to ASN.1 object
-  var obj = asn1.fromDer(msg.body, strict);
+  // convert BER to ASN.1 object
+  var obj = asn1.fromPemBer(msg.body, {strict: strict});
 
   return pki.certificationRequestFromAsn1(obj, computeHash);
 };

tests/unit/pkcs7.js

@@ -24,6 +24,22 @@ var UTIL = require('../../lib/util');
       'gEAm2mfSF5xFPLEqqFkvKTM4w8PfhnF0ehmfQNApvoWQRQanNWLCT+Q9GHx6DCFj\r\n' +
       'TUHl+53x88BrCl1E7FhYPs92\r\n' +
       '-----END PKCS7-----\r\n',
+    p7ZeroPadded:
+      '-----BEGIN PKCS7-----\r\n' +
+      'MIICTgYJKoZIhvcNAQcDoIICPzCCAjsCAQAxggHGMIIBwgIBADCBqTCBmzELMAkG\r\n' +
+      'A1UEBhMCREUxEjAQBgNVBAgMCUZyYW5jb25pYTEQMA4GA1UEBwwHQW5zYmFjaDEV\r\n' +
+      'MBMGA1UECgwMU3RlZmFuIFNpZWdsMRIwEAYDVQQLDAlHZWllcmxlaW4xFjAUBgNV\r\n' +
+      'BAMMDUdlaWVybGVpbiBERVYxIzAhBgkqhkiG9w0BCQEWFHN0ZXNpZUBicm9rZW5w\r\n' +
+      'aXBlLmRlAgkA1FQcQNg14vMwDQYJKoZIhvcNAQEBBQAEggEAJhWQz5SniCd1w3A8\r\n' +
+      'uKVZEfc8Tp21I7FMfFqou+UOVsZCq7kcEa9uv2DIj3o7zD8wbLK1fuyFi4SJxTwx\r\n' +
+      'kR0a6V4bbonIpXPPJ1f615dc4LydAi2tv5w14LJ1Js5XCgGVnkAmQHDaW3EHXB7X\r\n' +
+      'T4w9PR3+tcS/5YAnWaM6Es38zCKHd7TnHpuakplIkwSK9rBFAyA1g/IyTPI+ktrE\r\n' +
+      'EHcVuJcz/7eTlF6wJEa2HL8F1TVWuL0p/0GsJP/8y0MYGdCdtr+TIVo//3YGhoBl\r\n' +
+      'N4tnheFT/jRAzfCZtflDdgAukW24CekrJ1sG2M42p5cKQ5rGFQtzNy/n8EjtUutO\r\n' +
+      'HD5YITBsBgkqhkiG9w0BBwEwHQYJYIZIAWUDBAEqBBBmlpfy3WrYj3uWW7+xNEiH\r\n' +
+      'gEAm2mfSF5xFPLEqqFkvKTM4w8PfhnF0ehmfQNApvoWQRQanNWLCT+Q9GHx6DCFj\r\n' +
+      'TUHl+53x88BrCl1E7FhYPs92AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\r\n' +
+      '-----END PKCS7-----\r\n',
     certificate:
       '-----BEGIN CERTIFICATE-----\r\n' +
       'MIIDtDCCApwCCQDUVBxA2DXi8zANBgkqhkiG9w0BAQUFADCBmzELMAkGA1UEBhMC\r\n' +
@@ -412,6 +428,18 @@ var UTIL = require('../../lib/util');
       ASSERT.equal(p7.encryptedContent.parameter.data.length, 16); // IV
     });
 
+    it('should import padded message from PEM', function() {
+      ASSERT.doesNotThrow(function() {
+        var p7 = PKCS7.messageFromPem(_pem.p7);
+        var p7ZeroPadded = PKCS7.messageFromPem(_pem.p7ZeroPadded);
+        ASSERT.deepEqual(p7.type, p7ZeroPadded.type);
+        ASSERT.deepEqual(p7.version, p7ZeroPadded.version);
+        ASSERT.deepEqual(p7.recipients, p7ZeroPadded.recipients);
+        ASSERT.deepEqual(p7.encryptedContent, p7ZeroPadded.encryptedContent);
+        ASSERT.deepEqual(p7.rawCapture, p7ZeroPadded.rawCapture);
+      });
+    });
+
     it('should import indefinite length message from PEM', function() {
       ASSERT.doesNotThrow(function() {
         var p7 = PKCS7.messageFromPem(_pem.p7IndefiniteLength);

tests/unit/rsa.js

@@ -830,7 +830,7 @@ var UTIL = require('../../lib/util');
         ASSERT.throws(function() {
           publicKey.verify(md.digest().getBytes(), S);
         },
-        /^Error: Unparsed DER bytes remain after ASN.1 parsing.$/);
+        /^Error: Unparsed bytes remain after ASN.1 parsing.$/);
       }
 
       function _checkBadDigestInfo(publicKey, S, skipTailingGarbage) {