From 686e5a87df61bc215d2a071de2e446f0188206e8 Mon Sep 17 00:00:00 2001
From: Jack Magne <jmagne@dhcp-16-213.sjc.redhat.com>
Date: Mon, 17 Mar 2014 19:07:07 -0700
Subject: [PATCH]  Further work on TPS Processor, format operation.

Review changes included. Mostly minor, exception handling related.
---
 .../src/org/dogtagpki/tps/main/TPSException.java   |  5 +-
 .../src/org/dogtagpki/server/tps/TPSServlet.java   |  8 +-
 .../src/org/dogtagpki/server/tps/TPSSession.java   |  4 +-
 .../src/org/dogtagpki/server/tps/TPSSubsystem.java | 10 ++-
 .../org/dogtagpki/server/tps/engine/TPSEngine.java |  5 +-
 .../server/tps/processor/TPSFormatProcessor.java   |  6 +-
 .../server/tps/processor/TPSProcessor.java         | 96 ++++++++++++----------
 7 files changed, 76 insertions(+), 58 deletions(-)

diff --git a/base/common/src/org/dogtagpki/tps/main/TPSException.java b/base/common/src/org/dogtagpki/tps/main/TPSException.java
index 0ba7895..ee3ef57 100644
--- a/base/common/src/org/dogtagpki/tps/main/TPSException.java
+++ b/base/common/src/org/dogtagpki/tps/main/TPSException.java
@@ -16,11 +16,11 @@
 // All rights reserved.
 // --- END COPYRIGHT BLOCK ---
 package org.dogtagpki.tps.main;
+
 import org.dogtagpki.tps.msg.EndOp.TPSStatus;
 
 import com.netscape.certsrv.base.EBaseException;
 
-
 public class TPSException extends EBaseException {
 
     private static final long serialVersionUID = -678878301521643436L;
@@ -30,7 +30,8 @@ public class TPSException extends EBaseException {
         super(msg);
         status = TPSStatus.STATUS_ERROR_CONTACT_ADMIN;
     }
-    public TPSException(String msg,TPSStatus theStatus ) {
+
+    public TPSException(String msg, TPSStatus theStatus) {
 
         super(msg);
         status = theStatus;
diff --git a/base/tps-tomcat/src/org/dogtagpki/server/tps/TPSServlet.java b/base/tps-tomcat/src/org/dogtagpki/server/tps/TPSServlet.java
index d5c3983..d56fc7e 100644
--- a/base/tps-tomcat/src/org/dogtagpki/server/tps/TPSServlet.java
+++ b/base/tps-tomcat/src/org/dogtagpki/server/tps/TPSServlet.java
@@ -48,13 +48,7 @@ public class TPSServlet extends HttpServlet {
 
         CMS.debug("TPSSession created: " + session);
 
-        if (session != null) {
-            try {
-                session.process();
-            } catch (Exception e) {
-                CMS.debug("TPSServlet.service: Can't process incoming message exception occured: " + e);
-            }
-        }
+        session.process();
 
         CMS.debug("After session.process() exiting ...");
 
diff --git a/base/tps-tomcat/src/org/dogtagpki/server/tps/TPSSession.java b/base/tps-tomcat/src/org/dogtagpki/server/tps/TPSSession.java
index 4e338d1..79ca435 100644
--- a/base/tps-tomcat/src/org/dogtagpki/server/tps/TPSSession.java
+++ b/base/tps-tomcat/src/org/dogtagpki/server/tps/TPSSession.java
@@ -104,9 +104,9 @@ public class TPSSession {
             switch (op_type) {
             case OP_FORMAT:
 
-                TPSFormatProcessor processor = new TPSFormatProcessor();
+                TPSFormatProcessor processor = new TPSFormatProcessor(this);
                 BeginOp beginOp = (BeginOp) firstMsg;
-                status = processor.process(this, beginOp);
+                status = processor.process(beginOp);
 
             case OP_ENROLL:
                 break;
diff --git a/base/tps-tomcat/src/org/dogtagpki/server/tps/TPSSubsystem.java b/base/tps-tomcat/src/org/dogtagpki/server/tps/TPSSubsystem.java
index c8386bf..10920ed 100644
--- a/base/tps-tomcat/src/org/dogtagpki/server/tps/TPSSubsystem.java
+++ b/base/tps-tomcat/src/org/dogtagpki/server/tps/TPSSubsystem.java
@@ -17,15 +17,16 @@
 // --- END COPYRIGHT BLOCK ---
 package org.dogtagpki.server.tps;
 
+import org.dogtagpki.server.tps.cms.ConnectionManager;
 import org.dogtagpki.server.tps.config.ConfigDatabase;
 import org.dogtagpki.server.tps.config.ConnectionDatabase;
 import org.dogtagpki.server.tps.config.ProfileDatabase;
 import org.dogtagpki.server.tps.config.ProfileMappingDatabase;
-import org.dogtagpki.server.tps.cms.ConnectionManager;
 import org.dogtagpki.server.tps.dbs.ActivityDatabase;
 import org.dogtagpki.server.tps.dbs.AuthenticatorDatabase;
 import org.dogtagpki.server.tps.dbs.TPSCertDatabase;
 import org.dogtagpki.server.tps.dbs.TokenDatabase;
+import org.dogtagpki.server.tps.engine.TPSEngine;
 import org.mozilla.jss.CryptoManager;
 import org.mozilla.jss.CryptoManager.NotInitializedException;
 import org.mozilla.jss.crypto.ObjectNotFoundException;
@@ -65,6 +66,7 @@ public class TPSSubsystem implements IAuthority, ISubsystem {
     public ProfileMappingDatabase profileMappingDatabase;
     public TokenDatabase tokenDatabase;
     public ConnectionManager connManager;
+    public TPSEngine engine;
 
     @Override
     public String getId() {
@@ -98,6 +100,8 @@ public class TPSSubsystem implements IAuthority, ISubsystem {
         connectionDatabase = new ConnectionDatabase();
         profileDatabase = new ProfileDatabase();
         profileMappingDatabase = new ProfileMappingDatabase();
+        engine = new TPSEngine();
+        engine.init();
     }
 
     @Override
@@ -196,4 +200,8 @@ public class TPSSubsystem implements IAuthority, ISubsystem {
         CryptoManager cm = CryptoManager.getInstance();
         return cm.findCertByNickname(nickname);
     }
+
+    public TPSEngine getEngine() {
+        return engine;
+    }
 }
diff --git a/base/tps-tomcat/src/org/dogtagpki/server/tps/engine/TPSEngine.java b/base/tps-tomcat/src/org/dogtagpki/server/tps/engine/TPSEngine.java
index 0b22a9d..7d60bcd 100644
--- a/base/tps-tomcat/src/org/dogtagpki/server/tps/engine/TPSEngine.java
+++ b/base/tps-tomcat/src/org/dogtagpki/server/tps/engine/TPSEngine.java
@@ -92,6 +92,9 @@ public class TPSEngine {
     public static final String OP_FORMAT_PREFIX = "op.format";
 
 
+    public void init() {
+        //ToDo
+    }
 
     public TPSEngine() {
     }
@@ -103,7 +106,7 @@ public class TPSEngine {
         return rc;
     }
 
-    public static boolean isTokenPresent(String cuid) {
+    public boolean isTokenPresent(String cuid) {
         boolean present = false;
 
         return present;
diff --git a/base/tps-tomcat/src/org/dogtagpki/server/tps/processor/TPSFormatProcessor.java b/base/tps-tomcat/src/org/dogtagpki/server/tps/processor/TPSFormatProcessor.java
index 5615513..8a75f73 100644
--- a/base/tps-tomcat/src/org/dogtagpki/server/tps/processor/TPSFormatProcessor.java
+++ b/base/tps-tomcat/src/org/dogtagpki/server/tps/processor/TPSFormatProcessor.java
@@ -28,13 +28,13 @@ import com.netscape.certsrv.apps.CMS;
 
 public class TPSFormatProcessor extends TPSProcessor {
 
-    public TPSFormatProcessor() {
+    public TPSFormatProcessor(TPSSession session) {
+        super(session);
     }
 
 
-    public TPSStatus process(TPSSession theSession, BeginOp beginMsg) throws TPSException, IOException {
+    public TPSStatus process(BeginOp beginMsg) throws TPSException, IOException {
         CMS.debug("In TPS_Format_Processor.Process.");
-        setSession(theSession);
         return super.format(beginMsg);
     }
 
diff --git a/base/tps-tomcat/src/org/dogtagpki/server/tps/processor/TPSProcessor.java b/base/tps-tomcat/src/org/dogtagpki/server/tps/processor/TPSProcessor.java
index f1569b6..b6340af 100644
--- a/base/tps-tomcat/src/org/dogtagpki/server/tps/processor/TPSProcessor.java
+++ b/base/tps-tomcat/src/org/dogtagpki/server/tps/processor/TPSProcessor.java
@@ -21,6 +21,7 @@ import java.io.IOException;
 import java.util.Map;
 
 import org.dogtagpki.server.tps.TPSSession;
+import org.dogtagpki.server.tps.TPSSubsystem;
 import org.dogtagpki.server.tps.channel.SecureChannel.SecurityLevel;
 import org.dogtagpki.server.tps.engine.TPSEngine;
 import org.dogtagpki.tps.apdu.APDU;
@@ -39,6 +40,7 @@ import org.dogtagpki.tps.msg.TokenPDUResponse;
 
 import com.netscape.certsrv.apps.CMS;
 import com.netscape.certsrv.base.EBaseException;
+import com.netscape.certsrv.base.EPropertyNotFound;
 import com.netscape.certsrv.base.IConfigStore;
 
 public abstract class TPSProcessor {
@@ -50,11 +52,12 @@ public abstract class TPSProcessor {
     public static final int CPLC_MSN_INDEX = 41;
     public static final int CPLC_MSN_SIZE = 4;
 
-    private boolean isExternalReg = false;
+    private boolean isExternalReg;
 
-    private TPSSession session = null;
+    private TPSSession session;
 
-    public TPSProcessor() {
+    public TPSProcessor(TPSSession session) {
+        this.session = session;
     }
 
     protected void setSession(TPSSession session) {
@@ -217,17 +220,18 @@ public abstract class TPSProcessor {
 
     }
 
-    protected TPSBuffer getAppletVersion() throws IOException {
+    protected TPSBuffer getAppletVersion() throws IOException, TPSException {
+        //We return null if no applet present
+        // This is not an error, the token can be blank.
 
         CMS.debug("In TPSProcessor.getAppletVersion");
-        TPSBuffer build_id = null;
 
         GetVersion get_version_apdu = new GetVersion();
 
         APDUResponse respApdu = handleAPDURequest(get_version_apdu);
 
         if (checkTokenPDUResponse(respApdu) != true) {
-            CMS.debug("TPSProcessor.getAppletVersion: Error in getAppletVersion!");
+            CMS.debug("TPSProcessor.getAppletVersion: No applet version found on card!");
             return null;
         }
 
@@ -235,10 +239,10 @@ public abstract class TPSProcessor {
 
         if (apdu_data.size() != 6) {
             CMS.debug("TPSProcessor.getAppletVersion: incorrect return data size!");
-            return null;
+            throw new TPSException("TPSProcessor.getAppletVersion: invalid applet version string returned!");
         }
 
-        build_id = apdu_data.substr(0, 4);
+        TPSBuffer build_id = apdu_data.substr(0, 4);
 
         CMS.debug("TPSProcessor.getAppletVersion: returning: " + build_id.toHexString());
 
@@ -267,16 +271,15 @@ public abstract class TPSProcessor {
 
     protected TPSStatus format(BeginOp message) throws TPSException, IOException {
 
-        if (session == null) {
-            throw new TPSException("TPSProcessor.format, no TPSSession object specified!");
-        }
-
         IConfigStore configStore = CMS.getConfigStore();
 
         String CardManagerAID = null;
         String NetKeyAID = null;
         String appletVersion = null;
 
+        TPSSubsystem tps = (TPSSubsystem) CMS.getSubsystem(TPSSubsystem.ID);
+        TPSEngine engine = tps.getEngine();
+
         String External_Reg_Cfg = TPSEngine.CFG_EXTERNAL_REG + "." + "enable";
         boolean isExternalReg = false;
 
@@ -362,6 +365,9 @@ public abstract class TPSProcessor {
         if (status != null) {
             CMS.debug("TPS_Processor.format: status: " + status.toHexString());
             major_version = status.at(0);
+            if (session == null) {
+                throw new TPSException("TPSProcessor.format, no TPSSession object specified!");
+            }
             minor_version = status.at(1);
             app_major_version = status.at(2);
             app_minor_version = status.at(3);
@@ -379,9 +385,7 @@ public abstract class TPSProcessor {
             //ToDo, Do some authentication
         }
 
-        String tokenType = null;
-
-        tokenType = this.getTokenType(TPSEngine.OP_FORMAT_PREFIX, major_version, minor_version, cuid, msn,
+        String tokenType = getTokenType(TPSEngine.OP_FORMAT_PREFIX, major_version, minor_version, cuid, msn,
                 message.getExtensions());
 
         CMS.debug("TPS_Processor.format: calculated tokenType: " + tokenType);
@@ -396,7 +400,7 @@ public abstract class TPSProcessor {
             return ret;
         }
 
-        if (TPSEngine.isTokenPresent(cuid)) {
+        if (engine.isTokenPresent(cuid)) {
             //ToDo
 
         } else {
@@ -442,7 +446,7 @@ public abstract class TPSProcessor {
         return ret;
     }
 
-    boolean checkProfileStateOK(String tokenType) {
+    boolean checkProfileStateOK(String tokenType) throws TPSException {
 
         boolean result = true;
         IConfigStore configStore = CMS.getConfigStore();
@@ -452,10 +456,13 @@ public abstract class TPSProcessor {
 
         try {
             profileState = configStore.getString(profileConfig);
-        } catch (EBaseException e) {
-
+        } catch (EPropertyNotFound e) {
             CMS.debug("TPSProcessor.checkProfileStateOK:  getting profileState, assuming entry does not exist, continuing! "
                     + e);
+        } catch (EBaseException e) {
+            //Default TPSException will return a "contact admin" error code.
+            throw new TPSException(
+                    "TPSProcessor.checkProfileStateOK: internal error in getting profile state from config.");
         }
 
         if (profileState != null && !profileState.equals("Enabled")) {
@@ -467,15 +474,19 @@ public abstract class TPSProcessor {
 
     }
 
-    boolean checkAllowNoAppletToken(String operation, String tokenType) {
+    boolean checkAllowNoAppletToken(String operation, String tokenType) throws TPSException {
         boolean allow = true;
         IConfigStore configStore = CMS.getConfigStore();
 
         String noAppletConfig = operation + "." + tokenType + "." + TPSEngine.CFG_ALLOW_NO_APPLET;
 
         try {
-            allow = configStore.getBoolean(noAppletConfig, false);
+            allow = configStore.getBoolean(noAppletConfig);
+        } catch (EPropertyNotFound e) {
+            CMS.debug("TPSProcessor.checkAllowNoAppletToken: config param not found. Continuing..");
+
         } catch (EBaseException e) {
+            throw new TPSException("PSProcessor.checkAllowNoAppletToken: Internal error getting config param.");
         }
         CMS.debug("TPSProcessor.checkAllowNoAppletToken: returning: " + allow);
 
@@ -489,20 +500,23 @@ public abstract class TPSProcessor {
         String appletRequiredConfig = operation + "." + tokenType + "." + TPSEngine.CFG_APPLET_UPDATE_REQUIRED_VERSION;
 
         try {
-            requiredVersion = configStore.getString(appletRequiredConfig, null);
-        } catch (EBaseException e) {
-        }
-        CMS.debug("TPSProcessor.checkForAppletUpgrade: returning: " + requiredVersion);
-
-        if (requiredVersion == null) {
+            requiredVersion = configStore.getString(appletRequiredConfig);
+        } catch (EPropertyNotFound e) {
             throw new TPSException("TPSProcessor.checkForAppletUpgrade: Can't find applet required Version.",
                     TPSStatus.STATUS_ERROR_UPGRADE_APPLET);
+
+        } catch (EBaseException e) {
+            throw new TPSException(
+                    "TPSProcessor.checkForAppletUpgrade: Can't find applet required Version. Internal error obtaining version.",
+                    TPSStatus.STATUS_ERROR_UPGRADE_APPLET);
         }
 
+        CMS.debug("TPSProcessor.checkForAppletUpgrade: returning: " + requiredVersion);
+
         return requiredVersion;
     }
 
-    protected boolean checkAllowUnknownToken(String operation) {
+    protected boolean checkAllowUnknownToken(String operation) throws TPSException {
         boolean allow = true;
 
         IConfigStore configStore = CMS.getConfigStore();
@@ -510,8 +524,11 @@ public abstract class TPSProcessor {
         String unknownConfig = operation + "." + TPSEngine.CFG_ALLOW_UNKNOWN_TOKEN;
 
         try {
-            allow = configStore.getBoolean(unknownConfig, true);
+            allow = configStore.getBoolean(unknownConfig);
+        } catch (EPropertyNotFound e) {
+            CMS.debug("TPSProcessor.checkAllowUnknownToken: config value not found continue..");
         } catch (EBaseException e) {
+            throw new TPSException("TPSProcessor.checkAllowUnknownToken: Internal error getting config value.");
         }
 
         CMS.debug("checkAllowUnknownToken: returning: " + allow);
@@ -528,11 +545,12 @@ public abstract class TPSProcessor {
 
         try {
             directory = configStore.getString(directoryConfig, null);
+        } catch (EPropertyNotFound e) {
+            throw new TPSException("TPSProcessor.getAppletDirctory: Required config param missing.",
+                    TPSStatus.STATUS_ERROR_UPGRADE_APPLET);
         } catch (EBaseException e) {
-        }
-
-        if (directory == null) {
-            throw new TPSException("TPSProcessor.getAppletDirectory", TPSStatus.STATUS_ERROR_UPGRADE_APPLET);
+            throw new TPSException("TPSProcessor.getAppletDirctory: Internal error finding config value.",
+                    TPSStatus.STATUS_ERROR_UPGRADE_APPLET);
         }
 
         CMS.debug("getAppletDirectory: returning: " + directory);
@@ -564,7 +582,7 @@ public abstract class TPSProcessor {
         return isExternalReg;
     }
 
-    public TPSStatus process(TPSSession session, BeginOp beginMsg) throws TPSException, IOException {
+    public TPSStatus process(BeginOp beginMsg) throws TPSException, IOException {
         return TPSStatus.STATUS_NO_ERROR;
     }
 
@@ -747,10 +765,7 @@ public abstract class TPSProcessor {
 
                 int major = 0;
 
-                try {
-                    Integer.parseInt(majorVersion);
-                } catch (NumberFormatException e) {
-                }
+                major = Integer.parseInt(majorVersion);
 
                 if (major != major_version) {
                     continue;
@@ -772,10 +787,7 @@ public abstract class TPSProcessor {
 
                 int minor = 0;
 
-                try {
-                    minor = Integer.parseInt(minorVersion);
-                } catch (NumberFormatException e) {
-                }
+                minor = Integer.parseInt(minorVersion);
 
                 if (minor != minor_version) {
                     continue;
-- 
1.8.3.1

