diff --git a/openstackclient/compute/v2/keypair.py b/openstackclient/compute/v2/keypair.py index 7dabf78d9c..3a5513ef3a 100644 --- a/openstackclient/compute/v2/keypair.py +++ b/openstackclient/compute/v2/keypair.py @@ -15,11 +15,13 @@ """Keypair action implementations""" +import collections import io import logging import os -import sys +from cryptography.hazmat.primitives.asymmetric import ed25519 +from cryptography.hazmat.primitives import serialization from openstack import utils as sdk_utils from osc_lib.command import command from osc_lib import exceptions @@ -30,6 +32,27 @@ from openstackclient.identity import common as identity_common LOG = logging.getLogger(__name__) +Keypair = collections.namedtuple('Keypair', 'private_key public_key') + + +def _generate_keypair(): + """Generate a Ed25519 keypair in OpenSSH format. + + :returns: A `Keypair` named tuple with the generated private and public + keys. + """ + key = ed25519.Ed25519PrivateKey.generate() + private_key = key.private_bytes( + serialization.Encoding.PEM, + serialization.PrivateFormat.OpenSSH, + serialization.NoEncryption() + ).decode() + public_key = key.public_key().public_bytes( + serialization.Encoding.OpenSSH, + serialization.PublicFormat.OpenSSH + ).decode() + + return Keypair(private_key, public_key) def _get_keypair_columns(item, hide_pub_key=False, hide_priv_key=False): @@ -59,30 +82,37 @@ class CreateKeypair(command.ShowOne): key_group.add_argument( '--public-key', metavar='', - help=_("Filename for public key to add. If not used, " - "creates a private key.") + help=_( + "Filename for public key to add. " + "If not used, generates a private key in ssh-ed25519 format. " + "To generate keys in other formats, including the legacy " + "ssh-rsa format, you must use an external tool such as " + "ssh-keygen and specify this argument." + ), ) key_group.add_argument( '--private-key', metavar='', - help=_("Filename for private key to save. If not used, " - "print private key in console.") + help=_( + "Filename for private key to save. " + "If not used, print private key in console." + ) ) parser.add_argument( '--type', metavar='', choices=['ssh', 'x509'], help=_( - "Keypair type. Can be ssh or x509. " - "(Supported by API versions '2.2' - '2.latest')" + 'Keypair type ' + '(supported by --os-compute-api-version 2.2 or above)' ), ) parser.add_argument( '--user', metavar='', help=_( - 'The owner of the keypair. (admin only) (name or ID). ' - 'Requires ``--os-compute-api-version`` 2.10 or greater.' + 'The owner of the keypair (admin only) (name or ID) ' + '(supported by --os-compute-api-version 2.10 or above)' ), ) identity_common.add_user_domain_option_to_parser(parser) @@ -96,19 +126,43 @@ class CreateKeypair(command.ShowOne): 'name': parsed_args.name } - public_key = parsed_args.public_key - if public_key: + if parsed_args.public_key: + generated_keypair = None try: with io.open(os.path.expanduser(parsed_args.public_key)) as p: public_key = p.read() except IOError as e: msg = _("Key file %(public_key)s not found: %(exception)s") raise exceptions.CommandError( - msg % {"public_key": parsed_args.public_key, - "exception": e} + msg % { + "public_key": parsed_args.public_key, + "exception": e, + } ) kwargs['public_key'] = public_key + else: + generated_keypair = _generate_keypair() + kwargs['public_key'] = generated_keypair.public_key + + # If user have us a file, save private key into specified file + if parsed_args.private_key: + try: + with io.open( + os.path.expanduser(parsed_args.private_key), 'w+' + ) as p: + p.write(generated_keypair.private_key) + except IOError as e: + msg = _( + "Key file %(private_key)s can not be saved: " + "%(exception)s" + ) + raise exceptions.CommandError( + msg % { + "private_key": parsed_args.private_key, + "exception": e, + } + ) if parsed_args.type: if not sdk_utils.supports_microversion(compute_client, '2.2'): @@ -136,32 +190,17 @@ class CreateKeypair(command.ShowOne): keypair = compute_client.create_keypair(**kwargs) - private_key = parsed_args.private_key - # Save private key into specified file - if private_key: - try: - with io.open( - os.path.expanduser(parsed_args.private_key), 'w+' - ) as p: - p.write(keypair.private_key) - except IOError as e: - msg = _("Key file %(private_key)s can not be saved: " - "%(exception)s") - raise exceptions.CommandError( - msg % {"private_key": parsed_args.private_key, - "exception": e} - ) # NOTE(dtroyer): how do we want to handle the display of the private # key when it needs to be communicated back to the user # For now, duplicate nova keypair-add command output - if public_key or private_key: + if parsed_args.public_key or parsed_args.private_key: display_columns, columns = _get_keypair_columns( keypair, hide_pub_key=True, hide_priv_key=True) data = utils.get_item_properties(keypair, columns) return (display_columns, data) else: - sys.stdout.write(keypair.private_key) + self.app.stdout.write(generated_keypair.private_key) return ({}, {}) @@ -405,5 +444,5 @@ class ShowKeypair(command.ShowOne): data = utils.get_item_properties(keypair, columns) return (display_columns, data) else: - sys.stdout.write(keypair.public_key) + self.app.stdout.write(keypair.public_key) return ({}, {}) diff --git a/openstackclient/tests/functional/compute/v2/test_keypair.py b/openstackclient/tests/functional/compute/v2/test_keypair.py index 828d5dad18..e1d1297734 100644 --- a/openstackclient/tests/functional/compute/v2/test_keypair.py +++ b/openstackclient/tests/functional/compute/v2/test_keypair.py @@ -117,24 +117,28 @@ class KeypairTests(KeypairBase): self.assertIsNotNone(cmd_output.get('user_id')) self.assertIsNotNone(cmd_output.get('fingerprint')) pk_content = f.read() - self.assertInOutput('-----BEGIN RSA PRIVATE KEY-----', pk_content) + self.assertInOutput( + '-----BEGIN OPENSSH PRIVATE KEY-----', pk_content, + ) self.assertRegex(pk_content, "[0-9A-Za-z+/]+[=]{0,3}\n") - self.assertInOutput('-----END RSA PRIVATE KEY-----', pk_content) + self.assertInOutput( + '-----END OPENSSH PRIVATE KEY-----', pk_content, + ) def test_keypair_create(self): """Test keypair create command. Test steps: 1) Create keypair in setUp - 2) Check RSA private key in output + 2) Check Ed25519 private key in output 3) Check for new keypair in keypairs list """ NewName = data_utils.rand_name('TestKeyPairCreated') raw_output = self.openstack('keypair create ' + NewName) self.addCleanup(self.openstack, 'keypair delete ' + NewName) - self.assertInOutput('-----BEGIN RSA PRIVATE KEY-----', raw_output) + self.assertInOutput('-----BEGIN OPENSSH PRIVATE KEY-----', raw_output) self.assertRegex(raw_output, "[0-9A-Za-z+/]+[=]{0,3}\n") - self.assertInOutput('-----END RSA PRIVATE KEY-----', raw_output) + self.assertInOutput('-----END OPENSSH PRIVATE KEY-----', raw_output) self.assertIn(NewName, self.keypair_list()) def test_keypair_delete_not_existing(self): diff --git a/openstackclient/tests/unit/compute/v2/fakes.py b/openstackclient/tests/unit/compute/v2/fakes.py index 08d4a5749b..356cc29c21 100644 --- a/openstackclient/tests/unit/compute/v2/fakes.py +++ b/openstackclient/tests/unit/compute/v2/fakes.py @@ -793,7 +793,7 @@ class FakeKeypair(object): """Fake one or more keypairs.""" @staticmethod - def create_one_keypair(attrs=None, no_pri=False): + def create_one_keypair(attrs=None): """Create a fake keypair :param dict attrs: @@ -811,8 +811,6 @@ class FakeKeypair(object): 'public_key': 'dummy', 'user_id': 'user' } - if not no_pri: - keypair_info['private_key'] = 'private_key' # Overwrite default attributes. keypair_info.update(attrs) diff --git a/openstackclient/tests/unit/compute/v2/test_keypair.py b/openstackclient/tests/unit/compute/v2/test_keypair.py index 65d9396aee..1c2923b20c 100644 --- a/openstackclient/tests/unit/compute/v2/test_keypair.py +++ b/openstackclient/tests/unit/compute/v2/test_keypair.py @@ -54,10 +54,10 @@ class TestKeypair(compute_fakes.TestComputev2): class TestKeypairCreate(TestKeypair): - keypair = compute_fakes.FakeKeypair.create_one_keypair() - def setUp(self): - super(TestKeypairCreate, self).setUp() + super().setUp() + + self.keypair = compute_fakes.FakeKeypair.create_one_keypair() self.columns = ( 'fingerprint', @@ -77,8 +77,11 @@ class TestKeypairCreate(TestKeypair): self.sdk_client.create_keypair.return_value = self.keypair - def test_key_pair_create_no_options(self): - + @mock.patch.object( + keypair, '_generate_keypair', + return_value=keypair.Keypair('private', 'public'), + ) + def test_key_pair_create_no_options(self, mock_generate): arglist = [ self.keypair.name, ] @@ -90,18 +93,14 @@ class TestKeypairCreate(TestKeypair): columns, data = self.cmd.take_action(parsed_args) self.sdk_client.create_keypair.assert_called_with( - name=self.keypair.name + name=self.keypair.name, + public_key=mock_generate.return_value.public_key, ) self.assertEqual({}, columns) self.assertEqual({}, data) def test_keypair_create_public_key(self): - # overwrite the setup one because we want to omit private_key - self.keypair = compute_fakes.FakeKeypair.create_one_keypair( - no_pri=True) - self.sdk_client.create_keypair.return_value = self.keypair - self.data = ( self.keypair.fingerprint, self.keypair.name, @@ -135,7 +134,11 @@ class TestKeypairCreate(TestKeypair): self.assertEqual(self.columns, columns) self.assertEqual(self.data, data) - def test_keypair_create_private_key(self): + @mock.patch.object( + keypair, '_generate_keypair', + return_value=keypair.Keypair('private', 'public'), + ) + def test_keypair_create_private_key(self, mock_generate): tmp_pk_file = '/tmp/kp-file-' + uuid.uuid4().hex arglist = [ '--private-key', tmp_pk_file, @@ -156,10 +159,13 @@ class TestKeypairCreate(TestKeypair): self.sdk_client.create_keypair.assert_called_with( name=self.keypair.name, + public_key=mock_generate.return_value.public_key, ) mock_open.assert_called_once_with(tmp_pk_file, 'w+') - m_file.write.assert_called_once_with(self.keypair.private_key) + m_file.write.assert_called_once_with( + mock_generate.return_value.private_key, + ) self.assertEqual(self.columns, columns) self.assertEqual(self.data, data) @@ -167,8 +173,6 @@ class TestKeypairCreate(TestKeypair): @mock.patch.object(sdk_utils, 'supports_microversion', return_value=True) def test_keypair_create_with_key_type(self, sm_mock): for key_type in ['x509', 'ssh']: - self.keypair = compute_fakes.FakeKeypair.create_one_keypair( - no_pri=True) self.sdk_client.create_keypair.return_value = self.keypair self.data = ( @@ -233,8 +237,12 @@ class TestKeypairCreate(TestKeypair): '--os-compute-api-version 2.2 or greater is required', str(ex)) + @mock.patch.object( + keypair, '_generate_keypair', + return_value=keypair.Keypair('private', 'public'), + ) @mock.patch.object(sdk_utils, 'supports_microversion', return_value=True) - def test_key_pair_create_with_user(self, sm_mock): + def test_key_pair_create_with_user(self, sm_mock, mock_generate): arglist = [ '--user', identity_fakes.user_name, self.keypair.name, @@ -250,6 +258,7 @@ class TestKeypairCreate(TestKeypair): self.sdk_client.create_keypair.assert_called_with( name=self.keypair.name, user_id=identity_fakes.user_id, + public_key=mock_generate.return_value.public_key, ) self.assertEqual({}, columns) @@ -673,9 +682,6 @@ class TestKeypairShow(TestKeypair): self.cmd, arglist, verifylist) def test_keypair_show(self): - # overwrite the setup one because we want to omit private_key - self.keypair = compute_fakes.FakeKeypair.create_one_keypair( - no_pri=True) self.sdk_client.find_keypair.return_value = self.keypair self.data = ( @@ -704,7 +710,6 @@ class TestKeypairShow(TestKeypair): self.assertEqual(self.data, data) def test_keypair_show_public(self): - arglist = [ '--public-key', self.keypair.name @@ -723,10 +728,6 @@ class TestKeypairShow(TestKeypair): @mock.patch.object(sdk_utils, 'supports_microversion', return_value=True) def test_keypair_show_with_user(self, sm_mock): - - # overwrite the setup one because we want to omit private_key - self.keypair = compute_fakes.FakeKeypair.create_one_keypair( - no_pri=True) self.sdk_client.find_keypair.return_value = self.keypair self.data = ( diff --git a/releasenotes/notes/keypair-create-client-side-generation-73d8dd36192f70c9.yaml b/releasenotes/notes/keypair-create-client-side-generation-73d8dd36192f70c9.yaml new file mode 100644 index 0000000000..bf5fd5b759 --- /dev/null +++ b/releasenotes/notes/keypair-create-client-side-generation-73d8dd36192f70c9.yaml @@ -0,0 +1,11 @@ +--- +features: + - | + The ``openstack keypair create`` command will now generate keypairs on the + client side in ssh-ed25519 format. The Compute service no longer supports + server-side key generation starting with ``--os-compute-api-version 2.92`` + while the use of ssh-ed25519 is necessary as support for ssh-rsa has been + disabled by default starting in OpenSSH 8.8, which prevents its use in + guests using this version of OpenSSH in the default configuration. + ssh-ed25519 support is widespread and is supported by OpenSSH 6.5 or later + and Dropbear 2020.79 or later. diff --git a/requirements.txt b/requirements.txt index 1ae8cec422..458fb41195 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,6 +4,7 @@ pbr!=2.1.0,>=2.0.0 # Apache-2.0 +cryptography>=2.7 # BSD/Apache-2.0 cliff>=3.5.0 # Apache-2.0 iso8601>=0.1.11 # MIT openstacksdk>=0.103.0 # Apache-2.0