>From 49bdc8643bb54779d0c6ea72c7e3ca4bc7f06083 Mon Sep 17 00:00:00 2001
From: "Endi S. Dewata" <edewata@redhat.com>
Date: Tue, 21 Jun 2016 18:39:25 +0200
Subject: [PATCH] Fixed KRA cloning issue.

The pki pkcs12-import CLI has been modified not to import
certificates that already exist in the NSS database unless
specifically requested with the --overwrite parameter. This
will avoid changing the trust flags of the CA signing
certificate during KRA cloning.

The some other classes have been modified to provide better
debugging information.

https://fedorahosted.org/pki/ticket/2374
---
 base/common/python/pki/cli/pkcs12.py               | 19 +++++++++++++-
 base/common/python/pki/nssdb.py                    | 22 ++++++++++++----
 .../netscape/cmstools/pkcs12/PKCS12ImportCLI.java  |  6 +++--
 .../cms/servlet/csadmin/ConfigurationUtils.java    | 29 ++++++++++++++++++++--
 .../cmscore/ldapconn/LdapJssSSLSocketFactory.java  | 18 ++++++++------
 .../src/netscape/security/pkcs/PKCS12Util.java     | 21 +++++++++++-----
 6 files changed, 91 insertions(+), 24 deletions(-)

diff --git a/base/common/python/pki/cli/pkcs12.py b/base/common/python/pki/cli/pkcs12.py
index a7c32cc2b1218021bc15b5ea030df24c8b7143b9..3fcea35a4cc45641ec53d3aba933735c1b2c065a 100644
--- a/base/common/python/pki/cli/pkcs12.py
+++ b/base/common/python/pki/cli/pkcs12.py
@@ -55,6 +55,7 @@ class PKCS12ImportCLI(pki.cli.CLI):
         print('      --no-trust-flags               Do not include trust flags')
         print('      --no-user-certs                Do not import user certificates')
         print('      --no-ca-certs                  Do not import CA certificates')
+        print('      --overwrite                    Overwrite existing certificates')
         print('  -v, --verbose                      Run in verbose mode.')
         print('      --debug                        Run in debug mode.')
         print('      --help                         Show help message.')
@@ -65,7 +66,7 @@ class PKCS12ImportCLI(pki.cli.CLI):
         try:
             opts, _ = getopt.gnu_getopt(args, 'v', [
                 'pkcs12-file=', 'pkcs12-password=', 'pkcs12-password-file=',
-                'no-trust-flags', 'no-user-certs', 'no-ca-certs',
+                'no-trust-flags', 'no-user-certs', 'no-ca-certs', 'overwrite',
                 'verbose', 'debug', 'help'])
 
         except getopt.GetoptError as e:
@@ -79,6 +80,7 @@ class PKCS12ImportCLI(pki.cli.CLI):
         no_trust_flags = False
         import_user_certs = True
         import_ca_certs = True
+        overwrite = False
         debug = False
 
         for o, a in opts:
@@ -100,6 +102,9 @@ class PKCS12ImportCLI(pki.cli.CLI):
             elif o == '--no-ca-certs':
                 import_ca_certs = False
 
+            elif o == '--overwrite':
+                overwrite = True
+
             elif o in ('-v', '--verbose'):
                 self.set_verbose(True)
 
@@ -221,6 +226,15 @@ class PKCS12ImportCLI(pki.cli.CLI):
                     cert_id = cert_info['id']
                     nickname = cert_info['nickname']
 
+                    cert = nssdb.get_cert(nickname)
+
+                    if cert:
+                        if not overwrite:
+                            print('WARNING: cert %s already exists' % nickname)
+                            continue
+
+                        nssdb.remove_cert(nickname)
+
                     if 'trust_flags' in cert_info:
                         trust_flags = cert_info['trust_flags']
                     else:
@@ -292,6 +306,9 @@ class PKCS12ImportCLI(pki.cli.CLI):
             if no_trust_flags:
                 cmd.extend(['--no-trust-flags'])
 
+            if overwrite:
+                cmd.extend(['--overwrite'])
+
             if self.verbose:
                 cmd.extend(['--verbose'])
 
diff --git a/base/common/python/pki/nssdb.py b/base/common/python/pki/nssdb.py
index 0c27c3f19b6d938e4e335aaf0541d0ca0d0c1796..f563fd81e5584eb218e4a7f9a2ff3eff222296a2 100644
--- a/base/common/python/pki/nssdb.py
+++ b/base/common/python/pki/nssdb.py
@@ -423,12 +423,20 @@ class NSSDatabase(object):
             output_format_option
         ])
 
-        cert_data = subprocess.check_output(cmd)
+        try:
+            cert_data = subprocess.check_output(cmd)
 
-        if output_format == 'base64':
-            cert_data = base64.b64encode(cert_data)
+            if output_format == 'base64':
+                cert_data = base64.b64encode(cert_data)
 
-        return cert_data
+            return cert_data
+
+        except subprocess.CalledProcessError:
+            # All certutil errors return the same code (i.e. 255).
+            # For now assume it was caused by missing certificate.
+            # TODO: Check error message. If it's caused by other
+            # issue, throw exception.
+            return None
 
     def remove_cert(self, nickname):
 
@@ -576,7 +584,8 @@ class NSSDatabase(object):
                       pkcs12_password=None,
                       pkcs12_password_file=None,
                       no_user_certs=False,
-                      no_ca_certs=False):
+                      no_ca_certs=False,
+                      overwrite=False):
 
         tmpdir = tempfile.mkdtemp()
 
@@ -613,6 +622,9 @@ class NSSDatabase(object):
             if no_ca_certs:
                 cmd.extend(['--no-ca-certs'])
 
+            if overwrite:
+                cmd.extend(['--overwrite'])
+
             subprocess.check_call(cmd)
 
         finally:
diff --git a/base/java-tools/src/com/netscape/cmstools/pkcs12/PKCS12ImportCLI.java b/base/java-tools/src/com/netscape/cmstools/pkcs12/PKCS12ImportCLI.java
index ae574d3870a610cdf009b852732644675424a700..862fffb64b23b9f1f02d7923af3584f387167636 100644
--- a/base/java-tools/src/com/netscape/cmstools/pkcs12/PKCS12ImportCLI.java
+++ b/base/java-tools/src/com/netscape/cmstools/pkcs12/PKCS12ImportCLI.java
@@ -61,6 +61,7 @@ public class PKCS12ImportCLI extends CLI {
         options.addOption(option);
 
         options.addOption(null, "no-trust-flags", false, "Do not include trust flags");
+        options.addOption(null, "overwrite", false, "Overwrite existing certificates");
 
         options.addOption("v", "verbose", false, "Run in verbose mode.");
         options.addOption(null, "debug", false, "Run in debug mode.");
@@ -125,6 +126,7 @@ public class PKCS12ImportCLI extends CLI {
         Password password = new Password(passwordString.toCharArray());
 
         boolean trustFlagsEnabled = !cmd.hasOption("no-trust-flags");
+        boolean overwrite = cmd.hasOption("overwrite");
 
         try {
             PKCS12Util util = new PKCS12Util();
@@ -134,12 +136,12 @@ public class PKCS12ImportCLI extends CLI {
 
             if (nicknames.length == 0) {
                 // store all certificates
-                util.storeIntoNSS(pkcs12);
+                util.storeIntoNSS(pkcs12, overwrite);
 
             } else {
                 // load specified certificates
                 for (String nickname : nicknames) {
-                    util.storeCertIntoNSS(pkcs12, nickname);
+                    util.storeCertIntoNSS(pkcs12, nickname, overwrite);
                 }
             }
 
diff --git a/base/server/cms/src/com/netscape/cms/servlet/csadmin/ConfigurationUtils.java b/base/server/cms/src/com/netscape/cms/servlet/csadmin/ConfigurationUtils.java
index 2da4e48658354116eb8ef1427133bc928226bd0f..308f3e7f8ad0bb1f9a15062a2f6e307bbfe7432b 100644
--- a/base/server/cms/src/com/netscape/cms/servlet/csadmin/ConfigurationUtils.java
+++ b/base/server/cms/src/com/netscape/cms/servlet/csadmin/ConfigurationUtils.java
@@ -686,6 +686,7 @@ public class ConfigurationUtils {
     public static boolean updateConfigEntries(String hostname, int port, boolean https,
             String servlet, MultivaluedMap<String, String> content, IConfigStore config)
                     throws Exception {
+
         CMS.debug("updateConfigEntries start");
         String c = post(hostname, port, https, servlet, content, null, null);
 
@@ -704,6 +705,7 @@ public class ConfigurationUtils {
 
                 cstype = config.getString("cs.type", "");
 
+                CMS.debug("Master's configuration:");
                 Document doc = parser.getDocument();
                 NodeList list = doc.getElementsByTagName("name");
                 int len = list.getLength();
@@ -711,9 +713,12 @@ public class ConfigurationUtils {
                     Node n = list.item(i);
                     NodeList nn = n.getChildNodes();
                     String name = nn.item(0).getNodeValue();
+                    CMS.debug(" - " + name);
+
                     Node parent = n.getParentNode();
                     nn = parent.getChildNodes();
                     int len1 = nn.getLength();
+
                     String v = "";
                     for (int j = 0; j < len1; j++) {
                         Node nv = nn.item(j);
@@ -729,33 +734,43 @@ public class ConfigurationUtils {
                     if (name.equals("internaldb.basedn")) {
                         config.putString(name, v);
                         config.putString("preop.internaldb.master.basedn", v);
+
                     } else if (name.startsWith("internaldb")) {
                         config.putString(name.replaceFirst("internaldb", "preop.internaldb.master"), v);
+
                     } else if (name.equals("instanceId")) {
                         config.putString("preop.master.instanceId", v);
+
                     } else if (name.equals("cloning.signing.nickname")) {
                         config.putString("preop.master.signing.nickname", v);
                         config.putString("preop.cert.signing.nickname", v);
+
                     } else if (name.equals("cloning.ocsp_signing.nickname")) {
                         config.putString("preop.master.ocsp_signing.nickname", v);
                         config.putString("preop.cert.ocsp_signing.nickname", v);
+
                     } else if (name.equals("cloning.subsystem.nickname")) {
                         config.putString("preop.master.subsystem.nickname", v);
                         config.putString("preop.cert.subsystem.nickname", v);
+
                     } else if (name.equals("cloning.transport.nickname")) {
                         config.putString("preop.master.transport.nickname", v);
                         config.putString("kra.transportUnit.nickName", v);
                         config.putString("preop.cert.transport.nickname", v);
+
                     } else if (name.equals("cloning.storage.nickname")) {
                         config.putString("preop.master.storage.nickname", v);
                         config.putString("kra.storageUnit.nickName", v);
                         config.putString("preop.cert.storage.nickname", v);
+
                     } else if (name.equals("cloning.audit_signing.nickname")) {
                         config.putString("preop.master.audit_signing.nickname", v);
                         config.putString("preop.cert.audit_signing.nickname", v);
                         config.putString(name, v);
+
                     } else if (name.startsWith("cloning.ca")) {
                         config.putString(name.replaceFirst("cloning", "preop"), v);
+
                     } else if (name.equals("cloning.signing.keyalgorithm")) {
                         config.putString(name.replaceFirst("cloning", "preop.cert"), v);
                         if (cstype.equals("CA")) {
@@ -767,13 +782,16 @@ public class ConfigurationUtils {
                     } else if (name.equals("cloning.transport.keyalgorithm")) {
                         config.putString(name.replaceFirst("cloning", "preop.cert"), v);
                         config.putString("kra.transportUnit.signingAlgorithm", v);
+
                     } else if (name.equals("cloning.ocsp_signing.keyalgorithm")) {
                         config.putString(name.replaceFirst("cloning", "preop.cert"), v);
                         if (cstype.equals("CA")) {
                             config.putString("ca.ocsp_signing.defaultSigningAlgorithm", v);
                         }
+
                     } else if (name.startsWith("cloning")) {
                         config.putString(name.replaceFirst("cloning", "preop.cert"), v);
+
                     } else {
                         config.putString(name, v);
                     }
@@ -1183,12 +1201,15 @@ public class ConfigurationUtils {
         return org.mozilla.jss.crypto.PrivateKey.Type.RSA;
     }
 
-    public static boolean isCASigningCert(String name) {
+    public static boolean isCASigningCert(String name) throws EBaseException {
         IConfigStore cs = CMS.getConfigStore();
         try {
             String nickname = cs.getString("preop.master.signing.nickname");
+            CMS.debug("Property preop.master.signing.nickname: " + nickname);
             if (nickname.equals(name)) return true;
-        } catch(Exception e) {
+
+        } catch (EPropertyNotFound e) {
+            CMS.debug("Property preop.master.signing.nickname not found -> cert " + name + " is not CA signing cert");
             // nickname may not exist if this is not cloning a CA
         };
 
@@ -1245,6 +1266,8 @@ public class ConfigurationUtils {
         IConfigStore cs = CMS.getConfigStore();
         String certList = cs.getString("preop.cert.list", "");
         StringTokenizer st = new StringTokenizer(certList, ",");
+
+        CMS.debug("Master certs:");
         while (st.hasMoreTokens()) {
             String s = st.nextToken();
             if (s.equals("sslserver"))
@@ -1256,7 +1279,9 @@ public class ConfigurationUtils {
             name = "preop.cert." + s + ".dn";
             String dn = cs.getString(name);
             list.add(dn);
+            CMS.debug(" - " + name + ": " + dn);
         }
+
         return list;
     }
 
diff --git a/base/server/cmscore/src/com/netscape/cmscore/ldapconn/LdapJssSSLSocketFactory.java b/base/server/cmscore/src/com/netscape/cmscore/ldapconn/LdapJssSSLSocketFactory.java
index 720882a1534a95b016f9f5debd4d1ffb2be9e808..182812cfbaf223d7a136cce01f19bc2cf9dce8bd 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/ldapconn/LdapJssSSLSocketFactory.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/ldapconn/LdapJssSSLSocketFactory.java
@@ -21,9 +21,6 @@ import java.io.IOException;
 import java.net.Socket;
 import java.net.UnknownHostException;
 
-import netscape.ldap.LDAPException;
-import netscape.ldap.LDAPSSLSocketFactoryExt;
-
 import org.mozilla.jss.ssl.SSLHandshakeCompletedEvent;
 import org.mozilla.jss.ssl.SSLHandshakeCompletedListener;
 import org.mozilla.jss.ssl.SSLSocket;
@@ -31,6 +28,9 @@ import org.mozilla.jss.ssl.SSLSocket;
 import com.netscape.certsrv.apps.CMS;
 import com.netscape.certsrv.logging.ILogger;
 
+import netscape.ldap.LDAPException;
+import netscape.ldap.LDAPSSLSocketFactoryExt;
+
 /**
  * Uses HCL ssl socket.
  *
@@ -65,17 +65,18 @@ public class LdapJssSSLSocketFactory implements LDAPSSLSocketFactoryExt {
 
             if (mClientAuthCertNickname != null) {
                 mClientAuth = true;
-                CMS.debug(
-                        "LdapJssSSLSocket set client auth cert nickname" +
-                                mClientAuthCertNickname);
+                CMS.debug("LdapJssSSLSocket: set client auth cert nickname " +
+                        mClientAuthCertNickname);
                 s.setClientCertNickname(mClientAuthCertNickname);
             }
             s.forceHandshake();
+
         } catch (UnknownHostException e) {
             log(ILogger.LL_FAILURE,
                     CMS.getLogMessage("CMSCORE_LDAPCONN_UNKNOWN_HOST"));
             throw new LDAPException(
-                    "Cannot Create JSS SSL Socket - Unknown host");
+                    "Cannot Create JSS SSL Socket - Unknown host: " + e);
+
         } catch (IOException e) {
             if (s != null) {
                 try {
@@ -85,8 +86,9 @@ public class LdapJssSSLSocketFactory implements LDAPSSLSocketFactoryExt {
                 }
             }
             log(ILogger.LL_FAILURE, CMS.getLogMessage("CMSCORE_LDAPCONN_IO_ERROR", e.toString()));
-            throw new LDAPException("IO Error creating JSS SSL Socket");
+            throw new LDAPException("IO Error creating JSS SSL Socket: " + e);
         }
+
         return s;
     }
 
diff --git a/base/util/src/netscape/security/pkcs/PKCS12Util.java b/base/util/src/netscape/security/pkcs/PKCS12Util.java
index b1b0f0768c8feae3508b1d5e7fb06f152f0ccd29..178a861c1c0b4d8b2e5df182258704013f0f9292 100644
--- a/base/util/src/netscape/security/pkcs/PKCS12Util.java
+++ b/base/util/src/netscape/security/pkcs/PKCS12Util.java
@@ -635,14 +635,23 @@ public class PKCS12Util {
         wrapper.unwrapPrivate(encpkey, getPrivateKeyType(publicKey), publicKey);
     }
 
-    public void storeCertIntoNSS(PKCS12 pkcs12, PKCS12CertInfo certInfo) throws Exception {
+    public void storeCertIntoNSS(PKCS12 pkcs12, PKCS12CertInfo certInfo, boolean overwrite) throws Exception {
 
         CryptoManager cm = CryptoManager.getInstance();
+        CryptoToken ct = cm.getInternalKeyStorageToken();
+        CryptoStore store = ct.getCryptoStore();
 
-        X509Certificate cert;
         BigInteger id = certInfo.getID();
         PKCS12KeyInfo keyInfo = pkcs12.getKeyInfoByID(id);
 
+        for (X509Certificate cert : cm.findCertsByNickname(certInfo.nickname)) {
+            if (!overwrite) {
+                return;
+            }
+            store.deleteCert(cert);
+        }
+
+        X509Certificate cert;
         if (keyInfo != null) { // cert has key
             logger.fine("Importing user key for " + certInfo.nickname);
             importKey(pkcs12, keyInfo);
@@ -660,19 +669,19 @@ public class PKCS12Util {
             setTrustFlags(cert, certInfo.trustFlags);
     }
 
-    public void storeCertIntoNSS(PKCS12 pkcs12, String nickname) throws Exception {
+    public void storeCertIntoNSS(PKCS12 pkcs12, String nickname, boolean overwrite) throws Exception {
         Collection<PKCS12CertInfo> certInfos = pkcs12.getCertInfosByNickname(nickname);
         for (PKCS12CertInfo certInfo : certInfos) {
-            storeCertIntoNSS(pkcs12, certInfo);
+            storeCertIntoNSS(pkcs12, certInfo, overwrite);
         }
     }
 
-    public void storeIntoNSS(PKCS12 pkcs12) throws Exception {
+    public void storeIntoNSS(PKCS12 pkcs12, boolean overwrite) throws Exception {
 
         logger.info("Storing data into NSS database");
 
         for (PKCS12CertInfo certInfo : pkcs12.getCertInfos()) {
-            storeCertIntoNSS(pkcs12, certInfo);
+            storeCertIntoNSS(pkcs12, certInfo, overwrite);
         }
     }
 }
-- 
2.4.11

