Dev: bootstrap: use ssh agent by default

codecov.yml

@@ -8,7 +8,7 @@ coverage:
         threshold: 0.35%
 codecov:
   notify:
-    after_n_builds: 29
+    after_n_builds: 30
 comment:
-  after_n_builds: 29
+  after_n_builds: 30
   layout: "condensed_header, flags, files, condensed_footer"

crmsh/config.py

@@ -240,7 +240,7 @@ def get(self, value):
         'pager': opt_program('PAGER', ('less', 'more', 'pg')),
         'user': opt_string(''),
         'hosts': opt_list([]), # 'alice@host1, bob@host2'
-        'no_generating_ssh_key': opt_boolean('no'),
+        'no_generating_ssh_key': opt_boolean('no'), # deprecated
         'skill_level': opt_choice('expert', ('operator', 'administrator', 'expert')),
         'sort_elements': opt_boolean('yes'),
         'check_frequency': opt_choice('always', ('always', 'on-verify', 'never')),

crmsh/prun/prun.py

@@ -12,7 +12,7 @@
 
 _DEFAULT_CONCURRENCY = 32
 
-_SUDO_SFTP_SERVER = 'sudo PATH=/usr/lib/ssh:/usr/lib/openssh:/usr/libexec/ssh:/usr/libexec/openssh /bin/sh -c "exec sftp-server"'
+_SUDO_SFTP_SERVER = 'sudo --preserve-env=SSH_AUTH_SOCK PATH=/usr/lib/ssh:/usr/lib/openssh:/usr/libexec/ssh:/usr/libexec/openssh /bin/sh -c "exec sftp-server"'
 
 
 class ProcessResult:
@@ -117,11 +117,11 @@ def _build_run_task(remote: str, cmdline: str) -> Task:
         )
     else:
         local_sudoer, remote_sudoer = UserOfHost.instance().user_pair_for_ssh(remote)
-        shell = 'ssh {} {}@{} sudo -H /bin/sh'.format(crmsh.constants.SSH_OPTION, remote_sudoer, remote)
+        shell = 'ssh -A {} {}@{} sudo -H /bin/sh'.format(crmsh.constants.SSH_OPTION, remote_sudoer, remote)
         if local_sudoer == crmsh.userdir.getuser():
             args = ['/bin/sh', '-c', shell]
         elif os.geteuid() == 0:
-            args = ['su', local_sudoer, '--login', '-c', shell]
+            args = ['su', local_sudoer, '--login', '-c', shell, '-w', 'SSH_AUTH_SOCK']
         else:
             raise AssertionError('trying to run su as a non-root user')
         return Task(
@@ -171,7 +171,7 @@ def pcopy_to_remote(
             ssh = tempfile.NamedTemporaryFile('w', encoding='utf-8', delete=False)
             os.fchmod(ssh.fileno(), 0o700)
             ssh.write(f'''#!/bin/sh
-exec sudo -u {local_sudoer} ssh "$@"''')
+exec sudo --preserve-env=SSH_AUTH_SOCK -u {local_sudoer} ssh "$@"''')
         # It is necessary to close the file before executing, or we will get an EBUSY.
             ssh.close()
             tasks = [_build_copy_task("-S '{}'".format(ssh.name), script, host) for host in hosts]
@@ -233,7 +233,7 @@ def pfetch_from_remote(
             ssh = tempfile.NamedTemporaryFile('w', encoding='utf-8', delete=False)
             os.fchmod(ssh.fileno(), 0o700)
             ssh.write(f'''#!/bin/sh
-    exec sudo -u {local_sudoer} ssh "$@"''')
+    exec sudo --preserve-env=SSH_AUTH_SOCK -u {local_sudoer} ssh "$@"''')
             # It is necessary to close the file before executing
             ssh.close()
             tasks = [_build_fetch_task("-S '{}'".format(ssh.name), host, src, dst, flags) for host in hosts]

crmsh/prun/runner.py

@@ -37,6 +37,9 @@
         # Caller can pass arbitrary data to context, it is kept untouched.
         self.context = context
 
+    def __repr__(self):
+        return f"Task({self.args}, {self.input}, {self.stdout_config}, {self.stderr_config}, {self.context})"
+
 
 class Runner:
     def __init__(self, concurrency):

crmsh/report/core.py

@@ -406,12 +406,6 @@ def find_ssh_user(context: Context) -> None:
         if ret is not None:
             logger.debug("passwordless ssh to %s is OK", n)
             context.passwordless_shell_for_nodes[n] = ret
-        elif user_of_host.use_ssh_agent() and 'SSH_AUTH_SOCK' not in os.environ:
-            with StringIO() as buf:
-                buf.write('Environment variable SSH_AUTH_SOCK does not exist.')
-                if 'SUDO_USER' in os.environ:
-                    buf.write(' Please check whether ssh-agent is available and consider using "sudo --preserve-env=SSH_AUTH_SOCK".')
-                logger.warning('%s', buf.getvalue())
         else:
             logger.warning("passwordless ssh to node %s does not work", n)
             if not crmutils.can_ask():

crmsh/sh.py

@@ -57,13 +57,7 @@ def __init__(self, cmd: str, host: typing.Optional[str], user: str, msg: str):
         self.user = user
 
     def diagnose(self) -> str:
-        with StringIO() as buf:
-            if user_of_host.instance().use_ssh_agent():
-                if 'SSH_AUTH_SOCK' not in os.environ:
-                    buf.write('Environment variable SSH_AUTH_SOCK does not exist.')
-                    if 'SUDO_USER' in os.environ:
-                        buf.write(' Please check whether ssh-agent is available and consider using "sudo --preserve-env=SSH_AUTH_SOCK".')
-            return buf.getvalue()
+        return ''
 
 
 class NonInteractiveSSHAuthorizationError(AuthorizationError):
@@ -511,5 +505,10 @@ def subprocess_run_without_input(self, host: str, user: typing.Optional[str], cm
 
 
 def cluster_shell():
-    return ClusterShell(LocalShell(), user_of_host.instance(), raise_ssh_error=True)
+    return ClusterShell(
+        LocalShell(additional_environ={'SSH_AUTH_SOCK': os.environ.get('SSH_AUTH_SOCK', '')}),
+        user_of_host.instance(),
+        forward_ssh_agent=True,
+        raise_ssh_error=True,
+    )
 

crmsh/ssh_key.py

@@ -48,11 +48,15 @@
     def public_key(self) -> str:
         raise NotImplementedError
 
+    def fingerprint(self) -> str:
+        raise NotImplementedError
+
 
 class KeyFile(Key):
     def __init__(self, path: str):
         self._path = os.path.realpath(path)
         self._public_key = None
+        self._fingerprint = None
 
     def public_key_file(self) -> typing.Optional[str]:
         return self._path
@@ -65,6 +69,21 @@
                 self._public_key = f.read().strip()
             return self._public_key
 
+    def fingerprint(self) -> str:
+        if self._fingerprint:
+            return self._fingerprint
+        else:
+            result = subprocess.run(
+                ['ssh-keygen', '-l', '-f', self.public_key_file()],
+                stdin=subprocess.DEVNULL,
+                stdout=subprocess.PIPE,
+            )
+            if result.returncode == 0:
+                self._fingerprint = result.stdout.decode('utf-8', 'backslashreplace').strip()
+                return self._fingerprint
+            else:
+                raise ValueError(f'Failed to generate fingerprint: {result.returncode}.')
+
     def __eq__(self, other):
         return isinstance(other, KeyFile) and self._path == other._path and self.public_key() == other.public_key()
 
@@ -75,10 +94,27 @@
 class InMemoryPublicKey(Key):
     def __init__(self, content: str):
         self.content = content.strip()
+        self._fingerprint = None
 
     def public_key(self) -> str:
         return self.content
 
+    def fingerprint(self) -> str:
+        if self._fingerprint:
+            return self._fingerprint
+        else:
+            child = subprocess.Popen(
+                ['ssh-keygen', '-l', '-f', '/dev/stdin'],
+                stdin=subprocess.PIPE,
+                stdout=subprocess.PIPE,
+            )
+            stdout, _ = child.communicate(self.public_key().encode('utf-8'))
+            if child.returncode == 0:
+                self._fingerprint = stdout.decode('utf-8', 'backslashreplace').strip()
+                return self._fingerprint
+            else:
+                raise ValueError(f'Failed to generate fingerprint: {child.returncode}.')
+
     def __eq__(self, other):
         return isinstance(other, InMemoryPublicKey) and self.content == other.content
 
@@ -168,7 +204,9 @@
             self.socket_path = None
         else:
             self.socket_path = socket_path
-        self.shell = sh.LocalShell(additional_environ={'SSH_AUTH_SOCK': self.socket_path} if self.socket_path else None)
+        self.shell = sh.LocalShell(
+            additional_environ={'SSH_AUTH_SOCK': self.socket_path} if self.socket_path is not None else None,
+        )
 
     def list(self) -> typing.List[Key]:
         cmd = 'ssh-add -L'
@@ -274,7 +312,7 @@
         host: typing.Optional[str],
         user: str,
         generate_key_pair: bool = False
-) -> typing.List[str]:
+) -> typing.List[KeyFile]:
     """
     Fetch the public key file list for the specified user on the specified host.
 
@@ -294,7 +332,7 @@
     if not public_keys:
         host_str = f'@{host}' if host else ' locally'
         raise Error(f'No public key file found for {user}{host_str}')
-    return public_keys
+    return [KeyFile(p) for p in public_keys]
 
 
 def fetch_public_key_content_list(

crmsh/ui_cluster.py

@@ -410,8 +410,8 @@ def do_init(self, context, *args):
                             help="Use the given watchdog device or driver name")
         parser.add_argument("-x", "--skip-csync2-sync", dest="skip_csync2", action="store_true",
                             help="Skip csync2 initialization (an experimental option)")
-        parser.add_argument('--use-ssh-agent', action='store_true', dest='use_ssh_agent',
-                            help="Use an existing key from ssh-agent instead of creating new key pairs")
+        parser.add_argument('--use-ssh-agent', action=argparse.BooleanOptionalAction, dest='use_ssh_agent', default=True,
+                            help="Try to use an existing key from ssh-agent (default)")
 
         network_group = parser.add_argument_group("Network configuration", "Options for configuring the network and messaging layer.")
         network_group.add_argument("-i", "--interface", dest="nic_addr_list", metavar="IF", action=CustomAppendAction, default=[], help=constants.INTERFACE_HELP)
@@ -513,8 +513,8 @@ def do_join(self, context, *args):
         parser.add_argument("-h", "--help", action="store_true", dest="help", help="Show this help message")
         parser.add_argument("-q", "--quiet", help="Be quiet (don't describe what's happening, just do it)", action="store_true", dest="quiet")
         parser.add_argument("-y", "--yes", help='Answer "yes" to all prompts (use with caution)', action="store_true", dest="yes_to_all")
-        parser.add_argument('--use-ssh-agent', action='store_true', dest='use_ssh_agent',
-                            help="Use an existing key from ssh-agent instead of creating new key pairs")
+        parser.add_argument('--use-ssh-agent', action=argparse.BooleanOptionalAction, dest='use_ssh_agent', default=True,
+                            help="Try to use an existing key from ssh-agent (default)")
 
         network_group = parser.add_argument_group("Network configuration", "Options for configuring the network and messaging layer.")
         network_group.add_argument(
@@ -729,8 +729,8 @@ def do_geo_join(self, context, *args):
         parser.add_argument("-y", "--yes", help='Answer "yes" to all prompts (use with caution)', action="store_true", dest="yes_to_all")
         parser.add_argument("-c", "--cluster-node", metavar="[USER@]HOST", help="An already-configured geo cluster or arbitrator", dest="cluster_node")
         parser.add_argument("-s", "--clusters", help="Geo cluster description (see geo-init for details)", dest="clusters", metavar="DESC")
-        parser.add_argument('--use-ssh-agent', action='store_true', dest='use_ssh_agent',
-                            help="Use an existing key from ssh-agent instead of creating new key pairs")
+        parser.add_argument('--use-ssh-agent', action=argparse.BooleanOptionalAction, dest='use_ssh_agent', default=True,
+                            help="Try to use an existing key from ssh-agent (default)")
         options, args = parse_options(parser, args)
         if options is None or args is None:
             return
@@ -768,8 +768,8 @@ def do_geo_init_arbitrator(self, context, *args):
         parser.add_argument("-q", "--quiet", help="Be quiet (don't describe what's happening, just do it)", action="store_true", dest="quiet")
         parser.add_argument("-y", "--yes", help='Answer "yes" to all prompts (use with caution)', action="store_true", dest="yes_to_all")
         parser.add_argument("-c", "--cluster-node", metavar="[USER@]HOST", help="An already-configured geo cluster", dest="cluster_node")
-        parser.add_argument('--use-ssh-agent', action='store_true', dest='use_ssh_agent',
-                            help="Use an existing key from ssh-agent instead of creating new key pairs")
+        parser.add_argument('--use-ssh-agent', action=argparse.BooleanOptionalAction, dest='use_ssh_agent', default=True,
+                            help="Try to use an existing key from ssh-agent (default)")
         options, args = parse_options(parser, args)
         if options is None or args is None:
             return

crmsh/user_of_host.py

@@ -51,7 +51,7 @@ def user_pair_for_ssh(self, host: str) -> typing.Tuple[str, str]:
         local_user = None
         remote_user = None
         try:
-            local_user = 'root' if self.use_ssh_agent() else self.user_of(self.this_node())
+            local_user = self.user_of(self.this_node())
             remote_user = self.user_of(host)
             return local_user, remote_user
         except UserNotFoundError:
@@ -71,10 +71,6 @@ def user_pair_for_ssh(self, host: str) -> typing.Tuple[str, str]:
             else:
                 return cached
 
-    @staticmethod
-    def use_ssh_agent() -> bool:
-        return config.get_option('core', 'no_generating_ssh_key')
-
     @staticmethod
     def _get_user_of_host_from_config(host):
         try:

crmsh/utils.py

@@ -8,7 +8,6 @@
 from tempfile import mkstemp
 import subprocess
 import re
-import glob
 import time
 import datetime
 import shutil
@@ -41,8 +40,6 @@
 from . import constants
 from . import options
 from . import term
-from . import ssh_key
-from .constants import SSH_OPTION
 from . import log
 from .prun import prun
 from .sh import ShellUtils
@@ -136,24 +133,6 @@ def user_pair_for_ssh(host):
         raise ValueError('Can not create ssh session from {} to {}.'.format(this_node(), host))
 
 
-def ssh_copy_id_no_raise(local_user, remote_user, remote_node, shell: sh.LocalShell = None):
-    if shell is None:
-        shell = sh.LocalShell()
-    if check_ssh_passwd_need(local_user, remote_user, remote_node, shell):
-        local_public_key = ssh_key.fetch_public_key_file_list(None, local_user)[0]
-        logger.info("Configuring SSH passwordless with {}@{}".format(remote_user, remote_node))
-        cmd = f"ssh-copy-id -i {local_public_key} '{remote_user}@{remote_node}' &> /dev/null"
-        result = shell.su_subprocess_run(local_user, cmd, tty=True)
-        return result.returncode
-    else:
-        return 0
-
-
-def ssh_copy_id(local_user, remote_user, remote_node):
-    if 0 != ssh_copy_id_no_raise(local_user, remote_user, remote_node):
-        fatal("Failed to login to remote host {}@{}".format(remote_user, remote_node))
-
-
 @memoize
 def this_node():
     'returns name of this node (hostname)'
@@ -3022,12 +3001,10 @@ class HostUserConfig:
     """
     def __init__(self):
         self._hosts_users = dict()
-        self._no_generating_ssh_key = False
         self.load()
 
     def load(self):
         self._load_hosts_users()
-        self._load_no_generating_ssh_key()
 
     def _load_hosts_users(self):
         users = list()
@@ -3044,13 +3021,9 @@ def _load_hosts_users(self):
             hosts.append(parts[1])
         self._hosts_users = {host: user for user, host in zip(users, hosts)}
 
-    def _load_no_generating_ssh_key(self):
-        self._no_generating_ssh_key = config.get_option('core', 'no_generating_ssh_key')
-
     def save_local(self):
         value = [f'{user}@{host}' for host, user in sorted(self._hosts_users.items(), key=lambda x: x[0])]
         config.set_option('core', 'hosts', value)
-        config.set_option('core', 'no_generating_ssh_key', self._no_generating_ssh_key)
         debug_on = config.get_option('core', 'debug')
         if debug_on:
             config.set_option('core', 'debug', 'false')
@@ -3062,25 +3035,16 @@ def save_remote(self, remote_hosts: typing.Iterable[str]):
         self.save_local()
         value = [f'{user}@{host}' for host, user in sorted(self._hosts_users.items(), key=lambda x: x[0])]
         crmsh.parallax.parallax_call(remote_hosts, "crm options set core.hosts '{}'".format(', '.join(value)))
-        crmsh.parallax.parallax_call(remote_hosts, "crm options set core.no_generating_ssh_key '{}'".format(
-            'yes' if self._no_generating_ssh_key else 'no'
-        ))
 
     def clear(self):
         self._hosts_users = dict()
-        self._no_generating_ssh_key = False
 
     def get(self, host):
         return self._hosts_users[host]
 
     def add(self, user, host):
         self._hosts_users[host] = user
 
-    def set_no_generating_ssh_key(self, value: bool):
-        self._no_generating_ssh_key = value
-
-    def get_no_generating_ssh_key(self) -> bool:
-        return self._no_generating_ssh_key
 
 def parse_user_at_host(s: str):
     i = s.find('@')