Ticket #2534 Automatic recovery of encryption cert - CA and TPS tokendb shows different certificate status

This patch fixes the reported issue so now the auto-recovered certificate will reflect the actual status of the certificate.  Also, since the externalReg tracks its own recovered certificate status, it is consolidated with the certificate status tracking mechanism added in this patch so that they can be uniformly managed.

7 files changed, 158 insertions, 185 deletions

diff --git a/base/tps/src/org/dogtagpki/server/tps/TPSTokendb.java b/base/tps/src/org/dogtagpki/server/tps/TPSTokendb.java
index 729e81e07..ca0a409a7 100644
--- a/base/tps/src/org/dogtagpki/server/tps/TPSTokendb.java
+++ b/base/tps/src/org/dogtagpki/server/tps/TPSTokendb.java
@@ -25,15 +25,12 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 
-import netscape.security.x509.RevocationReason;
-
 import org.dogtagpki.server.tps.cms.CARemoteRequestHandler;
 import org.dogtagpki.server.tps.cms.CARevokeCertResponse;
 import org.dogtagpki.server.tps.dbs.ActivityDatabase;
 import org.dogtagpki.server.tps.dbs.TPSCertRecord;
+import org.dogtagpki.server.tps.dbs.TokenCertStatus;
 import org.dogtagpki.server.tps.dbs.TokenRecord;
-import org.dogtagpki.server.tps.main.ExternalRegAttrs;
-import org.dogtagpki.server.tps.main.ExternalRegCertToRecover;
 import org.dogtagpki.tps.main.TPSException;
 import org.dogtagpki.tps.msg.EndOpMsg.TPSStatus;
 
@@ -43,6 +40,8 @@ import com.netscape.certsrv.base.IConfigStore;
 import com.netscape.certsrv.dbs.EDBRecordNotFoundException;
 import com.netscape.certsrv.tps.token.TokenStatus;
 
+import netscape.security.x509.RevocationReason;
+
 /*
  * TPSTokendb class offers a collection of tokendb management convenience routines
  */
@@ -217,64 +216,24 @@ public class TPSTokendb {
         tps.tokenDatabase.updateRecord(id, tokenRecord);
     }
 
-    /*
-     * tdbAddCertificatesForCUID adds certificates issued for the token CUID
-     * - this instance pre-process the cert records to update the cert statuses
-     * @param cuid the cuid of the token
-     * @param certs an ArrayList of TPSCertRecord
-     * @param erAttrs the ExternalRegAttrs collection
-     */
-    public void tdbAddCertificatesForCUID(String cuid, ArrayList<TPSCertRecord> certs, ExternalRegAttrs erAttrs)
-            throws TPSException {
-        String method = "TPSTokendb.tdbAddCertificatesForCUID (with erAttrs): ";
-        String logMsg = "";
-        CMS.debug(method + "begins");
-        if (cuid == null || certs== null || certs.isEmpty() || erAttrs == null) {
-            logMsg =  "params cuid, certs and erAttrs cannot be null or empty";
-            CMS.debug(method + logMsg);
-            throw new TPSException(method + logMsg, TPSStatus.STATUS_ERROR_CONTACT_ADMIN);
-        }
-        CMS.debug("TPSTokendb.tdbAddCertificatesForCUID: number of certs to update:"+ certs.size());
-
-        // update cert status first
-        for (TPSCertRecord cert : certs) {
-            ExternalRegCertToRecover.CertStatus status = ExternalRegCertToRecover.CertStatus.UNINITIALIZED;
-            status = erAttrs.getCertStatus(cert.getSerialNumber());
-            if (status == ExternalRegCertToRecover.CertStatus.UNINITIALIZED) {
-                //cert not found in ExternalReg; don't reset status; don't report
-                continue;
-            }
-
-            //cert is one of the ExternalReg recovered certs, update the status
-            CMS.debug(method + "found and set status for:" + cert.getSerialNumber());
-            cert.setStatus(status.toString());
-
-        }
-
-        tdbAddCertificatesForCUID(cuid, certs);
-        CMS.debug(method + "ends");
-
-
-    }
-
     public void tdbAddCertificatesForCUID(String cuid, ArrayList<TPSCertRecord> certs)
             throws TPSException {
         String method = "TPSTokendb.tdbAddCertificatesForCUID: ";
         CMS.debug(method + "begins");
         boolean tokenExist = isTokenPresent(cuid);
-        if (!tokenExist){
-            CMS.debug("TPSTokendb.tdbAddCertificatesForCUID: token not found: "+ cuid);
-            throw new TPSException("TPSTokendb:tdbUpdateCertificates: token "+ cuid + " does not exist");
+        if (!tokenExist) {
+            CMS.debug(method + " token not found: " + cuid);
+            throw new TPSException(method + " token " + cuid + " does not exist");
         }
 
-        CMS.debug("TPSTokendb.tdbAddCertificatesForCUID: found token "+ cuid);
-        CMS.debug("TPSTokendb.tdbAddCertificatesForCUID: number of certs to update:"+ certs.size());
+        CMS.debug(method + " found token " + cuid);
+        CMS.debug(method + " number of certs to update:" + certs.size());
         try {
-            for (TPSCertRecord cert: certs) {
-               // cert.setOrigin(cuid);
+            for (TPSCertRecord cert : certs) {
+                // cert.setOrigin(cuid);
 
                 try {
-                tps.certDatabase.addRecord(cert.getId(), cert);
+                    tps.certDatabase.addRecord(cert.getId(), cert);
                 } catch (Exception e) {
 
                     //If this is due to a dup, try to update the record.
@@ -282,7 +241,7 @@ public class TPSTokendb {
                 }
             }
         } catch (Exception e) {
-            CMS.debug("TPSTokendb.tdbAddCertificatesForCUID: "+ e);
+            CMS.debug(method + e);
             // TODO: what if it throws in the middle of the cert list -- some cert records already updated?
             throw new TPSException(e.getMessage());
         }
@@ -312,11 +271,12 @@ public class TPSTokendb {
 
     public ArrayList<TPSCertRecord> tdbGetCertRecordsByCert(String serial, String issuer)
             throws TPSException {
+        String method = "TPSTokendb.tdbGetCertRecordsByCert:";
         if (serial == null)
-            throw new TPSException("TPSTokendb.tdbGetCertificatesBySerial: serial null");
+            throw new TPSException(method + " serial null");
 
         if (issuer == null) {
-            throw new TPSException("TPSTokendb.tdbGetCertificatesBySerial: issuer null");
+            throw new TPSException(method + " issuer null");
         }
 
         Map<String, String> attributes = new HashMap<String, String>();
@@ -328,7 +288,7 @@ public class TPSTokendb {
         try {
             records = tps.certDatabase.findRecords(null, attributes).iterator();
         } catch (Exception e) {
-            CMS.debug("TPSTokendb.tdbGetCertificatesByCUID:" + e);
+            CMS.debug(method + e);
             throw new TPSException(e.getMessage());
         }
 
@@ -393,7 +353,7 @@ public class TPSTokendb {
             if (cert.getTokenID().equals(cuid))
                 continue;
 
-            if (cert.getStatus().equals("active"))
+            if (cert.getStatus().equals(TokenCertStatus.ACTIVE.toString()))
                 return false;
         }
 
@@ -444,9 +404,11 @@ public class TPSTokendb {
 
             // update certificate status
             if (revokeReason == RevocationReason.CERTIFICATE_HOLD) {
-                updateCertsStatus(cert.getSerialNumber(), cert.getIssuedBy(), "revoked_on_hold");
+                updateCertsStatus(cert.getSerialNumber(), cert.getIssuedBy(),
+                        TokenCertStatus.ONHOLD.toString());
             } else {
-                updateCertsStatus(cert.getSerialNumber(), cert.getIssuedBy(), "revoked");
+                updateCertsStatus(cert.getSerialNumber(), cert.getIssuedBy(),
+                        TokenCertStatus.REVOKED.toString());
             }
 
             logMsg = "certificate revoked: " + cert.getSerialNumber();
@@ -484,7 +446,7 @@ public class TPSTokendb {
             logMsg = "called to unrevoke";
             CMS.debug(method + ": " + logMsg);
 
-            if (!cert.getStatus().equalsIgnoreCase("revoked_on_hold")) {
+            if (!cert.getStatus().equalsIgnoreCase(TokenCertStatus.ONHOLD.toString())) {
                 logMsg = "certificate record current status is not revoked_on_hold; cannot unrevoke";
                 CMS.debug(method + ": " + logMsg);
                 return; // TODO: continue or bail?
@@ -501,7 +463,8 @@ public class TPSTokendb {
             CMS.debug(method + ": response status: " + response.getStatus());
 
             // update certificate status
-            updateCertsStatus(cert.getSerialNumber(), cert.getIssuedBy(), "active");
+            updateCertsStatus(cert.getSerialNumber(), cert.getIssuedBy(),
+                    TokenCertStatus.ACTIVE.toString());
 
             logMsg = "certificate unrevoked: " + cert.getSerialNumber();
             CMS.debug(method + ": " + logMsg);
@@ -523,11 +486,17 @@ public class TPSTokendb {
     private void checkShouldRevoke(TokenRecord tokenRecord, TPSCertRecord cert, String tokenReason,
             String ipAddress, String remoteUser) throws Exception {
 
+        String method = "TPSTokendb.checkShouldRevoke:";
         IConfigStore configStore = CMS.getConfigStore();
 
         if (cert == null) {
             throw new TPSException("Missing token certificate");
         }
+        if (cert.getStatus().equalsIgnoreCase(TokenCertStatus.REVOKED.toString())) {
+            throw new TPSException(
+                    method + "certificate " + cert.getSerialNumber() +
+                            " already revoked.");
+        }
 
         String tokenType = cert.getType();
         String keyType = cert.getKeyType();
@@ -606,8 +575,6 @@ public class TPSTokendb {
             }
         }
 
-        IConfigStore configStore = CMS.getConfigStore();
-
         for (TPSCertRecord cert : certRecords) {
             if (isRevoke) {
                 revokeCert(tokenRecord, cert, tokenReason, ipAddress, remoteUser);
diff --git a/base/tps/src/org/dogtagpki/server/tps/dbs/TokenCertStatus.java b/base/tps/src/org/dogtagpki/server/tps/dbs/TokenCertStatus.java
new file mode 100644
index 000000000..853740c9a
--- /dev/null
+++ b/base/tps/src/org/dogtagpki/server/tps/dbs/TokenCertStatus.java
@@ -0,0 +1,43 @@
+// --- BEGIN COPYRIGHT BLOCK ---
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; version 2 of the License.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this program; if not, write to the Free Software Foundation, Inc.,
+// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+//
+// (C) 2014 Red Hat, Inc.
+// All rights reserved.
+// --- END COPYRIGHT BLOCK ---
+package org.dogtagpki.server.tps.dbs;
+
+/*
+ * TokenCertStatus - certificate statuses in the tokendb
+ *
+ * @author cfu
+ */
+public enum TokenCertStatus {
+    UNINITIALIZED("uninitialized"),
+    ACTIVE("active"),
+    REVOKED("revoked"),
+    ONHOLD("revoked_on_hold"),
+    EXPIRED("expired")
+    ;
+
+    private final String certStatusString;
+
+    private TokenCertStatus(final String status) {
+        this.certStatusString = status;
+    }
+
+    @Override
+    public String toString() {
+        return certStatusString;
+    }
+}
diff --git a/base/tps/src/org/dogtagpki/server/tps/main/ExternalRegAttrs.java b/base/tps/src/org/dogtagpki/server/tps/main/ExternalRegAttrs.java
index b90888839..2cb2e5029 100644
--- a/base/tps/src/org/dogtagpki/server/tps/main/ExternalRegAttrs.java
+++ b/base/tps/src/org/dogtagpki/server/tps/main/ExternalRegAttrs.java
@@ -1,11 +1,8 @@
 package org.dogtagpki.server.tps.main;
 
-import java.math.BigInteger;
 import java.util.ArrayList;
 
 import org.dogtagpki.server.tps.engine.TPSEngine;
-import org.dogtagpki.tps.main.TPSException;
-import org.dogtagpki.tps.msg.EndOpMsg.TPSStatus;
 
 import com.netscape.certsrv.apps.CMS;
 import com.netscape.certsrv.base.EBaseException;
@@ -110,36 +107,4 @@ public class ExternalRegAttrs {
         return isDelegation;
     }
 
-    /*
-     *
-     * @param serialString serial number in hex
-     */
-    public ExternalRegCertToRecover.CertStatus getCertStatus(String serialString) throws TPSException {
-        String method = "ExternalRegAttrs.getCertStatus:";
-        String logMsg = "";
-        CMS.debug(method + "begins. getCertsToRecoverCount=" + getCertsToRecoverCount());
-        if (serialString == null) {
-            logMsg = "parameter serialString cannnot be null";
-            CMS.debug(method + logMsg);
-            throw new TPSException(method + logMsg, TPSStatus.STATUS_ERROR_CONTACT_ADMIN);
-        } else
-            CMS.debug(method + "searching for serialString =" + serialString);
-        if (serialString.startsWith("0x")) {
-            serialString = serialString.substring(2);
-        }
-        BigInteger serial = new BigInteger(serialString, 16);
-        CMS.debug(method + "searching for serial=" + serial);
-        for (ExternalRegCertToRecover cert: certsToRecover) {
-            CMS.debug(method + "cert.getSerial()=" + cert.getSerial());
-            if (serial.compareTo(cert.getSerial()) == 0) {
-                CMS.debug(method + " cert found... returning status: " + cert.getCertStatus().toString());
-                return cert.getCertStatus();
-            }
-        }
-        logMsg = "cert not found in ExternalReg, status not reset";
-        CMS.debug(method + logMsg);
-        // no match means cert was not one of the ExternalReg recovered certs; so don't reset
-        // use UNINITIALIZED to mean not found, as all certs in externalReg must have been set by now
-        return ExternalRegCertToRecover.CertStatus.UNINITIALIZED;
-    }
 }
diff --git a/base/tps/src/org/dogtagpki/server/tps/main/ExternalRegCertToRecover.java b/base/tps/src/org/dogtagpki/server/tps/main/ExternalRegCertToRecover.java
index a445012eb..5fc8de52a 100644
--- a/base/tps/src/org/dogtagpki/server/tps/main/ExternalRegCertToRecover.java
+++ b/base/tps/src/org/dogtagpki/server/tps/main/ExternalRegCertToRecover.java
@@ -9,7 +9,6 @@ public class ExternalRegCertToRecover {
     String caConn;
     String kraConn;
     boolean isRetainable;
-    CertStatus certStatus = CertStatus.UNINITIALIZED;
 
     public ExternalRegCertToRecover() {
         isRetainable = false;
@@ -54,32 +53,6 @@ public class ExternalRegCertToRecover {
     public boolean getIsRetainable() {
         return isRetainable;
     }
-
-    public void setCertStatus(CertStatus status) {
-        this.certStatus = status;
-    }
-
-    public CertStatus getCertStatus() {
-        return certStatus;
-    }
-
-    public enum CertStatus {
-        UNINITIALIZED("uninitialized"),
-        ACTIVE("active"),
-        REVOKED("revoked"),
-        EXPIRED("expired")
-        ;
-
-        private final String certStatusString;
-        private CertStatus(final String status) {
-            this.certStatusString = status;
-        }
-
-        @Override
-        public String toString() {
-            return certStatusString;
-        }
-    }
 }
 
 
diff --git a/base/tps/src/org/dogtagpki/server/tps/processor/EnrolledCertsInfo.java b/base/tps/src/org/dogtagpki/server/tps/processor/EnrolledCertsInfo.java
index ae9919ddb..35793c71f 100644
--- a/base/tps/src/org/dogtagpki/server/tps/processor/EnrolledCertsInfo.java
+++ b/base/tps/src/org/dogtagpki/server/tps/processor/EnrolledCertsInfo.java
@@ -21,9 +21,8 @@ import java.io.IOException;
 import java.math.BigInteger;
 import java.util.ArrayList;
 
-import netscape.security.x509.X509CertImpl;
-
 import org.dogtagpki.server.tps.dbs.TPSCertRecord;
+import org.dogtagpki.server.tps.dbs.TokenCertStatus;
 import org.dogtagpki.server.tps.main.PKCS11Obj;
 import org.dogtagpki.tps.main.TPSBuffer;
 import org.dogtagpki.tps.main.Util;
@@ -31,10 +30,13 @@ import org.dogtagpki.tps.main.Util;
 import com.netscape.certsrv.apps.CMS;
 import com.netscape.certsrv.base.EBaseException;
 
+import netscape.security.x509.X509CertImpl;
+
 public class EnrolledCertsInfo {
 
     EnrolledCertsInfo() {
         certificates = new ArrayList<X509CertImpl>();
+        certStatuses = new ArrayList<TokenCertStatus>();
         ktypes = new ArrayList<String>();
         origins = new ArrayList<String>();
         tokenTypes = new ArrayList<String>();
@@ -57,6 +59,7 @@ public class EnrolledCertsInfo {
     private ArrayList<String> ktypes;
     private ArrayList<String> tokenTypes;
     private ArrayList<X509CertImpl> certificates;
+    private ArrayList<TokenCertStatus> certStatuses;
 
     private ArrayList<CertEnrollInfo> externalRegRecoveryEnrollList;
 
@@ -156,6 +159,10 @@ public class EnrolledCertsInfo {
         certificates.remove(x509Cert);
     }
 
+    public void addCertStatus(TokenCertStatus status) {
+        certStatuses.add(status);
+    }
+
     public void setStartProgress(int startP) {
         startProgress = startP;
 
@@ -220,6 +227,16 @@ public class EnrolledCertsInfo {
                 //certRecord.setType("");
             }
 
+            //cert status
+            if ((!certStatuses.isEmpty()) && index < certStatuses.size() && certStatuses.get(index) != null) {
+                CMS.debug("EnrolledCertsInfo.toTPSCertRecords: cert status=" + certStatuses.get(index));
+                certRecord.setStatus(certStatuses.get(index).toString());
+            } else {
+                CMS.debug("EnrolledCertsInfo.toTPSCertRecords: certStatus not found for index:" + index
+                        + "; set to default active");
+                certRecord.setStatus(TokenCertStatus.ACTIVE.toString());
+            }
+
             //Issuer
             String issuedBy = cert.getIssuerDN().toString();
             certRecord.setIssuedBy(issuedBy);
@@ -238,9 +255,6 @@ public class EnrolledCertsInfo {
             certRecord.setValidNotAfter(cert.getNotAfter());
             CMS.debug("EnrolledCertsInfo.toTPSCertRecords: notAfter ="+ cert.getNotAfter().toString());
 
-            //status
-            certRecord.setStatus("active");
-
             /* certificate
             byte[] certBytes = null;
             try {
diff --git a/base/tps/src/org/dogtagpki/server/tps/processor/TPSEnrollProcessor.java b/base/tps/src/org/dogtagpki/server/tps/processor/TPSEnrollProcessor.java
index 31d3eedad..af3e92e08 100644
--- a/base/tps/src/org/dogtagpki/server/tps/processor/TPSEnrollProcessor.java
+++ b/base/tps/src/org/dogtagpki/server/tps/processor/TPSEnrollProcessor.java
@@ -15,11 +15,6 @@ import java.util.Map;
 import java.util.Random;
 import java.util.zip.DataFormatException;
 
-import netscape.security.provider.RSAPublicKey;
-//import org.mozilla.jss.pkcs11.PK11ECPublicKey;
-import netscape.security.util.BigInt;
-import netscape.security.x509.X509CertImpl;
-
 import org.dogtagpki.server.tps.TPSSession;
 import org.dogtagpki.server.tps.TPSSubsystem;
 import org.dogtagpki.server.tps.TPSTokenPolicy;
@@ -35,13 +30,13 @@ import org.dogtagpki.server.tps.cms.KRARecoverKeyResponse;
 import org.dogtagpki.server.tps.cms.KRAServerSideKeyGenResponse;
 import org.dogtagpki.server.tps.dbs.ActivityDatabase;
 import org.dogtagpki.server.tps.dbs.TPSCertRecord;
+import org.dogtagpki.server.tps.dbs.TokenCertStatus;
 import org.dogtagpki.server.tps.dbs.TokenRecord;
 import org.dogtagpki.server.tps.engine.TPSEngine;
 import org.dogtagpki.server.tps.engine.TPSEngine.ENROLL_MODES;
 import org.dogtagpki.server.tps.main.AttributeSpec;
 import org.dogtagpki.server.tps.main.ExternalRegAttrs;
 import org.dogtagpki.server.tps.main.ExternalRegCertToRecover;
-import org.dogtagpki.server.tps.main.ExternalRegCertToRecover.CertStatus;
 import org.dogtagpki.server.tps.main.ObjectSpec;
 import org.dogtagpki.server.tps.main.PKCS11Obj;
 import org.dogtagpki.server.tps.mapping.BaseMappingResolver;
@@ -58,8 +53,6 @@ import org.mozilla.jss.pkcs11.PK11PubKey;
 import org.mozilla.jss.pkcs11.PK11RSAPublicKey;
 import org.mozilla.jss.pkix.primitive.SubjectPublicKeyInfo;
 
-import sun.security.pkcs11.wrapper.PKCS11Constants;
-
 import com.netscape.certsrv.apps.CMS;
 import com.netscape.certsrv.base.EBaseException;
 import com.netscape.certsrv.base.EPropertyNotFound;
@@ -67,6 +60,13 @@ import com.netscape.certsrv.base.IConfigStore;
 import com.netscape.certsrv.tps.token.TokenStatus;
 import com.netscape.cmsutil.util.Utils;
 
+import netscape.security.provider.RSAPublicKey;
+//import org.mozilla.jss.pkcs11.PK11ECPublicKey;
+import netscape.security.util.BigInt;
+import netscape.security.x509.RevocationReason;
+import netscape.security.x509.X509CertImpl;
+import sun.security.pkcs11.wrapper.PKCS11Constants;
+
 public class TPSEnrollProcessor extends TPSProcessor {
 
     public TPSEnrollProcessor(TPSSession session) {
@@ -543,6 +543,7 @@ public class TPSEnrollProcessor extends TPSProcessor {
 
         statusUpdate(99, "PROGRESS_SET_LIFECYCLE");
         channel.setLifeycleState((byte) 0x0f);
+
         //update the tokendb with new certs
         CMS.debug(method + " updating tokendb with certs.");
         try {
@@ -555,10 +556,7 @@ public class TPSEnrollProcessor extends TPSProcessor {
         }
         CMS.debug(method + " adding certs to token with tdbAddCertificatesForCUID...");
         ArrayList<TPSCertRecord> certRecords = certsInfo.toTPSCertRecords(tokenRecord.getId(), tokenRecord.getUserID());
-        if (isExternalReg)
-            tps.tdb.tdbAddCertificatesForCUID(tokenRecord.getId(), certRecords, erAttrs);
-        else
-            tps.tdb.tdbAddCertificatesForCUID(tokenRecord.getId(), certRecords);
+        tps.tdb.tdbAddCertificatesForCUID(tokenRecord.getId(), certRecords);
         CMS.debug(method + " tokendb updated with certs to the cuid so that it reflects what's on the token");
 
         logMsg = "appletVersion=" + lastObjVer + "; tokenType =" + selectedTokenType + "; userid =" + userid;
@@ -1250,27 +1248,16 @@ public class TPSEnrollProcessor extends TPSProcessor {
                 return TPSStatus.STATUS_ERROR_RECOVERY_FAILED;
             }
 
-            if (certResp.isCertRevoked()) {
-                CMS.debug(method + " cert revoked");
-                if (!allowRecoverInvalidCert()) {
-                    logMsg = "revoked cert not allowed on token per policy;";
-                    CMS.debug(method + logMsg);
-                    return TPSStatus.STATUS_ERROR_RECOVERY_FAILED;
-                }
-                erCert.setCertStatus(CertStatus.REVOKED);
-                CMS.debug(method + " erCert status =" + erCert.getCertStatus());
-            } else {
-                CMS.debug(method + " cert not revoked ");
-                erCert.setCertStatus(CertStatus.ACTIVE);
-
-                // check if expired or not yet valid
-                if (!certResp.isCertValid()) {
-                    logMsg = "cert expired or not yet valid";
-                    CMS.debug(logMsg);
-                    erCert.setCertStatus(CertStatus.EXPIRED); // it could be not yet valid
-                }
+            TokenCertStatus recoveredCertStatus = getRetrievedCertStatus(certResp);
+            if ((recoveredCertStatus != TokenCertStatus.ACTIVE) &&
+                    !allowRecoverInvalidCert()) {
+                logMsg = "invalid cert not allowed on token per policy; serial=" + serial.toString() + "; cert status=" + recoveredCertStatus.toString();
+                CMS.debug(method + logMsg);
+                return TPSStatus.STATUS_ERROR_RECOVERY_FAILED;
             }
 
+            certsInfo.addCertStatus(recoveredCertStatus);
+
             // default: externalReg.recover.byKeyID=false
             String b64cert = null;
             if (getExternalRegRecoverByKeyID() == false) {
@@ -1364,8 +1351,6 @@ public class TPSEnrollProcessor extends TPSProcessor {
                     int newCertId = pkcs11obj.getNextFreeCertIdNumber();
                     certsInfo.setCurrentCertIndex(i);
 
-                    //certsInfo.setCurrentCertIndex(i);
-
                     CMS.debug(method + "before calling generateCertificate, certsInfo.getCurrentCertIndex() ="
                             + certsInfo.getCurrentCertIndex());
                     generateCertificate(certsInfo, channel, appletInfo,
@@ -1946,7 +1931,7 @@ public class TPSEnrollProcessor extends TPSProcessor {
                             actualCertIndex, cEnrollInfo);
 
                     // unrevoke cert if needed
-                    if (certToRecover.getStatus().equalsIgnoreCase("revoked_on_hold")) {
+                    if (certToRecover.getStatus().equalsIgnoreCase(TokenCertStatus.ONHOLD.toString())) {
                         logMsg = "unrevoking cert...";
                         CMS.debug(method + ":" + logMsg);
 
@@ -1961,7 +1946,8 @@ public class TPSEnrollProcessor extends TPSProcessor {
                             CMS.debug(method + ": response status =" + response.getStatus());
                             auditRevoke(certToRecover.getTokenID(), false /*off-hold*/, -1 /*na*/,
                                     String.valueOf(response.getStatus()), serialToRecover, caConnId, null);
-
+                            // successful unrevoke should mark the cert "active"
+                            certsInfo.addCertStatus(TokenCertStatus.ACTIVE);
                         } catch (EBaseException e) {
                             logMsg = "failed getting CARemoteRequestHandler";
                             CMS.debug(method + ":" + logMsg);
@@ -1970,17 +1956,6 @@ public class TPSEnrollProcessor extends TPSProcessor {
                             throw new TPSException(method + ":" + logMsg, TPSStatus.STATUS_ERROR_RECOVERY_FAILED);
                         }
                     }
-
-                    try {
-                        // set cert status to active
-                        tps.tdb.updateCertsStatus(certToRecover.getSerialNumber(),
-                                certToRecover.getIssuedBy(),
-                                "active");
-                    } catch (Exception e) {
-                        logMsg = "failed tdbUpdateCertEntry";
-                        CMS.debug(method + ":" + logMsg);
-                        throw new TPSException(method + ":" + logMsg, TPSStatus.STATUS_ERROR_RECOVERY_FAILED);
-                    }
                 } else {
 
                 }
@@ -2020,7 +1995,7 @@ public class TPSEnrollProcessor extends TPSProcessor {
 
         certsInfo.setNumCertsToEnroll(keyTypeNum);
 
-        CMS.debug("TPSEnrollProcessor.generateCertificate: Number of certs to enroll: " + keyTypeNum);
+        CMS.debug("TPSEnrollProcessor.generateCertificates: Number of certs to enroll: " + keyTypeNum);
 
         for (int i = 0; i < keyTypeNum; i++) {
             String keyType = getConfiguredKeyType(i);
@@ -2477,6 +2452,7 @@ public class TPSEnrollProcessor extends TPSProcessor {
         // enrollment/recovery begins
         CMS.debug("TPSEnrollProcessor.enrollOneCertificate:: enrollment begins");
         X509CertImpl x509Cert = null;
+        TokenCertStatus certStatus = TokenCertStatus.ACTIVE; // track cert status
         byte[] cert_bytes = null;
         try {
 
@@ -2613,14 +2589,14 @@ public class TPSEnrollProcessor extends TPSProcessor {
 
                 cert_bytes = Utils.base64decode(retCertB64);
 
-                TPSBuffer cert_bytes_buf = new TPSBuffer(cert_bytes);
+                //TPSBuffer cert_bytes_buf = new TPSBuffer(cert_bytes);
                 //CMS.debug("TPSEnrollProcessor.enrollOneCertificate: retCertB64: " + cert_bytes_buf.toHexString());
                 CMS.debug("TPSEnrollProcessor.enrollOneCertificate: retCertB64 base64decode done");
 
                 x509Cert = caEnrollResp.getCert();
-                if (x509Cert != null)
+                if (x509Cert != null) {
                     CMS.debug("TPSEnrollProcessor.enrollOneCertificate:: new cert retrieved");
-                else {
+                } else {
                     CMS.debug("TPSEnrollProcessor.enrollOneCertificate:: new cert not found");
                     throw new TPSException("TPSEnrollProcessor.enrollOneCertificate: new cert not found",
                             TPSStatus.STATUS_ERROR_MAC_ENROLL_PDU);
@@ -2660,14 +2636,16 @@ public class TPSEnrollProcessor extends TPSProcessor {
                     CMS.debug("TPSEnrollProcessor.enrollOneCertificate: recovering:  retCertB64 retrieved from certResp");
                     cert_bytes = Utils.base64decode(retCertB64);
 
-                    TPSBuffer cert_bytes_buf = new TPSBuffer(cert_bytes);
                     CMS.debug("TPSEnrollProcessor.enrollOneCertificate: recovering: retCertB64 base64decode done");
+                    //TPSBuffer cert_bytes_buf = new TPSBuffer(cert_bytes);
                     //CMS.debug("TPSEnrollProcessor.enrollOneCertificate: recovering: retCertB64: "
                     //        + cert_bytes_buf.toHexString());
 
                     x509Cert = certResp.getCert();
                     if (x509Cert != null) {
                         CMS.debug("TPSEnrollProcessor.enrollOneCertificate:: recovering new cert retrieved");
+                        // recovered cert might have different status
+                        certStatus = getRetrievedCertStatus(certResp);
                         auditEnrollment(userid, "retrieval", aInfo, "success",
                                 channel.getKeyInfoData().toHexStringPlain(), x509Cert.getSerialNumber(),
                                 certResp.getConnID(), null);
@@ -2713,8 +2691,8 @@ public class TPSEnrollProcessor extends TPSProcessor {
                     }
 
                     cert_bytes = Utils.base64decode(retCertB64);
-                    TPSBuffer cert_bytes_buf = new TPSBuffer(cert_bytes);
                     CMS.debug("TPSEnrollProcessor.enrollOneCertificate: renewing: retCertB64 base64decode done");
+                    //TPSBuffer cert_bytes_buf = new TPSBuffer(cert_bytes);
                     //CMS.debug("TPSEnrollProcessor.enrollOneCertificate: renewing: retCertB64: "
                     //        + cert_bytes_buf.toHexString());
 
@@ -2758,6 +2736,7 @@ public class TPSEnrollProcessor extends TPSProcessor {
                 certsInfo.addOrigin(aInfo.getCUIDhexStringPlain());
             }
 
+            certsInfo.addCertStatus(certStatus);
             certsInfo.addTokenType(selectedTokenType);
 
             SubjectPublicKeyInfo publicKeyInfo = null;
@@ -2892,6 +2871,37 @@ public class TPSEnrollProcessor extends TPSProcessor {
 
     }
 
+    /*
+     * getRetrievedCertStatus
+     * @returns TokenCertStatus certificate status of the cert retrieved in certResponse
+     */
+    TokenCertStatus getRetrievedCertStatus(CARetrieveCertResponse certResponse)
+            throws TPSException {
+        String method = "TPSEnrollProcessor.getRetrievedCertStatus";
+        CMS.debug(method + " begins");
+        if (certResponse == null) {
+            throw new TPSException(
+                    "TPSEnrollProcessor.getRetrievedCertStatus: invalid input data! certResponse cannot be null",
+                    TPSStatus.STATUS_ERROR_MAC_ENROLL_PDU);
+        }
+        TokenCertStatus ret = TokenCertStatus.ACTIVE;
+        if (!certResponse.isCertValid()) {
+            CMS.debug(method + ": cert expired");
+            ret = TokenCertStatus.EXPIRED;
+        }
+        //This would overwrite the "EXPIRED" status,
+        //but "REVOKED" would be a more serious invalid status
+        if (certResponse.isCertRevoked()) {
+            String revReason = certResponse.getRevocationReason();
+            CMS.debug(method + ": cert revoked; reason=" + revReason);
+            if (RevocationReason.fromInt(Integer.parseInt(revReason)) == RevocationReason.CERTIFICATE_HOLD)
+                ret = TokenCertStatus.ONHOLD;
+            else
+                ret = TokenCertStatus.REVOKED;
+        }
+        return ret;
+    }
+
     private void importPrivateKeyPKCS8(KRARecoverKeyResponse keyResp, CertEnrollInfo cEnrollInfo,
             SecureChannel channel,
             boolean isECC) throws TPSException, IOException {
diff --git a/base/tps/src/org/dogtagpki/server/tps/processor/TPSProcessor.java b/base/tps/src/org/dogtagpki/server/tps/processor/TPSProcessor.java
index 582e3f90c..2b42dc613 100644
--- a/base/tps/src/org/dogtagpki/server/tps/processor/TPSProcessor.java
+++ b/base/tps/src/org/dogtagpki/server/tps/processor/TPSProcessor.java
@@ -33,8 +33,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
-import netscape.security.x509.RevocationReason;
-
 import org.dogtagpki.server.tps.TPSSession;
 import org.dogtagpki.server.tps.TPSSubsystem;
 import org.dogtagpki.server.tps.authentication.AuthUIParameter;
@@ -50,6 +48,7 @@ import org.dogtagpki.server.tps.cms.TKSRemoteRequestHandler;
 import org.dogtagpki.server.tps.config.ProfileDatabase;
 import org.dogtagpki.server.tps.dbs.ActivityDatabase;
 import org.dogtagpki.server.tps.dbs.TPSCertRecord;
+import org.dogtagpki.server.tps.dbs.TokenCertStatus;
 import org.dogtagpki.server.tps.dbs.TokenRecord;
 import org.dogtagpki.server.tps.engine.TPSEngine;
 import org.dogtagpki.server.tps.main.ExternalRegAttrs;
@@ -98,6 +97,8 @@ import com.netscape.certsrv.tps.token.TokenStatus;
 import com.netscape.cms.servlet.tks.SecureChannelProtocol;
 import com.netscape.symkey.SessionKey;
 
+import netscape.security.x509.RevocationReason;
+
 public class TPSProcessor {
 
     public static final int RESULT_NO_ERROR = 0;
@@ -1563,7 +1564,7 @@ public class TPSProcessor {
              * if the certificates are revoked_on_hold, don't do anything because the certificates may
              * be referenced by more than one token.
              */
-            if (cert.getStatus().equals("revoked_on_hold")) {
+            if (cert.getStatus().equals(TokenCertStatus.ONHOLD.toString())) {
                 CMS.debug(method + ": cert " + cert.getSerialNumber()
                         + " has status revoked_on_hold; remove from tokendb and move on");
                 try {