fix(apparmor): forbid breaking out of confinement with ssh again

[okurz]

This fixes a security regression introduced in 4ec6f2a allowing to
execute ssh globally in the complete worker process. Allowing to ssh out
from the main worker process would deny most of the confinement benefit
of using apparmor.

Related to
#7249 (comment)

[okurz]

@Vogtinator

[Martchus]

Maybe a stupid question: Don't certain backends need to make SSH connections? I suppose they don't do them via the ssh command, though. But does it really matter what tool/library is used? As long as we can make SSH connections in general I don't see a big improvement here.

I guess it makes generally sense to trim down the number of executables to a minimum but the title "forbid breaking out of confinement" makes it sound like this is about a specific attack vector being prevented here (SSH connections in general).

[Vogtinator]

This will break ssh backends again

[Vogtinator]

Maybe a stupid question: Don't certain backends need to make SSH connections? I suppose they don't do them via the ssh command, though.

They actually do: The worker spawns Xvnc, with icewm and xterm-console inside, then runs ssh.

4ec6f2a was necessary to make this work again.

But does it really matter what tool/library is used? As long as we can make SSH connections in general I don't see a big improvement here.

Correct. Any code, including test code can just connect to whereever using TCP sockets.

[Vogtinator]

x3270 does not invoke ssh

[kalikiana]

But does it really matter what tool/library is used? As long as we can make SSH connections in general I don't see a big improvement here.

Correct. Any code, including test code can just connect to whereever using TCP sockets.

Let's verify this properly then. https://openqa.opensuse.org/tests/5827767 is the test that was used to verify #7249 - are there other cases we should consider?

[Vogtinator]

Let's verify this properly then. https://openqa.opensuse.org/tests/5827767 is the test that was used to verify #7249 - are there other cases we should consider?

That should work, also svirt, pvm and generalhw. Also any git repo referenced through e.g. CASEDIR may do a clone over ssh.

[okurz]

Maybe a stupid question: Don't certain backends need to make SSH connections?

Of course. See #7255 (comment)

I suppose they don't do them via the ssh command, though. But does it really matter what tool/library is used? As long as we can make SSH connections in general I don't see a big improvement here.

What "improvement" do you mean? I am trying to break a security regression from 4ec6f2a

I guess it makes generally sense to trim down the number of executables to a minimum but the title "forbid breaking out of confinement" makes it sound like this is about a specific attack vector being prevented here (SSH connections in general).

please read the commit message and tell me where I can phrase it better. This PR is intended to be a security regression fix, not a general improvement.

[Martchus]

Then I don't get what regression this fixes considering code can make SSH connections anyway (e.g. using libopenssh directly or some custom implementation).

Considering @Vogtinator's comments this will even break existing tests and allows using ssh in the wrong place.

[okurz]

Then I don't get what regression this fixes considering code can make SSH connections anyway (e.g. using libopenssh directly or some custom implementation).

Considering @Vogtinator's comments this will even break existing tests and allows using ssh in the wrong place.

You should know even better. We have apparmor enforced in o3 except for https://progress.opensuse.org/projects/openqav3/wiki/#o3-s390-and-other-eg-bare-metal-workers-running-as-podman-containers-on-o3-worker-openqaworker23 . And we don't enforce apparmor within the whole OSD domain. Why is that then?

[Vogtinator]

We have apparmor enforced in o3 except for https://progress.opensuse.org/projects/openqav3/wiki/#o3-s390-and-other-eg-bare-metal-workers-running-as-podman-containers-on-o3-worker-openqaworker23

The apparmor denials caused failures in the containerized workers and adjusting the policy on ow23 (the container host!) made it work again. Not sure which part of that is intentional.

In any case, the tests should work even outside of containers, like on local workstations with apparmor enabled.

[okurz]

We have apparmor enforced in o3 except for https://progress.opensuse.org/projects/openqav3/wiki/#o3-s390-and-other-eg-bare-metal-workers-running-as-podman-containers-on-o3-worker-openqaworker23

The apparmor denials caused failures in the containerized workers and adjusting the policy on ow23 (the container host!) made it work again. Not sure which part of that is intentional.

In any case, the tests should work even outside of containers, like on local workstations with apparmor enabled.

That all sounds nice and dandy but doesn't explain why someone (not me) made the decision that o3 can have enforced apparmor profiles for all standard qemu workers, apparmor disabled but confined in containers in o3 for non-qemu and no apparmor enabled at all within OSD

[Vogtinator]

We have apparmor enforced in o3 except for https://progress.opensuse.org/projects/openqav3/wiki/#o3-s390-and-other-eg-bare-metal-workers-running-as-podman-containers-on-o3-worker-openqaworker23

The apparmor denials caused failures in the containerized workers and adjusting the policy on ow23 (the container host!) made it work again. Not sure which part of that is intentional.
In any case, the tests should work even outside of containers, like on local workstations with apparmor enabled.

That all sounds nice and dandy but doesn't explain why someone (not me) made the decision that o3 can have enforced apparmor profiles for all standard qemu workers, apparmor disabled but confined in containers in o3 for non-qemu

It's enabled on all workers FWICT? For the container workers I suspect the difference in behaviour started with Leap 16. Maybe some product regression there (the container engines no longer support AppArmor?).

and no apparmor enabled at all within OSD

Would be nice if that could change.