From 9cbc97194ed793229681b1f1b27569674855e617 Mon Sep 17 00:00:00 2001
From: Christina Fu <cfu@redhat.com>
Date: Sun, 26 Mar 2017 17:34:51 -0400
Subject: [PATCH] Bug #2615 CMC: cleanup code for Encrypted Decrypted POP This
 patch adds more error checking and debugging

---
 .../netscape/cms/profile/common/EnrollProfile.java | 190 ++++++++++++++++-----
 .../cms/servlet/common/CMCOutputTemplate.java      |  75 +++++++-
 .../com/netscape/cmsutil/crypto/CryptoUtil.java    |  18 ++
 3 files changed, 240 insertions(+), 43 deletions(-)

diff --git a/base/server/cms/src/com/netscape/cms/profile/common/EnrollProfile.java b/base/server/cms/src/com/netscape/cms/profile/common/EnrollProfile.java
index 5c07ecb..f4a59d2 100644
--- a/base/server/cms/src/com/netscape/cms/profile/common/EnrollProfile.java
+++ b/base/server/cms/src/com/netscape/cms/profile/common/EnrollProfile.java
@@ -352,8 +352,13 @@ public abstract class EnrollProfile extends BasicProfile
      */
     public void setPOPchallenge(IRequest req) throws EBaseException {
         String method = "EnrollProfile: setPOPchallenge: ";
+        String msg = "";
 
         CMS.debug(method + " getting user public key in request");
+        if (req == null) {
+            CMS.debug(method + "method parameters cannot be null");
+            throw new EBaseException(method + msg);
+        }
         byte[] req_key_data = req.getExtDataInByteArray(IEnrollProfile.REQUEST_KEY);
         netscape.security.x509.CertificateX509Key pubKey = null;
         if (req_key_data != null) {
@@ -368,16 +373,23 @@ public abstract class EnrollProfile extends BasicProfile
             PublicKey issuanceProtPubKey = authority.getIssuanceProtPubKey();
             if (issuanceProtPubKey != null)
                 CMS.debug(method + "issuanceProtPubKey not null");
-            else
-                CMS.debug(method + "issuanceProtPubKey null");
+            else {
+                msg = method + "issuanceProtPubKey null";
+                CMS.debug(msg);
+                throw new EBaseException(method + msg);
+            }
 
-            String msg = "";
             try {
                 CryptoToken token = null;
                 String tokenName = CMS.getConfigStore().getString("cmc.token", CryptoUtil.INTERNAL_TOKEN_NAME);
                 token = CryptoUtil.getCryptoToken(tokenName);
 
                 PublicKey userPubKey = X509Key.parsePublicKey(new DerValue(req_key_data));
+                if (userPubKey == null) {
+                    msg = method + "userPubKey null after X509Key.parsePublicKey";
+                    CMS.debug(msg);
+                    throw new EBaseException(msg);
+                }
 
                 SymmetricKey symKey = CryptoUtil.generateKey(token);
                 byte[] pop_encreyptedData = CryptoUtil.encryptUsingSymmetricKey(
@@ -385,7 +397,7 @@ public abstract class EnrollProfile extends BasicProfile
                 if (pop_encreyptedData == null) {
                     msg = method + "pop_encreyptedData null";
                     CMS.debug(msg);
-                    throw new Exception(msg);
+                    throw new EBaseException(msg);
                 }
 
                 byte[] pop_sysPubEncreyptedSession = CryptoUtil.wrapUsingPublicKey(
@@ -393,7 +405,7 @@ public abstract class EnrollProfile extends BasicProfile
                 if (pop_sysPubEncreyptedSession == null) {
                     msg = method + "pop_sysPubEncreyptedSession null";
                     CMS.debug(msg);
-                    throw new Exception(msg);
+                    throw new EBaseException(msg);
                 }
 
                 byte[] pop_userPubEncreyptedSession = CryptoUtil.wrapUsingPublicKey(
@@ -401,7 +413,7 @@ public abstract class EnrollProfile extends BasicProfile
                 if (pop_userPubEncreyptedSession == null) {
                     msg = method + "pop_userPubEncreyptedSession null";
                     CMS.debug(msg);
-                    throw new Exception(msg);
+                    throw new EBaseException(msg);
                 }
                 CMS.debug(method + "POP challenge fields generated successfully...setting request extData");
 
@@ -507,11 +519,15 @@ public abstract class EnrollProfile extends BasicProfile
             throws EProfileException {
 
         String method = "EnrollProfile: parseCMC: ";
+        String msg = ""; // for capturing debug and throw info
+
         /* cert request must not be null */
         if (certreq == null) {
-            CMS.debug(method + "certreq null");
+            msg = method + "certreq null";
+            CMS.debug(msg);
             throw new EProfileException(
-                    CMS.getUserMessage(locale, "CMS_PROFILE_INVALID_REQUEST"));
+                    CMS.getUserMessage(locale, "CMS_PROFILE_INVALID_REQUEST") +
+                            msg);
         }
         //CMS.debug(method + " Start parseCMC(): " + certreq);
         CMS.debug(method + "starts");
@@ -584,38 +600,63 @@ public abstract class EnrollProfile extends BasicProfile
                     } //for
 
                     /**
-                     * TODO: cfu: phase 2 should add enforcement for
-                     * id_cmc_identityProofV2 and id_cmc_identification control
-                     * when needed
-                     */
-
-                    /**
                      * now do the actual control processing
                      * (the postponed processing is so that we can capture
                      * the identification, if included)
                      */
 
-                    UTF8String ident_s = null;
-                    if (id_cmc_decryptedPOP && (decPopVals != null)) {
+                    if (id_cmc_decryptedPOP) {
+                        if (decPopVals != null) {
 
-                        DecryptedPOP decPop = (DecryptedPOP) (ASN1Util.decode(DecryptedPOP.getTemplate(),
-                                ASN1Util.encode(decPopVals.elementAt(0))));
-                        CMS.debug(method + "DecryptedPOP encoded");
+                            DecryptedPOP decPop = (DecryptedPOP) (ASN1Util.decode(DecryptedPOP.getTemplate(),
+                                    ASN1Util.encode(decPopVals.elementAt(0))));
+                            CMS.debug(method + "DecryptedPOP encoded");
 
-                        Integer reqId = verifyDecryptedPOP(decPop);
-                        if (reqId != null) {
-                            context.put("decryptedPopReqId", reqId);
+                            Integer reqId = verifyDecryptedPOP(locale, decPop);
+                            if (reqId != null) {
+                                context.put("decryptedPopReqId", reqId);
+                            }
+                        } else { //decPopVals == null
+                            msg = "id_cmc_decryptedPOP contains invalid DecryptedPOP";
+                            CMS.debug(method + msg);
+                            SEQUENCE bpids = getRequestBpids(reqSeq);
+                            context.put("decryptedPOP", bpids);
                         }
                         return null;
                     }
 
+                    UTF8String ident_s = null;
                     if (id_cmc_identification) {
+                        if (ident == null) {
+                            msg = "id_cmc_identification contains null attribute value";
+                            CMS.debug(method + msg);
+                            SEQUENCE bpids = getRequestBpids(reqSeq);
+                            context.put("identification", bpids);
+                            return null;
+                        }
                         ident_s = (UTF8String) (ASN1Util.decode(UTF8String.getTemplate(),
                                 ASN1Util.encode(ident.elementAt(0))));
+                        if (ident_s == null) {
+                            msg = "id_cmc_identification contains invalid content";
+                            CMS.debug(method + msg);
+                            SEQUENCE bpids = getRequestBpids(reqSeq);
+                            context.put("identification", bpids);
+                            return null;
+                        }
                     }
 
                     // either V2 or not V2; can't be both
                     if (id_cmc_identityProofV2 && (attr != null)) {
+                        if (!id_cmc_identification) {
+                            SEQUENCE bpids = getRequestBpids(reqSeq);
+                            context.put("identification", bpids);
+                            msg = "id_cmc_identityProofV2 must be accompanied by id_cmc_identification in this server";
+                            CMS.debug(method + msg);
+                            throw new EProfileException(
+                                    CMS.getUserMessage(locale, "CMS_PROFILE_INVALID_REQUEST") +
+                                            msg);
+                        }
+
                         boolean valid = verifyIdentityProofV2(attr, ident_s,
                                 reqSeq);
                         if (!valid) {
@@ -687,11 +728,24 @@ public abstract class EnrollProfile extends BasicProfile
      *
      * @author cfu
      */
-    private Integer verifyDecryptedPOP(DecryptedPOP decPop) {
+    private Integer verifyDecryptedPOP(Locale locale, DecryptedPOP decPop)
+            throws EProfileException {
         String method = "EnrollProfile: verifyDecryptedPOP: ";
         CMS.debug(method + "begins");
+        String msg = "";
+
+        if (decPop == null) {
+            CMS.debug(method + "method parameters cannot be null");
+            return null;
+        }
 
+        // iBody contains the request id
         INTEGER iBody = decPop.getBodyPartID();
+        if (iBody == null) {
+            msg = method + "iBody null after decPop.getBodyPartID";
+            CMS.debug(msg);
+            return null;
+        }
         CMS.debug(method + "request id from decryptedPOP =" +
                 iBody.toString());
         Integer reqId = new Integer(iBody.toString());
@@ -703,30 +757,36 @@ public abstract class EnrollProfile extends BasicProfile
         try {
             req = reqQueue.findRequest(new RequestId(reqId));
         } catch (Exception e) {
-            CMS.debug(method + e);
+            msg = method + "after findRequest: " + e;
+            CMS.debug(msg);
             return null;
         }
 
         // now verify the POP witness
         byte[] pop_encreyptedData = req.getExtDataInByteArray("pop_encreyptedData");
         if (pop_encreyptedData == null) {
-            CMS.debug(method +
-                    "pop_encreyptedData not found in request:" + reqId.toString());
+            msg = method +
+                    "pop_encreyptedData not found in request:" +
+                    reqId.toString();
+            CMS.debug(msg);
             return null;
         }
 
         byte[] pop_sysPubEncreyptedSession = req.getExtDataInByteArray("pop_sysPubEncreyptedSession");
         if (pop_sysPubEncreyptedSession == null) {
-            CMS.debug(method +
-                    "pop_sysPubEncreyptedSession not found in request:" + reqId.toString());
+            msg = method +
+                    "pop_sysPubEncreyptedSession not found in request:" +
+                    reqId.toString();
+            CMS.debug(msg);
             return null;
         }
 
         byte[] cmc_msg = req.getExtDataInByteArray(IEnrollProfile.CTX_CERT_REQUEST);
         if (pop_sysPubEncreyptedSession == null) {
-            CMS.debug(method +
+            msg = method +
                     "pop_sysPubEncreyptedSession not found in request:" +
-                    reqId.toString());
+                    reqId.toString();
+            CMS.debug(msg);
             return null;
         }
 
@@ -734,8 +794,11 @@ public abstract class EnrollProfile extends BasicProfile
         PrivateKey issuanceProtPrivKey = authority.getIssuanceProtPrivKey();
         if (issuanceProtPrivKey != null)
             CMS.debug(method + "issuanceProtPrivKey not null");
-        else
-            CMS.debug(method + "issuanceProtPrivKey null");
+        else {
+            msg = method + "issuanceProtPrivKey null";
+            CMS.debug(msg);
+            return null;
+        }
 
         try {
             CryptoToken token = null;
@@ -747,25 +810,46 @@ public abstract class EnrollProfile extends BasicProfile
                     SymmetricKey.Usage.DECRYPT,
                     issuanceProtPrivKey,
                     pop_sysPubEncreyptedSession);
+            if (symKey == null) {
+                msg = "symKey null after CryptoUtil.unwrap returned";
+                CMS.debug(msg);
+                return null;
+            }
+
             byte[] challenge_b = CryptoUtil.decryptUsingSymmetricKey(
                     token, pop_encreyptedData, symKey);
             if (challenge_b == null) {
-                CMS.debug(method + "decryptUsingSymmetricKey returned null");
+                msg = method + "challenge_b null after decryptUsingSymmetricKey returned";
+                CMS.debug(msg);
                 return null;
             }
 
             MessageDigest digest = MessageDigest.getInstance(CryptoUtil.getDefaultHashAlgName());
+            if (digest == null) {
+                msg = method + "digest null after decryptUsingSymmetricKey returned";
+                CMS.debug(msg);
+                return null;
+            }
             HMACDigest hmacDigest = new HMACDigest(digest, challenge_b);
             hmacDigest.update(cmc_msg);
             byte[] proofValue = hmacDigest.digest();
+            if (proofValue == null) {
+                msg = method + "proofValue null after hmacDigest.digest returned";
+                CMS.debug(msg);
+                return null;
+            }
             boolean witnessChecked = Arrays.equals(proofValue, witness_os.toByteArray());
             if (!witnessChecked) {
-                CMS.debug(method + "POP challenge witness verification failure");
+                msg = method + "POP challenge witness verification failure";
+                CMS.debug(msg);
                 return null;
             }
         } catch (Exception e) {
-            CMS.debug(method + e);
-            return null;
+            msg = method + e;
+            CMS.debug(msg);
+            throw new EProfileException(
+                    CMS.getUserMessage(locale, "CMS_PROFILE_INVALID_REQUEST") +
+                            e);
         }
 
         CMS.debug(method + "POP challenge verified!");
@@ -908,11 +992,24 @@ public abstract class EnrollProfile extends BasicProfile
         byte[] key = null;
         CMS.debug(method + "in verifyDigest: hashAlg=" + hashAlg.toString() +
                 "; macAlg=" + macAlg.toString());
+
+        if ((sharedSecret == null) ||
+            (text == null) ||
+            (bv == null) ||
+            (hashAlg == null) ||
+            (macAlg == null)) {
+            CMS.debug(method + "method parameters cannot be null");
+            return false;
+        }
         key = hashAlg.digest(sharedSecret);
 
         byte[] finalDigest = null;
         HMACDigest hmacDigest = new HMACDigest(macAlg, key);
         hmacDigest.update(text);
+        if (hmacDigest == null) {
+            CMS.debug(method + " hmacDigest null after hmacDigest.update");
+            return false;
+        }
         finalDigest = hmacDigest.digest();
 
         if (finalDigest.length != bv.length) {
@@ -963,14 +1060,15 @@ public abstract class EnrollProfile extends BasicProfile
             SEQUENCE reqSeq) {
         String method = "EnrollProfile:verifyIdentityProofV2: ";
         CMS.debug(method + " begins");
-
-        String ident_string = null;
-        if (ident != null) {
-            ident_string = ident.toString();
-            // cfu: REMOVE
-            CMS.debug(method + "received ident String: " + ident_string);
+        if ((attr == null) ||
+                (ident == null) ||
+                (reqSeq == null)) {
+            CMS.debug(method + "method parameters cannot be null");
+            return false;
         }
 
+        String ident_string = ident.toString();
+
         SET vals = attr.getValues(); // getting the IdentityProofV2 structure
         if (vals.size() < 1) {
             return false;
@@ -1002,6 +1100,10 @@ public abstract class EnrollProfile extends BasicProfile
             CMS.debug(method + " Illegal access: " + name);
             return false;
         }
+        if (tokenClass == null) {
+            CMS.debug(method + " Failed to retrieve shared secret plugin class");
+            return false;
+        }
 
         String token = null;
         if (ident_string != null)
@@ -1033,6 +1135,10 @@ public abstract class EnrollProfile extends BasicProfile
             // TODO: check against CA allowed algs later
 
             OCTET_STRING witness = idV2val.getWitness();
+            if (witness == null) {
+                CMS.debug(method + " witness reurned by idV2val.getWitness is null");
+                return false;
+            }
 
             byte[] witness_bytes = witness.toByteArray();
             byte[] request_bytes = ASN1Util.encode(reqSeq); // PKIData reqSequence field
diff --git a/base/server/cms/src/com/netscape/cms/servlet/common/CMCOutputTemplate.java b/base/server/cms/src/com/netscape/cms/servlet/common/CMCOutputTemplate.java
index f5bef2a..ac690f2 100644
--- a/base/server/cms/src/com/netscape/cms/servlet/common/CMCOutputTemplate.java
+++ b/base/server/cms/src/com/netscape/cms/servlet/common/CMCOutputTemplate.java
@@ -191,7 +191,8 @@ public class CMCOutputTemplate {
             EncryptedPOP encPop = null;
             if (reqs != null) {
                 for (int i = 0; i < reqs.length; i++) {
-                    CMS.debug(method + " error_codes[i]=" + error_codes[i]);
+                    CMS.debug(method + " error_codes[" +i+"]="
+                            + error_codes[i]);
                     if (error_codes[i] == 0) {
                         success_bpids.addElement(new INTEGER(
                                 reqs[i].getExtDataInBigInteger("bodyPartId")));
@@ -215,6 +216,46 @@ public class CMCOutputTemplate {
 
             TaggedAttribute tagattr = null;
             CMCStatusInfo cmcStatusInfo = null;
+
+//cfu
+
+            SEQUENCE decryptedPOPBpids = (SEQUENCE) context.get("decryptedPOP");
+            if (decryptedPOPBpids != null && decryptedPOPBpids.size() > 0) {
+                OtherInfo otherInfo = new OtherInfo(OtherInfo.FAIL,
+                        new INTEGER(OtherInfo.POP_FAILED), null);
+                cmcStatusInfo = new CMCStatusInfo(CMCStatusInfo.FAILED,
+                        decryptedPOPBpids, (String) null, otherInfo);
+                tagattr = new TaggedAttribute(
+                        new INTEGER(bpid++),
+                        OBJECT_IDENTIFIER.id_cmc_cMCStatusInfo, cmcStatusInfo);
+                controlSeq.addElement(tagattr);
+            }
+
+            SEQUENCE identificationBpids = (SEQUENCE) context.get("identification");
+            if (identificationBpids != null && identificationBpids.size() > 0) {
+                OtherInfo otherInfo = new OtherInfo(OtherInfo.FAIL,
+                        new INTEGER(OtherInfo.BAD_IDENTITY), null);
+                cmcStatusInfo = new CMCStatusInfo(CMCStatusInfo.FAILED,
+                        identificationBpids, (String) null, otherInfo);
+                tagattr = new TaggedAttribute(
+                        new INTEGER(bpid++),
+                        OBJECT_IDENTIFIER.id_cmc_cMCStatusInfo, cmcStatusInfo);
+                controlSeq.addElement(tagattr);
+            }
+
+            SEQUENCE identityV2Bpids = (SEQUENCE) context.get("identityProofV2");
+            if (identityV2Bpids != null && identityV2Bpids.size() > 0) {
+                OtherInfo otherInfo = new OtherInfo(OtherInfo.FAIL,
+                        new INTEGER(OtherInfo.BAD_IDENTITY), null);
+                cmcStatusInfo = new CMCStatusInfo(CMCStatusInfo.FAILED,
+                        identityV2Bpids, (String) null, otherInfo);
+                tagattr = new TaggedAttribute(
+                        new INTEGER(bpid++),
+                        OBJECT_IDENTIFIER.id_cmc_cMCStatusInfo, cmcStatusInfo);
+                controlSeq.addElement(tagattr);
+            }
+
+
             SEQUENCE identityBpids = (SEQUENCE) context.get("identityProof");
             if (identityBpids != null && identityBpids.size() > 0) {
                 OtherInfo otherInfo = new OtherInfo(OtherInfo.FAIL,
@@ -257,6 +298,7 @@ public class CMCOutputTemplate {
                 PendInfo pendInfo = new PendInfo(reqId, new Date());
                 otherInfo = new OtherInfo(OtherInfo.PEND, null,
                         pendInfo);
+                // cfu: inject POP_REQUIRED when working on V2 status
                 cmcStatusInfo = new CMCStatusInfo(CMCStatusInfo.PENDING,
                         pending_bpids, (String) null, otherInfo);
                 tagattr = new TaggedAttribute(
@@ -417,15 +459,23 @@ public class CMCOutputTemplate {
     public EncryptedPOP constructEncryptedPop(IRequest req)
             throws EBaseException {
         String method = "CMCOutputTemplate: constructEncryptedPop: ";
+        String msg = "";
         CMS.debug(method + "begins");
         EncryptedPOP encPop = null;
 
+        if (req == null) {
+            msg = method + "method parameters cannot be null";
+            CMS.debug(msg);
+            throw new EBaseException(msg);
+        }
+
         boolean popChallengeRequired = req.getExtDataInBoolean("cmc_POPchallengeRequired", false);
         if (!popChallengeRequired) {
             CMS.debug(method + "popChallengeRequired false");
             return null;
         }
         CMS.debug(method + "popChallengeRequired true");
+
         byte[] cmc_msg = req.getExtDataInByteArray(IEnrollProfile.CTX_CERT_REQUEST);
         byte[] pop_encreyptedData = req.getExtDataInByteArray("pop_encreyptedData");
         //don't need this for encryptedPOP, but need to check for existence anyway
@@ -441,12 +491,26 @@ public class CMCOutputTemplate {
                 EnvelopedData envData = CryptoUtil.createEnvelopedData(
                         pop_encreyptedData,
                         pop_userPubEncreyptedSession);
+                if (envData == null) {
+                    msg = "envData null returned by createEnvelopedData";
+                    throw new EBaseException(method + msg);
+                }
                 ContentInfo ci = new ContentInfo(envData);
+                if (ci == null) {
+                    msg = "ci null from new ContentInfo";
+                    CMS.debug(msg);
+                    throw new EBaseException(method + msg);
+                }
                 CMS.debug(method + "now we can compose encryptedPOP");
 
                 TaggedRequest.Template tReqTemplate = new TaggedRequest.Template();
                 TaggedRequest tReq = (TaggedRequest) tReqTemplate.decode(
                         new ByteArrayInputStream(cmc_msg));
+                if (tReq == null) {
+                    msg = "tReq null from tReqTemplate.decode";
+                    CMS.debug(msg);
+                    throw new EBaseException(method + msg);
+                }
 
                 encPop = new EncryptedPOP(
                         tReq,
@@ -454,12 +518,21 @@ public class CMCOutputTemplate {
                         CryptoUtil.getDefaultEncAlg(),
                         CryptoUtil.getDefaultHashAlg(),
                         new OCTET_STRING(req.getExtDataInByteArray("pop_witness")));
+                if (encPop == null) {
+                    msg = "encPop null returned by new EncryptedPOP";
+                    CMS.debug(msg);
+                    throw new EBaseException(method + msg);
+                }
 
             } catch (Exception e) {
                 CMS.debug(method + " excepton:" + e);
                 throw new EBaseException(method + " excepton:" + e);
             }
 
+        } else {
+            msg = "popChallengeRequired required, but one more more of the pop_ data not found in request";
+            CMS.debug(method + msg);
+            throw new EBaseException(method + msg);
         }
 
         return encPop;
diff --git a/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java b/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
index 716a3f2..3588852 100644
--- a/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
+++ b/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
@@ -2403,12 +2403,25 @@ public class CryptoUtil {
     public static EnvelopedData createEnvelopedData(byte[] encContent, byte[] encSymKey)
             throws Exception {
         String method = "CryptoUtl: createEnvelopedData: ";
+        String msg = "";
         System.out.println(method + "begins");
+        if ((encContent == null) ||
+                (encSymKey == null)) {
+            msg = method + "method parameters cannot be null";
+            System.out.println(msg);
+
+            throw new Exception(method + msg);
+        }
 
         EncryptedContentInfo encCInfo = new EncryptedContentInfo(
                 ContentInfo.DATA,
                 getDefaultEncAlg(),
                 new OCTET_STRING(encContent));
+        if (encCInfo == null) {
+            msg = method + "encCInfo null from new EncryptedContentInfo";
+            System.out.println(msg);
+            throw new Exception(method + msg);
+        }
 
         Name name = new Name();
         name.addCommonName("unUsedIssuerName"); //unused; okay for cmc EncryptedPOP
@@ -2417,6 +2430,11 @@ public class CryptoUtil {
                 new IssuerAndSerialNumber(name, new INTEGER(0)), //unUsed
                 new AlgorithmIdentifier(RSA_ENCRYPTION, new NULL()),
                 new OCTET_STRING(encSymKey));
+        if (recipient == null) {
+            msg = method + "recipient null from new RecipientInfo";
+            System.out.println(msg);
+            throw new Exception(method + msg);
+        }
 
         SET recipients = new SET();
         recipients.addElement(recipient);
-- 
2.7.4

