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

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

[Mikko Rapeli]

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

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
                 else:
-- 
2.34.1

[PATCH 2/2] runqemu: limit slirp host port forwarding to localhost 127.0.0.1

[Mikko Rapeli]

With default slirp port forwarding config qemu listens on TCP ports
2222 and 2323 on all IP addresses available on the build host. Most
use cases with runqemu only need it for localhost and it is not
safe to run qemu images with root login without password enabled
and listening on all available, possibly Internet reachable network
interfaces. Limit qemu port forwarding to localhost 127.0.0.1 IP
address. Now qemu machine SSH and telnet ports are only
reachable from the build host machine, not full Internet.

If qemu machine needs to be reachable from network, then it can
be enabled via local.conf or machine config variable QB_SLIRP_OPT:

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

Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 scripts/runqemu | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/runqemu b/scripts/runqemu
index a6ea578564..7bd9465593 100755
--- a/scripts/runqemu
+++ b/scripts/runqemu
@@ -1071,7 +1071,7 @@ class BaseConfig(object):
         logger.info("Network configuration:%s", netconf)
         self.kernel_cmdline_script += netconf
         # Port mapping
-        hostfwd = ",hostfwd=tcp::2222-:22,hostfwd=tcp::2323-:23"
+        hostfwd = ",hostfwd=tcp:127.0.0.1:2222-:22,hostfwd=tcp:127.0.0.1:2323-:23"
         qb_slirp_opt_default = "-netdev user,id=net0%s,tftp=%s" % (hostfwd, self.get('DEPLOY_DIR_IMAGE'))
         qb_slirp_opt = self.get('QB_SLIRP_OPT') or qb_slirp_opt_default
         # Figure out the port
-- 
2.34.1

Re: [OE-core] [PATCH 2/2] runqemu: limit slirp host port forwarding to localhost 127.0.0.1

[Quentin Schulz]

Hi Mikko,

On 11/14/22 16:50, Mikko Rapeli wrote:
> With default slirp port forwarding config qemu listens on TCP ports
> 2222 and 2323 on all IP addresses available on the build host. Most
> use cases with runqemu only need it for localhost and it is not
> safe to run qemu images with root login without password enabled
> and listening on all available, possibly Internet reachable network
> interfaces. Limit qemu port forwarding to localhost 127.0.0.1 IP
> address. Now qemu machine SSH and telnet ports are only
> reachable from the build host machine, not full Internet.
> 
> If qemu machine needs to be reachable from network, then it can
> be enabled via local.conf or machine config variable QB_SLIRP_OPT:
> 
> QB_SLIRP_OPT = "-netdev user,id=net0,hostfwd=tcp::2222-:22"
> 
> Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
> ---
>   scripts/runqemu | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/runqemu b/scripts/runqemu
> index a6ea578564..7bd9465593 100755
> --- a/scripts/runqemu
> +++ b/scripts/runqemu
> @@ -1071,7 +1071,7 @@ class BaseConfig(object):
>           logger.info("Network configuration:%s", netconf)
>           self.kernel_cmdline_script += netconf
>           # Port mapping
> -        hostfwd = ",hostfwd=tcp::2222-:22,hostfwd=tcp::2323-:23"
> +        hostfwd = ",hostfwd=tcp:127.0.0.1:2222-:22,hostfwd=tcp:127.0.0.1:2323-:23"

With the additional knowledge we gathered in the last patches, I believe 
it would be a good thing to say a few words/update the documentation.

See 
https://lore.kernel.org/yocto-docs/fedb4cc0-44d6-d7d8-bc26-c8de5bee06ca@theobroma-systems.com/T/#t 
for a patch I believe might make it to master soon? I think we should 
say what the default value entails (even if this patch isnt' taken) and 
maybe point/refer to the QEMU documentation for the meaning of options 
in QB_SLIRP_OPT. I believe some/all of options listed 
https://www.qemu.org/docs/master/system/invocation.html are possible?

What do you think?

Cheers,
Quentin

Re: [OE-core] [PATCH 2/2] runqemu: limit slirp host port forwarding to localhost 127.0.0.1

[Mikko Rapeli]

Hi,

On Thu, Nov 17, 2022 at 02:17:13PM +0100, Quentin Schulz wrote:
> Hi Mikko,
> 
> On 11/14/22 16:50, Mikko Rapeli wrote:
> > With default slirp port forwarding config qemu listens on TCP ports
> > 2222 and 2323 on all IP addresses available on the build host. Most
> > use cases with runqemu only need it for localhost and it is not
> > safe to run qemu images with root login without password enabled
> > and listening on all available, possibly Internet reachable network
> > interfaces. Limit qemu port forwarding to localhost 127.0.0.1 IP
> > address. Now qemu machine SSH and telnet ports are only
> > reachable from the build host machine, not full Internet.
> > 
> > If qemu machine needs to be reachable from network, then it can
> > be enabled via local.conf or machine config variable QB_SLIRP_OPT:
> > 
> > QB_SLIRP_OPT = "-netdev user,id=net0,hostfwd=tcp::2222-:22"
> > 
> > Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
> > ---
> >   scripts/runqemu | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/scripts/runqemu b/scripts/runqemu
> > index a6ea578564..7bd9465593 100755
> > --- a/scripts/runqemu
> > +++ b/scripts/runqemu
> > @@ -1071,7 +1071,7 @@ class BaseConfig(object):
> >           logger.info("Network configuration:%s", netconf)
> >           self.kernel_cmdline_script += netconf
> >           # Port mapping
> > -        hostfwd = ",hostfwd=tcp::2222-:22,hostfwd=tcp::2323-:23"
> > +        hostfwd = ",hostfwd=tcp:127.0.0.1:2222-:22,hostfwd=tcp:127.0.0.1:2323-:23"
> 
> With the additional knowledge we gathered in the last patches, I believe it
> would be a good thing to say a few words/update the documentation.
> 
> See https://lore.kernel.org/yocto-docs/fedb4cc0-44d6-d7d8-bc26-c8de5bee06ca@theobroma-systems.com/T/#t
> for a patch I believe might make it to master soon? I think we should say
> what the default value entails (even if this patch isnt' taken) and maybe
> point/refer to the QEMU documentation for the meaning of options in
> QB_SLIRP_OPT. I believe some/all of options listed
> https://www.qemu.org/docs/master/system/invocation.html are possible?
> 
> What do you think?

Yes, I agree, and saw that change too. I'll try to document this once
change gets integrated.

Cheers,

-Mikko

> Cheers,
> Quentin

Re: [OE-core] [PATCH v2 1/2] qemurunner.py: support setting slirp host IP address

[Quentin Schulz]

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

Re: [OE-core] [PATCH v2 1/2] qemurunner.py: support setting slirp host IP address

[Mikko Rapeli]

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