Skip to content

Commit

Permalink
Make interface.generate_*_keypair funcs protected
Browse files Browse the repository at this point in the history 
'interface.generate_and_write{rsa,ed25519.ecdsa}_keypair' functions
were recently changed to disable default password prompts and
generally make the private key encryption behavior more explicit.

This change also made it so that a default call of the functions
would leave private keys unencrypted, which is not desirable as per
the principle of 'secure defaults' (see heated discussion in #288).

This commit makes the corresponding functions protected, so that
they may still be used internally (in
securesystemslib/in-toto/tuf).

Corresponding public interface functions that neither offer
unexpected prompts, and encrypt by default will be added in
subsequent commits.
  • Loading branch information
@lukpueh
lukpueh committed Nov 4, 2020
1 parent fae4ebc commit dd2c5fc
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 34 deletions.
6 changes: 3 additions & 3 deletions securesystemslib/interface.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def _get_key_file_decryption_password(password, prompt, path):



def generate_and_write_rsa_keypair(filepath=None, bits=DEFAULT_RSA_KEY_BITS,
def _generate_and_write_rsa_keypair(filepath=None, bits=DEFAULT_RSA_KEY_BITS,
password=None, prompt=False):
"""Generates RSA key pair and writes PEM-encoded keys to disk.
Expand Down Expand Up @@ -341,7 +341,7 @@ def import_rsa_publickey_from_file(filepath, scheme='rsassa-pss-sha256',



def generate_and_write_ed25519_keypair(filepath=None, password=None,
def _generate_and_write_ed25519_keypair(filepath=None, password=None,
prompt=False):
"""Generates ed25519 key pair and writes custom JSON-formatted keys to disk.
Expand Down Expand Up @@ -496,7 +496,7 @@ 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):
"""Generates ecdsa key pair and writes custom JSON-formatted keys to disk.
Expand Down
6 changes: 3 additions & 3 deletions tests/check_public_interfaces.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def test_interface(self):

with self.assertRaises(
securesystemslib.exceptions.UnsupportedLibraryError):
securesystemslib.interface.generate_and_write_rsa_keypair(password='pw')
securesystemslib.interface._generate_and_write_rsa_keypair(password='pw')

with self.assertRaises(
securesystemslib.exceptions.UnsupportedLibraryError):
Expand All @@ -75,7 +75,7 @@ def test_interface(self):

with self.assertRaises(
securesystemslib.exceptions.UnsupportedLibraryError):
securesystemslib.interface.generate_and_write_ed25519_keypair(
securesystemslib.interface._generate_and_write_ed25519_keypair(
password='pw')

with self.assertRaises(
Expand All @@ -88,7 +88,7 @@ def test_interface(self):

with self.assertRaises(
securesystemslib.exceptions.UnsupportedLibraryError):
securesystemslib.interface.generate_and_write_ecdsa_keypair(
securesystemslib.interface._generate_and_write_ecdsa_keypair(
password='pw')

with self.assertRaises(
Expand Down
56 changes: 28 additions & 28 deletions tests/test_interface.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@
from securesystemslib.exceptions import Error, FormatError, CryptoError

from securesystemslib.interface import (
generate_and_write_rsa_keypair,
_generate_and_write_rsa_keypair,
import_rsa_privatekey_from_file,
import_rsa_publickey_from_file,
generate_and_write_ed25519_keypair,
_generate_and_write_ed25519_keypair,
import_ed25519_publickey_from_file,
import_ed25519_privatekey_from_file,
generate_and_write_ecdsa_keypair,
_generate_and_write_ecdsa_keypair,
import_ecdsa_publickey_from_file,
import_ecdsa_privatekey_from_file,
import_publickeys_from_file,
Expand Down Expand Up @@ -101,12 +101,12 @@ def tearDown(self):


def test_rsa(self):
"""Test RSA key generation and import interface functions. """
"""Test RSA key _generation and import interface functions. """

# TEST: Generate default keys and import
# Assert location and format
fn_default = "default"
fn_default_ret = generate_and_write_rsa_keypair(filepath=fn_default)
fn_default_ret = _generate_and_write_rsa_keypair(filepath=fn_default)

pub = import_rsa_publickey_from_file(fn_default + ".pub")
priv = import_rsa_privatekey_from_file(fn_default)
Expand All @@ -123,13 +123,13 @@ def test_rsa(self):
# Assert importable without password
fn_empty_prompt = "empty_prompt"
with mock.patch("securesystemslib.interface.get_password", return_value=""):
generate_and_write_rsa_keypair(filepath=fn_empty_prompt, prompt=True)
_generate_and_write_rsa_keypair(filepath=fn_empty_prompt, prompt=True)
import_rsa_privatekey_from_file(fn_empty_prompt)


# TEST: Generate keys with auto-filename, i.e. keyid
# Assert filename is keyid
fn_keyid = generate_and_write_rsa_keypair()
fn_keyid = _generate_and_write_rsa_keypair()
pub = import_rsa_publickey_from_file(fn_keyid + ".pub")
priv = import_rsa_privatekey_from_file(fn_keyid)
self.assertTrue(
Expand All @@ -140,7 +140,7 @@ def test_rsa(self):
# Assert length
bits = 4096
fn_bits = "bits"
generate_and_write_rsa_keypair(filepath=fn_bits, bits=bits)
_generate_and_write_rsa_keypair(filepath=fn_bits, bits=bits)

priv = import_rsa_privatekey_from_file(fn_bits)
# NOTE: Parse PEM with pyca/cryptography to get the key size property
Expand All @@ -158,10 +158,10 @@ def test_rsa(self):
fn_prompt = "prompt"

# ... a passed pw ...
generate_and_write_rsa_keypair(filepath=fn_encrypted, password=pw)
_generate_and_write_rsa_keypair(filepath=fn_encrypted, password=pw)
with mock.patch("securesystemslib.interface.get_password", return_value=pw):
# ... and a prompted pw.
generate_and_write_rsa_keypair(filepath=fn_prompt, prompt=True)
_generate_and_write_rsa_keypair(filepath=fn_prompt, prompt=True)

# Assert that both private keys are importable using the prompted pw ...
import_rsa_privatekey_from_file(fn_prompt, prompt=True)
Expand Down Expand Up @@ -194,7 +194,7 @@ def test_rsa(self):
"passing 'password' and 'prompt=True' is not allowed")]):

with self.assertRaises(ValueError, msg="(row {})".format(idx)) as ctx:
generate_and_write_rsa_keypair(**kwargs)
_generate_and_write_rsa_keypair(**kwargs)

self.assertEqual(err_msg, str(ctx.exception),
"expected: '{}' got: '{}' (row {})".format(
Expand All @@ -208,7 +208,7 @@ def test_rsa(self):
{"password": 123456}, # Not a string
{"prompt": "not-a-bool"}]):
with self.assertRaises(FormatError, msg="(row {})".format(idx)):
generate_and_write_rsa_keypair(**kwargs)
_generate_and_write_rsa_keypair(**kwargs)


# TEST: Import errors
Expand Down Expand Up @@ -280,12 +280,12 @@ def test_rsa(self):


def test_ed25519(self):
"""Test ed25519 key generation and import interface functions. """
"""Test ed25519 key _generation and import interface functions. """

# TEST: Generate default keys and import
# Assert location and format
fn_default = "default"
fn_default_ret = generate_and_write_ed25519_keypair(filepath=fn_default)
fn_default_ret = _generate_and_write_ed25519_keypair(filepath=fn_default)

pub = import_ed25519_publickey_from_file(fn_default + ".pub")
priv = import_ed25519_privatekey_from_file(fn_default)
Expand All @@ -302,14 +302,14 @@ def test_ed25519(self):
# 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_ed25519_keypair(filepath=fn_empty_prompt)
_generate_and_write_ed25519_keypair(filepath=fn_empty_prompt)
import_ed25519_privatekey_from_file(fn_empty_prompt, prompt=True)
import_ed25519_privatekey_from_file(fn_empty_prompt)


# TEST: Generate keys with auto-filename, i.e. keyid
# Assert filename is keyid
fn_keyid = generate_and_write_ed25519_keypair()
fn_keyid = _generate_and_write_ed25519_keypair()
pub = import_ed25519_publickey_from_file(fn_keyid + ".pub")
priv = import_ed25519_privatekey_from_file(fn_keyid)
self.assertTrue(
Expand All @@ -321,10 +321,10 @@ def test_ed25519(self):
fn_encrypted = "encrypted"
fn_prompt = "prompt"
# ... a passed pw ...
generate_and_write_ed25519_keypair(filepath=fn_encrypted, password=pw)
_generate_and_write_ed25519_keypair(filepath=fn_encrypted, password=pw)
with mock.patch("securesystemslib.interface.get_password", return_value=pw):
# ... and a prompted pw.
generate_and_write_ed25519_keypair(filepath=fn_prompt, prompt=True)
_generate_and_write_ed25519_keypair(filepath=fn_prompt, prompt=True)

# Assert that both private keys are importable using the prompted pw ...
import_ed25519_privatekey_from_file(fn_prompt, prompt=True)
Expand Down Expand Up @@ -370,7 +370,7 @@ def test_ed25519(self):
"passing 'password' and 'prompt=True' is not allowed")]):

with self.assertRaises(ValueError, msg="(row {})".format(idx)) as ctx:
generate_and_write_ed25519_keypair(**kwargs)
_generate_and_write_ed25519_keypair(**kwargs)

self.assertEqual(err_msg, str(ctx.exception),
"expected: '{}' got: '{}' (row {})".format(
Expand All @@ -382,7 +382,7 @@ def test_ed25519(self):
{"password": 123456}, # Not a string
{"prompt": "not-a-bool"}]):
with self.assertRaises(FormatError, msg="(row {})".format(idx)):
generate_and_write_ed25519_keypair(**kwargs)
_generate_and_write_ed25519_keypair(**kwargs)


# TEST: Import errors
Expand Down Expand Up @@ -459,11 +459,11 @@ def test_ed25519(self):


def test_ecdsa(self):
"""Test ecdsa key generation and import interface functions. """
"""Test ecdsa key _generation and import interface functions. """
# TEST: Generate default keys and import
# Assert location and format
fn_default = "default"
fn_default_ret = generate_and_write_ecdsa_keypair(filepath=fn_default)
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)
Expand All @@ -479,14 +479,14 @@ def test_ecdsa(self):
# 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)
_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()
fn_keyid = _generate_and_write_ecdsa_keypair()
pub = import_ecdsa_publickey_from_file(fn_keyid + ".pub")
priv = import_ecdsa_privatekey_from_file(fn_keyid)
self.assertTrue(
Expand All @@ -498,10 +498,10 @@ def test_ecdsa(self):
fn_encrypted = "encrypted"
fn_prompt = "prompt"
# ... a passed pw ...
generate_and_write_ecdsa_keypair(filepath=fn_encrypted, password=pw)
_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, prompt=True)
_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, prompt=True)
Expand Down Expand Up @@ -540,7 +540,7 @@ def test_ecdsa(self):
"passing 'password' and 'prompt=True' is not allowed")]):

with self.assertRaises(ValueError, msg="(row {})".format(idx)) as ctx:
generate_and_write_ecdsa_keypair(**kwargs)
_generate_and_write_ecdsa_keypair(**kwargs)

self.assertEqual(err_msg, str(ctx.exception),
"expected: '{}' got: '{}' (row {})".format(
Expand All @@ -552,7 +552,7 @@ def test_ecdsa(self):
{"password": 123456}, # Not a string
{"prompt": "not-a-bool"}]):
with self.assertRaises(FormatError, msg="(row {})".format(idx)):
generate_and_write_ecdsa_keypair(**kwargs)
_generate_and_write_ecdsa_keypair(**kwargs)


# TEST: Import errors
Expand Down

0 comments on commit dd2c5fc

Please sign in to comment.