Consolidate key interface password handling and general overhaul

securesystemslib/interface.py

@@ -662,7 +662,8 @@ def import_ed25519_privatekey_from_file(filepath, password=None, prompt=False,
 
 
 
-def generate_and_write_ecdsa_keypair(filepath=None, password=None):
+def generate_and_write_ecdsa_keypair(filepath=None, password=None,
+    prompt=False):
   """
   <Purpose>
     Generate an ECDSA keypair, where the encrypted key (using 'password' as the
@@ -701,6 +702,8 @@ def generate_and_write_ecdsa_keypair(filepath=None, password=None):
     The 'filepath' of the written key.
   """
 
+  password = _get_key_file_encryption_password(password, prompt, filepath)
+
   # Generate a new ECDSA key object.  The 'cryptography' library is currently
   # supported and performs the actual cryptographic operations.
   ecdsa_key = securesystemslib.keys.generate_ecdsa_key()
@@ -716,23 +719,6 @@ def generate_and_write_ecdsa_keypair(filepath=None, password=None):
   # Raise 'securesystemslib.exceptions.FormatError' if there is a mismatch.
   securesystemslib.formats.PATH_SCHEMA.check_match(filepath)
 
-  # If the caller does not provide a password argument, prompt for one.
-  if password is None:
-
-    # It is safe to specify the full path of 'filepath' in the prompt and not
-    # worry about leaking sensitive information about the key's location.
-    # However, care should be taken when including the full path in exceptions
-    # and log files.
-    password = get_password('Enter a password for the ECDSA'
-        ' key (' + TERM_RED + filepath + TERM_RESET + '): ',
-        confirm=True)
-
-  else:
-    logger.debug('The password has been specified.  Not prompting for one')
-
-  # Does 'password' have the correct format?
-  securesystemslib.formats.PASSWORD_SCHEMA.check_match(password)
-
   # If the parent directory of filepath does not exist,
   # create it (and all its parent directories, if necessary).
   securesystemslib.util.ensure_parent_dir(filepath)
@@ -759,10 +745,16 @@ def generate_and_write_ecdsa_keypair(filepath=None, password=None):
   # Write the encrypted key string, conformant to
   # 'securesystemslib.formats.ENCRYPTEDKEY_SCHEMA', to '<filepath>'.
   file_object = tempfile.TemporaryFile()
+
+  if password is not None:
+    ecdsa_key = securesystemslib.keys.encrypt_key(ecdsa_key, password)
+
+  else:
+    ecdsa_key = json.dumps(ecdsa_key)
+
   # Raise 'securesystemslib.exceptions.CryptoError' if 'ecdsa_key' cannot be
   # encrypted.
-  encrypted_key = securesystemslib.keys.encrypt_key(ecdsa_key, password)
-  file_object.write(encrypted_key.encode('utf-8'))
+  file_object.write(ecdsa_key.encode('utf-8'))
   securesystemslib.util.persist_temp_file(file_object, filepath)
 
   return filepath
@@ -814,7 +806,7 @@ def import_ecdsa_publickey_from_file(filepath):
 
 
 
-def import_ecdsa_privatekey_from_file(filepath, password=None,
+def import_ecdsa_privatekey_from_file(filepath, password=None, prompt=False,
     storage_backend=None):
   """
   <Purpose>
@@ -858,37 +850,25 @@ def import_ecdsa_privatekey_from_file(filepath, password=None,
   # Raise 'securesystemslib.exceptions.FormatError' if there is a mismatch.
   securesystemslib.formats.PATH_SCHEMA.check_match(filepath)
 
-  # If the caller does not provide a password argument, prompt for one.
-  # Password confirmation disabled here, which should ideally happen only
-  # when creating encrypted key files (i.e., improve usability).
-  if password is None:
-
-    # It is safe to specify the full path of 'filepath' in the prompt and not
-    # worry about leaking sensitive information about the key's location.
-    # However, care should be taken when including the full path in exceptions
-    # and log files.
-    password = get_password('Enter a password for the encrypted ECDSA'
-        ' key (' + TERM_RED + filepath + TERM_RESET + '): ',
-        confirm=False)
-
-  # Does 'password' have the correct format?
-  securesystemslib.formats.PASSWORD_SCHEMA.check_match(password)
+  password = _get_key_file_decryption_password(password, prompt, filepath)
 
   if storage_backend is None:
     storage_backend = securesystemslib.storage.FilesystemBackend()
 
   # Store the encrypted contents of 'filepath' prior to calling the decryption
   # routine.
-  encrypted_key = None
-
   with storage_backend.get(filepath) as file_object:
-    encrypted_key = file_object.read()
+    key_data = file_object.read().decode('utf-8')
 
   # Decrypt the loaded key file, calling the 'cryptography' library to generate
   # the derived encryption key from 'password'.  Raise
   # 'securesystemslib.exceptions.CryptoError' if the decryption fails.
-  key_object = securesystemslib.keys.decrypt_key(encrypted_key.decode('utf-8'),
-      password)
+  if password is not None:
+    key_object = securesystemslib.keys.decrypt_key(key_data, password)
+
+  else:
+    key_object = securesystemslib.util.load_json_string(key_data)
+
 
   # Raise an exception if an unexpected key type is imported.
   # NOTE: we support keytype's of ecdsa-sha2-nistp256 and ecdsa-sha2-nistp384

tests/test_interface.py

@@ -459,19 +459,13 @@ def test_ed25519(self):
 
   def test_ecdsa(self):
     """Test ecdsa key generation and import interface functions. """
-    # NOTE: Unlike rsa and ed25519, the ecdsa (key creation and private key
-    # import) interface only supports encrypted keys, even if the passed or
-    # prompted password is an empty string.
-
-    # TEST: Generate pw encrypted keys and import
+    # TEST: Generate default keys and import
     # Assert location and format
-    pw = "pw"
     fn_default = "default"
-    fn_default_ret = generate_and_write_ecdsa_keypair(
-        filepath=fn_default, password=pw)
+    fn_default_ret = generate_and_write_ecdsa_keypair(filepath=fn_default)
 
     pub = import_ecdsa_publickey_from_file(fn_default + ".pub")
-    priv = import_ecdsa_privatekey_from_file(fn_default, password=pw)
+    priv = import_ecdsa_privatekey_from_file(fn_default)
 
     self.assertEqual(fn_default, fn_default_ret)
     self.assertTrue(ECDSAKEY_SCHEMA.matches(pub))
@@ -480,13 +474,20 @@ def test_ecdsa(self):
     # NOTE: There is no private key schema, at least check it has a value
     self.assertTrue(priv["keyval"]["private"])
 
+    # TEST: Generate unencrypted keys with empty prompt
+    # Assert importable with empty prompt password and without password
+    fn_empty_prompt = "empty_prompt"
+    with mock.patch("securesystemslib.interface.get_password", return_value=""):
+      generate_and_write_ecdsa_keypair(filepath=fn_empty_prompt)
+      import_ecdsa_privatekey_from_file(fn_empty_prompt, prompt=True)
+    import_ecdsa_privatekey_from_file(fn_empty_prompt)
+
 
     # TEST: Generate keys with auto-filename, i.e. keyid
     # Assert filename is keyid
-    fn_keyid = generate_and_write_ecdsa_keypair(password=pw)
+    fn_keyid = generate_and_write_ecdsa_keypair()
     pub = import_ecdsa_publickey_from_file(fn_keyid + ".pub")
-    priv = import_ecdsa_privatekey_from_file(
-        fn_keyid, password=pw)
+    priv = import_ecdsa_privatekey_from_file(fn_keyid)
     self.assertTrue(
         os.path.basename(fn_keyid) == pub["keyid"] == priv["keyid"])
 
@@ -499,11 +500,11 @@ def test_ecdsa(self):
     generate_and_write_ecdsa_keypair(filepath=fn_encrypted, password=pw)
     with mock.patch("securesystemslib.interface.get_password", return_value=pw):
       # ... and a prompted pw.
-      generate_and_write_ecdsa_keypair(filepath=fn_prompt)
+      generate_and_write_ecdsa_keypair(filepath=fn_prompt, prompt=True)
 
       # Assert that both private keys are importable using the prompted pw ...
-      import_ecdsa_privatekey_from_file(fn_prompt)
-      import_ecdsa_privatekey_from_file(fn_encrypted)
+      import_ecdsa_privatekey_from_file(fn_prompt, prompt=True)
+      import_ecdsa_privatekey_from_file(fn_encrypted, prompt=True)
 
     # ... and the passed pw.
     import_ecdsa_privatekey_from_file(fn_prompt, password=pw)
@@ -529,11 +530,26 @@ def test_ecdsa(self):
 
 
     # TEST: Generation errors
+    for idx, (kwargs, err_msg) in enumerate([
+        # Error on empty password
+        ({"password": ""},
+          "encryption password must be 1 or more characters long"),
+        # Error on 'password' and 'prompt=True'
+        ({"password": pw, "prompt": True},
+          "passing 'password' and 'prompt=True' is not allowed")]):
+
+      with self.assertRaises(ValueError, msg="(row {})".format(idx)) as ctx:
+        generate_and_write_ecdsa_keypair(**kwargs)
+
+      self.assertEqual(err_msg, str(ctx.exception),
+          "expected: '{}' got: '{}' (row {})".format(
+          err_msg, ctx.exception, idx))
 
     # Error on bad argument format
     for idx, kwargs in enumerate([
         {"filepath": 123456}, # Not a string
-        {"password": 123456}]): # Not a string
+        {"password": 123456}, # Not a string
+        {"prompt": "not-a-bool"}]):
       with self.assertRaises(FormatError, msg="(row {})".format(idx)):
         generate_and_write_ecdsa_keypair(**kwargs)
 
@@ -542,8 +558,8 @@ def test_ecdsa(self):
 
     # Error on public key import...
     for idx, (fn, err_msg) in enumerate([
-        # Error on invalid custom json key format
-        (fn_default, "Cannot deserialize to a Python object"),
+        # Error on invalid json (custom key format)
+        (fn_encrypted, "Cannot deserialize to a Python object"),
         # Error on invalid custom key format
         (self.path_no_key, "Missing key")]):
       with self.assertRaises(Error, msg="(row {})".format(idx)) as ctx:
@@ -557,14 +573,23 @@ def test_ecdsa(self):
     # Error on private key import...
     for idx, (args, kwargs, err, err_msg) in enumerate([
         # Error on not an ecdsa private key
-        ([self.path_ed25519], {"password": "password"}, FormatError,
-          "Invalid key type loaded"),
-        # Error on encrypted but empty pw passed
-        ([fn_default], {"password": ""}, CryptoError,
+        ([self.path_ed25519], {}, Error,
+          "Cannot deserialize to a Python object"),
+        # Error on not encrypted
+        ([fn_default], {"password": pw}, CryptoError,
+          "Invalid encrypted file."),
+        # Error on encrypted but no pw
+        ([fn_encrypted], {}, Error,
+          "Cannot deserialize to a Python object"),
+        # Error on encrypted but empty pw
+        ([fn_encrypted], {"password": ""}, CryptoError,
+          "Decryption failed."),
+        # Error on encrypted but bad pw passed
+        ([fn_encrypted], {"password": "bad pw"}, CryptoError,
           "Decryption failed."),
-        # Error on encrypted but wrong pw passed
-        ([fn_default], {"password": "bad pw"}, CryptoError,
-          "Decryption failed.")]):
+        # Error on pw and prompt
+        ([fn_default], {"password": pw, "prompt": True}, ValueError,
+          "passing 'password' and 'prompt=True' is not allowed")]):
 
       with self.assertRaises(err, msg="(row {})".format(idx)) as ctx:
         import_ecdsa_privatekey_from_file(*args, **kwargs)
@@ -573,6 +598,16 @@ def test_ecdsa(self):
           "expected: '{}' got: '{}' (row {})".format(
           err_msg, ctx.exception, idx))
 
+    # Error on encrypted but bad pw prompted
+    err_msg = ("Decryption failed")
+    with self.assertRaises(CryptoError) as ctx, mock.patch(
+        "securesystemslib.interface.get_password", return_value="bad_pw"):
+      import_ecdsa_privatekey_from_file(fn_encrypted, prompt=True)
+
+    self.assertTrue(err_msg in str(ctx.exception),
+        "expected: '{}' got: '{}'".format(err_msg, ctx.exception))
+
+
     # Error on bad path format
     with self.assertRaises(FormatError):
       import_ecdsa_publickey_from_file(123456)
@@ -583,6 +618,10 @@ def test_ecdsa(self):
     with self.assertRaises(FormatError): # bad password
       import_ecdsa_privatekey_from_file(fn_default, password=123456)
 
+    # Error on bad prompt format
+    with self.assertRaises(FormatError):
+      import_ecdsa_privatekey_from_file(fn_default, prompt="not-a-bool")
+
 
 
   def test_import_publickeys_from_file(self):