Re: [PATCH v4 4/9] dts: add ssh pexpect library

[Bruce Richardson <bruce.richardson@intel.com>]

On Wed, Sep 14, 2022 at 12:03:34AM +0000, Honnappa Nagarahalli wrote:
>    <snip>
> 
> 
>      On Fri, Jul 29, 2022 at 10:55:45AM +0000, Juraj Linkeš wrote:
>      <snip>
>      > +                self.session = pxssh.pxssh(encoding="utf-8")
>      > +                self.session.login(
>      > +                    self.node,
>      > +                    self.username,
>      > +                    self.password,
>      > +                    original_prompt="[$#>]",
>      > +
>      password_regex=r"(?i)(?:password:)|(?:passphrase for
>      key)|(?i)(password for .+:)",
>      > +                )
>      > +                [1]self.logger.info(f"Connection to {self.node}
>      succeeded")
>      > +            self.send_expect("stty -echo", "#")
>      > +            self.send_expect("stty columns 1000", "#")
>      First of all, thanks for those changes! Having DTS inside DPDK makes
>      test synchronization a lot easier. I'm happy to say (unsurprisingly)
>      that it works with my RISC-V HiFive Unmatched board like a charm.
> 
> 
>      Though there is a small issue with the lines above. They assume "#"
>      as
>      the prompt sign, even though original_prompt was set to "[$#>]".
>      This
>      touches on two problems:
>      1. # is usually a root prompt - is DTS assumed to be run with root
>         privileges? DPDK may (in theory) run without them with some
>      permission
>         adjustment (hugetlb, VFIO container, etc.). If we assume DTS
>      needs
>         root access, this has to be both documented and validated before
>         running the whole suite. Otherwise it'll be hard to debug.
> 
> 
>    Around a year ago there were some attempts to get DTS to not require
>    root. This ended up running into issues because DTS sets up drivers for
>    you, which requires root as far as I know, as well as setting up
>    hugepages, which I think also requires root. The current version of DTS
>    can probably run without root, but it will probably stop working as
>    soon as DTS starts interacting with PCI devices. Elevating privileges
>    using pkexec or sudo is less portable and would require supporting a
>    lot more forms of authentication (kerberos/ldap for enterprise
>    deployments, passwords, 2fa, etc). It is much easier to say that the
>    default SSH agent must provide root access to the SUT and Traffic
>    Generator either with a password or pre-configured passwordless
>    authentication (ssh keys, kerberos, etc).
> 
>    [Honnappa] One of the feedback we collected asks to deprecate the use
>    of clear text passwords in config files and root user. It suggests to
>    use keys and sudo. It is a ‘Must Have’ item.
> 
> 
>    I agree it should be documented. I honestly didn't consider that anyone
>    would try running DTS as a non-root user.
> 
>    [Honnappa] +1 for supporting root users for now and documenting.
> 
> 
>      2. Different shells use different prompts on different distros.
>      Hence
>         perhaps there should be a regex here (same as with
>      original_prompt)
>         and there could be a conf.yaml option to modify it on a per-host
>         basis?
> 
> 
>    As far as customizing the prompts, I think that is doable via a
>    configuration option.
>    As far as different shells, I don't think we were planning to support
>    anything besides either bash or posix-compatible shells. At the moment
>    all of the community lab systems use bash, and for ease of test
>    development it will be easier to mandate that everyone uses one shell.
>    Otherwise DTS CI will need to run once for each shell to catch issues,
>    which in my opinion are resources better spent on more in-depth testing
>    of DTS and DPDK.
> 
>    [Honnappa] +1 for using just bash, we can document this as well.
>

I would agree overall. Just supporting one shell is fine - certainly for
now. Also completely agree that we need to remove hard-coded passwords and
ideally non-root. However, I think for the initial versions the main thing
should be removing the passwords so I would be ok for keeping the "root"
login requirement, so long as we support using ssh keys for login rather
than hard-coded passwords.

/Bruce