>From e4f69b5f2feb45adc0bdeb7da34e8a7d1536ed84 Mon Sep 17 00:00:00 2001
From: Ade Lee <alee@redhat.com>
Date: Thu, 21 Mar 2013 02:54:53 -0400
Subject: [PATCH] Address various issues from review.

Ticket #532
---
 base/common/python/pki/encoder.py                  |   5 +-
 base/common/python/pki/system.py                   |   8 +-
 .../certsrv/system/ConfigurationResponse.java      |   3 +-
 base/deploy/src/scriptlets/configuration.py        |   5 +-
 base/deploy/src/scriptlets/pkihelper.py            | 119 ++++++++++-----------
 base/ocsp/shared/webapps/ocsp/WEB-INF/web.xml      |   3 +-
 6 files changed, 67 insertions(+), 76 deletions(-)

diff --git a/base/common/python/pki/encoder.py b/base/common/python/pki/encoder.py
index 56e0b0222a7d291b38a98fd48f4317c5ffcee72e..7fee57e7106adf841b9d3dd3f58d8eb4843d6ee7 100644
--- a/base/common/python/pki/encoder.py
+++ b/base/common/python/pki/encoder.py
@@ -1,9 +1,8 @@
 import json
 import pki.system
 
-TYPES = { 'ConfigurationRequest': pki.system.ConfigurationRequest,
-          'ConfigurationResponse': pki.system.ConfigurationResponse }
-NOTYPES = { 'SystemCertData': pki.system.SystemCertData }
+TYPES = {}
+NOTYPES = {}
 
 class CustomTypeEncoder(json.JSONEncoder):
     """A custom JSONEncoder class that knows how to encode core custom
diff --git a/base/common/python/pki/system.py b/base/common/python/pki/system.py
index dbf549220f2189f8099f590d6becfa5a606f63df..8f61421a86ec823c9b033a829c35fcb261e43105 100644
--- a/base/common/python/pki/system.py
+++ b/base/common/python/pki/system.py
@@ -19,6 +19,8 @@
 # All rights reserved.
 #
 
+import pki.encoder as encoder
+
 class SecurityDomainInfo:
 
     def __init__(self):
@@ -46,17 +48,17 @@ class ConfigurationRequest:
         self.secureConn = "off"
         self.importAdminCert = "false"
         self.generateServerCert = "true"
-        pass
+        encoder.TYPES['ConfigurationRequest'] = ConfigurationRequest
 
 class ConfigurationResponse:
 
     def __init__(self):
-        pass
+        encoder.TYPES['ConfigurationResponse']= ConfigurationResponse
 
 class SystemCertData:
 
     def __init__(self):
-        pass
+        encoder.NOTYPES['SystemCertData'] = SystemCertData
 
 class SystemConfigClient:
 
diff --git a/base/common/src/com/netscape/certsrv/system/ConfigurationResponse.java b/base/common/src/com/netscape/certsrv/system/ConfigurationResponse.java
index cdd9de205edbfaa1d8de8bcfbf8d8d1482743096..e967914ce37b92a05b441503a7dfb9ff1da08780 100644
--- a/base/common/src/com/netscape/certsrv/system/ConfigurationResponse.java
+++ b/base/common/src/com/netscape/certsrv/system/ConfigurationResponse.java
@@ -19,7 +19,6 @@ package com.netscape.certsrv.system;
 
 import java.security.cert.CertificateEncodingException;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.List;
 
 import javax.xml.bind.annotation.XmlAccessType;
@@ -56,7 +55,7 @@ public class ConfigurationResponse {
     /**
      * @return the systemCerts
      */
-    public Collection<SystemCertData> getSystemCerts() {
+    public List<SystemCertData> getSystemCerts() {
         return systemCerts;
     }
 
diff --git a/base/deploy/src/scriptlets/configuration.py b/base/deploy/src/scriptlets/configuration.py
index 8fc062dfbab2d090abf95cf66c40adb7d4f026bd..7bd1b017a8bef03aa584160912ed50546295a88a 100644
--- a/base/deploy/src/scriptlets/configuration.py
+++ b/base/deploy/src/scriptlets/configuration.py
@@ -105,6 +105,7 @@ class PkiScriptlet(pkiscriptlet.AbstractBasePkiScriptlet):
         if config.str2bool(master['pki_enable_java_debugger']):
             config.wait_to_attach_an_external_java_debugger()
 
+        config_client = util.config_client()
         # Construct PKI Subsystem Configuration Data
         data = None
         if master['pki_instance_type'] == "Apache":
@@ -120,10 +121,10 @@ class PkiScriptlet(pkiscriptlet.AbstractBasePkiScriptlet):
                 return rv
         elif master['pki_instance_type'] == "Tomcat":
             # CA, KRA, OCSP, or TKS
-            data = util.config_client.construct_pki_configuration_data()
+            data = config_client.construct_pki_configuration_data()
         
         # Configure the substem
-        util.config_client.configure_pki_data(
+        config_client.configure_pki_data(
             json.dumps(data, cls=pki.encoder.CustomTypeEncoder))
 
         return self.rv
diff --git a/base/deploy/src/scriptlets/pkihelper.py b/base/deploy/src/scriptlets/pkihelper.py
index 9b67277a73814196a7c2851f339a4b02441a69ea..92707c55365faa2f7851fe1bd84b1edc37d2a67c 100644
--- a/base/deploy/src/scriptlets/pkihelper.py
+++ b/base/deploy/src/scriptlets/pkihelper.py
@@ -1003,17 +1003,15 @@ class instance:
         try:
             client = pki.system.SystemStatusClient(self.connection)
             response = client.getStatus()
+            config.pki_log.debug(response,
+                extra=config.PKI_INDENTATION_LEVEL_3)
+
             root = ET.fromstring(response)
             status = root.findtext("Status")
-
-            config.pki_log.debug(log.PKI_CONFIG_RESPONSE_STATUS, 
-                                 str(status),
-                                 extra=config.PKI_INDENTATION_LEVEL_3)
             return status
-        except requests.exceptions.ConnectionError, e:
-            config.pki_log.debug(log.PKI_CONFIG_RESPONSE_STATUS,
-                                 "Connection Error: " + str(e),
-                                 extra=config.PKI_INDENTATION_LEVEL_3)
+        except requests.exceptions.ConnectionError:
+            config.pki_log.debug("No connection", 
+                extra=config.PKI_INDENTATION_LEVEL_3)
             return None
 
     def wait_for_startup(self, timeout):
@@ -2320,15 +2318,15 @@ class certutil:
     def import_cert(self, nickname, trust, input_file, password_file,
                     path=None, token=None, critical_failure=True):
         try:
-            command = "certutil -A "
+            command = ["certutil","-A"]
             if path:
-                command = command + " -d " + path
+                command.extend(["-d", path])
 
             if token:
-                command = command + " -h '" + token + "'"
+                command.extend(["-h", token])
 
             if nickname:
-                command = command + " -n '" + nickname + "'"
+                command.extend(["-n", nickname ])
             else:
                 config.pki_log.error(
                     log.PKIHELPER_CERTUTIL_MISSING_NICKNAME,
@@ -2336,7 +2334,7 @@ class certutil:
                 sys.exit(1)
 
             if trust:
-                command = command + " -t '" + trust + "'"
+                command.extend(["-t", trust])
             else:
                 config.pki_log.error(
                     log.PKIHELPER_CERTUTIL_MISSING_TRUSTARGS,
@@ -2344,7 +2342,7 @@ class certutil:
                 sys.exit(1)
 
             if input_file:
-                command = command + " -i " + input_file
+                command.extend(["-i", input_file])
             else:
                 config.pki_log.error(
                     log.PKIHELPER_CERTUTIL_MISSING_INPUT_FILE,
@@ -2352,16 +2350,16 @@ class certutil:
                 sys.exit(1)
 
             if password_file:
-                command = command + " -f " + password_file
+                command.extend(["-f", password_file])
             else:
                 config.pki_log.error(
                     log.PKIHELPER_CERTUTIL_MISSING_PASSWORD_FILE,
                     extra=config.PKI_INDENTATION_LEVEL_2)
                 sys.exit(1)
 
-            config.pki_log.debug("Executing: " + command, 
+            config.pki_log.info(command, 
                 extra=config.PKI_INDENTATION_LEVEL_2)
-            subprocess.call(command, shell=True)
+            subprocess.call(command)
         except subprocess.CalledProcessError as exc:
             config.pki_log.error(log.PKI_SUBPROCESS_ERROR_1, exc,
                                  extra=config.PKI_INDENTATION_LEVEL_2)
@@ -2380,17 +2378,17 @@ class certutil:
                                      ascii_format = None, token = None,
                                      critical_failure=True):
         try:
-            command = "certutil -R"
+            command = ["certutil", "-R"]
             if path:
-                command = command + " -d " + path
+                command.extend(["-d", path])
             else:
-                command = command + " -d ."
+                command.extend(["-d", "."])
 
             if token:
-                command = command + " -h '" + token + "'"
+                command.extend(["-h", token])
 
             if subject:
-                command = command + " -s '" + subject + "'"
+                command.extend(["-s", subject])
             else:
                 config.pki_log.error(
                     log.PKIHELPER_CERTUTIL_MISSING_SUBJECT,
@@ -2398,10 +2396,10 @@ class certutil:
                 sys.exit(1)
 
             if key_size:
-                command = command + " -g " + str(key_size)
+                command.extend(["-g", str(key_size)])
 
             if noise_file:
-                command = command + " -z " + noise_file
+                command.extend(["-z", noise_file])
             else:
                 config.pki_log.error(
                     log.PKIHELPER_CERTUTIL_MISSING_NOISE_FILE,
@@ -2409,7 +2407,7 @@ class certutil:
                 sys.exit(1)
 
             if password_file:
-                command = command + " -f " + password_file
+                command.extend(["-f", password_file])
             else:
                 config.pki_log.error(
                     log.PKIHELPER_CERTUTIL_MISSING_PASSWORD_FILE,
@@ -2417,14 +2415,11 @@ class certutil:
                 sys.exit(1)
 
             if output_file:
-                command = command + " -o " + output_file
+                command.extend(["-o", output_file])
 
             # set acsii output
             if ascii_format:
-                command = command + " -a"
-
-            # mask the command-line output of this command
-            command = command + " " + "> /dev/null 2>&1"
+                command.append("-a")
 
             # Display this "certutil" command
             config.pki_log.info(
@@ -2444,7 +2439,8 @@ class certutil:
                     extra=config.PKI_INDENTATION_LEVEL_2)
                 sys.exit(1)
             # Execute this "certutil" command
-            subprocess.call(command, shell=True)
+            with open(os.devnull, "w") as fnull:
+                subprocess.call(command, stdout=fnull, stderr=fnull)
         except subprocess.CalledProcessError as exc:
             config.pki_log.error(log.PKI_SUBPROCESS_ERROR_1, exc,
                                  extra=config.PKI_INDENTATION_LEVEL_2)
@@ -2462,41 +2458,42 @@ class pk12util:
     def create_file(self, out_file, nickname, out_pwfile,
                     db_pwfile, path=None):
         try:
-            command = "pk12util "
+            command = ["pk12util"]
             if path:
-                command = command + " -d " + path
+                command.extend(["-d", path])
             if out_file:
-                command = command + " -o " + out_file
+                command.extend(["-o", out_file])
             else:
                 config.pki_log.error(
                     log.PKIHELPER_PK12UTIL_MISSING_OUTFILE,
                     extra=config.PKI_INDENTATION_LEVEL_2)
                 sys.exit(1)
             if nickname:
-                command = command + " -n \"" + nickname + "\""
+                command.extend(["-n", nickname])
             else:
                 config.pki_log.error(
                     log.PKIHELPER_PK12UTIL_MISSING_NICKNAME,
                     extra=config.PKI_INDENTATION_LEVEL_2)
                 sys.exit(1)
             if out_pwfile:
-                command = command + " -w " + out_pwfile
+                command.extend(["-w", out_pwfile])
             else:
                 config.pki_log.error(
                     log.PKIHELPER_PK12UTIL_MISSING_OUTPWFILE,
                     extra=config.PKI_INDENTATION_LEVEL_2)
                 sys.exit(1)
             if db_pwfile:
-                command = command + " -k " + db_pwfile
+                command.extend(["-k", db_pwfile])
             else:
                 config.pki_log.error(
                     log.PKIHELPER_PK12UTIL_MISSING_DBPWFILE,
                     extra=config.PKI_INDENTATION_LEVEL_2)
                 sys.exit(1)
 
-            config.pki_log.debug("Executing: " + command, 
+            config.pki_log.info(command, 
                     extra=config.PKI_INDENTATION_LEVEL_2)
-            subprocess.call(command, shell=True)
+            with open(os.devnull, "w") as fnull:
+                subprocess.call(command, stdout=fnull, stderr=fnull)
         except subprocess.CalledProcessError as exc:
             config.pki_log.error(log.PKI_SUBPROCESS_ERROR_1, exc,
                                  extra=config.PKI_INDENTATION_LEVEL_2)
@@ -3012,9 +3009,8 @@ class config_client:
                             extra=config.PKI_INDENTATION_LEVEL_2)
                         directory.create(
                             os.path.dirname(master['pki_external_csr_path']))
-                        f =  open(master['pki_external_csr_path'], "w")
-                        f.write(cdata['request'])
-                        f.close()
+                        with open(master['pki_external_csr_path'], "w") as f:
+                            f.write(cdata['request'])
                         return
                 else:
                     config.pki_log.debug(log.PKI_CONFIG_CDATA_TAG +\
@@ -3050,16 +3046,14 @@ class config_client:
         config.pki_log.debug(log.PKI_CONFIG_ADMIN_CERT_SAVE +\
                                " '" + admin_cert_file + "'",
                              extra=config.PKI_INDENTATION_LEVEL_2)
-        f = open(admin_cert_file, "w")
-        f.write(admin_cert)
-        f.close()
+        with open(admin_cert_file, "w") as f:
+            f.write(admin_cert)
 
         # convert the cert file to binary
-        command = "AtoB "+ admin_cert_file + " " + admin_cert_bin_file
-        config.pki_log.debug(log.PKI_CONFIG_ADMIN_CERT_ATOB +\
-            " '" + command + "'",
+        command = ["AtoB", admin_cert_file, admin_cert_bin_file]
+        config.pki_log.info(command,
             extra=config.PKI_INDENTATION_LEVEL_2)
-        os.system(command)
+        subprocess.call(command)
 
         os.chmod(admin_cert_file,
                  config.PKI_DEPLOYMENT_DEFAULT_FILE_PERMISSIONS)
@@ -3161,9 +3155,8 @@ class config_client:
                     print(
                         log.PKI_CONFIG_EXTERNAL_CA_LOAD + " " +\
                         "'" + master['pki_external_ca_cert_path'] + "'")
-                    f = open(master['pki_external_ca_cert_path'])
-                    external_cert = f.read()
-                    f.close()
+                    with open(master['pki_external_ca_cert_path']) as f:
+                        external_cert = f.read()
                     cert1.cert = external_cert
 
                     # Load the 'External CA Signing Certificate Chain' (Step 2)
@@ -3171,9 +3164,8 @@ class config_client:
                         log.PKI_CONFIG_EXTERNAL_CA_CHAIN_LOAD + " " +\
                         "'" + master['pki_external_ca_cert_chain_path'] +\
                         "'")
-                    f = open(master['pki_external_ca_cert_chain_path'])
-                    external_cert_chain = f.read()
-                    f.close()
+                    with open(master['pki_external_ca_cert_chain_path']) as f:
+                        external_cert_chain = f.read()
 
                     cert1.certChain = external_cert_chain
                 systemCerts.append(cert1)
@@ -3305,9 +3297,8 @@ class config_client:
         if config.str2bool(master['pki_import_admin_cert']):
             data.importAdminCert = "true"
             # read config from file
-            f = open(master['pki_admin_cert_file'])
-            b64 = f.read().replace('\n','')
-            f.close()
+            with open(master['pki_admin_cert_file']) as f:
+                b64 = f.read().replace('\n','')
             data.adminCert = b64
         else:
             data.importAdminCert = "false"
@@ -3334,14 +3325,13 @@ class config_client:
                                      None, None, True)
 
                 # convert output to ascii
-                command = "BtoA " + output_file + " " + output_file + ".asc"
-                config.pki_log.debug(command, 
+                command = ["BtoA", output_file, output_file + ".asc"]
+                config.pki_log.info(command, 
                                      extra=config.PKI_INDENTATION_LEVEL_2)
-                os.system(command)
+                subprocess.call(command)
                 
-                f = open(output_file + ".asc")
-                b64 = f.read().replace('\n','')
-                f.close()
+                with open(output_file + ".asc") as f:
+                    b64 = f.read().replace('\n','')
 
                 data.adminCertRequest = b64
             else:
@@ -3404,4 +3394,3 @@ pk12util = pk12util()
 security_domain = security_domain()
 kra_connector = kra_connector()
 systemd = systemd()
-config_client = config_client()
diff --git a/base/ocsp/shared/webapps/ocsp/WEB-INF/web.xml b/base/ocsp/shared/webapps/ocsp/WEB-INF/web.xml
index 570f4db6c44a83acf6b5738d3fb3b1e766cae7ca..68f482b4798cc8bb341c7b932b8883d9c557971b 100644
--- a/base/ocsp/shared/webapps/ocsp/WEB-INF/web.xml
+++ b/base/ocsp/shared/webapps/ocsp/WEB-INF/web.xml
@@ -571,7 +571,8 @@
       <url-pattern>   /admin/console/config/savepkcs12 </url-pattern>
    </servlet-mapping>
 
-   <servlet-name>  ocspGetStatus  </servlet-name>
+   <servlet-mapping>
+      <servlet-name>  ocspGetStatus  </servlet-name>
       <url-pattern>   /admin/ocsp/getStatus  </url-pattern>
    </servlet-mapping>
 
-- 
1.8.1.4

