Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ssh v1.0.1 #23

Closed
wants to merge 28 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
e8426ae
Fix filepath with spaces bug. Improve deletion warnings
vthiebaut10 Jan 6, 2022
9323307
update version to 1.0.1
vthiebaut10 Jan 6, 2022
365dc0f
Small change just to create remote branch
vthiebaut10 Jan 21, 2022
d3f5920
Merge pull request #22 from vthiebaut10/fix_space_bug
vthiebaut10 Jan 21, 2022
15a57d9
1.0.1 changes
vthiebaut10 Jan 25, 2022
988faa2
Print error messages from the client log
vthiebaut10 Feb 4, 2022
1967288
Prepend -v to ssh_args if az ssh vm is run in debug mode
vthiebaut10 Feb 4, 2022
091cf57
Refactor extension
vthiebaut10 Feb 14, 2022
238b746
Address style errors
vthiebaut10 Feb 14, 2022
30b42bf
Make sure credentials_folder is absolute
vthiebaut10 Feb 14, 2022
c3c04a1
Properly append config entries
vthiebaut10 Feb 14, 2022
ea82f53
fix unit tests
vthiebaut10 Mar 1, 2022
e709c5c
Fix a few style issues
vthiebaut10 Mar 1, 2022
f933d61
update history
vthiebaut10 Mar 1, 2022
22067bb
check if config folder exists even if credential folder is provided
vthiebaut10 Mar 1, 2022
f022fb4
Merge branch 'Azure:main' into ssh-v1.0.1
vthiebaut10 Mar 1, 2022
a678cce
Add license header to ssh_info.py
vthiebaut10 Mar 1, 2022
25f48cb
fix function that print error messages from the log
vthiebaut10 Mar 1, 2022
38da127
address review comments
vthiebaut10 Mar 1, 2022
126f83e
Add green message at the end of az ssh cert
vthiebaut10 Mar 2, 2022
5fab4c2
Address review comments
vthiebaut10 Mar 2, 2022
963105b
Remove colorama dependency, throw error before writing config file if…
vthiebaut10 Mar 2, 2022
d61c18d
Remove comment
vthiebaut10 Mar 2, 2022
912d2dd
Allow * as ip for config
vthiebaut10 Mar 8, 2022
30792f9
Disable known pylint errors
vthiebaut10 Mar 8, 2022
a904acf
Roll back to version 1.0.0
vthiebaut10 Mar 9, 2022
2919211
Warn users when using incompatible openssh versions
vthiebaut10 Mar 9, 2022
958c81d
Address review comments
vthiebaut10 Mar 9, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/ssh/HISTORY.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
Release History
===============
1.0.1
-----
* Added --ssh-client-folder
* Fixed bug of when there are spaces in the paths
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handle file paths with spaces

* Abs path for config
* Show error messages from the ssh log
* Fix bug with config not being able to write non english characters
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path containing non english characters


1.0.0
-----
* Delete all keys and certificates created during execution of ssh vm.
Expand Down
6 changes: 6 additions & 0 deletions src/ssh/azext_ssh/_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ def load_arguments(self, _):
c.argument('cert_file', options_list=['--certificate-file', '-c'],
help='Path to a certificate file used for authentication when using local user credentials.')
c.argument('port', options_list=['--port'], help='SSH port')
c.argument('ssh_client_folder', options_list=['--ssh-client-folder'],
help='Path to folder that contains ssh executables (ssh.exe, ssh-keygen.exe, etc). '
'Default to ssh pre-installed if not provided or if executables can\'t be found in the provided folder.')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can delete this line. It's too much information giving to customers.

c.positional('ssh_args', nargs='*', help='Additional arguments passed to OpenSSH')

with self.argument_context('ssh config') as c:
Expand All @@ -37,6 +40,9 @@ def load_arguments(self, _):
help='Folder where new generated keys will be stored.')
c.argument('cert_file', options_list=['--certificate-file', '-c'], help='Path to certificate file')
c.argument('port', options_list=['--port'], help='SSH port')
c.argument('ssh_client_folder', options_list=['--ssh-client-folder'],
help='Path to folder that contains ssh executables (ssh.exe, ssh-keygen.exe, etc). '
'Default to ssh pre-installed if not provided or if executables can\'t be found in the provided folder.')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment


with self.argument_context('ssh cert') as c:
c.argument('cert_path', options_list=['--file', '-f'],
Expand Down
2 changes: 1 addition & 1 deletion src/ssh/azext_ssh/azext_metadata.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"azext.isPreview": true,
"azext.isPreview": false,
"azext.minCliCoreVersion": "2.4.0"
}
49 changes: 31 additions & 18 deletions src/ssh/azext_ssh/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,29 @@

def ssh_vm(cmd, resource_group_name=None, vm_name=None, ssh_ip=None, public_key_file=None,
private_key_file=None, use_private_ip=False, local_user=None, cert_file=None, port=None,
ssh_args=None):
ssh_client_folder=None, ssh_args=[]):

if '--debug' in cmd.cli_ctx.data['safe_params'] and set(['-v', '-vv', '-vvv']).isdisjoint(ssh_args):
ssh_args = ['-v'] + ssh_args

_assert_args(resource_group_name, vm_name, ssh_ip, cert_file, local_user)
credentials_folder = None
op_call = functools.partial(ssh_utils.start_ssh_connection, port, ssh_args)
_do_ssh_op(cmd, resource_group_name, vm_name, ssh_ip, public_key_file, private_key_file, use_private_ip,
local_user, cert_file, credentials_folder, op_call)
local_user, cert_file, credentials_folder, ssh_client_folder, op_call)


def ssh_config(cmd, config_path, resource_group_name=None, vm_name=None, ssh_ip=None,
public_key_file=None, private_key_file=None, overwrite=False, use_private_ip=False,
local_user=None, cert_file=None, port=None, credentials_folder=None):
local_user=None, cert_file=None, port=None, credentials_folder=None, ssh_client_folder=None):
_assert_args(resource_group_name, vm_name, ssh_ip, cert_file, local_user)
# If user provides their own key pair, certificate will be written in the same folder as public key.
if (public_key_file or private_key_file) and credentials_folder:
raise azclierror.ArgumentUsageError("--keys-destination-folder can't be used in conjunction with "
"--public-key-file/-p or --private-key-file/-i.")

config_path = os.path.abspath(config_path)

op_call = functools.partial(ssh_utils.write_ssh_config, config_path, resource_group_name, vm_name, overwrite, port)
# Default credential location
if not credentials_folder:
Expand All @@ -51,10 +57,10 @@ def ssh_config(cmd, config_path, resource_group_name=None, vm_name=None, ssh_ip=
credentials_folder = os.path.join(config_folder, os.path.join("az_ssh_config", folder_name))

_do_ssh_op(cmd, resource_group_name, vm_name, ssh_ip, public_key_file, private_key_file, use_private_ip,
local_user, cert_file, credentials_folder, op_call)
local_user, cert_file, credentials_folder, ssh_client_folder, op_call)


def ssh_cert(cmd, cert_path=None, public_key_file=None):
def ssh_cert(cmd, cert_path=None, public_key_file=None, ssh_client_folder=None):
if not cert_path and not public_key_file:
raise azclierror.RequiredArgumentMissingError("--file or --public-key-file must be provided.")
if cert_path and not os.path.isdir(os.path.dirname(cert_path)):
Expand All @@ -63,15 +69,21 @@ def ssh_cert(cmd, cert_path=None, public_key_file=None):
keys_folder = None
if not public_key_file:
keys_folder = os.path.dirname(cert_path)
logger.warning("The generated SSH keys are stored at %s. Please delete SSH keys when the certificate "
"is no longer being used.", keys_folder)
public_key_file, _, _ = _check_or_create_public_private_files(public_key_file, None, keys_folder)
cert_file, _ = _get_and_write_certificate(cmd, public_key_file, cert_path)
print(cert_file + "\n")
print(f"The generated SSH keys are saved in {keys_folder}. Please delete SSH keys when the certificate "
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In terms of color coding does logger.warning and print look same on the terminal?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bagajjal logger.warning() prints in yellow. I changed it to print because I felt that this message was out of scope for warning, and I wanted to be printed no matter what the log level is set to.

"is no longer being used.")

public_key_file, _, _ = _check_or_create_public_private_files(public_key_file, None, keys_folder, ssh_client_folder)
cert_file, _ = _get_and_write_certificate(cmd, public_key_file, cert_path, ssh_client_folder)
try:
cert_expiration = ssh_utils.get_certificate_start_and_end_times(cert_file, ssh_client_folder)[1]
print(f"Generated SSH certificate {cert_file} is valid until {cert_expiration} in local time.")
except Exception as e:
logger.warning("Couldn't determine certificate validity. Error: %s", str(e))
print(cert_file + "\n")


def _do_ssh_op(cmd, resource_group, vm_name, ssh_ip, public_key_file, private_key_file, use_private_ip,
username, cert_file, credentials_folder, op_call):
username, cert_file, credentials_folder, ssh_client_folder, op_call):
# Get ssh_ip before getting public key to avoid getting "ResourceNotFound" exception after creating the keys
ssh_ip = ssh_ip or ip_utils.get_ssh_ip(cmd, resource_group, vm_name, use_private_ip)

Expand All @@ -89,13 +101,14 @@ def _do_ssh_op(cmd, resource_group, vm_name, ssh_ip, public_key_file, private_ke
delete_cert = True
public_key_file, private_key_file, delete_keys = _check_or_create_public_private_files(public_key_file,
private_key_file,
credentials_folder)
cert_file, username = _get_and_write_certificate(cmd, public_key_file, None)
credentials_folder,
ssh_client_folder)
cert_file, username = _get_and_write_certificate(cmd, public_key_file, None, ssh_client_folder)

op_call(ssh_ip, username, cert_file, private_key_file, delete_keys, delete_cert)
op_call(ssh_ip, username, cert_file, private_key_file, ssh_client_folder, delete_keys, delete_cert)


def _get_and_write_certificate(cmd, public_key_file, cert_file):
def _get_and_write_certificate(cmd, public_key_file, cert_file, ssh_client_folder):
cloudtoscope = {
"azurecloud": "https://pas.windows.net/CheckMyAccess/Linux/.default",
"azurechinacloud": "https://pas.chinacloudapi.cn/CheckMyAccess/Linux/.default",
Expand Down Expand Up @@ -126,7 +139,7 @@ def _get_and_write_certificate(cmd, public_key_file, cert_file):
cert_file = public_key_file + "-aadcert.pub"
_write_cert_file(certificate, cert_file)
# instead we use the validprincipals from the cert due to mismatched upn and email in guest scenarios
username = ssh_utils.get_ssh_cert_principals(cert_file)[0]
username = ssh_utils.get_ssh_cert_principals(cert_file, ssh_client_folder)[0]
return cert_file, username.lower()


Expand Down Expand Up @@ -172,7 +185,7 @@ def _assert_args(resource_group, vm_name, ssh_ip, cert_file, username):
raise azclierror.FileOperationError(f"Certificate file {cert_file} not found")


def _check_or_create_public_private_files(public_key_file, private_key_file, credentials_folder):
def _check_or_create_public_private_files(public_key_file, private_key_file, credentials_folder, ssh_client_folder=None):
delete_keys = False
# If nothing is passed, then create a directory with a ephemeral keypair
if not public_key_file and not private_key_file:
Expand All @@ -189,7 +202,7 @@ def _check_or_create_public_private_files(public_key_file, private_key_file, cre
os.makedirs(credentials_folder)
public_key_file = os.path.join(credentials_folder, "id_rsa.pub")
private_key_file = os.path.join(credentials_folder, "id_rsa")
ssh_utils.create_ssh_keyfile(private_key_file)
ssh_utils.create_ssh_keyfile(private_key_file, ssh_client_folder)

if not public_key_file:
if private_key_file:
Expand Down
Loading