>From 8ec7d8a42c99c072a613ef5001f730e4fa824727 Mon Sep 17 00:00:00 2001
From: Abhishek Koneru <akoneru@redhat.com>
Date: Wed, 27 Jun 2012 10:54:57 -0400
Subject: [PATCH] Fixes for review comments for Patch16

---
 .../src/com/netscape/cmscore/apps/CMSEngine.java   |   14 ++-
 .../netscape/cmscore/security/JssSubsystem.java    |  102 +++++++-------------
 .../src/com/netscape/cmstools/DRMTool.java         |   16 +--
 .../com/netscape/pkisilent/common/ComCrypto.java   |    8 +-
 .../security/extensions/GenericASN1Extension.java  |    5 +-
 .../src/netscape/security/x509/X509CRLImpl.java    |    2 +-
 6 files changed, 60 insertions(+), 87 deletions(-)

diff --git a/base/common/src/com/netscape/cmscore/apps/CMSEngine.java b/base/common/src/com/netscape/cmscore/apps/CMSEngine.java
index b534d4a7100668841f01cfb39336eed195687e66..cbdfee8fc65e435ffae9f568473674d068b1c480 100644
--- a/base/common/src/com/netscape/cmscore/apps/CMSEngine.java
+++ b/base/common/src/com/netscape/cmscore/apps/CMSEngine.java
@@ -308,14 +308,12 @@ public class CMSEngine implements ICMSEngine {
                 CMS.debug("CMSEngine: getPasswordStore(): password store not initialized before.");
                 String pwdClass = mConfig.getString("passwordClass");
 
-                if (pwdClass != null) {
-                    try {
-                        mPasswordStore = (IPasswordStore) Class.forName(pwdClass).newInstance();
-                    } catch (Exception e) {
-                        CMS.debug("CMSEngine: getPasswordStore(): password store initialization failure:"
-                                + e.toString());
-                        throw e;
-                    }
+                try {
+                    mPasswordStore = (IPasswordStore) Class.forName(pwdClass).newInstance();
+                } catch (Exception e) {
+                    CMS.debug("CMSEngine: getPasswordStore(): password store initialization failure:"
+                            + e.toString());
+                    throw e;
                 }
             } else {
                 CMS.debug("CMSEngine: getPasswordStore(): password store initialized before.");
diff --git a/base/common/src/com/netscape/cmscore/security/JssSubsystem.java b/base/common/src/com/netscape/cmscore/security/JssSubsystem.java
index 9d97fefe9c004d16f6775455d3d1d5dc57c0f455..e3b744220fc7791ad09a896b1e6eae0d16ce335a 100644
--- a/base/common/src/com/netscape/cmscore/security/JssSubsystem.java
+++ b/base/common/src/com/netscape/cmscore/security/JssSubsystem.java
@@ -301,9 +301,7 @@ public final class JssSubsystem implements ICryptoSubsystem {
 
         certDir = config.getString(CONFIG_DIR, null);
 
-        CryptoManager.InitializationValues vals =
-                new CryptoManager.InitializationValues(certDir,
-                                                   "", "", "secmod.db");
+        CryptoManager.InitializationValues vals = new CryptoManager.InitializationValues(certDir, "", "", "secmod.db");
 
         vals.removeSunProvider = false;
         vals.installJSSProvider = true;
@@ -313,8 +311,7 @@ public final class JssSubsystem implements ICryptoSubsystem {
             // do nothing
         } catch (Exception e) {
             String[] params = { mId, e.toString() };
-            EBaseException ex = new EBaseException(
-                    CMS.getUserMessage("CMS_BASE_CREATE_SERVICE_FAILED", params));
+            EBaseException ex = new EBaseException(CMS.getUserMessage("CMS_BASE_CREATE_SERVICE_FAILED", params));
 
             log(ILogger.LL_FAILURE, CMS.getLogMessage("CMSCORE_SECURITY_GENERAL_ERROR", ex.toString()));
             throw ex;
@@ -325,8 +322,7 @@ public final class JssSubsystem implements ICryptoSubsystem {
             initSSL();
         } catch (CryptoManager.NotInitializedException e) {
             String[] params = { mId, e.toString() };
-            EBaseException ex = new EBaseException(
-                    CMS.getUserMessage("CMS_BASE_CREATE_SERVICE_FAILED", params));
+            EBaseException ex = new EBaseException(CMS.getUserMessage("CMS_BASE_CREATE_SERVICE_FAILED", params));
 
             log(ILogger.LL_FAILURE, CMS.getLogMessage("CMSCORE_SECURITY_GENERAL_ERROR", ex.toString()));
             throw ex;
@@ -751,8 +747,7 @@ public final class JssSubsystem implements ICryptoSubsystem {
 
     public String getSignatureAlgorithm(String nickname) throws EBaseException {
         try {
-            X509Certificate cert =
-                    CryptoManager.getInstance().findCertByNickname(nickname);
+            X509Certificate cert = CryptoManager.getInstance().findCertByNickname(nickname);
             X509CertImpl impl = new X509CertImpl(cert.getEncoded());
 
             return impl.getSigAlgName();
@@ -773,10 +768,8 @@ public final class JssSubsystem implements ICryptoSubsystem {
 
     public KeyPair getKeyPair(String nickname) throws EBaseException {
         try {
-            X509Certificate cert =
-                    CryptoManager.getInstance().findCertByNickname(nickname);
-            PrivateKey priKey =
-                    CryptoManager.getInstance().findPrivKeyByCert(cert);
+            X509Certificate cert = CryptoManager.getInstance().findCertByNickname(nickname);
+            PrivateKey priKey = CryptoManager.getInstance().findPrivKeyByCert(cert);
             PublicKey publicKey = cert.getPublicKey();
 
             return new KeyPair(publicKey, priKey);
@@ -822,7 +815,6 @@ public final class JssSubsystem implements ICryptoSubsystem {
 
         try {
             KeyPair kp = KeyCertUtil.generateKeyPair(token, kpAlg, keySize, pqg);
-
             return kp;
         } catch (InvalidParameterException e) {
             log(ILogger.LL_FAILURE, CMS.getLogMessage("CMSCORE_SECURITY_KEY_PAIR", e.toString()));
@@ -856,11 +848,9 @@ public final class JssSubsystem implements ICryptoSubsystem {
     public String getCertRequest(String subjectName, KeyPair kp)
             throws EBaseException {
         try {
-            netscape.security.pkcs.PKCS10 pkcs =
-                    KeyCertUtil.getCertRequest(subjectName, kp);
+            netscape.security.pkcs.PKCS10 pkcs = KeyCertUtil.getCertRequest(subjectName, kp);
             ByteArrayOutputStream bs = new ByteArrayOutputStream();
             PrintStream ps = new PrintStream(bs);
-
             pkcs.print(ps);
             return bs.toString();
         } catch (NoSuchAlgorithmException e) {
@@ -1154,8 +1144,8 @@ public final class JssSubsystem implements ICryptoSubsystem {
             if (mNicknameMapCertsTable != null) {
                 mNicknameMapCertsTable.clear();
             } else {
-                CMS.debug("mNicknameMapCertsTable is null");
-                throw new EBaseException("mNicknameMapCertsTable is null");
+                CMS.debug("JssSubsystem::getRootCerts() - mNicknameMapCertsTable is null");
+                throw new EBaseException("JssSubsystem::getRootCerts() - mNicknameMapCertsTable is null");
             }
 
             // a temp hashtable with vectors
@@ -1171,8 +1161,7 @@ public final class JssSubsystem implements ICryptoSubsystem {
                 for (int i = 0; i < list.length; i++) {
                     try {
                         @SuppressWarnings("unused")
-                        PrivateKey key =
-                                CryptoManager.getInstance().findPrivKeyByCert(list[i]); // check for errors
+                        PrivateKey key = CryptoManager.getInstance().findPrivKeyByCert(list[i]); // check for errors
                         Debug.trace("JssSubsystem getRootCerts: find private key "
                                 + list[i].getNickname());
                     } catch (ObjectNotFoundException e) {
@@ -1298,8 +1287,8 @@ public final class JssSubsystem implements ICryptoSubsystem {
         if (mNicknameMapUserCertsTable != null) {
             mNicknameMapUserCertsTable.clear();
         } else {
-            CMS.debug("mNicknameMapCertsTable is null");
-            throw new EBaseException("mNicknameMapCertsTable is null");
+            CMS.debug("JssSubsystem:: getAllCertsManage() : mNicknameMapCertsTable is null");
+            throw new EBaseException("JssSubsystem:: getAllCertsManage() : mNicknameMapCertsTable is null");
         }
 
         try {
@@ -1366,17 +1355,15 @@ public final class JssSubsystem implements ICryptoSubsystem {
         X509Certificate[] certs;
 
         try {
-            certs =
-                    CryptoManager.getInstance().getCACerts();
+            certs = CryptoManager.getInstance().getCACerts();
         } catch (NotInitializedException e) {
             log(ILogger.LL_FAILURE, CMS.getLogMessage("CMSCORE_SECURITY_GET_CA_CERT", e.toString()));
             throw new EBaseException(CMS.getUserMessage("CMS_BASE_CRYPTOMANAGER_UNINITIALIZED"));
         }
 
         if (mNicknameMapCertsTable == null) {
-            CMS.debug("JssSubsystem::getCACerts() - "
-                     + "mNicknameMapCertsTable is null!");
-            throw new EBaseException("mNicknameMapCertsTable is null");
+            CMS.debug("JssSubsystem::getCACerts() - " + "mNicknameMapCertsTable is null!");
+            throw new EBaseException("JssSubsystem::getCACerts() - mNicknameMapCertsTable is null");
         } else {
             mNicknameMapCertsTable.clear();
         }
@@ -1578,18 +1565,15 @@ public final class JssSubsystem implements ICryptoSubsystem {
     public void deleteCert(String nickname, String notAfterTime) throws EBaseException {
         boolean isUserCert = false;
         X509Certificate[] certs = null;
-        ;
 
         try {
             if (mNicknameMapCertsTable != null) {
-                certs =
-                        mNicknameMapCertsTable.get(nickname);
+                certs = mNicknameMapCertsTable.get(nickname);
             }
 
             if (certs == null) {
                 if (mNicknameMapUserCertsTable != null) {
-                    certs =
-                            mNicknameMapUserCertsTable.get(nickname);
+                    certs = mNicknameMapUserCertsTable.get(nickname);
                     if (certs != null) {
                         CMS.debug("in mNicknameMapUserCertsTable, isUserCert is true");
                         isUserCert = true;
@@ -1720,8 +1704,7 @@ public final class JssSubsystem implements ICryptoSubsystem {
 
     public String getSubjectDN(String nickname) throws EBaseException {
         try {
-            X509Certificate cert =
-                    CryptoManager.getInstance().findCertByNickname(nickname);
+            X509Certificate cert = CryptoManager.getInstance().findCertByNickname(nickname);
             X509CertImpl impl = new X509CertImpl(cert.getEncoded());
 
             return impl.getSubjectDN().getName();
@@ -1766,8 +1749,7 @@ public final class JssSubsystem implements ICryptoSubsystem {
             nickname = nickname.substring(index + 1);
         }
         try {
-            X509Certificate[] certs =
-                    CryptoManager.getInstance().findCertsByNickname(nickname);
+            X509Certificate[] certs = CryptoManager.getInstance().findCertsByNickname(nickname);
 
             X509CertImpl impl = null;
             int i = 0;
@@ -1779,10 +1761,8 @@ public final class JssSubsystem implements ICryptoSubsystem {
                         return certs[i];
                 }
             } else {
-                EBaseException e =
-                        new EBaseException(CMS.getUserMessage("CMS_BASE_CERT_NOT_FOUND"));
-                log(ILogger.LL_FAILURE,
-                        CMS.getLogMessage("CMSCORE_SECURITY_PRINT_CERT", e.toString()));
+                EBaseException e = new EBaseException(CMS.getUserMessage("CMS_BASE_CERT_NOT_FOUND"));
+                log(ILogger.LL_FAILURE, CMS.getLogMessage("CMSCORE_SECURITY_PRINT_CERT", e.toString()));
                 throw e;
             }
         } catch (NotInitializedException e) {
@@ -1807,8 +1787,7 @@ public final class JssSubsystem implements ICryptoSubsystem {
             nickname = nickname.substring(index + 1);
         }
         try {
-            X509Certificate[] certs =
-                    CryptoManager.getInstance().findCertsByNickname(nickname);
+            X509Certificate[] certs = CryptoManager.getInstance().findCertsByNickname(nickname);
 
             X509CertImpl impl = null;
             int i = 0;
@@ -1820,10 +1799,8 @@ public final class JssSubsystem implements ICryptoSubsystem {
                         break;
                 }
             } else {
-                EBaseException e =
-                        new EBaseException(CMS.getUserMessage("CMS_BASE_CERT_NOT_FOUND"));
-                log(ILogger.LL_FAILURE,
-                        CMS.getLogMessage("CMSCORE_SECURITY_PRINT_CERT", e.toString()));
+                EBaseException e = new EBaseException(CMS.getUserMessage("CMS_BASE_CERT_NOT_FOUND"));
+                log(ILogger.LL_FAILURE, CMS.getLogMessage("CMSCORE_SECURITY_PRINT_CERT", e.toString()));
                 throw e;
             }
 
@@ -1856,8 +1833,7 @@ public final class JssSubsystem implements ICryptoSubsystem {
             nickname = nickname.substring(index + 1);
         }
         try {
-            X509Certificate[] certs =
-                    CryptoManager.getInstance().findCertsByNickname(nickname);
+            X509Certificate[] certs = CryptoManager.getInstance().findCertsByNickname(nickname);
 
             X509CertImpl impl = null;
             if (certs != null && certs.length > 0) {
@@ -1868,10 +1844,8 @@ public final class JssSubsystem implements ICryptoSubsystem {
                         break;
                 }
             } else {
-                EBaseException e =
-                        new EBaseException(CMS.getUserMessage("CMS_BASE_CERT_NOT_FOUND"));
-                log(ILogger.LL_FAILURE,
-                        CMS.getLogMessage("CMSCORE_SECURITY_PRINT_CERT", e.toString()));
+                EBaseException e = new EBaseException(CMS.getUserMessage("CMS_BASE_CERT_NOT_FOUND"));
+                log(ILogger.LL_FAILURE, CMS.getLogMessage("CMSCORE_SECURITY_PRINT_CERT", e.toString()));
                 throw e;
             }
             CertPrettyPrint print = null;
@@ -1903,8 +1877,7 @@ public final class JssSubsystem implements ICryptoSubsystem {
             nickname = nickname.substring(index + 1);
         }
         try {
-            X509Certificate[] certs =
-                    CryptoManager.getInstance().findCertsByNickname(nickname);
+            X509Certificate[] certs = CryptoManager.getInstance().findCertsByNickname(nickname);
 
             X509CertImpl impl = null;
             if (certs != null && certs.length > 0) {
@@ -1915,10 +1888,8 @@ public final class JssSubsystem implements ICryptoSubsystem {
                         break;
                 }
             } else {
-                EBaseException e =
-                        new EBaseException(CMS.getUserMessage("CMS_BASE_CERT_NOT_FOUND"));
-                log(ILogger.LL_FAILURE,
-                        CMS.getLogMessage("CMSCORE_SECURITY_PRINT_CERT", e.toString()));
+                EBaseException e = new EBaseException(CMS.getUserMessage("CMS_BASE_CERT_NOT_FOUND"));
+                log(ILogger.LL_FAILURE, CMS.getLogMessage("CMSCORE_SECURITY_PRINT_CERT", e.toString()));
                 throw e;
             }
             CertPrettyPrint print = null;
@@ -1953,8 +1924,7 @@ public final class JssSubsystem implements ICryptoSubsystem {
     public String getCertPrettyPrint(String nickname, String date,
             Locale locale) throws EBaseException {
         try {
-            X509Certificate[] certs =
-                    CryptoManager.getInstance().findCertsByNickname(nickname);
+            X509Certificate[] certs = CryptoManager.getInstance().findCertsByNickname(nickname);
 
             if ((certs == null || certs.length == 0) &&
                     mNicknameMapCertsTable != null) {
@@ -2080,8 +2050,7 @@ public final class JssSubsystem implements ICryptoSubsystem {
 
         try {
             certInfo = cert.getCertInfo();
-            SignatureAlgorithm sigAlg =
-                    (SignatureAlgorithm) data.get(Constants.PR_SIGNATURE_ALGORITHM);
+            SignatureAlgorithm sigAlg = (SignatureAlgorithm) data.get(Constants.PR_SIGNATURE_ALGORITHM);
 
             signedCert = KeyCertUtil.signCert(priKey, certInfo, sigAlg);
         } catch (NoSuchTokenException e) {
@@ -2108,15 +2077,14 @@ public final class JssSubsystem implements ICryptoSubsystem {
             if (certinfo == null)
                 return false;
             else {
-                CertificateExtensions exts =
-                        (CertificateExtensions) certinfo.get(X509CertInfo.EXTENSIONS);
+                CertificateExtensions exts = (CertificateExtensions) certinfo.get(X509CertInfo.EXTENSIONS);
 
                 if (exts == null)
                     return false;
                 else {
                     try {
-                        BasicConstraintsExtension ext = (BasicConstraintsExtension)
-                                exts.get(BasicConstraintsExtension.NAME);
+                        BasicConstraintsExtension ext = (BasicConstraintsExtension) exts
+                                .get(BasicConstraintsExtension.NAME);
 
                         if (ext == null)
                             return false;
diff --git a/base/java-tools/src/com/netscape/cmstools/DRMTool.java b/base/java-tools/src/com/netscape/cmstools/DRMTool.java
index 8b415e98d3318878086c542d9edb776cac4a8f97..4403dc8ecf62b63c382abefb688f88ab94fd1c45 100644
--- a/base/java-tools/src/com/netscape/cmstools/DRMTool.java
+++ b/base/java-tools/src/com/netscape/cmstools/DRMTool.java
@@ -4059,7 +4059,7 @@ public class DRMTool {
         PrintWriter writer = null;
         String input = null;
         String line = null;
-        String previous_line = "";
+        String previous_line = null;
         String output = null;
         String data = null;
         String record_type = null;
@@ -4223,9 +4223,13 @@ public class DRMTool {
                         // if one is not already present,
                         // compose and write out the missing
                         // 'extdata_requestnotes' line
-                        create_extdata_request_notes(record_type,
-                                                      previous_line,
-                                                      writer);
+                        if (previous_line != null) {
+                            create_extdata_request_notes(record_type,
+                                    previous_line,
+                                    writer);
+                        } else {
+                            return FAILURE;
+                        }
 
                         // ALWAYS pass through the original
                         // 'extdata-requesttype' line UNCHANGED
@@ -4253,7 +4257,7 @@ public class DRMTool {
                         if (output == null) {
                             return FAILURE;
                         }
-                    } else if (previous_line.length() > 0 &&
+                    } else if (previous_line != null &&
                                previous_line.startsWith(
                                        DRM_LDIF_EXTDATA_AUTH_TOKEN_USER)) {
                         output = output_extdata_auth_token_user(record_type,
@@ -4261,7 +4265,7 @@ public class DRMTool {
                         if (output == null) {
                             return FAILURE;
                         }
-                    } else if (previous_line.length() > 0 &&
+                    } else if (previous_line != null &&
                                previous_line.startsWith(
                                        DRM_LDIF_EXTDATA_AUTH_TOKEN_USER_DN)) {
                         output = output_extdata_auth_token_user_dn(record_type,
diff --git a/base/silent/src/com/netscape/pkisilent/common/ComCrypto.java b/base/silent/src/com/netscape/pkisilent/common/ComCrypto.java
index 07fe8464e0f6575ed51830d2bbf4b8501a2154c6..48b364349bc3f7b12909fefd2bf2dd64778c5b3f 100644
--- a/base/silent/src/com/netscape/pkisilent/common/ComCrypto.java
+++ b/base/silent/src/com/netscape/pkisilent/common/ComCrypto.java
@@ -241,8 +241,8 @@ public class ComCrypto {
             }
 
             if (manager == null) {
-                System.out.println("Manager object is null");
-                throw new Exception("Manager Object is null");
+                System.out.println("ComCrypto: importCert :Manager object is null");
+                throw new Exception("ComCrypto: importCert :Manager Object is null");
             }
 
             manager.importCertPackage(tmp.getBytes(), cn);
@@ -271,8 +271,8 @@ public class ComCrypto {
             }
 
             if (manager == null) {
-                System.out.println("Manager object is null");
-                throw new Exception("Manager object is null");
+                System.out.println("ComCrypto: importCACert :Manager object is null");
+                throw new Exception("ComCrypto: importCACert :Manager object is null");
             }
 
             manager.importCACertPackage(tmp.getBytes());
diff --git a/base/util/src/netscape/security/extensions/GenericASN1Extension.java b/base/util/src/netscape/security/extensions/GenericASN1Extension.java
index dfd34de2dbc42a90d613e6cd5d53d7bf63e32924..43b2e0b2b8352638627f95420827c2ae1affc684 100644
--- a/base/util/src/netscape/security/extensions/GenericASN1Extension.java
+++ b/base/util/src/netscape/security/extensions/GenericASN1Extension.java
@@ -410,7 +410,7 @@ public class GenericASN1Extension extends Extension
     }
 
     private String getFromFile(String fname) throws IOException {
-        String s = "";
+        String s = null;
         byte[] buff = null;
         int i = 0;
         int j = 0;
@@ -434,6 +434,9 @@ public class GenericASN1Extension extends Extension
                 fis.close();
             }
         }
+        if (s == null) {
+            return "";
+        }
         for (i = 0, j = 0; j < s.length(); j++) {
             int ch = s.charAt(j);
             if (ch == 10 || ch == 13 || ch == 9)
diff --git a/base/util/src/netscape/security/x509/X509CRLImpl.java b/base/util/src/netscape/security/x509/X509CRLImpl.java
index f830ef24ad63a579f94ac0b1bb5830ab7873b80a..46c3edfcf1a4e4c4785d651b349aa165004c2f9a 100755
--- a/base/util/src/netscape/security/x509/X509CRLImpl.java
+++ b/base/util/src/netscape/security/x509/X509CRLImpl.java
@@ -404,7 +404,7 @@ public class X509CRLImpl extends X509CRL {
         Signature sigVerf = null;
 
         String sigAlg = sigAlgId.getName();
-        if ("Mozilla-JSS".equals(sigProvider)) {
+        if (sigProvider != null && sigProvider.equals("Mozilla-JSS")) {
             if (sigAlg.equals("MD5withRSA")) {
                 sigAlg = "MD5/RSA";
             } else if (sigAlg.equals("MD2withRSA")) {
-- 
1.7.10.2

