cleanup decryption key handling

Author: joseph-neeraj

lib/mcapi/crypto/field-level-crypto.js

@@ -258,7 +258,7 @@ function validateFingerprint(config, contains) {
     config[propertiesFingerprint[0]] !== "publicKey"
   ) {
     throw Error(
-      "Config not valid: propertiesFingerprint should be: 'certificate' or 'publicKey'"
+      `Config not valid: ${propertiesFingerprint[0]} should be: 'certificate' or 'publicKey'`
     );
   }
 }

lib/mcapi/utils/utils.js

@@ -1,5 +1,6 @@
 const forge = require("node-forge");
 const fs = require("fs");
+const path = require("path")
 const c = require("../utils/constants");
 
 /**
@@ -195,25 +196,40 @@ function deleteRoot(obj, paths, properties) {
 
 module.exports.getPrivateKey = function (config) {
   if (config.privateKey) {
-    return loadPrivateKey(config.privateKey);
-  } else if (config.keyStore) {
-    if (config.keyStore.includes(".p12")) {
+    return getUnencryptedPrivateKey(config.privateKey);
+  } 
+  
+  if (config.keyStore) {
+    if (config.keyStore.includes(".p12") || (config.keyStoreAlias && config.keyStorePassword)) {
       return getPrivateKey12(
         config.keyStore,
         config.keyStoreAlias,
         config.keyStorePassword
       );
     }
-    if (config.keyStore.includes(".pem")) {
-      return getPrivateKeyPem(config.keyStore);
-    }
-    if (config.keyStore.includes(".der")) {
-      return getPrivateKeyDer(config.keyStore);
-    }
+
+    return getUnencryptedPrivateKey(config.keyStore)
   }
+
   return null;
 };
 
+function getUnencryptedPrivateKey(filePath) {
+  const fileContent = fs.readFileSync(filePath)
+
+  if (!fileContent || fileContent.length < 1) {
+    throw new Error("private key file content is empty")
+  }
+
+  // key is PEM
+  if(fileContent.toString('utf-8').startsWith("-----BEGIN")) {
+    return forge.pki.privateKeyFromPem(fileContent.toString('utf-8'));
+  }
+
+  // attempt to read DER
+  return forge.pki.privateKeyFromAsn1(forge.asn1.fromDer(fileContent.toString('binary')));
+}
+
 function getPrivateKey12(p12Path, alias, password) {
   const p12Content = fs.readFileSync(p12Path, "binary");
 
@@ -248,40 +264,6 @@ function getPrivateKey12(p12Path, alias, password) {
   return keyObj.key;
 }
 
-function getPrivateKeyPem(pemPath) {
-  let pemContent = fs.readFileSync(pemPath, "binary");
-
-  if (!pemContent || pemContent.length <= 1) {
-    throw new Error("pem keystore content is empty");
-  }
-
-  pemContent = pemContent.replace("\n", "");
-  pemContent = pemContent.replace("\r\n", "");
-
-  return forge.pki.privateKeyFromPem(pemContent);
-}
-
-function getPrivateKeyDer(derPath) {
-  const derContent = fs.readFileSync(derPath, "binary");
-
-  if (!derContent || derContent.length <= 1) {
-    throw new Error("der keystore content is empty");
-  }
-
-  const pkeyAsn1 = forge.asn1.fromDer(derContent);
-  return forge.pki.privateKeyFromAsn1(pkeyAsn1);
-}
-
-function loadPrivateKey(privateKeyPath) {
-  const privateKeyContent = fs.readFileSync(privateKeyPath, "binary");
-
-  if (!privateKeyContent || privateKeyContent.length <= 1) {
-    throw new Error("Private key content not valid");
-  }
-
-  return forge.pki.privateKeyFromAsn1(forge.asn1.fromDer(privateKeyContent));
-}
-
 module.exports.readPublicCertificate = function (publicCertificatePath) {
   const certificateContent = fs.readFileSync(publicCertificatePath);
   if (!certificateContent || certificateContent.length <= 1) {
@@ -351,13 +333,10 @@ module.exports.checkConfigFieldsArePopulated = function(config, propertiesBasic,
       return config[elem] !== null && typeof config[elem] !== "undefined";
     });
   };
-  if (typeof config !== "object" || config === null) {
+  if (typeof config !== "object" || config == null) {
     throw Error("Config not valid: config should be an object.");
   }
-  if (
-    config["paths"] === null ||
-    typeof config["paths"] === "undefined" ||
-    !(config["paths"] instanceof Array)
+  if (!Array.isArray(config.paths)
   ) {
     throw Error("Config not valid: paths should be an array of path element.");
   }

test/field-level-crypto.test.js

@@ -131,7 +131,7 @@ describe("Field Level Crypto", () => {
       delete config["publicKeyFingerprintType"];
       assert.throws(
         () => new Crypto(config),
-        /Config not valid: propertiesFingerprint should be: 'certificate' or 'publicKey'/
+        /Config not valid: publicKeyFingerprintType should be: 'certificate' or 'publicKey'/
       );
     });
 
@@ -147,7 +147,7 @@ describe("Field Level Crypto", () => {
       config.publicKeyFingerprintType = "foobar";
       assert.throws(
         () => new Crypto(config),
-        /Config not valid: propertiesFingerprint should be: 'certificate' or 'publicKey'/
+        /Config not valid: publicKeyFingerprintType should be: 'certificate' or 'publicKey'/
       );
     });
 

test/res/empty.p12

@@ -453,7 +453,25 @@ describe("Utils", () => {
     });
   });
 
-  describe("#getPrivateKey12", () => {
+  describe("#getPrivateKey", () => {
+    it("empty p12 file", () => {
+      assert.throws(() => {
+        utils.getPrivateKey({ 
+          keyStore: "./test/res/empty.p12",
+          keyStoreAlias: "mykeyalias",
+          keyStorePassword: "Password1", });
+      }, /p12 keystore content is empty/);
+    });
+    
+    it("empty p12 file unknown extension", () => {
+      assert.throws(() => {
+        utils.getPrivateKey({ 
+          keyStore: "./test/res/keys/pkcs12/empty_key_container.unknown_extension",
+          keyStoreAlias: "mykeyalias",
+          keyStorePassword: "Password1", });
+      }, /p12 keystore content is empty/);
+    });
+
     it("empty alias", () => {
       assert.throws(() => {
         utils.getPrivateKey({
@@ -489,50 +507,82 @@ describe("Utils", () => {
         });
       }, /No key found for alias \[mykeyalias1\]/);
     });
-  });
 
-  describe("#getPrivateKeyPem", () => {
-    it("valid pkcs8 pem", () => {
+    it("valid p12, unknown extension", () => {
+      const pk = utils.getPrivateKey({
+        keyStore: "./test/res/keys/pkcs12/test_key_container.unknown_extension",
+        keyStoreAlias: "mykeyalias",
+        keyStorePassword: "Password1",
+      });
+      assert.ok(pk);
+    });
+
+    it("empty unencrypted file keyStore", () => {
+      assert.throws(() => {
+        utils.getPrivateKey({ keyStore: "./test/res/empty.pem" });
+      }, /private key file content is empty/);
+    });
+
+    it("valid pkcs8 pem in keyStore", () => {
       const pk = utils.getPrivateKey({
         keyStore: "./test/res/keys/pkcs8/test_key.pem",
       });
       assert.ok(pk);
     });
 
-    it("valid pkcs1 pem", () => {
+    it("valid pkcs1 pem in keyStore", () => {
       const pk = utils.getPrivateKey({
         keyStore: "./test/res/keys/pkcs1/test_key.pem",
       });
       assert.ok(pk);
     });
 
-    it("not valid key", () => {
-      assert.throws(() => {
-        utils.getPrivateKey({ keyStore: "./test/res/empty.pem" });
-      }, /pem keystore content is empty/);
+    it("valid pkcs8 der in keyStore", () => {
+      const pk = utils.getPrivateKey({
+        keyStore: "./test/res/keys/pkcs8/test_key.der",
+      });
+      assert.ok(pk);
     });
-  });
 
-  describe("#getPrivateKeyDer", () => {
-    it("valid pkcs8 der", () => {
+    it("valid pkcs1 der in keyStore", () => {
       const pk = utils.getPrivateKey({
-        keyStore: "./test/res/keys/pkcs8/test_key.der",
+        keyStore: "./test/res/keys/pkcs1/test_key.der",
       });
       assert.ok(pk);
     });
 
-    it("not valid key", () => {
+    it("empty unencrypted file in privateKey", () => {
       assert.throws(() => {
-        utils.getPrivateKey({ keyStore: "./test/res/empty.der" });
-      }, /der keystore content is empty/);
+        utils.getPrivateKey({ privateKey: "./test/res/empty.pem" });
+      }, /private key file content is empty/);
     });
-  });
 
-  describe("#loadPrivateKey", () => {
-    it("not valid key", () => {
-      assert.throws(() => {
-        utils.getPrivateKey({ privateKey: "./test/res/empty.key" });
-      }, /Private key content not valid/);
+    it("valid pkcs8 pem in privateKey", () => {
+      const pk = utils.getPrivateKey({
+        privateKey: "./test/res/keys/pkcs8/test_key.pem",
+      });
+      assert.ok(pk);
+    });
+
+    it("valid pkcs1 pem in privateKey", () => {
+      const pk = utils.getPrivateKey({
+        privateKey: "./test/res/keys/pkcs1/test_key.pem",
+      });
+      assert.ok(pk);
+    });
+
+    it("valid pkcs8 der in privateKey", () => {
+      const pk = utils.getPrivateKey({
+        privateKey: "./test/res/keys/pkcs8/test_key.der",
+      });
+      assert.ok(pk);
+    });
+
+    it("valid pkcs1 der in privateKey", () => {
+      const pk = utils.getPrivateKey({
+        privateKey: "./test/res/keys/pkcs1/test_key.der",
+      });
+      assert.ok(pk);
     });
   });