This is the cleanup patch that I promised. It contains as much error-checking
code as I can muster for now.
It is tested to work.
This is again an incremental patch from the previous CMC patch.
thanks,
Christina
From 9cbc97194ed793229681b1f1b27569674855e617 Mon Sep 17 00:00:00 2001
From: Christina Fu <c...@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
_______________________________________________
Pki-devel mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
