From eb7c0223ae7d7cf9e26c25c655d4bfc3ad9beaf5 Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Fri, 10 Dec 2021 13:30:54 -0500 Subject: [PATCH 01/18] Make --local-user mandatory for windows --- src/ssh/azext_ssh/custom.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/ssh/azext_ssh/custom.py b/src/ssh/azext_ssh/custom.py index 8ae5feb3e4f..f3e3f778dd1 100644 --- a/src/ssh/azext_ssh/custom.py +++ b/src/ssh/azext_ssh/custom.py @@ -216,6 +216,13 @@ def _assert_args(resource_group, vm_name, ssh_ip, resource_type, cert_file, user if cert_file and not os.path.isfile(cert_file): raise azclierror.FileOperationError(f"Certificate file {cert_file} not found") + if not username: + import platform + operating_system = platform.system() + if operating_system.lower() == 'windows': + raise azclierror.RequiredArgumentMissingError("SSH to AAD user is not currently supported on Windows." + "Please provide --local-user") + def _check_or_create_public_private_files(public_key_file, private_key_file, credentials_folder): delete_keys = False From a3b55a58c80c1b97d55e9b6d33208bc8f2fddcc3 Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Fri, 10 Dec 2021 15:50:09 -0500 Subject: [PATCH 02/18] local-user mandatory for windows, check resources --- src/ssh/azext_ssh/custom.py | 96 +++++++++++++++++++++++++++---------- 1 file changed, 70 insertions(+), 26 deletions(-) diff --git a/src/ssh/azext_ssh/custom.py b/src/ssh/azext_ssh/custom.py index f3e3f778dd1..9bbf0456587 100644 --- a/src/ssh/azext_ssh/custom.py +++ b/src/ssh/azext_ssh/custom.py @@ -17,6 +17,7 @@ from knack import log from azure.cli.core import azclierror from azure.cli.core import telemetry +from azure.core.exceptions import ResourceNotFoundError, HttpResponseError from . import ip_utils from . import rsa_parser @@ -37,7 +38,7 @@ def ssh_vm(cmd, resource_group_name=None, vm_name=None, ssh_ip=None, public_key_ _assert_args(resource_group_name, vm_name, ssh_ip, resource_type, cert_file, local_user) credentials_folder = None do_ssh_op = _decide_op_call(cmd, resource_group_name, vm_name, ssh_ip, resource_type, None, None, - ssh_client_path, ssh_args, delete_credentials, credentials_folder) + ssh_client_path, ssh_args, delete_credentials, credentials_folder, local_user) do_ssh_op(cmd, vm_name, resource_group_name, ssh_ip, public_key_file, private_key_file, local_user, cert_file, port, use_private_ip, credentials_folder) @@ -51,6 +52,10 @@ def ssh_config(cmd, config_path, resource_group_name=None, vm_name=None, ssh_ip= "--public-key-file/-p or --private-key-file/-i.") _assert_args(resource_group_name, vm_name, ssh_ip, resource_type, cert_file, local_user) + config_path = os.path.abspath(config_path) + + print(config_path) + # Default credential location if not credentials_folder: config_folder = os.path.dirname(config_path) @@ -64,7 +69,7 @@ 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 = _decide_op_call(cmd, resource_group_name, vm_name, ssh_ip, resource_type, config_path, overwrite, - None, None, False, credentials_folder) + None, None, False, credentials_folder, local_user) do_ssh_op(cmd, vm_name, resource_group_name, ssh_ip, public_key_file, private_key_file, local_user, cert_file, port, use_private_ip, credentials_folder) @@ -95,6 +100,21 @@ def ssh_arc(cmd, resource_group_name=None, vm_name=None, public_key_file=None, p credentials_folder = None + arc, arc_error, is_arc_server = _check_if_arc_server(cmd, resource_group_name, vm_name) + if not is_arc_server: + if isinstance(arc_error, ResourceNotFoundError): + raise azclierror.ResourceNotFoundError(f"The resource {vm_name} in the resource group " + f"{resource_group_name} was not found. Error:\n" + f"{str(arc_error)}") + raise azclierror.BadRequestError("Unable to determine that the target machine is an Arc Server. " + f"Error:\n{str(arc_error)}") + if arc and arc.properties and arc.properties and arc.properties.os_name: + os_type = arc.properties.os_name + # Note: This is a temporary check while AAD login is not enabled for Windows. + if os_type.lower() == 'windows' and not local_user: + raise azclierror.RequiredArgumentMissingError("SSH Login to AAD user is not currently supported for Windows. " + "Please provide --local-user.") + op_call = functools.partial(ssh_utils.start_ssh_connection, ssh_client_path=ssh_client_path, ssh_args=ssh_args, delete_credentials=delete_credentials) _do_ssh_op(cmd, vm_name, resource_group_name, None, public_key_file, private_key_file, local_user, cert_file, port, @@ -216,13 +236,6 @@ def _assert_args(resource_group, vm_name, ssh_ip, resource_type, cert_file, user if cert_file and not os.path.isfile(cert_file): raise azclierror.FileOperationError(f"Certificate file {cert_file} not found") - if not username: - import platform - operating_system = platform.system() - if operating_system.lower() == 'windows': - raise azclierror.RequiredArgumentMissingError("SSH to AAD user is not currently supported on Windows." - "Please provide --local-user") - def _check_or_create_public_private_files(public_key_file, private_key_file, credentials_folder): delete_keys = False @@ -398,35 +411,66 @@ def _arc_list_access_details(cmd, resource_group, vm_name): def _decide_op_call(cmd, resource_group_name, vm_name, ssh_ip, resource_type, config_path, overwrite, - ssh_client_path, ssh_args, delete_credentials, credentials_folder): + ssh_client_path, ssh_args, delete_credentials, credentials_folder, local_user): # If the user provides an IP address the target will be treated as an Azure VM even if it is an # Arc Server. Which just means that the Connectivity Proxy won't be used to establish connection. is_arc_server = False + is_azure_vm = False if ssh_ip: - is_arc_server = False + is_azure_vm = True + vm = None elif resource_type: if resource_type == "Microsoft.HybridCompute": - is_arc_server = True + arc, arc_error, is_arc_server = _check_if_arc_server(cmd, resource_group_name, vm_name) + if not is_arc_server: + if isinstance(arc_error, ResourceNotFoundError): + raise azclierror.ResourceNotFoundError(f"The resource {vm_name} in the resource group " + f"{resource_group_name} was not found. Error:\n" + f"{str(arc_error)}") + raise azclierror.BadRequestError("Unable to determine that the target machine is an Arc Server. " + f"Error:\n{str(arc_error)}") + + elif resource_type == "Microsoft.Compute": + vm, vm_error, is_azure_vm = _check_if_azure_vm(cmd, resource_group_name, vm_name) + if not is_azure_vm: + if isinstance(vm_error, ResourceNotFoundError): + raise azclierror.ResourceNotFoundError(f"The resource {vm_name} in the resource group " + f"{resource_group_name} was not found. Error:\n" + f"{str(vm_error)}") + raise azclierror.BadRequestError("Unable to determine that the target machine is an Azure VM. " + f"Error:\n{str(vm_error)}") else: - vm_error, is_azure_vm = _check_if_azure_vm(cmd, resource_group_name, vm_name) - arc_error, is_arc_server = _check_if_arc_server(cmd, resource_group_name, vm_name) + vm, vm_error, is_azure_vm = _check_if_azure_vm(cmd, resource_group_name, vm_name) + arc, arc_error, is_arc_server = _check_if_arc_server(cmd, resource_group_name, vm_name) if is_azure_vm and is_arc_server: raise azclierror.BadRequestError(f"{resource_group_name} has Azure VM and Arc Server with the " f"same name: {vm_name}. Please provide a --resource-type.") if not is_azure_vm and not is_arc_server: - from azure.core.exceptions import ResourceNotFoundError if isinstance(arc_error, ResourceNotFoundError) and isinstance(vm_error, ResourceNotFoundError): raise azclierror.ResourceNotFoundError(f"The resource {vm_name} in the resource group " - f"{resource_group_name} was not found. Erros:\n" + f"{resource_group_name} was not found. Errors:\n" f"{str(arc_error)}\n{str(vm_error)}") raise azclierror.BadRequestError("Unable to determine the target machine type as Azure VM or " f"Arc Server. Errors:\n{str(arc_error)}\n{str(vm_error)}") + # Note: We are not able to determine the os of the target if the user only provides an IP address. + os_type = None + if is_azure_vm and vm and vm.storage_profile and vm.storage_profile.os_disk and vm.storage_profile.os_disk.os_type: + os_type = vm.storage_profile.os_disk.os_type + + if is_arc_server and arc and arc.properties and arc.properties and arc.properties.os_name: + os_type = arc.properties.os_name + + # Note 2: This is a temporary check while AAD login is not enabled for Windows. + if os_type.lower() == 'windows' and not local_user: + raise azclierror.RequiredArgumentMissingError("SSH Login to AAD user is not currently supported for Windows. " + "Please provide --local-user.") + if config_path: op_call = functools.partial(ssh_utils.write_ssh_config, config_path=config_path, overwrite=overwrite, resource_group=resource_group_name, credentials_folder=credentials_folder) @@ -440,27 +484,27 @@ def _decide_op_call(cmd, resource_group_name, vm_name, ssh_ip, resource_type, co def _check_if_azure_vm(cmd, resource_group_name, vm_name): from azure.cli.core.commands import client_factory from azure.cli.core import profiles - from azure.core.exceptions import ResourceNotFoundError, HttpResponseError try: compute_client = client_factory.get_mgmt_service_client(cmd.cli_ctx, profiles.ResourceType.MGMT_COMPUTE) - compute_client.virtual_machines.get(resource_group_name, vm_name) + vm = compute_client.virtual_machines.get(resource_group_name, vm_name) except ResourceNotFoundError as e: - return e, False + return None, e, False # If user is not authorized to get the VM, it will throw a HttpResponseError except HttpResponseError as e: - return e, False - return None, True + return None, e, False + + return vm, None, True def _check_if_arc_server(cmd, resource_group_name, vm_name): - from azure.core.exceptions import ResourceNotFoundError, HttpResponseError from azext_ssh._client_factory import cf_machine client = cf_machine(cmd.cli_ctx) try: - client.get(resource_group_name=resource_group_name, machine_name=vm_name) + arc = client.get(resource_group_name=resource_group_name, machine_name=vm_name) except ResourceNotFoundError as e: - return e, False + return None, e, False # If user is not authorized to get the arc server, it will throw a HttpResponseError except HttpResponseError as e: - return e, False - return None, True + return None, e, False + + return arc, None, True From ba9d01a707b9b7a65a15f34fdac5b8f3f02aa203 Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Fri, 10 Dec 2021 16:11:56 -0500 Subject: [PATCH 03/18] az ssh config always stores relay on /az_ssh_config/myrg-myvm --- src/ssh/azext_ssh/custom.py | 2 -- src/ssh/azext_ssh/ssh_utils.py | 21 ++++++++------------- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/ssh/azext_ssh/custom.py b/src/ssh/azext_ssh/custom.py index 9bbf0456587..9f4b3fd061a 100644 --- a/src/ssh/azext_ssh/custom.py +++ b/src/ssh/azext_ssh/custom.py @@ -54,8 +54,6 @@ def ssh_config(cmd, config_path, resource_group_name=None, vm_name=None, ssh_ip= config_path = os.path.abspath(config_path) - print(config_path) - # Default credential location if not credentials_folder: config_folder = os.path.dirname(config_path) diff --git a/src/ssh/azext_ssh/ssh_utils.py b/src/ssh/azext_ssh/ssh_utils.py index c89bb1aeac5..4d08e2bfa39 100644 --- a/src/ssh/azext_ssh/ssh_utils.py +++ b/src/ssh/azext_ssh/ssh_utils.py @@ -124,8 +124,7 @@ def write_ssh_config(relay_info, proxy_path, vm_name, ip, username, relay_info_path = None relay_info_filename = None if is_arc: - relay_info_path, relay_info_filename = _prepare_relay_info_file(relay_info, cert_file, - private_key_file, credentials_folder, + relay_info_path, relay_info_filename = _prepare_relay_info_file(relay_info, credentials_folder, vm_name, resource_group) lines.append("Host " + resource_group + "-" + vm_name) @@ -276,16 +275,11 @@ def _terminate_cleanup(delete_keys, delete_cert, delete_credentials, cleanup_pro file_utils.delete_folder(temp_dir, f"Couldn't delete temporary folder {temp_dir}", True) -def _prepare_relay_info_file(relay_info, cert_file, private_key_file, credentials_folder, vm_name, resource_group): - if cert_file: - relay_info_dir = os.path.dirname(cert_file) - elif private_key_file: - relay_info_dir = os.path.dirname(private_key_file) - else: - # create the custom folder - relay_info_dir = credentials_folder - if not os.path.isdir(relay_info_dir): - os.makedirs(relay_info_dir) +def _prepare_relay_info_file(relay_info, credentials_folder, vm_name, resource_group): + # create the custom folder + relay_info_dir = credentials_folder + if not os.path.isdir(relay_info_dir): + os.makedirs(relay_info_dir) if vm_name and resource_group: relay_info_filename = resource_group + "-" + vm_name + "-relay_info" @@ -310,7 +304,8 @@ def _issue_config_cleanup_warning(delete_cert, delete_keys, is_arc, cert_file, r items_to_delete = f" (id_rsa, id_rsa.pub, id_rsa.pub-aadcert.pub, {relay_info_filename})" elif delete_cert: path_to_delete = os.path.dirname(cert_file) - items_to_delete = f" (id_rsa.pub-aadcert.pub, {relay_info_filename})" + relay_partial_path = relay_info_path.replace(path_to_delete, "") + items_to_delete = f" (id_rsa.pub-aadcert.pub, {relay_partial_path})" else: path_to_delete = relay_info_path items_to_delete = "" From c9a7e14c6707e2a362dbc5cf7c9abbc507e98dbe Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Tue, 14 Dec 2021 15:04:54 -0500 Subject: [PATCH 04/18] Add new parameter --arc-proxy-folder --- src/ssh/azext_ssh/_params.py | 3 +++ src/ssh/azext_ssh/custom.py | 44 ++++++++++++++++++++++++------------ src/ssh/setup.py | 2 +- 3 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/ssh/azext_ssh/_params.py b/src/ssh/azext_ssh/_params.py index a01c787d61f..efe220fa081 100644 --- a/src/ssh/azext_ssh/_params.py +++ b/src/ssh/azext_ssh/_params.py @@ -27,6 +27,7 @@ def load_arguments(self, _): help=('This is an internal argument. This argument is used by Azure Portal to provide a one click ' 'SSH login experience in Cloud shell.'), deprecate_info=c.deprecate(hide=True), action='store_true') + c.argument('arc_proxy_folder', options_list=['--arc-proxy-folder'], help='Path to the folder where arc proxy should be stored.') c.positional('ssh_args', nargs='*', help='Additional arguments passed to OpenSSH') with self.argument_context('ssh config') as c: @@ -47,6 +48,7 @@ def load_arguments(self, _): c.argument('resource_type', options_list=['--resource-type'], help='Resource type should be either Microsoft.Compute or Microsoft.HybridCompute') c.argument('cert_file', options_list=['--certificate-file', '-c'], help='Path to certificate file') + c.argument('arc_proxy_folder', options_list=['--arc-proxy-folder'], help='Path to the folder where the arc proxy should be saved. Defaults to .clientsshproxy folder under the user\'s home directory.') with self.argument_context('ssh cert') as c: c.argument('cert_path', options_list=['--file', '-f'], @@ -69,4 +71,5 @@ def load_arguments(self, _): help=('This is an internal argument. This argument is used by Azure Portal to provide a one click ' 'SSH login experience in Cloud shell.'), deprecate_info=c.deprecate(hide=True), action='store_true') + c.argument('arc_proxy_folder', options_list=['--arc-proxy-folder'], help='Path to the folder where arc proxy should be stored.') c.positional('ssh_args', nargs='*', help='Additional arguments passed to OpenSSH') diff --git a/src/ssh/azext_ssh/custom.py b/src/ssh/azext_ssh/custom.py index 9f4b3fd061a..922fe72968d 100644 --- a/src/ssh/azext_ssh/custom.py +++ b/src/ssh/azext_ssh/custom.py @@ -30,7 +30,7 @@ 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_client_path=None, delete_credentials=False, resource_type=None, ssh_args=None): + ssh_client_path=None, delete_credentials=False, resource_type=None, arc_proxy_folder=None, ssh_args=None): if delete_credentials and os.environ.get("AZUREPS_HOST_ENVIRONMENT") != "cloud-shell/1.0": raise azclierror.ArgumentUsageError("Can't use --delete-private-key outside an Azure Cloud Shell session.") @@ -40,12 +40,12 @@ def ssh_vm(cmd, resource_group_name=None, vm_name=None, ssh_ip=None, public_key_ do_ssh_op = _decide_op_call(cmd, resource_group_name, vm_name, ssh_ip, resource_type, None, None, ssh_client_path, ssh_args, delete_credentials, credentials_folder, local_user) do_ssh_op(cmd, vm_name, resource_group_name, ssh_ip, public_key_file, private_key_file, local_user, - cert_file, port, use_private_ip, credentials_folder) + cert_file, port, use_private_ip, credentials_folder, arc_proxy_folder) 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, resource_type=None, credentials_folder=None): + local_user=None, cert_file=None, port=None, resource_type=None, credentials_folder=None, arc_proxy_folder=None): if (public_key_file or private_key_file) and credentials_folder: raise azclierror.ArgumentUsageError("--keys-destination-folder can't be used in conjunction with " @@ -69,7 +69,7 @@ def ssh_config(cmd, config_path, resource_group_name=None, vm_name=None, ssh_ip= do_ssh_op = _decide_op_call(cmd, resource_group_name, vm_name, ssh_ip, resource_type, config_path, overwrite, None, None, False, credentials_folder, local_user) do_ssh_op(cmd, vm_name, resource_group_name, ssh_ip, public_key_file, private_key_file, local_user, - cert_file, port, use_private_ip, credentials_folder) + cert_file, port, use_private_ip, credentials_folder, arc_proxy_folder) def ssh_cert(cmd, cert_path=None, public_key_file=None): @@ -89,7 +89,7 @@ def ssh_cert(cmd, cert_path=None, public_key_file=None): def ssh_arc(cmd, resource_group_name=None, vm_name=None, public_key_file=None, private_key_file=None, - local_user=None, cert_file=None, port=None, ssh_client_path=None, delete_credentials=False, ssh_args=None): + local_user=None, cert_file=None, port=None, ssh_client_path=None, delete_credentials=False, arc_proxy_folder=None, ssh_args=None): if delete_credentials and os.environ.get("AZUREPS_HOST_ENVIRONMENT") != "cloud-shell/1.0": raise azclierror.ArgumentUsageError("Can't use --delete-private-key outside an Azure Cloud Shell session.") @@ -116,16 +116,19 @@ def ssh_arc(cmd, resource_group_name=None, vm_name=None, public_key_file=None, p op_call = functools.partial(ssh_utils.start_ssh_connection, ssh_client_path=ssh_client_path, ssh_args=ssh_args, delete_credentials=delete_credentials) _do_ssh_op(cmd, vm_name, resource_group_name, None, public_key_file, private_key_file, local_user, cert_file, port, - False, credentials_folder, op_call, True) + False, credentials_folder, arc_proxy_folder, op_call, True) def _do_ssh_op(cmd, vm_name, resource_group_name, ssh_ip, public_key_file, private_key_file, username, - cert_file, port, use_private_ip, credentials_folder, op_call, is_arc): + cert_file, port, use_private_ip, credentials_folder, arc_proxy_folder, op_call, is_arc): + + if not is_arc and arc_proxy_folder: + logger.warning("Target machine is not an Arc Server, --arc-proxy-folder value will be ignored.") proxy_path = None relay_info = None if is_arc: - proxy_path = _arc_get_client_side_proxy() + proxy_path = _arc_get_client_side_proxy(arc_proxy_folder) relay_info = _arc_list_access_details(cmd, resource_group_name, vm_name) else: ssh_ip = ssh_ip or ip_utils.get_ssh_ip(cmd, resource_group_name, vm_name, use_private_ip) @@ -298,7 +301,7 @@ def _get_modulus_exponent(public_key_file): # Downloads client side proxy to connect to Arc Connectivity Platform -def _arc_get_client_side_proxy(): +def _arc_get_client_side_proxy(arc_proxy_folder): import platform operating_system = platform.system() machine = platform.machine() @@ -322,8 +325,10 @@ def _arc_get_client_side_proxy(): proxy_name = f"sshProxy_{operating_system.lower()}_{architecture}" request_uri = (f"{consts.CLIENT_PROXY_STORAGE_URL}/{consts.CLIENT_PROXY_RELEASE}" f"/{proxy_name}_{consts.CLIENT_PROXY_VERSION}") - install_location = os.path.join(".clientsshproxy", proxy_name + "_" + consts.CLIENT_PROXY_VERSION.replace('.', '_')) - older_version_location = os.path.join(".clientsshproxy", proxy_name + "*") + install_location = proxy_name + "_" + consts.CLIENT_PROXY_VERSION.replace('.', '_') + older_version_location = proxy_name + "*" + #install_location = os.path.join(".clientsshproxy", proxy_name + "_" + consts.CLIENT_PROXY_VERSION.replace('.', '_')) + #older_version_location = os.path.join(".clientsshproxy", proxy_name + "*") if operating_system == 'Windows': request_uri = request_uri + ".exe" @@ -335,9 +340,18 @@ def _arc_get_client_side_proxy(): summary=f'{operating_system} is not supported for installing client proxy') raise azclierror.BadRequestError(f"Unsuported OS: {operating_system} platform is not currently supported") - install_location = os.path.expanduser(os.path.join('~', install_location)) - older_version_location = os.path.expanduser(os.path.join('~', older_version_location)) - install_dir = os.path.dirname(install_location) + + if not arc_proxy_folder: + install_location = os.path.expanduser(os.path.join('~', os.path.join(".clientsshproxy", install_location))) + older_version_location = os.path.expanduser(os.path.join('~', os.path.join(".clientsshproxy", older_version_location))) + install_dir = os.path.dirname(install_location) + else: + install_location = os.path.join(arc_proxy_folder, install_location) + older_version_location = os.path.join(arc_proxy_folder, older_version_location) + install_dir = arc_proxy_folder + + print(install_location) + print(older_version_location) # Only download new proxy if it doesn't exist already if not os.path.isfile(install_location): @@ -361,7 +375,7 @@ def _arc_get_client_side_proxy(): telemetry.add_extension_event('ssh', proxy_data) # if directory doesn't exist, create it - if not os.path.exists(install_dir): + if not os.path.isdir(install_dir): file_utils.create_directory(install_dir, f"Failed to create client proxy directory '{install_dir}'. ") # if directory exists, delete any older versions of the proxy else: diff --git a/src/ssh/setup.py b/src/ssh/setup.py index 860e04cb9b8..ebba436252b 100644 --- a/src/ssh/setup.py +++ b/src/ssh/setup.py @@ -7,7 +7,7 @@ from setuptools import setup, find_packages -VERSION = "0.2.1" +VERSION = "0.2.2" CLASSIFIERS = [ 'Development Status :: 4 - Beta', From b5d11ee4b906874b9c59de05e154954912d2f365 Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Tue, 14 Dec 2021 15:15:29 -0500 Subject: [PATCH 05/18] Add quotation marks around paths in config file --- src/ssh/azext_ssh/ssh_utils.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ssh/azext_ssh/ssh_utils.py b/src/ssh/azext_ssh/ssh_utils.py index 4d08e2bfa39..08168310354 100644 --- a/src/ssh/azext_ssh/ssh_utils.py +++ b/src/ssh/azext_ssh/ssh_utils.py @@ -116,9 +116,9 @@ def write_ssh_config(relay_info, proxy_path, vm_name, ip, username, common_lines = [] common_lines.append("\tUser " + username) if cert_file: - common_lines.append("\tCertificateFile " + cert_file) + common_lines.append("\tCertificateFile \"" + cert_file + "\"") if private_key_file: - common_lines.append("\tIdentityFile " + private_key_file) + common_lines.append("\tIdentityFile \"" + private_key_file + "\"") lines = [""] relay_info_path = None @@ -131,9 +131,9 @@ def write_ssh_config(relay_info, proxy_path, vm_name, ip, username, lines.append("\tHostName " + vm_name) lines = lines + common_lines if port: - lines.append("\tProxyCommand " + proxy_path + " " + "-r " + relay_info_path + " " + "-p " + port) + lines.append("\tProxyCommand \"" + proxy_path + "\" " + "-r \"" + relay_info_path + "\" " + "-p " + port) else: - lines.append("\tProxyCommand " + proxy_path + " " + "-r " + relay_info_path) + lines.append("\tProxyCommand \"" + proxy_path + "\" " + "-r \"" + relay_info_path + "\"") else: if resource_group and vm_name: lines.append("Host " + resource_group + "-" + vm_name) From a96356c62114c679f336f3b430fbef11ab18b049 Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Tue, 14 Dec 2021 15:28:04 -0500 Subject: [PATCH 06/18] Ignore capitalization on resource type --- src/ssh/azext_ssh/custom.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/ssh/azext_ssh/custom.py b/src/ssh/azext_ssh/custom.py index 922fe72968d..f9fa7713bf6 100644 --- a/src/ssh/azext_ssh/custom.py +++ b/src/ssh/azext_ssh/custom.py @@ -213,7 +213,7 @@ def _prepare_jwk_data(public_key_file): def _assert_args(resource_group, vm_name, ssh_ip, resource_type, cert_file, username): - if resource_type and resource_type != "Microsoft.Compute" and resource_type != "Microsoft.HybridCompute": + if resource_type and resource_type.lower() != "microsoft.compute" and resource_type.lower() != "microsoft.hybridcompute": raise azclierror.InvalidArgumentValueError("--resource-type must be either \"Microsoft.Compute\" " "for Azure VMs or \"Microsoft.HybridCompute\" for Arc Servers.") @@ -327,8 +327,6 @@ def _arc_get_client_side_proxy(arc_proxy_folder): f"/{proxy_name}_{consts.CLIENT_PROXY_VERSION}") install_location = proxy_name + "_" + consts.CLIENT_PROXY_VERSION.replace('.', '_') older_version_location = proxy_name + "*" - #install_location = os.path.join(".clientsshproxy", proxy_name + "_" + consts.CLIENT_PROXY_VERSION.replace('.', '_')) - #older_version_location = os.path.join(".clientsshproxy", proxy_name + "*") if operating_system == 'Windows': request_uri = request_uri + ".exe" @@ -339,7 +337,6 @@ def _arc_get_client_side_proxy(arc_proxy_folder): fault_type=consts.PROXY_UNSUPPORTED_OS_FAULT_TYPE, summary=f'{operating_system} is not supported for installing client proxy') raise azclierror.BadRequestError(f"Unsuported OS: {operating_system} platform is not currently supported") - if not arc_proxy_folder: install_location = os.path.expanduser(os.path.join('~', os.path.join(".clientsshproxy", install_location))) @@ -350,9 +347,6 @@ def _arc_get_client_side_proxy(arc_proxy_folder): older_version_location = os.path.join(arc_proxy_folder, older_version_location) install_dir = arc_proxy_folder - print(install_location) - print(older_version_location) - # Only download new proxy if it doesn't exist already if not os.path.isfile(install_location): t0 = time.time() @@ -435,7 +429,7 @@ def _decide_op_call(cmd, resource_group_name, vm_name, ssh_ip, resource_type, co vm = None elif resource_type: - if resource_type == "Microsoft.HybridCompute": + if resource_type.lower() == "microsoft.hybridcompute": arc, arc_error, is_arc_server = _check_if_arc_server(cmd, resource_group_name, vm_name) if not is_arc_server: if isinstance(arc_error, ResourceNotFoundError): @@ -445,7 +439,7 @@ def _decide_op_call(cmd, resource_group_name, vm_name, ssh_ip, resource_type, co raise azclierror.BadRequestError("Unable to determine that the target machine is an Arc Server. " f"Error:\n{str(arc_error)}") - elif resource_type == "Microsoft.Compute": + elif resource_type.lower() == "microsoft.compute": vm, vm_error, is_azure_vm = _check_if_azure_vm(cmd, resource_group_name, vm_name) if not is_azure_vm: if isinstance(vm_error, ResourceNotFoundError): @@ -496,6 +490,7 @@ def _decide_op_call(cmd, resource_group_name, vm_name, ssh_ip, resource_type, co def _check_if_azure_vm(cmd, resource_group_name, vm_name): from azure.cli.core.commands import client_factory from azure.cli.core import profiles + vm = None try: compute_client = client_factory.get_mgmt_service_client(cmd.cli_ctx, profiles.ResourceType.MGMT_COMPUTE) vm = compute_client.virtual_machines.get(resource_group_name, vm_name) @@ -511,6 +506,7 @@ def _check_if_azure_vm(cmd, resource_group_name, vm_name): def _check_if_arc_server(cmd, resource_group_name, vm_name): from azext_ssh._client_factory import cf_machine client = cf_machine(cmd.cli_ctx) + arc = None try: arc = client.get(resource_group_name=resource_group_name, machine_name=vm_name) except ResourceNotFoundError as e: From 7e578e5ad65fd7ca8b623290b4b20dfc1c6eb022 Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Tue, 14 Dec 2021 16:15:19 -0500 Subject: [PATCH 07/18] Sync validity of relay information with certificate validity --- src/ssh/HISTORY.md | 16 ++++++++++++ src/ssh/azext_ssh/custom.py | 50 ++++++++++++++++++++++++++----------- 2 files changed, 52 insertions(+), 14 deletions(-) diff --git a/src/ssh/HISTORY.md b/src/ssh/HISTORY.md index 688825d5019..dd7f85fea9b 100644 --- a/src/ssh/HISTORY.md +++ b/src/ssh/HISTORY.md @@ -1,5 +1,21 @@ Release History =============== +0.2.2 +----- +* Validate that target machine exists before attempting to connect. +* ssh config accepts relative path for --file. +* Make --local-user mandatory for Windows target machines. +* For ssh config, relay information is stored under az_ssh_config folder. +* New optional parameter --arc-proxy-folder to determine where arc proxy is stored. + +0.2.1 +----- +* SSHArc Private Preview 2 + +0.2.0 +----- +* SSHArc Private Preview 1 + 0.1.9 ----- * Add support for connecting to Arc Servers using AAD issued certificates. diff --git a/src/ssh/azext_ssh/custom.py b/src/ssh/azext_ssh/custom.py index f9fa7713bf6..d86f7fca056 100644 --- a/src/ssh/azext_ssh/custom.py +++ b/src/ssh/azext_ssh/custom.py @@ -125,11 +125,27 @@ def _do_ssh_op(cmd, vm_name, resource_group_name, ssh_ip, public_key_file, priva if not is_arc and arc_proxy_folder: logger.warning("Target machine is not an Arc Server, --arc-proxy-folder value will be ignored.") + # If user provides local user, no credentials should be deleted. + delete_keys = False + delete_cert = False + certificate_validity_in_seconds = None + if not username: + 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) + if is_arc: + try: + certificate_validity_in_seconds = _get_certificate_validity_in_seconds(cert_file) + except Exception as e: + logger.warning("Couldn't determine certificate validity. Error: " + str(e)) + proxy_path = None relay_info = None if is_arc: proxy_path = _arc_get_client_side_proxy(arc_proxy_folder) - relay_info = _arc_list_access_details(cmd, resource_group_name, vm_name) + relay_info = _arc_list_access_details(cmd, resource_group_name, vm_name, certificate_validity_in_seconds) else: ssh_ip = ssh_ip or ip_utils.get_ssh_ip(cmd, resource_group_name, vm_name, use_private_ip) if not ssh_ip: @@ -138,20 +154,22 @@ def _do_ssh_op(cmd, vm_name, resource_group_name, ssh_ip, public_key_file, priva raise azclierror.ResourceNotFoundError(f"VM '{vm_name}' does not have a public or private IP address to" "SSH to") - # If user provides local user, no credentials should be deleted. - delete_keys = False - delete_cert = False - if not username: - 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) - op_call(relay_info, proxy_path, vm_name, ssh_ip, username, cert_file, private_key_file, port, is_arc, delete_keys, delete_cert, public_key_file) +def _get_certificate_validity_in_seconds(cert_file): + import datetime + validity_str = ssh_utils.get_ssh_cert_validity(cert_file) + validity = None + if validity_str and "Valid: from " in validity_str and " to " in validity_str: + times = validity_str.replace("Valid: from ", "").split(" to ") + t0 = datetime.datetime.fromisoformat(times[0]) + t1 = datetime.datetime.fromisoformat(times[1]) + validity = t1 - t0 + return int(validity.total_seconds()) + + def _get_and_write_certificate(cmd, public_key_file, cert_file): cloudtoscope = { "azurecloud": "https://pas.windows.net/CheckMyAccess/Linux/.default", @@ -385,13 +403,17 @@ def _arc_get_client_side_proxy(arc_proxy_folder): # Get the Access Details to connect to Arc Connectivity platform from the HybridConnectivity RP -def _arc_list_access_details(cmd, resource_group, vm_name): +def _arc_list_access_details(cmd, resource_group, vm_name, certificate_validity): from azext_ssh._client_factory import cf_endpoint client = cf_endpoint(cmd.cli_ctx) try: t0 = time.time() - result = client.list_credentials(resource_group_name=resource_group, machine_name=vm_name, - endpoint_name="default") + if certificate_validity: + result = client.list_credentials(resource_group_name=resource_group, machine_name=vm_name, + endpoint_name="default", expiresin=certificate_validity) + else: + result = client.list_credentials(resource_group_name=resource_group, machine_name=vm_name, + endpoint_name="default") time_elapsed = time.time() - t0 telemetry.add_extension_event('ssh', {'Context.Default.AzureCLI.SSHListCredentialsTime': time_elapsed}) except Exception as e: From 9cff8a1440456122a614738880081cbb0c314b38 Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Tue, 14 Dec 2021 16:41:10 -0500 Subject: [PATCH 08/18] Style fixes --- src/ssh/azext_ssh/_params.py | 12 +++- src/ssh/azext_ssh/custom.py | 104 +++++++++++++++++++---------------- 2 files changed, 66 insertions(+), 50 deletions(-) diff --git a/src/ssh/azext_ssh/_params.py b/src/ssh/azext_ssh/_params.py index efe220fa081..004dc8dad79 100644 --- a/src/ssh/azext_ssh/_params.py +++ b/src/ssh/azext_ssh/_params.py @@ -27,7 +27,9 @@ def load_arguments(self, _): help=('This is an internal argument. This argument is used by Azure Portal to provide a one click ' 'SSH login experience in Cloud shell.'), deprecate_info=c.deprecate(hide=True), action='store_true') - c.argument('arc_proxy_folder', options_list=['--arc-proxy-folder'], help='Path to the folder where arc proxy should be stored.') + c.argument('arc_proxy_folder', options_list=['--arc-proxy-folder'], + help=('Path to the folder where the arc proxy should be saved. ' + 'Default to .clientsshproxy folder in user\'s home directory if not provided.')) c.positional('ssh_args', nargs='*', help='Additional arguments passed to OpenSSH') with self.argument_context('ssh config') as c: @@ -48,7 +50,9 @@ def load_arguments(self, _): c.argument('resource_type', options_list=['--resource-type'], help='Resource type should be either Microsoft.Compute or Microsoft.HybridCompute') c.argument('cert_file', options_list=['--certificate-file', '-c'], help='Path to certificate file') - c.argument('arc_proxy_folder', options_list=['--arc-proxy-folder'], help='Path to the folder where the arc proxy should be saved. Defaults to .clientsshproxy folder under the user\'s home directory.') + c.argument('arc_proxy_folder', options_list=['--arc-proxy-folder'], + help=('Path to the folder where the arc proxy should be saved. ' + 'Default to .clientsshproxy folder in user\'s home directory if not provided.')) with self.argument_context('ssh cert') as c: c.argument('cert_path', options_list=['--file', '-f'], @@ -71,5 +75,7 @@ def load_arguments(self, _): help=('This is an internal argument. This argument is used by Azure Portal to provide a one click ' 'SSH login experience in Cloud shell.'), deprecate_info=c.deprecate(hide=True), action='store_true') - c.argument('arc_proxy_folder', options_list=['--arc-proxy-folder'], help='Path to the folder where arc proxy should be stored.') + c.argument('arc_proxy_folder', options_list=['--arc-proxy-folder'], + help=('Path to the folder where the arc proxy should be saved. ' + 'Default to .clientsshproxy folder in user\'s home directory if not provided.')) c.positional('ssh_args', nargs='*', help='Additional arguments passed to OpenSSH') diff --git a/src/ssh/azext_ssh/custom.py b/src/ssh/azext_ssh/custom.py index d86f7fca056..faf1d04dbf7 100644 --- a/src/ssh/azext_ssh/custom.py +++ b/src/ssh/azext_ssh/custom.py @@ -45,7 +45,8 @@ def ssh_vm(cmd, resource_group_name=None, vm_name=None, ssh_ip=None, public_key_ 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, resource_type=None, credentials_folder=None, arc_proxy_folder=None): + local_user=None, cert_file=None, port=None, resource_type=None, credentials_folder=None, + arc_proxy_folder=None): if (public_key_file or private_key_file) and credentials_folder: raise azclierror.ArgumentUsageError("--keys-destination-folder can't be used in conjunction with " @@ -89,7 +90,8 @@ def ssh_cert(cmd, cert_path=None, public_key_file=None): def ssh_arc(cmd, resource_group_name=None, vm_name=None, public_key_file=None, private_key_file=None, - local_user=None, cert_file=None, port=None, ssh_client_path=None, delete_credentials=False, arc_proxy_folder=None, ssh_args=None): + local_user=None, cert_file=None, port=None, ssh_client_path=None, delete_credentials=False, + arc_proxy_folder=None, ssh_args=None): if delete_credentials and os.environ.get("AZUREPS_HOST_ENVIRONMENT") != "cloud-shell/1.0": raise azclierror.ArgumentUsageError("Can't use --delete-private-key outside an Azure Cloud Shell session.") @@ -139,8 +141,9 @@ def _do_ssh_op(cmd, vm_name, resource_group_name, ssh_ip, public_key_file, priva try: certificate_validity_in_seconds = _get_certificate_validity_in_seconds(cert_file) except Exception as e: - logger.warning("Couldn't determine certificate validity. Error: " + str(e)) - + logger.warning("Couldn't determine certificate validity. Error: %s", str(e)) + print(certificate_validity_in_seconds) + proxy_path = None relay_info = None if is_arc: @@ -231,7 +234,8 @@ def _prepare_jwk_data(public_key_file): def _assert_args(resource_group, vm_name, ssh_ip, resource_type, cert_file, username): - if resource_type and resource_type.lower() != "microsoft.compute" and resource_type.lower() != "microsoft.hybridcompute": + if resource_type and resource_type.lower() != "microsoft.compute" \ + and resource_type.lower() != "microsoft.hybridcompute": raise azclierror.InvalidArgumentValueError("--resource-type must be either \"Microsoft.Compute\" " "for Azure VMs or \"Microsoft.HybridCompute\" for Arc Servers.") @@ -320,6 +324,48 @@ def _get_modulus_exponent(public_key_file): # Downloads client side proxy to connect to Arc Connectivity Platform def _arc_get_client_side_proxy(arc_proxy_folder): + + request_uri, install_location, older_version_location = _get_proxy_filename_and_url(arc_proxy_folder) + install_dir = os.path.dirname(install_location) + + # Only download new proxy if it doesn't exist already + if not os.path.isfile(install_location): + t0 = time.time() + # download the executable + try: + with urllib.request.urlopen(request_uri) as response: + response_content = response.read() + response.close() + except Exception as e: + telemetry.set_exception(exception=e, fault_type=consts.PROXY_DOWNLOAD_FAILED_FAULT_TYPE, + summary=f'Failed to download proxy from {request_uri}') + raise azclierror.ClientRequestError(f"Failed to download client proxy executable from {request_uri}. " + "Error: " + str(e)) from e + time_elapsed = time.time() - t0 + + proxy_data = { + 'Context.Default.AzureCLI.SSHProxyDownloadTime': time_elapsed, + 'Context.Default.AzureCLI.SSHProxyVersion': consts.CLIENT_PROXY_VERSION + } + telemetry.add_extension_event('ssh', proxy_data) + + # if directory doesn't exist, create it + if not os.path.isdir(install_dir): + file_utils.create_directory(install_dir, f"Failed to create client proxy directory '{install_dir}'. ") + # if directory exists, delete any older versions of the proxy + else: + older_version_files = glob(older_version_location) + for f in older_version_files: + file_utils.delete_file(f, f"failed to delete older version file {f}", warning=True) + + # write executable in the install location + file_utils.write_to_file(install_location, 'wb', response_content, "Failed to create client proxy file. ") + os.chmod(install_location, os.stat(install_location).st_mode | stat.S_IXUSR) + + return install_location + + +def _get_proxy_filename_and_url(arc_proxy_folder): import platform operating_system = platform.system() machine = platform.machine() @@ -344,62 +390,26 @@ def _arc_get_client_side_proxy(arc_proxy_folder): request_uri = (f"{consts.CLIENT_PROXY_STORAGE_URL}/{consts.CLIENT_PROXY_RELEASE}" f"/{proxy_name}_{consts.CLIENT_PROXY_VERSION}") install_location = proxy_name + "_" + consts.CLIENT_PROXY_VERSION.replace('.', '_') - older_version_location = proxy_name + "*" + older_location = proxy_name + "*" if operating_system == 'Windows': request_uri = request_uri + ".exe" install_location = install_location + ".exe" - older_version_location = older_version_location + ".exe" + older_location = older_location + ".exe" elif operating_system not in ('Linux', 'Darwin'): telemetry.set_exception(exception='Unsuported OS for installing ssh client proxy', fault_type=consts.PROXY_UNSUPPORTED_OS_FAULT_TYPE, summary=f'{operating_system} is not supported for installing client proxy') raise azclierror.BadRequestError(f"Unsuported OS: {operating_system} platform is not currently supported") - + if not arc_proxy_folder: install_location = os.path.expanduser(os.path.join('~', os.path.join(".clientsshproxy", install_location))) - older_version_location = os.path.expanduser(os.path.join('~', os.path.join(".clientsshproxy", older_version_location))) - install_dir = os.path.dirname(install_location) + older_location = os.path.expanduser(os.path.join('~', os.path.join(".clientsshproxy", older_location))) else: install_location = os.path.join(arc_proxy_folder, install_location) - older_version_location = os.path.join(arc_proxy_folder, older_version_location) - install_dir = arc_proxy_folder - - # Only download new proxy if it doesn't exist already - if not os.path.isfile(install_location): - t0 = time.time() - # download the executable - try: - with urllib.request.urlopen(request_uri) as response: - response_content = response.read() - response.close() - except Exception as e: - telemetry.set_exception(exception=e, fault_type=consts.PROXY_DOWNLOAD_FAILED_FAULT_TYPE, - summary=f'Failed to download proxy from {request_uri}') - raise azclierror.ClientRequestError(f"Failed to download client proxy executable from {request_uri}. " - "Error: " + str(e)) from e - time_elapsed = time.time() - t0 - - proxy_data = { - 'Context.Default.AzureCLI.SSHProxyDownloadTime': time_elapsed, - 'Context.Default.AzureCLI.SSHProxyVersion': consts.CLIENT_PROXY_VERSION - } - telemetry.add_extension_event('ssh', proxy_data) + older_location = os.path.join(arc_proxy_folder, older_location) - # if directory doesn't exist, create it - if not os.path.isdir(install_dir): - file_utils.create_directory(install_dir, f"Failed to create client proxy directory '{install_dir}'. ") - # if directory exists, delete any older versions of the proxy - else: - older_version_files = glob(older_version_location) - for f in older_version_files: - file_utils.delete_file(f, f"failed to delete older version file {f}", warning=True) - - # write executable in the install location - file_utils.write_to_file(install_location, 'wb', response_content, "Failed to create client proxy file. ") - os.chmod(install_location, os.stat(install_location).st_mode | stat.S_IXUSR) - - return install_location + return request_uri, install_location, older_location # Get the Access Details to connect to Arc Connectivity platform from the HybridConnectivity RP From 44a20149a891556bfa32f204f49264730269b7ac Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Wed, 15 Dec 2021 15:08:24 -0500 Subject: [PATCH 09/18] Fixes based on review comments --- src/ssh/HISTORY.md | 1 + src/ssh/azext_ssh/_params.py | 6 +- src/ssh/azext_ssh/arc_utils.py | 152 +++++++++++++++++++++++++++ src/ssh/azext_ssh/custom.py | 182 ++++----------------------------- src/ssh/azext_ssh/ssh_utils.py | 56 ++++++---- 5 files changed, 214 insertions(+), 183 deletions(-) create mode 100644 src/ssh/azext_ssh/arc_utils.py diff --git a/src/ssh/HISTORY.md b/src/ssh/HISTORY.md index dd7f85fea9b..0ab8abbe59d 100644 --- a/src/ssh/HISTORY.md +++ b/src/ssh/HISTORY.md @@ -7,6 +7,7 @@ Release History * Make --local-user mandatory for Windows target machines. * For ssh config, relay information is stored under az_ssh_config folder. * New optional parameter --arc-proxy-folder to determine where arc proxy is stored. +* Relay information lifetime is synced with certificate lifetime for AAD login. 0.2.1 ----- diff --git a/src/ssh/azext_ssh/_params.py b/src/ssh/azext_ssh/_params.py index 004dc8dad79..14b88eecde6 100644 --- a/src/ssh/azext_ssh/_params.py +++ b/src/ssh/azext_ssh/_params.py @@ -27,7 +27,7 @@ def load_arguments(self, _): help=('This is an internal argument. This argument is used by Azure Portal to provide a one click ' 'SSH login experience in Cloud shell.'), deprecate_info=c.deprecate(hide=True), action='store_true') - c.argument('arc_proxy_folder', options_list=['--arc-proxy-folder'], + c.argument('ssh_proxy_folder', options_list=['--ssh-proxy-folder'], help=('Path to the folder where the arc proxy should be saved. ' 'Default to .clientsshproxy folder in user\'s home directory if not provided.')) c.positional('ssh_args', nargs='*', help='Additional arguments passed to OpenSSH') @@ -50,7 +50,7 @@ def load_arguments(self, _): c.argument('resource_type', options_list=['--resource-type'], help='Resource type should be either Microsoft.Compute or Microsoft.HybridCompute') c.argument('cert_file', options_list=['--certificate-file', '-c'], help='Path to certificate file') - c.argument('arc_proxy_folder', options_list=['--arc-proxy-folder'], + c.argument('ssh_proxy_folder', options_list=['--ssh-proxy-folder'], help=('Path to the folder where the arc proxy should be saved. ' 'Default to .clientsshproxy folder in user\'s home directory if not provided.')) @@ -75,7 +75,7 @@ def load_arguments(self, _): help=('This is an internal argument. This argument is used by Azure Portal to provide a one click ' 'SSH login experience in Cloud shell.'), deprecate_info=c.deprecate(hide=True), action='store_true') - c.argument('arc_proxy_folder', options_list=['--arc-proxy-folder'], + c.argument('ssh_proxy_folder', options_list=['--ssh-proxy-folder'], help=('Path to the folder where the arc proxy should be saved. ' 'Default to .clientsshproxy folder in user\'s home directory if not provided.')) c.positional('ssh_args', nargs='*', help='Additional arguments passed to OpenSSH') diff --git a/src/ssh/azext_ssh/arc_utils.py b/src/ssh/azext_ssh/arc_utils.py new file mode 100644 index 00000000000..bf3d42b0a89 --- /dev/null +++ b/src/ssh/azext_ssh/arc_utils.py @@ -0,0 +1,152 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +import time +import stat +import os +import urllib.request +import json +import base64 +from glob import glob + +from azure.cli.core import telemetry +from azure.cli.core import azclierror +from knack import log + +from . import file_utils +from . import constants as consts + +logger = log.get_logger(__name__) + +# Get the Access Details to connect to Arc Connectivity platform from the HybridConnectivity RP +def _arc_list_access_details(cmd, resource_group, vm_name, certificate_validity): + from azext_ssh._client_factory import cf_endpoint + client = cf_endpoint(cmd.cli_ctx) + + if not certificate_validity or certificate_validity > 3600: + certificate_validity = 3600 + + try: + t0 = time.time() + result = client.list_credentials(resource_group_name=resource_group, machine_name=vm_name, + endpoint_name="default", expiresin=certificate_validity) + time_elapsed = time.time() - t0 + telemetry.add_extension_event('ssh', {'Context.Default.AzureCLI.SSHListCredentialsTime': time_elapsed}) + except Exception as e: + telemetry.set_exception(exception='Call to listCredentials failed', + fault_type=consts.LIST_CREDENTIALS_FAILED_FAULT_TYPE, + summary=f'listCredentials failed with error: {str(e)}.') + raise azclierror.ClientRequestError(f"Request for Azure Relay Information Failed: {str(e)}") + + return result + + +# Downloads client side proxy to connect to Arc Connectivity Platform +def _arc_get_client_side_proxy(arc_proxy_folder): + + request_uri, install_location, older_version_location = _get_proxy_filename_and_url(arc_proxy_folder) + install_dir = os.path.dirname(install_location) + + # Only download new proxy if it doesn't exist already + if not os.path.isfile(install_location): + t0 = time.time() + # download the executable + try: + with urllib.request.urlopen(request_uri) as response: + response_content = response.read() + response.close() + except Exception as e: + telemetry.set_exception(exception=e, fault_type=consts.PROXY_DOWNLOAD_FAILED_FAULT_TYPE, + summary=f'Failed to download proxy from {request_uri}') + raise azclierror.ClientRequestError(f"Failed to download client proxy executable from {request_uri}. " + "Error: " + str(e)) from e + time_elapsed = time.time() - t0 + + proxy_data = { + 'Context.Default.AzureCLI.SSHProxyDownloadTime': time_elapsed, + 'Context.Default.AzureCLI.SSHProxyVersion': consts.CLIENT_PROXY_VERSION + } + telemetry.add_extension_event('ssh', proxy_data) + + # if directory doesn't exist, create it + if not os.path.isdir(install_dir): + file_utils.create_directory(install_dir, f"Failed to create client proxy directory '{install_dir}'. ") + # if directory exists, delete any older versions of the proxy + else: + older_version_files = glob(older_version_location) + for f in older_version_files: + file_utils.delete_file(f, f"failed to delete older version file {f}", warning=True) + + # write executable in the install location + file_utils.write_to_file(install_location, 'wb', response_content, "Failed to create client proxy file. ") + os.chmod(install_location, os.stat(install_location).st_mode | stat.S_IXUSR) + + return install_location + + +def _get_proxy_filename_and_url(arc_proxy_folder): + import platform + operating_system = platform.system() + machine = platform.machine() + + logger.debug("Platform OS: %s", operating_system) + logger.debug("Platform architecture: %s", machine) + + if machine.endswith('64'): + architecture = 'amd64' + elif machine.endswith('86'): + architecture = '386' + elif machine == '': + raise azclierror.BadRequestError("Couldn't identify the platform architecture.") + else: + telemetry.set_exception(exception='Unsuported architecture for installing proxy', + fault_type=consts.PROXY_UNSUPPORTED_ARCH_FAULT_TYPE, + summary=f'{machine} is not supported for installing client proxy') + raise azclierror.BadRequestError(f"Unsuported architecture: {machine} is not currently supported") + + # define the request url and install location based on the os and architecture + proxy_name = f"sshProxy_{operating_system.lower()}_{architecture}" + request_uri = (f"{consts.CLIENT_PROXY_STORAGE_URL}/{consts.CLIENT_PROXY_RELEASE}" + f"/{proxy_name}_{consts.CLIENT_PROXY_VERSION}") + install_location = proxy_name + "_" + consts.CLIENT_PROXY_VERSION.replace('.', '_') + older_location = proxy_name + "*" + + if operating_system == 'Windows': + request_uri = request_uri + ".exe" + install_location = install_location + ".exe" + older_location = older_location + ".exe" + elif operating_system not in ('Linux', 'Darwin'): + telemetry.set_exception(exception='Unsuported OS for installing ssh client proxy', + fault_type=consts.PROXY_UNSUPPORTED_OS_FAULT_TYPE, + summary=f'{operating_system} is not supported for installing client proxy') + raise azclierror.BadRequestError(f"Unsuported OS: {operating_system} platform is not currently supported") + + if not arc_proxy_folder: + install_location = os.path.expanduser(os.path.join('~', os.path.join(".clientsshproxy", install_location))) + older_location = os.path.expanduser(os.path.join('~', os.path.join(".clientsshproxy", older_location))) + else: + install_location = os.path.join(arc_proxy_folder, install_location) + older_location = os.path.join(arc_proxy_folder, older_location) + + return request_uri, install_location, older_location + + +def _arc_format_relay_info_string(relay_info): + relay_info_string = json.dumps( + { + "relay": { + "namespaceName": relay_info.namespace_name, + "namespaceNameSuffix": relay_info.namespace_name_suffix, + "hybridConnectionName": relay_info.hybrid_connection_name, + "accessKey": relay_info.access_key, + "expiresOn": relay_info.expires_on + } + }) + result_bytes = relay_info_string.encode("ascii") + enc = base64.b64encode(result_bytes) + base64_result_string = enc.decode("ascii") + return base64_result_string + + diff --git a/src/ssh/azext_ssh/custom.py b/src/ssh/azext_ssh/custom.py index faf1d04dbf7..7e8947419e7 100644 --- a/src/ssh/azext_ssh/custom.py +++ b/src/ssh/azext_ssh/custom.py @@ -8,11 +8,7 @@ import hashlib import json import tempfile -import urllib.request -import base64 -import stat import time -from glob import glob from knack import log from azure.cli.core import azclierror @@ -22,15 +18,14 @@ from . import ip_utils from . import rsa_parser from . import ssh_utils -from . import constants as consts -from . import file_utils +from . import arc_utils logger = log.get_logger(__name__) 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_client_path=None, delete_credentials=False, resource_type=None, arc_proxy_folder=None, ssh_args=None): + ssh_client_path=None, delete_credentials=False, resource_type=None, ssh_proxy_folder=None, ssh_args=None): if delete_credentials and os.environ.get("AZUREPS_HOST_ENVIRONMENT") != "cloud-shell/1.0": raise azclierror.ArgumentUsageError("Can't use --delete-private-key outside an Azure Cloud Shell session.") @@ -40,13 +35,13 @@ def ssh_vm(cmd, resource_group_name=None, vm_name=None, ssh_ip=None, public_key_ do_ssh_op = _decide_op_call(cmd, resource_group_name, vm_name, ssh_ip, resource_type, None, None, ssh_client_path, ssh_args, delete_credentials, credentials_folder, local_user) do_ssh_op(cmd, vm_name, resource_group_name, ssh_ip, public_key_file, private_key_file, local_user, - cert_file, port, use_private_ip, credentials_folder, arc_proxy_folder) + cert_file, port, use_private_ip, credentials_folder, ssh_proxy_folder) 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, resource_type=None, credentials_folder=None, - arc_proxy_folder=None): + ssh_proxy_folder=None): if (public_key_file or private_key_file) and credentials_folder: raise azclierror.ArgumentUsageError("--keys-destination-folder can't be used in conjunction with " @@ -70,7 +65,7 @@ def ssh_config(cmd, config_path, resource_group_name=None, vm_name=None, ssh_ip= do_ssh_op = _decide_op_call(cmd, resource_group_name, vm_name, ssh_ip, resource_type, config_path, overwrite, None, None, False, credentials_folder, local_user) do_ssh_op(cmd, vm_name, resource_group_name, ssh_ip, public_key_file, private_key_file, local_user, - cert_file, port, use_private_ip, credentials_folder, arc_proxy_folder) + cert_file, port, use_private_ip, credentials_folder, ssh_proxy_folder) def ssh_cert(cmd, cert_path=None, public_key_file=None): @@ -86,12 +81,17 @@ def ssh_cert(cmd, cert_path=None, public_key_file=None): "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") + try: + certificate_lifetime = ssh_utils._get_certificate_lifetime(cert_file) + print(f"Generated SSH certificate {cert_file} is valid for {certificate_lifetime}.") + except Exception as e: + logger.warning("Couldn't determine certificate validity. Error: %s", str(e)) + print(cert_file + "\n") def ssh_arc(cmd, resource_group_name=None, vm_name=None, public_key_file=None, private_key_file=None, local_user=None, cert_file=None, port=None, ssh_client_path=None, delete_credentials=False, - arc_proxy_folder=None, ssh_args=None): + ssh_proxy_folder=None, ssh_args=None): if delete_credentials and os.environ.get("AZUREPS_HOST_ENVIRONMENT") != "cloud-shell/1.0": raise azclierror.ArgumentUsageError("Can't use --delete-private-key outside an Azure Cloud Shell session.") @@ -118,19 +118,19 @@ def ssh_arc(cmd, resource_group_name=None, vm_name=None, public_key_file=None, p op_call = functools.partial(ssh_utils.start_ssh_connection, ssh_client_path=ssh_client_path, ssh_args=ssh_args, delete_credentials=delete_credentials) _do_ssh_op(cmd, vm_name, resource_group_name, None, public_key_file, private_key_file, local_user, cert_file, port, - False, credentials_folder, arc_proxy_folder, op_call, True) + False, credentials_folder, ssh_proxy_folder, op_call, True) def _do_ssh_op(cmd, vm_name, resource_group_name, ssh_ip, public_key_file, private_key_file, username, - cert_file, port, use_private_ip, credentials_folder, arc_proxy_folder, op_call, is_arc): + cert_file, port, use_private_ip, credentials_folder, ssh_proxy_folder, op_call, is_arc): - if not is_arc and arc_proxy_folder: - logger.warning("Target machine is not an Arc Server, --arc-proxy-folder value will be ignored.") + if not is_arc and ssh_proxy_folder: + logger.warning("Target machine is not an Arc Server, --ssh-proxy-folder value will be ignored.") # If user provides local user, no credentials should be deleted. delete_keys = False delete_cert = False - certificate_validity_in_seconds = None + cert_lifetime = None if not username: delete_cert = True public_key_file, private_key_file, delete_keys = _check_or_create_public_private_files(public_key_file, @@ -139,16 +139,16 @@ def _do_ssh_op(cmd, vm_name, resource_group_name, ssh_ip, public_key_file, priva cert_file, username = _get_and_write_certificate(cmd, public_key_file, None) if is_arc: try: - certificate_validity_in_seconds = _get_certificate_validity_in_seconds(cert_file) + cert_lifetime = ssh_utils._get_certificate_lifetime(cert_file).total_seconds() + print(cert_lifetime) except Exception as e: - logger.warning("Couldn't determine certificate validity. Error: %s", str(e)) - print(certificate_validity_in_seconds) + logger.warning("Couldn't determine certificate expiration. Error: %s", str(e)) proxy_path = None relay_info = None if is_arc: - proxy_path = _arc_get_client_side_proxy(arc_proxy_folder) - relay_info = _arc_list_access_details(cmd, resource_group_name, vm_name, certificate_validity_in_seconds) + proxy_path = arc_utils._arc_get_client_side_proxy(ssh_proxy_folder) + relay_info = arc_utils._arc_list_access_details(cmd, resource_group_name, vm_name, cert_lifetime) else: ssh_ip = ssh_ip or ip_utils.get_ssh_ip(cmd, resource_group_name, vm_name, use_private_ip) if not ssh_ip: @@ -161,18 +161,6 @@ def _do_ssh_op(cmd, vm_name, resource_group_name, ssh_ip, public_key_file, priva delete_cert, public_key_file) -def _get_certificate_validity_in_seconds(cert_file): - import datetime - validity_str = ssh_utils.get_ssh_cert_validity(cert_file) - validity = None - if validity_str and "Valid: from " in validity_str and " to " in validity_str: - times = validity_str.replace("Valid: from ", "").split(" to ") - t0 = datetime.datetime.fromisoformat(times[0]) - t1 = datetime.datetime.fromisoformat(times[1]) - validity = t1 - t0 - return int(validity.total_seconds()) - - def _get_and_write_certificate(cmd, public_key_file, cert_file): cloudtoscope = { "azurecloud": "https://pas.windows.net/CheckMyAccess/Linux/.default", @@ -322,132 +310,6 @@ def _get_modulus_exponent(public_key_file): return modulus, exponent -# Downloads client side proxy to connect to Arc Connectivity Platform -def _arc_get_client_side_proxy(arc_proxy_folder): - - request_uri, install_location, older_version_location = _get_proxy_filename_and_url(arc_proxy_folder) - install_dir = os.path.dirname(install_location) - - # Only download new proxy if it doesn't exist already - if not os.path.isfile(install_location): - t0 = time.time() - # download the executable - try: - with urllib.request.urlopen(request_uri) as response: - response_content = response.read() - response.close() - except Exception as e: - telemetry.set_exception(exception=e, fault_type=consts.PROXY_DOWNLOAD_FAILED_FAULT_TYPE, - summary=f'Failed to download proxy from {request_uri}') - raise azclierror.ClientRequestError(f"Failed to download client proxy executable from {request_uri}. " - "Error: " + str(e)) from e - time_elapsed = time.time() - t0 - - proxy_data = { - 'Context.Default.AzureCLI.SSHProxyDownloadTime': time_elapsed, - 'Context.Default.AzureCLI.SSHProxyVersion': consts.CLIENT_PROXY_VERSION - } - telemetry.add_extension_event('ssh', proxy_data) - - # if directory doesn't exist, create it - if not os.path.isdir(install_dir): - file_utils.create_directory(install_dir, f"Failed to create client proxy directory '{install_dir}'. ") - # if directory exists, delete any older versions of the proxy - else: - older_version_files = glob(older_version_location) - for f in older_version_files: - file_utils.delete_file(f, f"failed to delete older version file {f}", warning=True) - - # write executable in the install location - file_utils.write_to_file(install_location, 'wb', response_content, "Failed to create client proxy file. ") - os.chmod(install_location, os.stat(install_location).st_mode | stat.S_IXUSR) - - return install_location - - -def _get_proxy_filename_and_url(arc_proxy_folder): - import platform - operating_system = platform.system() - machine = platform.machine() - - logger.debug("Platform OS: %s", operating_system) - logger.debug("Platform architecture: %s", machine) - - if machine.endswith('64'): - architecture = 'amd64' - elif machine.endswith('86'): - architecture = '386' - elif machine == '': - raise azclierror.BadRequestError("Couldn't identify the platform architecture.") - else: - telemetry.set_exception(exception='Unsuported architecture for installing proxy', - fault_type=consts.PROXY_UNSUPPORTED_ARCH_FAULT_TYPE, - summary=f'{machine} is not supported for installing client proxy') - raise azclierror.BadRequestError(f"Unsuported architecture: {machine} is not currently supported") - - # define the request url and install location based on the os and architecture - proxy_name = f"sshProxy_{operating_system.lower()}_{architecture}" - request_uri = (f"{consts.CLIENT_PROXY_STORAGE_URL}/{consts.CLIENT_PROXY_RELEASE}" - f"/{proxy_name}_{consts.CLIENT_PROXY_VERSION}") - install_location = proxy_name + "_" + consts.CLIENT_PROXY_VERSION.replace('.', '_') - older_location = proxy_name + "*" - - if operating_system == 'Windows': - request_uri = request_uri + ".exe" - install_location = install_location + ".exe" - older_location = older_location + ".exe" - elif operating_system not in ('Linux', 'Darwin'): - telemetry.set_exception(exception='Unsuported OS for installing ssh client proxy', - fault_type=consts.PROXY_UNSUPPORTED_OS_FAULT_TYPE, - summary=f'{operating_system} is not supported for installing client proxy') - raise azclierror.BadRequestError(f"Unsuported OS: {operating_system} platform is not currently supported") - - if not arc_proxy_folder: - install_location = os.path.expanduser(os.path.join('~', os.path.join(".clientsshproxy", install_location))) - older_location = os.path.expanduser(os.path.join('~', os.path.join(".clientsshproxy", older_location))) - else: - install_location = os.path.join(arc_proxy_folder, install_location) - older_location = os.path.join(arc_proxy_folder, older_location) - - return request_uri, install_location, older_location - - -# Get the Access Details to connect to Arc Connectivity platform from the HybridConnectivity RP -def _arc_list_access_details(cmd, resource_group, vm_name, certificate_validity): - from azext_ssh._client_factory import cf_endpoint - client = cf_endpoint(cmd.cli_ctx) - try: - t0 = time.time() - if certificate_validity: - result = client.list_credentials(resource_group_name=resource_group, machine_name=vm_name, - endpoint_name="default", expiresin=certificate_validity) - else: - result = client.list_credentials(resource_group_name=resource_group, machine_name=vm_name, - endpoint_name="default") - time_elapsed = time.time() - t0 - telemetry.add_extension_event('ssh', {'Context.Default.AzureCLI.SSHListCredentialsTime': time_elapsed}) - except Exception as e: - telemetry.set_exception(exception='Call to listCredentials failed', - fault_type=consts.LIST_CREDENTIALS_FAILED_FAULT_TYPE, - summary=f'listCredentials failed with error: {str(e)}.') - raise azclierror.ClientRequestError(f"Request for Azure Relay Information Failed: {str(e)}") - - result_string = json.dumps( - { - "relay": { - "namespaceName": result.namespace_name, - "namespaceNameSuffix": result.namespace_name_suffix, - "hybridConnectionName": result.hybrid_connection_name, - "accessKey": result.access_key, - "expiresOn": result.expires_on - } - }) - result_bytes = result_string.encode("ascii") - enc = base64.b64encode(result_bytes) - base64_result_string = enc.decode("ascii") - return base64_result_string - - def _decide_op_call(cmd, resource_group_name, vm_name, ssh_ip, resource_type, config_path, overwrite, ssh_client_path, ssh_args, delete_credentials, credentials_folder, local_user): diff --git a/src/ssh/azext_ssh/ssh_utils.py b/src/ssh/azext_ssh/ssh_utils.py index 08168310354..7f2fc21ab73 100644 --- a/src/ssh/azext_ssh/ssh_utils.py +++ b/src/ssh/azext_ssh/ssh_utils.py @@ -8,6 +8,7 @@ import stat import multiprocessing as mp import time +import datetime import oschmod from knack import log @@ -15,6 +16,7 @@ from azure.cli.core import telemetry from . import file_utils +from . import arc_utils from . import constants as const logger = log.get_logger(__name__) @@ -34,7 +36,7 @@ def start_ssh_connection(relay_info, proxy_path, vm_name, ip, username, cert_fil env = os.environ.copy() if is_arc: - env['SSHPROXY_RELAY_INFO'] = relay_info + env['SSHPROXY_RELAY_INFO'] = arc_utils._arc_format_relay_info_string(relay_info) if port: pcommand = f"ProxyCommand={proxy_path} -p {port}" else: @@ -84,6 +86,26 @@ def get_ssh_cert_info(cert_file): return subprocess.check_output(command, shell=platform.system() == 'Windows').decode().splitlines() +def _get_ssh_cert_validity(cert_file): + if cert_file: + info = get_ssh_cert_info(cert_file) + for line in info: + if "Valid:" in line: + return line.strip() + return None + + +def _get_certificate_lifetime(cert_file): + validity_str = _get_ssh_cert_validity(cert_file) + lifetime = None + if validity_str and "Valid: from " in validity_str and " to " in validity_str: + times = validity_str.replace("Valid: from ", "").split(" to ") + t0 = datetime.datetime.fromisoformat(times[0]) + t1 = datetime.datetime.fromisoformat(times[1]) + lifetime = t1 - t0 + return lifetime + + def get_ssh_cert_principals(cert_file): info = get_ssh_cert_info(cert_file) principals = [] @@ -100,15 +122,6 @@ def get_ssh_cert_principals(cert_file): return principals -def get_ssh_cert_validity(cert_file): - if cert_file: - info = get_ssh_cert_info(cert_file) - for line in info: - if "Valid:" in line: - return line.strip() - return None - - def write_ssh_config(relay_info, proxy_path, vm_name, ip, username, cert_file, private_key_file, port, is_arc, delete_keys, delete_cert, _, config_path, overwrite, resource_group, credentials_folder): @@ -276,6 +289,7 @@ def _terminate_cleanup(delete_keys, delete_cert, delete_credentials, cleanup_pro def _prepare_relay_info_file(relay_info, credentials_folder, vm_name, resource_group): + # create the custom folder relay_info_dir = credentials_folder if not os.path.isdir(relay_info_dir): @@ -287,14 +301,22 @@ def _prepare_relay_info_file(relay_info, credentials_folder, vm_name, resource_g relay_info_path = os.path.join(relay_info_dir, relay_info_filename) # Overwrite relay_info if it already exists in that folder. file_utils.delete_file(relay_info_path, f"{relay_info_path} already exists, and couldn't be overwritten.") - file_utils.write_to_file(relay_info_path, 'w', relay_info, + file_utils.write_to_file(relay_info_path, 'w', arc_utils._arc_format_relay_info_string(relay_info), f"Couldn't write relay information to file {relay_info_path}.", 'utf-8') oschmod.set_mode(relay_info_path, stat.S_IRUSR) + #get expiration + expiration = datetime.datetime.fromtimestamp(relay_info.expires_on) + print(f"Generated file with Relay Information is valid for {expiration - datetime.datetime.now()}\n") + return relay_info_path, relay_info_filename def _issue_config_cleanup_warning(delete_cert, delete_keys, is_arc, cert_file, relay_info_filename, relay_info_path): + if delete_cert: + # Find a better way to format the delta time + print(f"Generated SSH certificate {cert_file} is valid for {_get_certificate_lifetime(cert_file)}.\n") + if delete_keys or delete_cert or is_arc: # Warn users to delete credentials once config file is no longer being used. # If user provided keys, only ask them to delete the certificate. @@ -316,15 +338,8 @@ def _issue_config_cleanup_warning(delete_cert, delete_keys, is_arc, cert_file, r path_to_delete = cert_file items_to_delete = "" - validity_warning = "" - if delete_cert: - validity = get_ssh_cert_validity(cert_file) - if validity: - validity_warning = f" {validity.lower()}" - - logger.warning("%s contains sensitive information%s%s\n" - "Please delete it once you no longer need this config file. ", - path_to_delete, items_to_delete, validity_warning) + print(f"{path_to_delete} contains sensitive information{items_to_delete}. " + "Please delete it once you no longer need this config file.\n") def _get_connection_status(log_file): @@ -335,3 +350,4 @@ def _get_connection_status(log_file): except: return False return match + From c767210714de365a0303a297a297ef6d59385950 Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Wed, 15 Dec 2021 15:32:16 -0500 Subject: [PATCH 10/18] Change the way we print the lifetime of the relay and certificate --- src/ssh/azext_ssh/ssh_utils.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/ssh/azext_ssh/ssh_utils.py b/src/ssh/azext_ssh/ssh_utils.py index 7f2fc21ab73..3d59add1b75 100644 --- a/src/ssh/azext_ssh/ssh_utils.py +++ b/src/ssh/azext_ssh/ssh_utils.py @@ -95,14 +95,22 @@ def _get_ssh_cert_validity(cert_file): return None -def _get_certificate_lifetime(cert_file): +def _get_certificate_start_and_end_times(cert_file): validity_str = _get_ssh_cert_validity(cert_file) - lifetime = None + times = None if validity_str and "Valid: from " in validity_str and " to " in validity_str: times = validity_str.replace("Valid: from ", "").split(" to ") t0 = datetime.datetime.fromisoformat(times[0]) t1 = datetime.datetime.fromisoformat(times[1]) - lifetime = t1 - t0 + times = (t0, t1) + return times + + +def _get_certificate_lifetime(cert_file): + times = _get_certificate_start_and_end_times(cert_file) + lifetime = None + if times: + lifetime = times[1] - times[0] return lifetime @@ -307,15 +315,14 @@ def _prepare_relay_info_file(relay_info, credentials_folder, vm_name, resource_g #get expiration expiration = datetime.datetime.fromtimestamp(relay_info.expires_on) - print(f"Generated file with Relay Information is valid for {expiration - datetime.datetime.now()}\n") + print(f"Generated file with Relay Information is valid until {expiration}.\n") return relay_info_path, relay_info_filename def _issue_config_cleanup_warning(delete_cert, delete_keys, is_arc, cert_file, relay_info_filename, relay_info_path): if delete_cert: - # Find a better way to format the delta time - print(f"Generated SSH certificate {cert_file} is valid for {_get_certificate_lifetime(cert_file)}.\n") + print(f"Generated SSH certificate {cert_file} is valid until {_get_certificate_start_and_end_times(cert_file)[1]}.\n") if delete_keys or delete_cert or is_arc: # Warn users to delete credentials once config file is no longer being used. From b3fddde8225993da7101a3f6ed65451e4493a96e Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Wed, 15 Dec 2021 15:43:40 -0500 Subject: [PATCH 11/18] Style check --- src/ssh/azext_ssh/arc_utils.py | 11 +++++------ src/ssh/azext_ssh/custom.py | 8 ++++---- src/ssh/azext_ssh/ssh_utils.py | 17 ++++++++--------- 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/ssh/azext_ssh/arc_utils.py b/src/ssh/azext_ssh/arc_utils.py index bf3d42b0a89..2ae8a9c4a73 100644 --- a/src/ssh/azext_ssh/arc_utils.py +++ b/src/ssh/azext_ssh/arc_utils.py @@ -20,8 +20,9 @@ logger = log.get_logger(__name__) + # Get the Access Details to connect to Arc Connectivity platform from the HybridConnectivity RP -def _arc_list_access_details(cmd, resource_group, vm_name, certificate_validity): +def arc_list_access_details(cmd, resource_group, vm_name, certificate_validity): from azext_ssh._client_factory import cf_endpoint client = cf_endpoint(cmd.cli_ctx) @@ -39,12 +40,12 @@ def _arc_list_access_details(cmd, resource_group, vm_name, certificate_validity) fault_type=consts.LIST_CREDENTIALS_FAILED_FAULT_TYPE, summary=f'listCredentials failed with error: {str(e)}.') raise azclierror.ClientRequestError(f"Request for Azure Relay Information Failed: {str(e)}") - + return result # Downloads client side proxy to connect to Arc Connectivity Platform -def _arc_get_client_side_proxy(arc_proxy_folder): +def arc_get_client_side_proxy(arc_proxy_folder): request_uri, install_location, older_version_location = _get_proxy_filename_and_url(arc_proxy_folder) install_dir = os.path.dirname(install_location) @@ -133,7 +134,7 @@ def _get_proxy_filename_and_url(arc_proxy_folder): return request_uri, install_location, older_location -def _arc_format_relay_info_string(relay_info): +def arc_format_relay_info_string(relay_info): relay_info_string = json.dumps( { "relay": { @@ -148,5 +149,3 @@ def _arc_format_relay_info_string(relay_info): enc = base64.b64encode(result_bytes) base64_result_string = enc.decode("ascii") return base64_result_string - - diff --git a/src/ssh/azext_ssh/custom.py b/src/ssh/azext_ssh/custom.py index 7e8947419e7..d188afaa176 100644 --- a/src/ssh/azext_ssh/custom.py +++ b/src/ssh/azext_ssh/custom.py @@ -82,7 +82,7 @@ def ssh_cert(cmd, cert_path=None, public_key_file=None): 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) try: - certificate_lifetime = ssh_utils._get_certificate_lifetime(cert_file) + certificate_lifetime = ssh_utils.get_certificate_lifetime(cert_file) print(f"Generated SSH certificate {cert_file} is valid for {certificate_lifetime}.") except Exception as e: logger.warning("Couldn't determine certificate validity. Error: %s", str(e)) @@ -139,7 +139,7 @@ def _do_ssh_op(cmd, vm_name, resource_group_name, ssh_ip, public_key_file, priva cert_file, username = _get_and_write_certificate(cmd, public_key_file, None) if is_arc: try: - cert_lifetime = ssh_utils._get_certificate_lifetime(cert_file).total_seconds() + cert_lifetime = ssh_utils.get_certificate_lifetime(cert_file).total_seconds() print(cert_lifetime) except Exception as e: logger.warning("Couldn't determine certificate expiration. Error: %s", str(e)) @@ -147,8 +147,8 @@ def _do_ssh_op(cmd, vm_name, resource_group_name, ssh_ip, public_key_file, priva proxy_path = None relay_info = None if is_arc: - proxy_path = arc_utils._arc_get_client_side_proxy(ssh_proxy_folder) - relay_info = arc_utils._arc_list_access_details(cmd, resource_group_name, vm_name, cert_lifetime) + proxy_path = arc_utils.arc_get_client_side_proxy(ssh_proxy_folder) + relay_info = arc_utils.arc_list_access_details(cmd, resource_group_name, vm_name, cert_lifetime) else: ssh_ip = ssh_ip or ip_utils.get_ssh_ip(cmd, resource_group_name, vm_name, use_private_ip) if not ssh_ip: diff --git a/src/ssh/azext_ssh/ssh_utils.py b/src/ssh/azext_ssh/ssh_utils.py index 3d59add1b75..7f4289d8be0 100644 --- a/src/ssh/azext_ssh/ssh_utils.py +++ b/src/ssh/azext_ssh/ssh_utils.py @@ -36,7 +36,7 @@ def start_ssh_connection(relay_info, proxy_path, vm_name, ip, username, cert_fil env = os.environ.copy() if is_arc: - env['SSHPROXY_RELAY_INFO'] = arc_utils._arc_format_relay_info_string(relay_info) + env['SSHPROXY_RELAY_INFO'] = arc_utils.arc_format_relay_info_string(relay_info) if port: pcommand = f"ProxyCommand={proxy_path} -p {port}" else: @@ -106,7 +106,7 @@ def _get_certificate_start_and_end_times(cert_file): return times -def _get_certificate_lifetime(cert_file): +def get_certificate_lifetime(cert_file): times = _get_certificate_start_and_end_times(cert_file) lifetime = None if times: @@ -297,7 +297,6 @@ def _terminate_cleanup(delete_keys, delete_cert, delete_credentials, cleanup_pro def _prepare_relay_info_file(relay_info, credentials_folder, vm_name, resource_group): - # create the custom folder relay_info_dir = credentials_folder if not os.path.isdir(relay_info_dir): @@ -309,21 +308,22 @@ def _prepare_relay_info_file(relay_info, credentials_folder, vm_name, resource_g relay_info_path = os.path.join(relay_info_dir, relay_info_filename) # Overwrite relay_info if it already exists in that folder. file_utils.delete_file(relay_info_path, f"{relay_info_path} already exists, and couldn't be overwritten.") - file_utils.write_to_file(relay_info_path, 'w', arc_utils._arc_format_relay_info_string(relay_info), + file_utils.write_to_file(relay_info_path, 'w', arc_utils.arc_format_relay_info_string(relay_info), f"Couldn't write relay information to file {relay_info_path}.", 'utf-8') oschmod.set_mode(relay_info_path, stat.S_IRUSR) - #get expiration + # Print the expiration of the relay information f expiration = datetime.datetime.fromtimestamp(relay_info.expires_on) - print(f"Generated file with Relay Information is valid until {expiration}.\n") + print(f"Generated file with Relay Information {relay_info_path} is valid until {expiration}.\n") return relay_info_path, relay_info_filename def _issue_config_cleanup_warning(delete_cert, delete_keys, is_arc, cert_file, relay_info_filename, relay_info_path): if delete_cert: - print(f"Generated SSH certificate {cert_file} is valid until {_get_certificate_start_and_end_times(cert_file)[1]}.\n") - + print(f"Generated SSH certificate {cert_file} is valid until", + f"{_get_certificate_start_and_end_times(cert_file)[1]}.\n") + if delete_keys or delete_cert or is_arc: # Warn users to delete credentials once config file is no longer being used. # If user provided keys, only ask them to delete the certificate. @@ -357,4 +357,3 @@ def _get_connection_status(log_file): except: return False return match - From a96c59fea58bb2ba06cfb9bb5fd690968b7e596a Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Wed, 15 Dec 2021 17:05:25 -0500 Subject: [PATCH 12/18] Fix error with IP as parameter --- src/ssh/azext_ssh/custom.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ssh/azext_ssh/custom.py b/src/ssh/azext_ssh/custom.py index d188afaa176..7fd6bf45651 100644 --- a/src/ssh/azext_ssh/custom.py +++ b/src/ssh/azext_ssh/custom.py @@ -140,7 +140,6 @@ def _do_ssh_op(cmd, vm_name, resource_group_name, ssh_ip, public_key_file, priva if is_arc: try: cert_lifetime = ssh_utils.get_certificate_lifetime(cert_file).total_seconds() - print(cert_lifetime) except Exception as e: logger.warning("Couldn't determine certificate expiration. Error: %s", str(e)) @@ -367,7 +366,7 @@ def _decide_op_call(cmd, resource_group_name, vm_name, ssh_ip, resource_type, co os_type = arc.properties.os_name # Note 2: This is a temporary check while AAD login is not enabled for Windows. - if os_type.lower() == 'windows' and not local_user: + if os_type and os_type.lower() == 'windows' and not local_user: raise azclierror.RequiredArgumentMissingError("SSH Login to AAD user is not currently supported for Windows. " "Please provide --local-user.") From 1b7e3eae67105f95aabe47db482d3c45aadf4a61 Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Wed, 15 Dec 2021 18:08:15 -0500 Subject: [PATCH 13/18] datetime.fromisoformat() fails for python lower than 3.7 --- src/ssh/azext_ssh/ssh_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ssh/azext_ssh/ssh_utils.py b/src/ssh/azext_ssh/ssh_utils.py index 7f4289d8be0..1db4353b875 100644 --- a/src/ssh/azext_ssh/ssh_utils.py +++ b/src/ssh/azext_ssh/ssh_utils.py @@ -100,8 +100,8 @@ def _get_certificate_start_and_end_times(cert_file): times = None if validity_str and "Valid: from " in validity_str and " to " in validity_str: times = validity_str.replace("Valid: from ", "").split(" to ") - t0 = datetime.datetime.fromisoformat(times[0]) - t1 = datetime.datetime.fromisoformat(times[1]) + t0 = datetime.datetime.strptime(times[0], '%Y-%m-%dT%X') + t1 = datetime.datetime.strptime(times[1], '%Y-%m-%dT%X') times = (t0, t1) return times From 679cd1e509be3e9d7ac20243fdd95c98b301b0d7 Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Thu, 16 Dec 2021 10:21:05 -0500 Subject: [PATCH 14/18] Changes based on review comments --- src/ssh/azext_ssh/_params.py | 6 +++--- .../{arc_utils.py => connectivity_utils.py} | 12 ++++++------ src/ssh/azext_ssh/constants.py | 2 ++ src/ssh/azext_ssh/custom.py | 15 +++++++-------- src/ssh/azext_ssh/ssh_utils.py | 10 +++++----- 5 files changed, 23 insertions(+), 22 deletions(-) rename src/ssh/azext_ssh/{arc_utils.py => connectivity_utils.py} (94%) diff --git a/src/ssh/azext_ssh/_params.py b/src/ssh/azext_ssh/_params.py index 14b88eecde6..f4c0856090f 100644 --- a/src/ssh/azext_ssh/_params.py +++ b/src/ssh/azext_ssh/_params.py @@ -28,7 +28,7 @@ def load_arguments(self, _): 'SSH login experience in Cloud shell.'), deprecate_info=c.deprecate(hide=True), action='store_true') c.argument('ssh_proxy_folder', options_list=['--ssh-proxy-folder'], - help=('Path to the folder where the arc proxy should be saved. ' + help=('Path to the folder where the ssh proxy should be saved. ' 'Default to .clientsshproxy folder in user\'s home directory if not provided.')) c.positional('ssh_args', nargs='*', help='Additional arguments passed to OpenSSH') @@ -51,7 +51,7 @@ def load_arguments(self, _): help='Resource type should be either Microsoft.Compute or Microsoft.HybridCompute') c.argument('cert_file', options_list=['--certificate-file', '-c'], help='Path to certificate file') c.argument('ssh_proxy_folder', options_list=['--ssh-proxy-folder'], - help=('Path to the folder where the arc proxy should be saved. ' + help=('Path to the folder where the ssh proxy should be saved. ' 'Default to .clientsshproxy folder in user\'s home directory if not provided.')) with self.argument_context('ssh cert') as c: @@ -76,6 +76,6 @@ def load_arguments(self, _): 'SSH login experience in Cloud shell.'), deprecate_info=c.deprecate(hide=True), action='store_true') c.argument('ssh_proxy_folder', options_list=['--ssh-proxy-folder'], - help=('Path to the folder where the arc proxy should be saved. ' + help=('Path to the folder where the ssh proxy should be saved. ' 'Default to .clientsshproxy folder in user\'s home directory if not provided.')) c.positional('ssh_args', nargs='*', help='Additional arguments passed to OpenSSH') diff --git a/src/ssh/azext_ssh/arc_utils.py b/src/ssh/azext_ssh/connectivity_utils.py similarity index 94% rename from src/ssh/azext_ssh/arc_utils.py rename to src/ssh/azext_ssh/connectivity_utils.py index 2ae8a9c4a73..d021a23728a 100644 --- a/src/ssh/azext_ssh/arc_utils.py +++ b/src/ssh/azext_ssh/connectivity_utils.py @@ -22,17 +22,17 @@ # Get the Access Details to connect to Arc Connectivity platform from the HybridConnectivity RP -def arc_list_access_details(cmd, resource_group, vm_name, certificate_validity): +def get_relay_information(cmd, resource_group, vm_name, certificate_validity_in_seconds): from azext_ssh._client_factory import cf_endpoint client = cf_endpoint(cmd.cli_ctx) - if not certificate_validity or certificate_validity > 3600: - certificate_validity = 3600 + if not certificate_validity_in_seconds or certificate_validity_in_seconds > 3600: + certificate_validity_in_seconds = consts.RELAY_INFO_MAXIMUM_DURATION_IN_SECONDS try: t0 = time.time() result = client.list_credentials(resource_group_name=resource_group, machine_name=vm_name, - endpoint_name="default", expiresin=certificate_validity) + endpoint_name="default", expiresin=certificate_validity_in_seconds) time_elapsed = time.time() - t0 telemetry.add_extension_event('ssh', {'Context.Default.AzureCLI.SSHListCredentialsTime': time_elapsed}) except Exception as e: @@ -45,7 +45,7 @@ def arc_list_access_details(cmd, resource_group, vm_name, certificate_validity): # Downloads client side proxy to connect to Arc Connectivity Platform -def arc_get_client_side_proxy(arc_proxy_folder): +def get_client_side_proxy(arc_proxy_folder): request_uri, install_location, older_version_location = _get_proxy_filename_and_url(arc_proxy_folder) install_dir = os.path.dirname(install_location) @@ -134,7 +134,7 @@ def _get_proxy_filename_and_url(arc_proxy_folder): return request_uri, install_location, older_location -def arc_format_relay_info_string(relay_info): +def format_relay_info_string(relay_info): relay_info_string = json.dumps( { "relay": { diff --git a/src/ssh/azext_ssh/constants.py b/src/ssh/azext_ssh/constants.py index 1731ec2df75..b2f364670f2 100644 --- a/src/ssh/azext_ssh/constants.py +++ b/src/ssh/azext_ssh/constants.py @@ -9,7 +9,9 @@ CLEANUP_TOTAL_TIME_LIMIT_IN_SECONDS = 120 CLEANUP_TIME_INTERVAL_IN_SECONDS = 10 CLEANUP_AWAIT_TERMINATION_IN_SECONDS = 30 +RELAY_INFO_MAXIMUM_DURATION_IN_SECONDS = 3600 PROXY_UNSUPPORTED_ARCH_FAULT_TYPE = 'client-proxy-unsupported-architecture-error' PROXY_UNSUPPORTED_OS_FAULT_TYPE = 'client-proxy-unsupported-os-error' PROXY_DOWNLOAD_FAILED_FAULT_TYPE = 'client-proxy-download-failed-error' LIST_CREDENTIALS_FAILED_FAULT_TYPE = 'get-relay-information-failed-error' + diff --git a/src/ssh/azext_ssh/custom.py b/src/ssh/azext_ssh/custom.py index 7fd6bf45651..2175281e4c2 100644 --- a/src/ssh/azext_ssh/custom.py +++ b/src/ssh/azext_ssh/custom.py @@ -18,7 +18,7 @@ from . import ip_utils from . import rsa_parser from . import ssh_utils -from . import arc_utils +from . import connectivity_utils logger = log.get_logger(__name__) @@ -82,8 +82,8 @@ def ssh_cert(cmd, cert_path=None, public_key_file=None): 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) try: - certificate_lifetime = ssh_utils.get_certificate_lifetime(cert_file) - print(f"Generated SSH certificate {cert_file} is valid for {certificate_lifetime}.") + cert_expiration = ssh_utils.get_certificate_start_and_end_times(cert_file)[1] + print(f"Generated SSH certificate {cert_file} is valid until {cert_expiration}.") except Exception as e: logger.warning("Couldn't determine certificate validity. Error: %s", str(e)) print(cert_file + "\n") @@ -146,15 +146,14 @@ def _do_ssh_op(cmd, vm_name, resource_group_name, ssh_ip, public_key_file, priva proxy_path = None relay_info = None if is_arc: - proxy_path = arc_utils.arc_get_client_side_proxy(ssh_proxy_folder) - relay_info = arc_utils.arc_list_access_details(cmd, resource_group_name, vm_name, cert_lifetime) + proxy_path = connectivity_utils.get_client_side_proxy(ssh_proxy_folder) + relay_info = connectivity_utils.get_relay_information(cmd, resource_group_name, vm_name, cert_lifetime) else: ssh_ip = ssh_ip or ip_utils.get_ssh_ip(cmd, resource_group_name, vm_name, use_private_ip) if not ssh_ip: if not use_private_ip: - raise azclierror.ResourceNotFoundError(f"VM '{vm_name}' does not have a public IP address to SSH to") - raise azclierror.ResourceNotFoundError(f"VM '{vm_name}' does not have a public or private IP address to" - "SSH to") + raise azclierror.ResourceNotFoundError(f"VM '{vm_name}' does not have a public IP address to SSH to.") + raise azclierror.ResourceNotFoundError("Internal Error. Couldn't determine the IP address.") op_call(relay_info, proxy_path, vm_name, ssh_ip, username, cert_file, private_key_file, port, is_arc, delete_keys, delete_cert, public_key_file) diff --git a/src/ssh/azext_ssh/ssh_utils.py b/src/ssh/azext_ssh/ssh_utils.py index 1db4353b875..b8a12a40b18 100644 --- a/src/ssh/azext_ssh/ssh_utils.py +++ b/src/ssh/azext_ssh/ssh_utils.py @@ -16,7 +16,7 @@ from azure.cli.core import telemetry from . import file_utils -from . import arc_utils +from . import connectivity_utils from . import constants as const logger = log.get_logger(__name__) @@ -36,7 +36,7 @@ def start_ssh_connection(relay_info, proxy_path, vm_name, ip, username, cert_fil env = os.environ.copy() if is_arc: - env['SSHPROXY_RELAY_INFO'] = arc_utils.arc_format_relay_info_string(relay_info) + env['SSHPROXY_RELAY_INFO'] = connectivity_utils.format_relay_info_string(relay_info) if port: pcommand = f"ProxyCommand={proxy_path} -p {port}" else: @@ -95,7 +95,7 @@ def _get_ssh_cert_validity(cert_file): return None -def _get_certificate_start_and_end_times(cert_file): +def get_certificate_start_and_end_times(cert_file): validity_str = _get_ssh_cert_validity(cert_file) times = None if validity_str and "Valid: from " in validity_str and " to " in validity_str: @@ -107,7 +107,7 @@ def _get_certificate_start_and_end_times(cert_file): def get_certificate_lifetime(cert_file): - times = _get_certificate_start_and_end_times(cert_file) + times = get_certificate_start_and_end_times(cert_file) lifetime = None if times: lifetime = times[1] - times[0] @@ -308,7 +308,7 @@ def _prepare_relay_info_file(relay_info, credentials_folder, vm_name, resource_g relay_info_path = os.path.join(relay_info_dir, relay_info_filename) # Overwrite relay_info if it already exists in that folder. file_utils.delete_file(relay_info_path, f"{relay_info_path} already exists, and couldn't be overwritten.") - file_utils.write_to_file(relay_info_path, 'w', arc_utils.arc_format_relay_info_string(relay_info), + file_utils.write_to_file(relay_info_path, 'w', connectivity_utils.format_relay_info_string(relay_info), f"Couldn't write relay information to file {relay_info_path}.", 'utf-8') oschmod.set_mode(relay_info_path, stat.S_IRUSR) From cd3829a0d70dbeecf6c6370fb1783c7472d698b4 Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Thu, 16 Dec 2021 10:34:04 -0500 Subject: [PATCH 15/18] Fix config console messages --- src/ssh/azext_ssh/ssh_utils.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/ssh/azext_ssh/ssh_utils.py b/src/ssh/azext_ssh/ssh_utils.py index b8a12a40b18..199fab36a9b 100644 --- a/src/ssh/azext_ssh/ssh_utils.py +++ b/src/ssh/azext_ssh/ssh_utils.py @@ -312,7 +312,7 @@ def _prepare_relay_info_file(relay_info, credentials_folder, vm_name, resource_g f"Couldn't write relay information to file {relay_info_path}.", 'utf-8') oschmod.set_mode(relay_info_path, stat.S_IRUSR) - # Print the expiration of the relay information f + # Print the expiration of the relay information expiration = datetime.datetime.fromtimestamp(relay_info.expires_on) print(f"Generated file with Relay Information {relay_info_path} is valid until {expiration}.\n") @@ -322,7 +322,7 @@ def _prepare_relay_info_file(relay_info, credentials_folder, vm_name, resource_g def _issue_config_cleanup_warning(delete_cert, delete_keys, is_arc, cert_file, relay_info_filename, relay_info_path): if delete_cert: print(f"Generated SSH certificate {cert_file} is valid until", - f"{_get_certificate_start_and_end_times(cert_file)[1]}.\n") + f"{get_certificate_start_and_end_times(cert_file)[1]}.\n") if delete_keys or delete_cert or is_arc: # Warn users to delete credentials once config file is no longer being used. @@ -332,9 +332,8 @@ def _issue_config_cleanup_warning(delete_cert, delete_keys, is_arc, cert_file, r path_to_delete = os.path.dirname(cert_file) items_to_delete = f" (id_rsa, id_rsa.pub, id_rsa.pub-aadcert.pub, {relay_info_filename})" elif delete_cert: - path_to_delete = os.path.dirname(cert_file) - relay_partial_path = relay_info_path.replace(path_to_delete, "") - items_to_delete = f" (id_rsa.pub-aadcert.pub, {relay_partial_path})" + path_to_delete = f"{cert_file} and {relay_info_path}" + items_to_delete = "" else: path_to_delete = relay_info_path items_to_delete = "" @@ -345,7 +344,7 @@ def _issue_config_cleanup_warning(delete_cert, delete_keys, is_arc, cert_file, r path_to_delete = cert_file items_to_delete = "" - print(f"{path_to_delete} contains sensitive information{items_to_delete}. " + print(f"{path_to_delete} contain sensitive information{items_to_delete}. " "Please delete it once you no longer need this config file.\n") From 19a621abef04246a4364df7578f6742bfc438b04 Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Thu, 16 Dec 2021 20:42:19 -0500 Subject: [PATCH 16/18] Use constant for validity --- src/ssh/azext_ssh/connectivity_utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ssh/azext_ssh/connectivity_utils.py b/src/ssh/azext_ssh/connectivity_utils.py index d021a23728a..1b6402a1c66 100644 --- a/src/ssh/azext_ssh/connectivity_utils.py +++ b/src/ssh/azext_ssh/connectivity_utils.py @@ -26,7 +26,8 @@ def get_relay_information(cmd, resource_group, vm_name, certificate_validity_in_ from azext_ssh._client_factory import cf_endpoint client = cf_endpoint(cmd.cli_ctx) - if not certificate_validity_in_seconds or certificate_validity_in_seconds > 3600: + if not certificate_validity_in_seconds or \ + certificate_validity_in_seconds > consts.RELAY_INFO_MAXIMUM_DURATION_IN_SECONDS: certificate_validity_in_seconds = consts.RELAY_INFO_MAXIMUM_DURATION_IN_SECONDS try: From 01fdf2bce59afa07cce6249d4e7ed3aa7d7add8e Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Thu, 16 Dec 2021 20:46:44 -0500 Subject: [PATCH 17/18] Change message for resource not found error --- src/ssh/azext_ssh/custom.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ssh/azext_ssh/custom.py b/src/ssh/azext_ssh/custom.py index 2175281e4c2..6c34dc8e814 100644 --- a/src/ssh/azext_ssh/custom.py +++ b/src/ssh/azext_ssh/custom.py @@ -351,8 +351,7 @@ def _decide_op_call(cmd, resource_group_name, vm_name, ssh_ip, resource_type, co if not is_azure_vm and not is_arc_server: if isinstance(arc_error, ResourceNotFoundError) and isinstance(vm_error, ResourceNotFoundError): raise azclierror.ResourceNotFoundError(f"The resource {vm_name} in the resource group " - f"{resource_group_name} was not found. Errors:\n" - f"{str(arc_error)}\n{str(vm_error)}") + f"{resource_group_name} was not found.") raise azclierror.BadRequestError("Unable to determine the target machine type as Azure VM or " f"Arc Server. Errors:\n{str(arc_error)}\n{str(vm_error)}") From b01b7287af86c3b46fce2cc17c276e50df86b898 Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Fri, 17 Dec 2021 10:55:40 -0500 Subject: [PATCH 18/18] Change format of expiration date from military time to standard time --- src/ssh/azext_ssh/ssh_utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ssh/azext_ssh/ssh_utils.py b/src/ssh/azext_ssh/ssh_utils.py index 199fab36a9b..b3e4bdcac14 100644 --- a/src/ssh/azext_ssh/ssh_utils.py +++ b/src/ssh/azext_ssh/ssh_utils.py @@ -314,6 +314,7 @@ def _prepare_relay_info_file(relay_info, credentials_folder, vm_name, resource_g # Print the expiration of the relay information expiration = datetime.datetime.fromtimestamp(relay_info.expires_on) + expiration = expiration.strftime("%Y-%m-%d %I:%M:%S %p") print(f"Generated file with Relay Information {relay_info_path} is valid until {expiration}.\n") return relay_info_path, relay_info_filename @@ -321,8 +322,10 @@ def _prepare_relay_info_file(relay_info, credentials_folder, vm_name, resource_g def _issue_config_cleanup_warning(delete_cert, delete_keys, is_arc, cert_file, relay_info_filename, relay_info_path): if delete_cert: + expiration = get_certificate_start_and_end_times(cert_file)[1] + expiration = expiration.strftime("%Y-%m-%d %I:%M:%S %p") print(f"Generated SSH certificate {cert_file} is valid until", - f"{get_certificate_start_and_end_times(cert_file)[1]}.\n") + f"{expiration}.\n") if delete_keys or delete_cert or is_arc: # Warn users to delete credentials once config file is no longer being used.