[v2,1/2] qemurunner.py: support setting slirp host IP address

By default host side IP address is not set and qemu listens
on all IP addresses on the host machine which is not a good
idea when images have root login enabled without password.
It make sense to listen only on localhost IP address 127.0.0.1 using
config change like:

QB_SLIRP_OPT = "-netdev user,id=net0,hostfwd=tcp:127.0.0.1:2222-:22"

This config works for qemu itself, but breaks runqemu which tries to
parse the host side port number from qemu process command line arguments.
So change the runqemu side hostfwd parsing for port number to ignore
the host IP address field.

Reviewed-by: Quentin Schulz <foss+yocto@0leil.net>
Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 meta/lib/oeqa/utils/qemurunner.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

v2: improved commit message

v1: https://lists.openembedded.org/g/openembedded-core/topic/95016024#173234

Hi Mikko,

On 11/14/22 16:50, Mikko Rapeli wrote:
> By default host side IP address is not set and qemu listens
> on all IP addresses on the host machine which is not a good
> idea when images have root login enabled without password.
> It make sense to listen only on localhost IP address 127.0.0.1 using
> config change like:
> 
> QB_SLIRP_OPT = "-netdev user,id=net0,hostfwd=tcp:127.0.0.1:2222-:22"
> 
> This config works for qemu itself, but breaks runqemu which tries to
> parse the host side port number from qemu process command line arguments.
> So change the runqemu side hostfwd parsing for port number to ignore
> the host IP address field.
> 
> Reviewed-by: Quentin Schulz <foss+yocto@0leil.net>
> Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
> ---
>   meta/lib/oeqa/utils/qemurunner.py | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> v2: improved commit message
> 
> v1: https://urldefense.com/v3/__https://lists.openembedded.org/g/openembedded-core/topic/95016024*173234__;Iw!!OOPJP91ZZw!lmurd9F5r43EHopuJkrpTJZMlC93fI8sXSNgnxmfVSRyTvP6unwa8Wn4-wxgjS9UM6EQUnGK5X8gzyVSyfm3WExF4pLQQuHl0L_BlQ$
> 
> diff --git a/meta/lib/oeqa/utils/qemurunner.py b/meta/lib/oeqa/utils/qemurunner.py
> index e602399232..f175f8a1de 100644
> --- a/meta/lib/oeqa/utils/qemurunner.py
> +++ b/meta/lib/oeqa/utils/qemurunner.py
> @@ -401,7 +401,8 @@ class QemuRunner:
>                   cmdline = re_control_char.sub(' ', cmdline)
>               try:
>                   if self.use_slirp:
> -                    tcp_ports = cmdline.split("hostfwd=tcp::")[1]
> +                    tcp_ports = cmdline.split("hostfwd=tcp:")[1]
> +                    tcp_ports = tcp_ports.split(":")[1]
>                       host_port = tcp_ports[:tcp_ports.find('-')]
>                       self.ip = "localhost:%s" % host_port

Wondering if we shouldn't also update the self.ip here to display the 
actual ip to use? I assume we just need to extract it from the first 
tcp_ports. I think it should just be tcp_ports.split(":")[0] ? and just 
need to check if it's empty/None in which case we set it to localhost?

What do you think?

Cheers,
Quentin

Hi,

On Thu, Nov 17, 2022 at 02:13:01PM +0100, Quentin Schulz wrote:
> Hi Mikko,
> 
> On 11/14/22 16:50, Mikko Rapeli wrote:
> > By default host side IP address is not set and qemu listens
> > on all IP addresses on the host machine which is not a good
> > idea when images have root login enabled without password.
> > It make sense to listen only on localhost IP address 127.0.0.1 using
> > config change like:
> > 
> > QB_SLIRP_OPT = "-netdev user,id=net0,hostfwd=tcp:127.0.0.1:2222-:22"
> > 
> > This config works for qemu itself, but breaks runqemu which tries to
> > parse the host side port number from qemu process command line arguments.
> > So change the runqemu side hostfwd parsing for port number to ignore
> > the host IP address field.
> > 
> > Reviewed-by: Quentin Schulz <foss+yocto@0leil.net>
> > Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
> > ---
> >   meta/lib/oeqa/utils/qemurunner.py | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > v2: improved commit message
> > 
> > v1: https://urldefense.com/v3/__https://lists.openembedded.org/g/openembedded-core/topic/95016024*173234__;Iw!!OOPJP91ZZw!lmurd9F5r43EHopuJkrpTJZMlC93fI8sXSNgnxmfVSRyTvP6unwa8Wn4-wxgjS9UM6EQUnGK5X8gzyVSyfm3WExF4pLQQuHl0L_BlQ$
> > 
> > diff --git a/meta/lib/oeqa/utils/qemurunner.py b/meta/lib/oeqa/utils/qemurunner.py
> > index e602399232..f175f8a1de 100644
> > --- a/meta/lib/oeqa/utils/qemurunner.py
> > +++ b/meta/lib/oeqa/utils/qemurunner.py
> > @@ -401,7 +401,8 @@ class QemuRunner:
> >                   cmdline = re_control_char.sub(' ', cmdline)
> >               try:
> >                   if self.use_slirp:
> > -                    tcp_ports = cmdline.split("hostfwd=tcp::")[1]
> > +                    tcp_ports = cmdline.split("hostfwd=tcp:")[1]
> > +                    tcp_ports = tcp_ports.split(":")[1]
> >                       host_port = tcp_ports[:tcp_ports.find('-')]
> >                       self.ip = "localhost:%s" % host_port
> 
> Wondering if we shouldn't also update the self.ip here to display the actual
> ip to use? I assume we just need to extract it from the first tcp_ports. I
> think it should just be tcp_ports.split(":")[0] ? and just need to check if
> it's empty/None in which case we set it to localhost?
> 
> What do you think?

Good idea! I'll send a new version.

Cheers,

-Mikko

> Cheers,
> Quentin