>From b3e34f912c7aa5a695710fdbf9ff8faf5f5a47af Mon Sep 17 00:00:00 2001
From: Christina Fu <cfu@redhat.com>
Date: Mon, 23 May 2016 16:22:54 -0700
Subject: [PATCH] Ticket 1665 - Cert Revocation Reasons not being updated when
 on-hold This patch fixes the following areas: * In the CA, when revokeCert is
 called, make it possible to move from on_hold to revoke. * In the servlet
 that handles TPS revoke (DoRevokeTPS), make sure it allows the on_hold cert
 to be put in the bucket to be revoked. * there are a few minor fixes such as
 typos and one have to do with the populate method in SubjectDNInput.java
 needs better handling of subject in case it's null. Note: This patch does not
 make attempt to allow agents to revoke certs that are on_hold from agent
 interface.  The search filter needs to be modified to allow that.

---
 base/ca/src/com/netscape/ca/CAService.java         | 17 ++++++++--
 .../netscape/certsrv/dbs/certdb/ICertRecord.java   |  5 +++
 .../certsrv/dbs/certdb/ICertificateRepository.java |  3 ++
 .../netscape/cms/profile/common/EnrollProfile.java |  4 +--
 .../netscape/cms/profile/input/SubjectDNInput.java |  2 +-
 .../com/netscape/cms/servlet/cert/DoRevokeTPS.java | 12 ++++---
 .../src/com/netscape/cmscore/dbs/CertRecord.java   | 28 ++++++++++++++++
 .../cmscore/dbs/CertificateRepository.java         | 38 ++++++++++++++++++----
 .../src/org/dogtagpki/server/tps/TPSTokendb.java   |  2 ++
 9 files changed, 95 insertions(+), 16 deletions(-)

diff --git a/base/ca/src/com/netscape/ca/CAService.java b/base/ca/src/com/netscape/ca/CAService.java
index 2b5d5f732b36f5cf6e1e5aa54552a5e4dcc5afaa..7fdce06136ba5cd27e65b48aeb3ed3e3cb3566e8 100644
--- a/base/ca/src/com/netscape/ca/CAService.java
+++ b/base/ca/src/com/netscape/ca/CAService.java
@@ -1001,9 +1001,11 @@ public class CAService implements ICAService, IService {
         Date revdate = crlentry.getRevocationDate();
         CRLExtensions crlentryexts = crlentry.getExtensions();
 
+        CMS.debug("CAService.revokeCert: revokeCert begins");
         CertRecord certRec = (CertRecord) mCA.getCertificateRepository().readCertificateRecord(serialno);
 
         if (certRec == null) {
+            CMS.debug("CAService.revokeCert: cert record not found");
             mCA.log(ILogger.LL_FAILURE, CMS.getLogMessage("CMSCORE_CA_CERT_NOT_FOUND", serialno.toString(16)));
             throw new ECAException(
                     CMS.getUserMessage("CMS_CA_CANT_FIND_CERT_SERIAL",
@@ -1013,14 +1015,24 @@ public class CAService implements ICAService, IService {
         // allow revoking certs that are on hold.
         String certStatus = certRec.getStatus();
 
-        if (certStatus.equals(ICertRecord.STATUS_REVOKED) ||
+        if ((certStatus.equals(ICertRecord.STATUS_REVOKED) &&
+            !certRec.isCertOnHold()) ||
                 certStatus.equals(ICertRecord.STATUS_REVOKED_EXPIRED)) {
+            CMS.debug("CAService.revokeCert: cert already revoked:" +
+                serialno.toString());
             throw new ECAException(CMS.getUserMessage("CMS_CA_CERT_ALREADY_REVOKED",
                     "0x" + Long.toHexString(serialno.longValue())));
         }
         try {
-            mCA.getCertificateRepository().markAsRevoked(serialno,
+            CMS.debug("CAService.revokeCert: about to call markAsRevoked");
+            if (certRec.isCertOnHold()) {
+                mCA.getCertificateRepository().markAsRevoked(serialno,
+                    new RevocationInfo(revdate, crlentryexts), true /*isOnHold*/);
+            } else {
+                mCA.getCertificateRepository().markAsRevoked(serialno,
                     new RevocationInfo(revdate, crlentryexts));
+            }
+            CMS.debug("CAService.revokeCert: cert revoked");
             mCA.log(ILogger.LL_INFO, CMS.getLogMessage("CMSCORE_CA_CERT_REVOKED",
                     serialno.toString(16)));
             // inform all CRLIssuingPoints about revoked certificate
@@ -1052,6 +1064,7 @@ public class CAService implements ICAService, IService {
                 }
             }
         } catch (EBaseException e) {
+            CMS.debug("CAService.revokeCert: " + e.toString());
             mCA.log(ILogger.LL_FAILURE,
                     CMS.getLogMessage("CMSCORE_CA_ERROR_REVOCATION", serialno.toString(), e.toString()));
             //e.printStackTrace();
diff --git a/base/common/src/com/netscape/certsrv/dbs/certdb/ICertRecord.java b/base/common/src/com/netscape/certsrv/dbs/certdb/ICertRecord.java
index 23f4e07d43bffd51e41a75d0939e5ad807400f9d..3a0c9559ec164ceb3fe40666402e1115f81740f4 100644
--- a/base/common/src/com/netscape/certsrv/dbs/certdb/ICertRecord.java
+++ b/base/common/src/com/netscape/certsrv/dbs/certdb/ICertRecord.java
@@ -176,4 +176,9 @@ public interface ICertRecord extends IDBObj {
      * @return revocation info
      */
     public IRevocationInfo getRevocationInfo();
+
+    /**
+     * is this cert on hold?
+     */
+    public boolean isCertOnHold();
 }
diff --git a/base/common/src/com/netscape/certsrv/dbs/certdb/ICertificateRepository.java b/base/common/src/com/netscape/certsrv/dbs/certdb/ICertificateRepository.java
index 066043b136afd0f5861c9a898d324b352b65a072..6dc6c396786f88b007d372cb99a182e3edb0b428 100644
--- a/base/common/src/com/netscape/certsrv/dbs/certdb/ICertificateRepository.java
+++ b/base/common/src/com/netscape/certsrv/dbs/certdb/ICertificateRepository.java
@@ -135,10 +135,13 @@ public interface ICertificateRepository extends IRepository {
      *
      * @param id serial number
      * @param info revocation information
+     * @param isOnHold boolean to indicate if the cert was revoked onHold
      * @exception EBaseException failed to mark
      */
     public void markAsRevoked(BigInteger id, IRevocationInfo info)
             throws EBaseException;
+    public void markAsRevoked(BigInteger id, IRevocationInfo info, boolean isOnHold)
+            throws EBaseException;
 
     /**
      * Updates certificate status.
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 1f0e4706957859a5b08ee87964ab61521ac279dd..7d7ce06614d82f2d972c6c83f2e1d0cd11bb1e6b 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
@@ -1170,7 +1170,7 @@ public abstract class EnrollProfile extends BasicProfile
             // keeping "aoluid" to be backward compatible
             req.setExtData("aoluid", sn);
             req.setExtData("uid", sn);
-            CMS.debug("EnrollPrifile: fillNSNKEY(): uid=" + sn);
+            CMS.debug("EnrollProfile: fillNSNKEY(): uid=" + sn);
 
         } catch (Exception e) {
             CMS.debug("EnrollProfile: Unable to fill NSNKEY: " + e);
@@ -1195,7 +1195,7 @@ public abstract class EnrollProfile extends BasicProfile
             //                              X500Name("CN="+sn)));
             req.setExtData("tokencuid", tcuid);
 
-            CMS.debug("EnrollPrifile: fillNSNKEY(): tokencuid=" + tcuid);
+            CMS.debug("EnrollProfile: fillNSNKEY(): tokencuid=" + tcuid);
 
         } catch (Exception e) {
             CMS.debug("EnrollProfile: Unable to fill NSHKEY: " + e);
diff --git a/base/server/cms/src/com/netscape/cms/profile/input/SubjectDNInput.java b/base/server/cms/src/com/netscape/cms/profile/input/SubjectDNInput.java
index a12351f8a6535ae85703b20620f92948c1d9d9e9..0f1341728a8760a614f49869b81f6f3a438c8609 100644
--- a/base/server/cms/src/com/netscape/cms/profile/input/SubjectDNInput.java
+++ b/base/server/cms/src/com/netscape/cms/profile/input/SubjectDNInput.java
@@ -94,7 +94,7 @@ public class SubjectDNInput extends EnrollInput implements IProfileInput {
         String subjectName = "";
 
         subjectName = ctx.get(VAL_SUBJECT);
-        if (subjectName.equals("")) {
+        if (subjectName == null || subjectName.equals("")) {
             throw new EProfileException(
                     CMS.getUserMessage(getLocale(request),
                             "CMS_PROFILE_SUBJECT_NAME_NOT_FOUND"));
diff --git a/base/server/cms/src/com/netscape/cms/servlet/cert/DoRevokeTPS.java b/base/server/cms/src/com/netscape/cms/servlet/cert/DoRevokeTPS.java
index a08f02be10e3c7a62a5847f78560ed288d304d7e..30bd2cde04309dfb86329821b3335036bbd944ee 100644
--- a/base/server/cms/src/com/netscape/cms/servlet/cert/DoRevokeTPS.java
+++ b/base/server/cms/src/com/netscape/cms/servlet/cert/DoRevokeTPS.java
@@ -51,6 +51,7 @@ import com.netscape.certsrv.ca.ICertificateAuthority;
 import com.netscape.certsrv.common.ICMSRequest;
 import com.netscape.certsrv.dbs.certdb.ICertRecord;
 import com.netscape.certsrv.dbs.certdb.ICertificateRepository;
+import com.netscape.certsrv.dbs.certdb.IRevocationInfo;
 import com.netscape.certsrv.logging.AuditFormat;
 import com.netscape.certsrv.logging.ILogger;
 import com.netscape.certsrv.publish.IPublisherProcessor;
@@ -332,6 +333,7 @@ public class DoRevokeTPS extends CMSServlet {
         String auditRequestType = auditRequestType(reason);
         RequestStatus auditApprovalStatus = null;
         String auditReasonNum = String.valueOf(reason);
+        String method = "DoRevokeTPS.process";
 
         if (revokeAll != null) {
             CMS.debug("DoRevokeTPS.process revokeAll" + revokeAll);
@@ -405,9 +407,10 @@ public class DoRevokeTPS extends CMSServlet {
                     rarg.addStringValue("serialNumber",
                             xcert.getSerialNumber().toString(16));
 
-                    if (rec.getStatus().equals(ICertRecord.STATUS_REVOKED)) {
+                    if (rec.getStatus().equals(ICertRecord.STATUS_REVOKED)
+                          && !rec.isCertOnHold()) {
                         alreadyRevokedCertFound = true;
-                        CMS.debug("Certificate 0x" + xcert.getSerialNumber().toString(16) + " has been revoked.");
+                        CMS.debug(method + "Certificate 0x" + xcert.getSerialNumber().toString(16) + " has already been revoked.");
                     } else {
                         oldCertsV.addElement(xcert);
 
@@ -416,7 +419,7 @@ public class DoRevokeTPS extends CMSServlet {
                                         CMS.getCurrentDate(), entryExtn);
 
                         revCertImplsV.addElement(revCertImpl);
-                        CMS.debug("Certificate 0x" + xcert.getSerialNumber().toString(16) + " is going to be revoked.");
+                        CMS.debug(method + "Certificate 0x" + xcert.getSerialNumber().toString(16) + " is going to be revoked.");
                         count++;
                     }
                 } else {
@@ -428,7 +431,7 @@ public class DoRevokeTPS extends CMSServlet {
                 // Situation where no certs were reoked here, but some certs
                 // requested happened to be already revoked. Don't return error.
                 if (alreadyRevokedCertFound == true && badCertsRequested == false) {
-                    CMS.debug("Only have previously revoked certs in the list.");
+                    CMS.debug(method + "Only have previously revoked certs in the list.");
                     // store a message in the signed audit log file
                     auditMessage = CMS.getLogMessage(
                             LOGGING_SIGNED_AUDIT_CERT_STATUS_CHANGE_REQUEST,
@@ -843,6 +846,7 @@ public class DoRevokeTPS extends CMSServlet {
         return;
     }
 
+
     /**
      * Signed Audit Log Requester ID
      *
diff --git a/base/server/cmscore/src/com/netscape/cmscore/dbs/CertRecord.java b/base/server/cmscore/src/com/netscape/cmscore/dbs/CertRecord.java
index 422d506d54d735adb8282dc98134f5c1f1836d42..a79f7a300f2069c85c36439b5ff31751a8723ffa 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/dbs/CertRecord.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/dbs/CertRecord.java
@@ -23,7 +23,11 @@ import java.util.Date;
 import java.util.Enumeration;
 import java.util.Vector;
 
+import netscape.security.x509.CRLExtensions;
+import netscape.security.x509.CRLReasonExtension;
+import netscape.security.x509.RevocationReason;
 import netscape.security.x509.X509CertImpl;
+import netscape.security.x509.X509ExtensionException;
 
 import com.netscape.certsrv.apps.CMS;
 import com.netscape.certsrv.base.EBaseException;
@@ -270,6 +274,30 @@ public class CertRecord implements IDBObj, ICertRecord {
         return mModifyTime;
     }
 
+    public boolean isCertOnHold() {
+        String method = "CertRecord.isCertOnHold:";
+        CMS.debug(method + " checking for cert serial: "
+             + getSerialNumber().toString());
+        IRevocationInfo revInfo = getRevocationInfo();
+        if (revInfo != null) {
+            CRLExtensions crlExts = revInfo.getCRLEntryExtensions();
+            if (crlExts == null) return false;
+            CRLReasonExtension reasonExt = null;
+            try {
+                reasonExt = (CRLReasonExtension) crlExts.get(CRLReasonExtension.NAME);
+            } catch (X509ExtensionException e) {
+                CMS.debug(method + " returning false:" + e.toString());
+                return false;
+            }
+            if (reasonExt.getReason() == RevocationReason.CERTIFICATE_HOLD) {
+                CMS.debug(method + " returning true");
+                return true;
+            }
+        }
+        CMS.debug(method + " returning false");
+        return false;
+    }
+
     /**
      * String representation
      */
diff --git a/base/server/cmscore/src/com/netscape/cmscore/dbs/CertificateRepository.java b/base/server/cmscore/src/com/netscape/cmscore/dbs/CertificateRepository.java
index f22beeb6755cbbbb7f446ba8fb74bb5c5af0e8e7..e6fa0ff149f1b90abfa7cb76147e19895eb86bda 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/dbs/CertificateRepository.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/dbs/CertificateRepository.java
@@ -1110,24 +1110,48 @@ public class CertificateRepository extends Repository
 
     /**
      * Marks certificate as revoked.
+     * isOnHold - boolean to indicate that the cert was revoked onHold
+     *   When a cert was originally onHold, some of the ldap attributes
+     *   already exist, so "MOD_REPLACE" is needed instead of "MOD_ADD"
      */
     public void markAsRevoked(BigInteger id, IRevocationInfo info)
             throws EBaseException {
+        markAsRevoked(id, info, false);
+    }
+    public void markAsRevoked(BigInteger id, IRevocationInfo info, boolean isOnHold)
+            throws EBaseException {
+        String method = "CertificateRepository.markAsRevoked:";
         ModificationSet mods = new ModificationSet();
-
-        mods.add(CertRecord.ATTR_REVO_INFO, Modification.MOD_ADD, info);
+        if (isOnHold) {
+            mods.add(CertRecord.ATTR_REVO_INFO, Modification.MOD_REPLACE, info);
+        } else { 
+            mods.add(CertRecord.ATTR_REVO_INFO, Modification.MOD_ADD, info);
+        }
         SessionContext ctx = SessionContext.getContext();
         String uid = (String) ctx.get(SessionContext.USER_ID);
 
-        if (uid == null) {
-            mods.add(CertRecord.ATTR_REVOKED_BY, Modification.MOD_ADD,
+        if (isOnHold) {
+            if (uid == null) {
+                mods.add(CertRecord.ATTR_REVOKED_BY, Modification.MOD_REPLACE,
                     "system");
+            } else {
+                mods.add(CertRecord.ATTR_REVOKED_BY, Modification.MOD_REPLACE,
+                    uid);
+            }
+            mods.add(CertRecord.ATTR_REVOKED_ON, Modification.MOD_REPLACE,
+                CMS.getCurrentDate());
         } else {
-            mods.add(CertRecord.ATTR_REVOKED_BY, Modification.MOD_ADD,
+            if (uid == null) {
+                mods.add(CertRecord.ATTR_REVOKED_BY, Modification.MOD_ADD,
+                    "system");
+            } else {
+                mods.add(CertRecord.ATTR_REVOKED_BY, Modification.MOD_ADD,
                     uid);
+            }
+            mods.add(CertRecord.ATTR_REVOKED_ON, Modification.MOD_ADD,
+                CMS.getCurrentDate());
         }
-        mods.add(CertRecord.ATTR_REVOKED_ON, Modification.MOD_ADD,
-                CMS.getCurrentDate());
+
         mods.add(CertRecord.ATTR_CERT_STATUS, Modification.MOD_REPLACE,
                 CertRecord.STATUS_REVOKED);
         modifyCertificateRecord(id, mods);
diff --git a/base/tps/src/org/dogtagpki/server/tps/TPSTokendb.java b/base/tps/src/org/dogtagpki/server/tps/TPSTokendb.java
index 2e46b037c73a09719d39168041ad649919cc0819..7997cc579687658de93b8040aabe7ade15465ae2 100644
--- a/base/tps/src/org/dogtagpki/server/tps/TPSTokendb.java
+++ b/base/tps/src/org/dogtagpki/server/tps/TPSTokendb.java
@@ -393,6 +393,7 @@ public class TPSTokendb {
 
             // get conn ID
             String config = "op.enroll." + cert.getType() + ".keyGen." + cert.getKeyType() + ".ca.conn";
+            CMS.debug(method + ": " + " getting config: " + config);
             String connID = configStore.getString(config);
 
             RevocationReason revokeReason = RevocationReason.UNSPECIFIED;
@@ -408,6 +409,7 @@ public class TPSTokendb {
             // get revoke reason
             config = "op.enroll." + cert.getType() + ".keyGen." + cert.getKeyType() +
                     ".recovery." + tokenReason + ".revokeCert.reason";
+            CMS.debug(method + ": " + " getting config: " + config);
             int reasonInt = configStore.getInteger(config, 0);
             revokeReason = RevocationReason.fromInt(reasonInt);
 
-- 
2.4.3

