Fixes for coments from review

5 files changed, 139 insertions, 97 deletions

diff --git a/base/common/python/pki/cryptoutil.py b/base/common/python/pki/cryptoutil.py
index b39259dd5..d7cd1670c 100644
--- a/base/common/python/pki/cryptoutil.py
+++ b/base/common/python/pki/cryptoutil.py
@@ -98,15 +98,16 @@ class NSSCryptoUtil(CryptoUtil):
         ''' Create an NSS database '''
         if os.path.exists(db_dir):
             if not over_write:
-                raise exceptions.ValueError(
-                        "Directory already exists and over_write is false")
+                raise exceptions.IOError(
+                        "Directory already exists.")
             if os.path.isdir(db_dir):
                 shutil.rmtree(db_dir)
             else:
                 os.remove(db_dir)
         os.makedirs(db_dir)
 
-        with tempfile.NamedTemporaryFile() as pwd_file:
+        home = os.path.expanduser("~")
+        with tempfile.NamedTemporaryFile(dir=home) as pwd_file:
             pwd_file.write(password)
             pwd_file.flush()
             command = ['certutil', '-N', '-d', db_dir, '-f', pwd_file.name]
@@ -145,7 +146,7 @@ class NSSCryptoUtil(CryptoUtil):
         # Get a PK11 slot based on the cipher
         slot = nss.get_best_slot(mechanism)
 
-        if sym_key == None:
+        if sym_key is None:
             sym_key = slot.key_gen(mechanism, None, slot.get_best_key_length(mechanism))
 
         # If initialization vector was supplied use it, otherwise set it to None
@@ -192,7 +193,7 @@ class NSSCryptoUtil(CryptoUtil):
 
     def symmetric_wrap(self, data, wrapping_key, mechanism=nss.CKM_DES3_CBC_PAD, nonce_iv=None):
         '''
-        :param data:           Data to be wrapped
+        :param data            Data to be wrapped
         :param wrapping_key    Symmetric key to wrap data
 
         Wrap (encrypt) data using the supplied symmetric key
@@ -203,13 +204,13 @@ class NSSCryptoUtil(CryptoUtil):
 
     def symmetric_unwrap(self, data, wrapping_key, mechanism=nss.CKM_DES3_CBC_PAD, nonce_iv=None):
         '''
-        :param data:           Data to be unwrapped
+        :param data            Data to be unwrapped
         :param wrapping_key    Symmetric key to unwrap data
         :param nonce_iv        iv data
 
         Unwrap (decrypt) data using the supplied symmetric key
         '''
-        if nonce_iv == None:
+        if nonce_iv is None:
             nonce_iv = self.nonce_iv
         else:
             nonce_iv = nss.data_to_hex(nonce_iv)
@@ -221,7 +222,7 @@ class NSSCryptoUtil(CryptoUtil):
 
     def asymmetric_wrap(self, data, wrapping_cert, mechanism=nss.CKM_DES3_CBC_PAD):
         '''
-        :param data:            Data to be wrapped
+        :param data             Data to be wrapped
         :param wrapping_cert    Public key to wrap data
         :param mechanism        algorithm of symmetric key to be wrapped
 
diff --git a/base/common/python/pki/key.py b/base/common/python/pki/key.py
index de9fcc959..cdcc7380f 100644
--- a/base/common/python/pki/key.py
+++ b/base/common/python/pki/key.py
@@ -69,7 +69,10 @@ class KeyData(object):
         ''' Return a KeyData object from a JSON dict '''
         key_data = cls()
         for key in attr_list:
-            setattr(key_data, key, attr_list[key])
+            if (key == "wrappedPrivateData") or (key == "nonceData"):
+                setattr(key_data, key, base64.decodestring(attr_list[key]))
+            else:
+                setattr(key_data, key, attr_list[key])
         return key_data
 
 class KeyInfo(object):
@@ -342,7 +345,7 @@ class KeyClient(object):
     def set_transport_cert(self, transport_cert_nick):
         ''' Set the transport certificate for crypto operations '''
         if transport_cert_nick is None:
-            raise ValueError("Transport cert nickname must be specified.")
+            raise TypeError("Transport certificate nickname must be specified.")
         self.transport_cert = self.crypto.get_cert(transport_cert_nick)
 
     @pki.handle_exceptions()
@@ -378,7 +381,7 @@ class KeyClient(object):
     def get_request_info(self, request_id):
         ''' Return a KeyRequestInfo object for a specific request. '''
         if request_id is None:
-            raise ValueError("request_id must be specified")
+            raise TypeError("Request ID must be specified")
 
         url = self.key_requests_url + '/' + request_id
         response = self.connection.get(url, self.headers)
@@ -388,7 +391,7 @@ class KeyClient(object):
     def get_key_info(self, key_id):
         ''' Get the info in the KeyRecord for a specific secret in the DRM. '''
         if key_id is None:
-            raise ValueError("key_id must be specified")
+            raise TypeError("Key ID must be specified")
 
         url = self.key_url + '/' + key_id
         response = self.connection.get(url, headers=self.headers)
@@ -398,9 +401,9 @@ class KeyClient(object):
     def get_active_key_info(self, client_key_id):
         ''' Get the info in the KeyRecord for the active secret in the DRM. '''
         if client_key_id is None:
-            raise ValueError("client_key_id must be specified")
+            raise TypeError("Client Key ID must be specified")
 
-        url = self.key_url + '/active/' + urllib.quote_plus(client_key_id)
+        url = self.key_url + '/active/' + urllib.quote(client_key_id)
         response = self.connection.get(url, headers=self.headers)
         return KeyInfo.from_json(response.json())
 
@@ -408,7 +411,7 @@ class KeyClient(object):
     def modify_key_status(self, key_id, status):
         ''' Modify the status of a key '''
         if (key_id is None) or (status is None):
-            raise ValueError("key_id and status must be specified")
+            raise TypeError("Key ID and status must be specified")
 
         url = self.key_url + '/' + key_id
         params = {'status':status}
@@ -418,7 +421,7 @@ class KeyClient(object):
     def approve_request(self, request_id):
         ''' Approve a secret recovery request '''
         if request_id is None:
-            raise ValueError("request_id must be specified")
+            raise TypeError("Request ID must be specified")
 
         url = self.key_requests_url + '/' + request_id + '/approve'
         self.connection.post(url, self.headers)
@@ -427,7 +430,7 @@ class KeyClient(object):
     def reject_request(self, request_id):
         ''' Reject a secret recovery request. '''
         if request_id is None:
-            raise ValueError("request_id must be specified")
+            raise TypeError("Request ID must be specified")
 
         url = self.key_requests_url + '/' + request_id + '/reject'
         self.connection.post(url, self.headers)
@@ -436,7 +439,7 @@ class KeyClient(object):
     def cancel_request(self, request_id):
         ''' Cancel a secret recovery request '''
         if request_id is None:
-            raise ValueError("request_id must be specified")
+            raise TypeError("Request ID must be specified")
 
         url = self.key_requests_url + '/' + request_id + '/cancel'
         self.connection.post(url, self.headers)
@@ -452,7 +455,7 @@ class KeyClient(object):
             returns a KeyRequestResponse object.
         '''
         if request is None:
-            raise ValueError("request must be specified")
+            raise TypeError("Request must be specified")
 
         url = self.key_requests_url
         key_request = json.dumps(request, cls=encoder.CustomTypeEncoder, sort_keys=True)
@@ -469,7 +472,7 @@ class KeyClient(object):
 
         '''
         if client_key_id is None:
-            raise ValueError("Must specify client_key_id")
+            raise TypeError("Must specify Client Key ID")
 
         if trans_wrapped_session_key is not None:
             twsk = base64.encodestring(trans_wrapped_session_key)
@@ -490,10 +493,7 @@ class KeyClient(object):
         return self.create_request(request)
 
     @pki.handle_exceptions()
-    def archive_key(self, client_key_id, data_type, private_data=None,
-                    wrapped_private_data=None, trans_wrapped_session_key=None,
-                    algorithm_oid=None, symkey_params=None,
-                    pki_archive_options=None,
+    def archive_key(self, client_key_id, data_type, private_data,
                     key_algorithm=None, key_size=None):
         ''' Archive a secret (symmetric key or passphrase) on the DRM.
 
@@ -509,12 +509,57 @@ class KeyClient(object):
             key_algorithm and key_size are applicable to symmetric keys only.
             If a symmetric key is being archived, these parameters are required.
 
-            Callers can invoke this method in one of three ways:
+            private_data is the raw secret to be archived.
+            It will be wrapped and sent to the DRM.
+
+            The function returns a KeyRequestResponse object containing a KeyRequestInfo
+            object with details about the archival request and key archived.
+        '''
+        if (client_key_id is None) or (data_type is None):
+            raise TypeError("Client Key ID and data type must be specified")
+
+        if data_type == KeyClient.SYMMETRIC_KEY_TYPE:
+            if (key_algorithm is None) or (key_size is None):
+                raise TypeError(
+                        "For symmetric keys, key algorithm and key_size must be specified")
 
-            1. Provide the private_data.  This is the secret to be archived.
-               It will be wrapped and sent to the DRM.
+        if private_data is None:
+            raise TypeError("No data provided to be archived")
+
+        session_key = self.crypto.generate_session_key()
+        trans_wrapped_session_key = \
+            self.crypto.asymmetric_wrap(session_key, self.transport_cert)
+        wrapped_private_data = self.crypto.symmetric_wrap(private_data, session_key)
+
+        twsk = base64.encodestring(trans_wrapped_session_key)
+        data = base64.encodestring(wrapped_private_data)
+
+        # TODO - generate_algorithm_oid here
+        # generate symkey_params here
+        algorithm_oid = "todo - fix me"
+        symkey_params = "todo - fix me"
+
+        request = KeyArchivalRequest(client_key_id=client_key_id,
+                         data_type=data_type,
+                         wrapped_private_data=data,
+                         trans_wrapped_session_key=twsk,
+                         algorithm_oid=algorithm_oid,
+                         symkey_params=symkey_params,
+                         key_algorithm=key_algorithm,
+                         key_size=key_size)
+        return self.create_request(request)
 
-            2. Provide the following pieces:
+    @pki.handle_exceptions()
+    def archive_wrapped_data(self, client_key_id, data_type,
+                    wrapped_private_data, trans_wrapped_session_key,
+                    algorithm_oid, symkey_params,
+                    key_algorithm=None, key_size=None):
+        ''' Archive a secret (symmetric key or passphrase) on the DRM.
+
+            Refer to archive_key() comments for a description of client_key_id,
+            data_type, key_algorithm and key_size.
+
+            The following parameters are also required:
                 - wrapped_private_data - which is the secret wrapped by a session
                 key (168 bit 3DES symmetric key)
                 - trans_wrapped_session_key - the above session key wrapped by the
@@ -522,58 +567,28 @@ class KeyClient(object):
                 - the algorithm_oid string for the symmetric key wrap
                 - the symkey_params for the symmetric key wrap
 
-            3. wrapped_private_data which consists of a PKIArchiveOptions structure,
-
-            private_data is the secret that is to be archived.
+            This function is useful if the caller wants to do their own wrapping
+            of the secret, or if the secret was generated on a separate client
+            machine and the wrapping was done there.
 
             The function returns a KeyRequestResponse object containing a KeyRequestInfo
             object with details about the archival request and key archived.
         '''
         if (client_key_id is None) or (data_type is None):
-            raise ValueError("client_key_id and data_type must be specified")
+            raise TypeError("Client Key ID and data type must be specified")
 
         if data_type == KeyClient.SYMMETRIC_KEY_TYPE:
             if (key_algorithm is None) or (key_size is None):
-                raise ValueError(
-                        "For symmetric keys, key algorithm and key_size must be specified")
+                raise TypeError(
+                        "For symmetric keys, key algorithm and key size must be specified")
 
-        if private_data is None:
-            if wrapped_private_data is None:
-                if pki_archive_options is None:
-                    raise ValueError("No data provided to be archived")
-                else:
-                    data = base64.encodestring(pki_archive_options)
-                    request = KeyArchivalRequest(client_key_id=client_key_id,
-                                     data_type=data_type,
-                                     pki_archive_options=data,
-                                     key_algorithm=key_algorithm,
-                                     key_size=key_size)
-            if trans_wrapped_session_key is None:
-                raise ValueError("Session Key not provided")
-            else:
-                twsk = base64.encodestring(trans_wrapped_session_key)
-                data = base64.encodestring(wrapped_private_data)
-                request = KeyArchivalRequest(client_key_id=client_key_id,
-                                 data_type=data_type,
-                                 wrapped_private_data=data,
-                                 trans_wrapped_session_key=twsk,
-                                 algorithm_oid=algorithm_oid,
-                                 symkey_params=symkey_params,
-                                 key_algorithm=key_algorithm,
-                                 key_size=key_size)
-        else:
-            session_key = self.crypto.generate_session_key()
-            trans_wrapped_session_key = \
-                self.crypto.asymmetric_wrap(session_key, self.transport_cert)
-            wrapped_private_data = self.crypto.symmetric_wrap(private_data, session_key)
+        if (wrapped_private_data is None) or (trans_wrapped_session_key is None) or \
+           (algorithm_oid is None)  or (symkey_params is None):
+            raise TypeError("All data and wrapping parameters must be specified")
 
-            twsk = base64.encodestring(trans_wrapped_session_key)
-            data = base64.encodestring(wrapped_private_data)
-
-            # TODO - generate_algorithm_oid here
-            # generate symkey_params here
-
-            request = KeyArchivalRequest(client_key_id=client_key_id,
+        twsk = base64.encodestring(trans_wrapped_session_key)
+        data = base64.encodestring(wrapped_private_data)
+        request = KeyArchivalRequest(client_key_id=client_key_id,
                             data_type=data_type,
                             wrapped_private_data=data,
                             trans_wrapped_session_key=twsk,
@@ -581,6 +596,40 @@ class KeyClient(object):
                             symkey_params=symkey_params,
                             key_algorithm=key_algorithm,
                             key_size=key_size)
+
+        return self.create_request(request)
+
+    @pki.handle_exceptions()
+    def archive_options_data(self, client_key_id, data_type, pki_archive_options,
+                    key_algorithm=None, key_size=None):
+        ''' Archive a secret (symmetric key or passphrase) on the DRM.
+
+            Refer to archive_key() comments for a description of client_key_id,
+            data_type, key_algorithm and key_size.
+
+            pki_archive_options is the data to be archived wrapped in a
+            PKIArchiveOptions structure,
+
+            The function returns a KeyRequestResponse object containing a KeyRequestInfo
+            object with details about the archival request and key archived.
+        '''
+        if (client_key_id is None) or (data_type is None):
+            raise TypeError("Client Key_ID and Data Type must be specified")
+
+        if data_type == KeyClient.SYMMETRIC_KEY_TYPE:
+            if (key_algorithm is None) or (key_size is None):
+                raise TypeError(
+                        "For symmetric keys, key algorithm and key_size must be specified")
+
+        if pki_archive_options is None:
+            raise TypeError("No data provided to be archived")
+
+        data = base64.encodestring(pki_archive_options)
+        request = KeyArchivalRequest(client_key_id=client_key_id,
+                           data_type=data_type,
+                           pki_archive_options=data,
+                           key_algorithm=key_algorithm,
+                           key_size=key_size)
         return self.create_request(request)
 
     @pki.handle_exceptions()
@@ -597,7 +646,7 @@ class KeyClient(object):
             is required.
         '''
         if key_id is None:
-            raise ValueError("key_id must be defined")
+            raise TypeError("Key ID must be defined")
 
         request = KeyRecoveryRequest(key_id=key_id,
                                      request_id=request_id,
@@ -619,7 +668,7 @@ class KeyClient(object):
             Returns a KeyData object containing the wrapped secret.
         '''
         if data is None:
-            raise ValueError("KeyRecoveryRequest must be specified")
+            raise TypeError("Key Recovery Request must be specified")
 
         url = self.key_url + '/retrieve'
         key_request = json.dumps(data, cls=encoder.CustomTypeEncoder, sort_keys=True)
@@ -655,7 +704,7 @@ class KeyClient(object):
         wrapped secret and some nonce data to be used as a salt when unwrapping.
         '''
         if key_id is None:
-            raise ValueError("key_id must be specified")
+            raise TypeError("Key ID must be specified")
 
         key_provided = True
         if trans_wrapped_session_key is None:
@@ -678,9 +727,9 @@ class KeyClient(object):
             return key_data, None
 
         unwrapped_key = self.crypto.symmetric_unwrap(
-                                base64.decodestring(key_data.wrappedPrivateData),
+                                key_data.wrappedPrivateData,
                                 session_key,
-                                nonce_iv=base64.decodestring(key_data.nonceData))
+                                nonce_iv=key_data.nonceData)
         return key_data, unwrapped_key
 
     @pki.handle_exceptions()
@@ -741,7 +790,7 @@ class KeyClient(object):
         The function returns a KeyData object.
         '''
         if (key_id is None) or (certificate is None) or (passphrase is None):
-            raise ValueError("key_id, certificate and passphrase must all be specified")
+            raise TypeError("Key ID, certificate and passphrase must all be specified")
 
         response = self.recover_key(key_id, b64certificate=certificate)
         request_id = response.get_request_id()
diff --git a/base/kra/functional/drmtest.py b/base/kra/functional/drmtest.py
index 08baf5011..169a7879f 100644
--- a/base/kra/functional/drmtest.py
+++ b/base/kra/functional/drmtest.py
@@ -48,7 +48,7 @@ def print_key_request(request):
 def print_key_info(key_info):
     ''' Prints the relevant fields of a KeyInfo object '''
     print "Key URL: " + str(key_info.keyURL)
-    print "Client ID: " + str(key_info.clientKeyID)
+    print "Client Key ID: " + str(key_info.clientKeyID)
     print "Algorithm: " + str(key_info.algorithm)
     print "Status: " + str(key_info.status)
     print "Owner Name: " + str(key_info.ownerName)
@@ -58,8 +58,8 @@ def print_key_data(key_data):
     ''' Prints the relevant fields of a KeyData object '''
     print "Key Algorithm: " + str(key_data.algorithm)
     print "Key Size: " + str(key_data.size)
-    print "Nonce Data: " + str(key_data.nonceData)
-    print "Wrapped Private Data: " + str(key_data.wrappedPrivateData)
+    print "Nonce Data: " + base64.encodestring(key_data.nonceData)
+    print "Wrapped Private Data: " + base64.encodestring(key_data.wrappedPrivateData)
 
 def main():
     ''' test code execution '''
@@ -105,8 +105,7 @@ def main():
 
     # Test 4: generate symkey -- same as barbican_encode()
     print "Now generating symkey on KRA"
-    #client_key_id = "Vek #1" + time.strftime('%X %x %Z')
-    client_key_id = "veka9"
+    client_key_id = "Vek #1" + time.strftime('%c')
     algorithm = "AES"
     key_size = 128
     usages = [key.SymKeyGenerationRequest.DECRYPT_USAGE, key.SymKeyGenerationRequest.ENCRYPT_USAGE]
@@ -135,9 +134,9 @@ def main():
     print "My key id is " + str(key_id)
     key_data, _unwrapped_key = keyclient.retrieve_key(key_id, trans_wrapped_session_key=wrapped_session_key)
     print_key_data(key_data)
-    unwrapped_key = crypto.symmetric_unwrap(base64.decodestring(key_data.wrappedPrivateData),
+    unwrapped_key = crypto.symmetric_unwrap(key_data.wrappedPrivateData,
                                             session_key,
-                                            nonce_iv=base64.decodestring(key_data.nonceData))
+                                            nonce_iv=key_data.nonceData)
     key1 = base64.encodestring(unwrapped_key)
 
     # Test 7: Recover key without providing trans_wrapped_session_key
@@ -208,18 +207,18 @@ def main():
         print "ResourceNotFoundException thrown - Code: " + exc.code + "Message: " + exc.message
 
     #Test 18: Generate a symmetric key with default parameters
-    client_key_id = "Vek #3" + time.strftime('%X %x %Z')
+    client_key_id = "Vek #3" + time.strftime('%c')
     response = keyclient.generate_symmetric_key(client_key_id)
     print_key_request(response.requestInfo)
 
     # Test 19: Try to archive key
     print "try to archive key"
     print "key to archive: " + key1
-    client_key_id = "Vek #4" + time.strftime('%X %x %Z')
+    client_key_id = "Vek #4" + time.strftime('%c')
 
     # this test is not quite working yet
     #response = keyclient.archive_key(client_key_id, keyclient.SYMMETRIC_KEY_TYPE,
-    #                                private_data=base64.decodestring(key1),
+    #                                base64.decodestring(key1),
     #                                key_algorithm=keyclient.AES_ALGORITHM,
     #                                key_size=128)
     #print_key_request(response.requestInfo)
diff --git a/base/kra/functional/src/com/netscape/cms/servlet/test/DRMTest.java b/base/kra/functional/src/com/netscape/cms/servlet/test/DRMTest.java
index 899c78a66..621d95d0b 100644
--- a/base/kra/functional/src/com/netscape/cms/servlet/test/DRMTest.java
+++ b/base/kra/functional/src/com/netscape/cms/servlet/test/DRMTest.java
@@ -257,7 +257,7 @@ public class DRMTest {
 
         // Test 4: Generate and archive a symmetric key
         log("Archiving symmetric key");
-        clientKeyId = "UUID: 123-45-6789 VEK " + Calendar.getInstance().getTime().toString();
+        clientKeyId = "UUID: 123-45-6789 VEK " + Calendar.getInstance().getTime();
         try {
             vek = CryptoUtil.generateKey(token, KeyGenAlgorithm.DES3);
             byte[] encoded = CryptoUtil.createPKIArchiveOptions(manager, token, transportCert, vek, null,
@@ -371,7 +371,7 @@ public class DRMTest {
 
         passphrase = "secret12345";
         // Test 12: Generate and archive a passphrase
-        clientKeyId = "UUID: 123-45-6789 RKEK " + Calendar.getInstance().getTime().toString();
+        clientKeyId = "UUID: 123-45-6789 RKEK " + Calendar.getInstance().getTime();
         try {
             byte[] encoded = CryptoUtil.createPKIArchiveOptions(manager, token, transportCert, null, passphrase,
                     KeyGenAlgorithm.DES3, ivps);
@@ -571,7 +571,7 @@ public class DRMTest {
         }
 
         // test 28: Generate symmetric key
-        clientKeyId = "Symmetric Key #1234f " + Calendar.getInstance().getTime().toString();
+        clientKeyId = "Symmetric Key #1234f " + Calendar.getInstance().getTime();
         List<String> usages = new ArrayList<String>();
         usages.add(SymKeyGenerationRequest.DECRYPT_USAGE);
         usages.add(SymKeyGenerationRequest.ENCRYPT_USAGE);
@@ -652,7 +652,7 @@ public class DRMTest {
 
         // Test 36: Generate and archive a symmetric key of type AES
         log("Archiving symmetric key");
-        clientKeyId = "UUID: 123-45-6789 VEK " + Calendar.getInstance().getTime().toString();
+        clientKeyId = "UUID: 123-45-6789 VEK " + Calendar.getInstance().getTime();
         try {
             KeyGenerator kg = token.getKeyGenerator(KeyGenAlgorithm.AES);
             kg.initialize(128);
diff --git a/base/kra/src/com/netscape/kra/SecurityDataService.java b/base/kra/src/com/netscape/kra/SecurityDataService.java
index 37229f09a..b9620f5d2 100644
--- a/base/kra/src/com/netscape/kra/SecurityDataService.java
+++ b/base/kra/src/com/netscape/kra/SecurityDataService.java
@@ -133,13 +133,6 @@ public class SecurityDataService implements IService {
             byte[] encoded = Utils.base64decode(pkiArchiveOptions);
 
             ArchiveOptions options = ArchiveOptions.toArchiveOptions(encoded);
-
-            //Check here just in case a null ArchiveOptions makes it this far
-            if (options == null) {
-                auditArchivalRequestProcessed(subjectID, ILogger.FAILURE, request.getRequestId(),
-                        clientKeyId, null, "Problem decoding PKIArchiveOptions");
-                throw new EBaseException("Problem decoding PKIArchiveOptions.");
-            }
             algStr = options.getSymmAlgOID();
             wrappedSessionKey = options.getEncSymmKey();
             secdata = options.getEncValue();