From 887d70ce1b8c4a00f62c2b4eec24326e487da5bd Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal@redhat.com>
Date: Thu, 15 Jun 2017 12:38:26 +1000
Subject: [PATCH 3/4] Fix regression in pkcs12 key bag creation

Commit 633c7c6519c925af7e3700adff29961d72435c7f changed the PKCS #12
file handing to never deal with raw private key material.
PKCS12Util.addKeyBag() was changed to export the PrivateKey handle,
or fail.  This change missed this case where a PKCS #12 file is
loaded from file, possibly modified, then written back to a file,
without involving an NSSDB.  One example is pkcs12-cert-del which
deletes a certificate and associated key from a PKCS #12 file.

Fix the PKCS12Util.addKeyBag() method to use the stored
EncryptedPricateKeyInfo if available, otherwise export the
PrivateKey handle.

Fixes: https://pagure.io/dogtagpki/issue/2741
Change-Id: Ib8098126bc5a79b5dae19103e25b270e2f10ab5a
(cherry picked from commit a411492fe5ad2030bb9f18db9a8ed8d1c45ee7de)
---
 .../src/netscape/security/pkcs/PKCS12Util.java     | 58 ++++++++++++++--------
 1 file changed, 37 insertions(+), 21 deletions(-)

diff --git a/base/util/src/netscape/security/pkcs/PKCS12Util.java b/base/util/src/netscape/security/pkcs/PKCS12Util.java
index 31c7126..1bc1bae 100644
--- a/base/util/src/netscape/security/pkcs/PKCS12Util.java
+++ b/base/util/src/netscape/security/pkcs/PKCS12Util.java
@@ -102,33 +102,49 @@ public class PKCS12Util {
         icert.setObjectSigningTrust(PKCS12.decodeFlags(flags[2]));
     }
 
-    /**
-     * Used during EXPORT to add a private key to the PKCS12.
+    /** Add a private key to the PKCS #12 object.
+     *
+     * The PKCS12KeyInfo object received comes about in two
+     * different scenarios:
+     *
+     * - The private key could be in encrypted byte[] form (e.g.
+     *   when we have merely loaded a PKCS #12 file for inspection
+     *   or e.g. to delete a certificate and its associated key).
+     *   In this case we simply re-use this encrypted private key
+     *   info byte[].
      *
-     * The private key is exported directly from the token, into
-     * an EncryptedPrivateKeyInfo value, then added as a
-     * "Shrouded Key Bag" to the PKCS #12 object.  Unencrypted
-     * key material is never seen.
+     * - The private key could be a be an NSS PrivateKey handle.  In
+     *   this case we must export the PrivateKey from the token to
+     *   obtain the EncryptedPrivateKeyInfo.
+     *
+     * The common final step is to add the encrypted private key
+     * data to a "Shrouded Key Bag" to the PKCS #12 object.
+     * Unencrypted key material is never seen.
      */
     public void addKeyBag(PKCS12KeyInfo keyInfo, Password password,
             SEQUENCE encSafeContents) throws Exception {
-        PrivateKey k = keyInfo.getPrivateKey();
-        if (k == null) {
-            logger.debug("NO PRIVATE KEY for " + keyInfo.subjectDN);
-            return;
-        }
-
         logger.debug("Creating key bag for " + keyInfo.subjectDN);
 
-        PasswordConverter passConverter = new PasswordConverter();
-        byte[] epkiBytes = CryptoManager.getInstance()
-            .getInternalKeyStorageToken()
-            .getCryptoStore()
-            .getEncryptedPrivateKeyInfo(
-                /* NSS has a bug that causes any AES CBC encryption
-                 * to use AES-256, but AlgorithmID contains chosen
-                 * alg.  To avoid mismatch, use AES_256_CBC. */
-                passConverter, password, EncryptionAlgorithm.AES_256_CBC, 0, k);
+        byte[] epkiBytes = keyInfo.getEncryptedPrivateKeyInfoBytes();
+        if (epkiBytes == null) {
+            PrivateKey k = keyInfo.getPrivateKey();
+            if (k == null) {
+                logger.debug("NO PRIVATE KEY for " + keyInfo.subjectDN);
+                return;
+            }
+            logger.debug("Encrypting private key for " + keyInfo.subjectDN);
+
+            PasswordConverter passConverter = new PasswordConverter();
+            epkiBytes = CryptoManager.getInstance()
+                .getInternalKeyStorageToken()
+                .getCryptoStore()
+                .getEncryptedPrivateKeyInfo(
+                    /* NSS has a bug that causes any AES CBC encryption
+                     * to use AES-256, but AlgorithmID contains chosen
+                     * alg.  To avoid mismatch, use AES_256_CBC. */
+                    passConverter, password,
+                    EncryptionAlgorithm.AES_256_CBC, 0, k);
+        }
 
         SET keyAttrs = createKeyBagAttrs(keyInfo);
 
-- 
1.8.3.1