>From ab1a231d3f61dc41ad03fbc5e9c0a362aa0ca3ea Mon Sep 17 00:00:00 2001
From: "Endi S. Dewata" <edewata@redhat.com>
Date: Wed, 11 May 2016 09:42:53 +0200
Subject: [PATCH] Fixed error handling ConfigurationUtils.handleCertRequest().

The ConfigurationUtils.handleCertRequest() has been modified
to throw an exception on error during CSR generation instead
of silently ignoring it. The method has also been renamed to
generateCertRequest() for clarity.
---
 .../cms/servlet/csadmin/ConfigurationUtils.java    | 58 ++++++++++++----------
 .../dogtagpki/server/rest/SystemConfigService.java |  7 ++-
 2 files changed, 36 insertions(+), 29 deletions(-)

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 c0f0ce1f405dd63232f1be6c15f8bd8d1a8d3c4b..88e85a01a8d21afeb4cd46a2250545e7b2084c3a 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
@@ -35,11 +35,9 @@ import java.security.InvalidKeyException;
 import java.security.KeyPair;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
-import java.security.NoSuchProviderException;
 import java.security.Principal;
 import java.security.PublicKey;
 import java.security.SecureRandom;
-import java.security.SignatureException;
 import java.security.cert.CertificateEncodingException;
 import java.security.cert.CertificateException;
 import java.security.cert.CertificateExpiredException;
@@ -119,6 +117,7 @@ import com.netscape.certsrv.apps.CMS;
 import com.netscape.certsrv.authentication.EAuthException;
 import com.netscape.certsrv.authentication.IAuthSubsystem;
 import com.netscape.certsrv.authorization.IAuthzSubsystem;
+import com.netscape.certsrv.base.BadRequestException;
 import com.netscape.certsrv.base.ConflictingOperationException;
 import com.netscape.certsrv.base.EBaseException;
 import com.netscape.certsrv.base.EPropertyNotFound;
@@ -2894,39 +2893,37 @@ public class ConfigurationUtils {
         cert.setRequest(formattedCertreq);
     }
 
-    public static void handleCertRequest(IConfigStore config, String certTag, Cert cert) throws EPropertyNotFound,
-            EBaseException, InvalidKeyException, NotInitializedException, TokenException, NoSuchAlgorithmException,
-            NoSuchProviderException, CertificateException, SignatureException, IOException {
+    public static void generateCertRequest(IConfigStore config, String certTag, Cert cert) throws Exception {
+
+        CMS.debug("generateCertRequest: getting public key for certificate " + certTag);
 
-        CMS.debug("ConfigurationUtils: handleCertRequest() begins");
-        // get public key
         String pubKeyType = config.getString(PCERT_PREFIX + certTag + ".keytype");
         String algorithm = config.getString(PCERT_PREFIX + certTag + ".keyalgorithm");
 
-        X509Key pubk = null;
+        X509Key pubk;
         if (pubKeyType.equals("rsa")) {
             pubk = getRSAX509Key(config, certTag);
+
         } else if (pubKeyType.equals("ecc")) {
             pubk = getECCX509Key(config, certTag);
+
         } else {
-            CMS.debug("handleCertRequest() - " + "pubKeyType " + pubKeyType + " is unsupported!");
-            return;
+            CMS.debug("generateCertRequest: Unsupported public key type: " + pubKeyType);
+            throw new BadRequestException("Unsupported public key type: " + pubKeyType);
         }
 
-        CMS.debug("handleCertRequest: tag=" + certTag);
-        if (pubk == null) {
-            CMS.debug("handleCertRequest: error getting public key null");
-            return;
-        }
+        // public key cannot be null here
 
-        // get private key
+        CMS.debug("generateCertRequest: getting private key for certificate " + certTag);
         String privKeyID = config.getString(PCERT_PREFIX + certTag + ".privkey.id");
-        CMS.debug("privKeyID=" + privKeyID);
+
+        CMS.debug("generateCertRequest: private key ID: " + privKeyID);
         byte[] keyIDb = CryptoUtil.string2byte(privKeyID);
 
         PrivateKey privk = CryptoUtil.findPrivateKeyFromID(keyIDb);
         if (privk == null) {
-            CMS.debug("handleCertRequest: error getting private key");
+            CMS.debug("generateCertRequest: Unable to find private key for certificate " + certTag);
+            throw new BadRequestException("Unable to find private key for certificate " + certTag);
         }
 
         // construct cert request
@@ -2936,12 +2933,14 @@ public class ConfigurationUtils {
 
         Extensions exts = null;
         if (certTag.equals("signing")) {
-            CMS.debug("handleCertRequest: certTag is siging -- about to call createBasicCAExtensions()");
+            CMS.debug("generateCertRequest: generating basic CA extensions");
             exts = createBasicCAExtensions(config);
         }
+
+        CMS.debug("generateCertRequest: generating PKCS #10 request");
         PKCS10 certReq = CryptoUtil.createCertificationRequest(caDN, pubk, privk, algorithm, exts);
 
-        CMS.debug("handleCertRequest: created cert request");
+        CMS.debug("generateCertRequest: storing cert request");
         byte[] certReqb = certReq.toByteArray();
         String certReqs = CryptoUtil.base64Encode(certReqb);
         String certReqf = CryptoUtil.reqFormat(certReqs);
@@ -2949,15 +2948,15 @@ public class ConfigurationUtils {
         String subsystem = config.getString(PCERT_PREFIX + certTag + ".subsystem");
         config.putString(subsystem + "." + certTag + ".certreq", certReqs);
         config.commit(false);
+
         cert.setRequest(certReqf);
-
     }
 
     /*
      * createBasicCAExtensions creates the basic Extensions needed for a CSR to a
      * CA signing certificate
      */
-    private static Extensions createBasicCAExtensions(IConfigStore config) throws IOException {
+    private static Extensions createBasicCAExtensions(IConfigStore config) throws Exception {
         Extensions exts = new Extensions();
         CMS.debug("ConfigurationUtils: createBasicCAExtensions: begins");
 
@@ -2989,27 +2988,32 @@ public class ConfigurationUtils {
         */
 
         // add a generic extension
-        Extension genExt = null;
         try {
             String oidString = config.getString(PCERT_PREFIX + "signing.ext.oid");
             String dataString = config.getString(PCERT_PREFIX + "signing.ext.data");
-            boolean critical = false;
+
             if (oidString != null && dataString != null) {
                 CMS.debug("ConfigurationUtils: createBasicCAExtensions: processing generic extension");
-                critical = config.getBoolean("preop.cert.signing.ext.critical");
+                boolean critical = config.getBoolean("preop.cert.signing.ext.critical");
                 ObjectIdentifier oid = new ObjectIdentifier(oidString);
 
                 byte data[] = CryptoUtil.hexString2Bytes(dataString);
                 DerOutputStream out = new DerOutputStream();
                 out.putOctetString(data);
-                genExt = new Extension(oid, critical, out.toByteArray());
+
+                Extension genExt = new Extension(oid, critical, out.toByteArray());
                 out.close();
 
                 exts.add(genExt);
                 CMS.debug("ConfigurationUtils: createBasicCAExtensions: generic extension added: " + oidString);
             }
+
+        } catch (EPropertyNotFound e) {
+            // generic extension not specified, ignore
+
         } catch (EBaseException e) {
-            CMS.debug("ConfigurationUtils: createBasicCAExtensions: generic extension not processed:" + e);
+            CMS.debug("ConfigurationUtils: createBasicCAExtensions: Unable to add generic extension: " + e);
+            throw new BadRequestException("Unable to add generic certificate extension: " + e, e);
         }
 
         return exts;
diff --git a/base/server/cms/src/org/dogtagpki/server/rest/SystemConfigService.java b/base/server/cms/src/org/dogtagpki/server/rest/SystemConfigService.java
index f9415f520eb264fece8846339ed9da2904c7dbfa..ef5d784bcd3ba6b4d588ab4c15b5362f8e82606d 100644
--- a/base/server/cms/src/org/dogtagpki/server/rest/SystemConfigService.java
+++ b/base/server/cms/src/org/dogtagpki/server/rest/SystemConfigService.java
@@ -492,7 +492,7 @@ public class SystemConfigService extends PKIService implements SystemConfigResou
                     // Handle Cert Requests for everything EXCEPT Stand-alone PKI (Step 2)
                     if (!request.getStepTwo()) {
                         // Stand-alone PKI (Step 1)
-                        ConfigurationUtils.handleCertRequest(cs, tag, cert);
+                        ConfigurationUtils.generateCertRequest(cs, tag, cert);
 
                         CMS.debug("Stand-alone " + csType + " Admin CSR");
                         String adminSubjectDN = request.getAdminSubjectDN();
@@ -505,7 +505,7 @@ public class SystemConfigService extends PKIService implements SystemConfigResou
                     }
 
                 } else {
-                    ConfigurationUtils.handleCertRequest(cs, tag, cert);
+                    ConfigurationUtils.generateCertRequest(cs, tag, cert);
                 }
 
                 if (request.isClone()) {
@@ -551,6 +551,9 @@ public class SystemConfigService extends PKIService implements SystemConfigResou
         } catch (NoSuchAlgorithmException e) {
             throw new BadRequestException("Invalid algorithm " + e);
 
+        } catch (PKIException e) {
+            throw e;
+
         } catch (Exception e) {
             CMS.debug(e);
             throw new PKIException("Error in setting certificate names and key sizes: " + e);
-- 
2.4.11

