Skip to content

Commit

Permalink
set acl permissions on workdir root
Browse files Browse the repository at this point in the history
In machines with umask set to `0027` it is necessary to
setup an acl to overide it so the workdir root directory
stays multi-user access. This is specially necessary on
machines accessed with a non-root user, that are hardened
to CIS Level 1 guidelines.

Fixes: #2496
Signed-off-by: Carlos Rodriguez-Fernandez <[email protected]>
  • Loading branch information
carlosrodfern committed Feb 22, 2024
1 parent d2df591 commit 981dd52
Show file tree
Hide file tree
Showing 12 changed files with 83 additions and 1 deletion.
12 changes: 12 additions & 0 deletions examples/plugins/provision.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ def go(self):

self._guest = GuestExample(data, name=self.name, parent=self.step)
self._guest.start()
self._guest.setup()

def guest(self):
"""
Expand All @@ -110,6 +111,7 @@ class GuestExample(tmt.Guest):
user ....... user name to log in
key ........ private key
password ... password
become ..... whether to run the scripts with sudo
These are by default imported into instance attributes (see the
class attribute '_keys' in tmt.Guest class).
Expand Down Expand Up @@ -190,6 +192,16 @@ def start(self):
raise tmt.utils.ProvisionError(
"All attempts to provision a machine with example failed.")

def setup(self):
"""
Setup the guest
This should include all necessary configurations inside the instance
to get the plans to work. For example, ensure the permissions in
TMT_WORKDIR_ROOT will work if the user is non-root.
"""
print("setup() called")

# For advanced development
def execute(self, command, **kwargs):
"""
Expand Down
12 changes: 12 additions & 0 deletions tests/provision/become/data/umask.fmf
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
summary: Check that become works when instance umask is restricted
description: Ensures that become works when used in virtual instances with umask set to 0027 like in hardened OSs.
provision+:
become: true
discover:
tests:
- name: Beakerlib test to generate report files on guest
test: ./umask.sh
framework: beakerlib
prepare:
how: shell
script: "echo 'umask 0027' >> /etc/profile; echo 'umask 0027' >> /etc/bashrc"
21 changes: 21 additions & 0 deletions tests/provision/become/data/umask.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#!/bin/bash
# vim: dict+=/usr/share/beakerlib/dictionary.vim cpt=.,w,b,u,t,i,k
. /usr/share/beakerlib/beakerlib.sh || exit 1

rlJournalStart
rlPhaseStartSetup
rlRun "tmp=\$(mktemp -d)" 0 "Create tmp directory"
rlRun "pushd $tmp"
rlRun "set -o pipefail"
rlPhaseEnd

rlPhaseStartTest
rlRun -s "echo HELLO > output" 0 "Say something"
rlAssertGrep "HELLO" "output"
rlPhaseEnd

rlPhaseStartCleanup
rlRun "popd"
rlRun "rm -r $tmp" 0 "Remove tmp directory"
rlPhaseEnd
rlJournalEnd
2 changes: 1 addition & 1 deletion tests/provision/become/main.fmf
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ tag+:
- provision-only
- provision-container
- provision-virtual
duration: 20m
duration: 24m
6 changes: 6 additions & 0 deletions tests/provision/become/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ rlJournalStart
rlRun "tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /prepare-finish/user/scripts"
rlPhaseEnd

if [[ "$PROVISION_HOW" == "virtual" ]]; then
rlPhaseStartTest "$PROVISION_HOW, umask with become=true"
rlRun "tmt --context provisiontest=$PROVISION_HOW run -rvvv plan --name /umask"
rlPhaseEnd
fi

rlPhaseStartCleanup
rlRun "popd"
if [[ "$PROVISION_HOW" == "container" ]]; then
Expand Down
25 changes: 25 additions & 0 deletions tmt/steps/provision/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
ShellScript,
cached_property,
configure_constant,
effective_workdir_root,
field,
key_to_option,
)
Expand Down Expand Up @@ -734,6 +735,13 @@ def start(self) -> None:
"""
self.debug(f"Doing nothing to start guest '{self.primary_address}'.")

def setup(self) -> None:
"""
Setup the guest
Setup the guest after it has been started. It is called after start().
"""

# A couple of requiremens for this field:
#
# * it should be valid, i.e. when someone tries to access it, the values
Expand Down Expand Up @@ -1365,6 +1373,23 @@ def is_ready(self) -> bool:
# Enough for now, ssh connection can be created later
return self.primary_address is not None

def setup(self) -> None:
if self.is_dry_run:
return
if not self.facts.is_superuser and self.become:
assert self.facts.package_manager is not None
self.execute(
Command(
'sudo',
f'{self.facts.package_manager.value}',
'install',
'-y',
'acl'))
workdir_root = effective_workdir_root()
acl_setup_script = ShellScript(f'mkdir -p {workdir_root}')
acl_setup_script += ShellScript(f'setfacl -d -m o:rX {workdir_root}')
self.execute(acl_setup_script)

def execute(self,
command: Union[tmt.utils.Command, tmt.utils.ShellScript],
cwd: Optional[Path] = None,
Expand Down
1 change: 1 addition & 0 deletions tmt/steps/provision/artemis.py
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,7 @@ def go(self) -> None:
name=self.name,
parent=self.step)
self._guest.start()
self._guest.setup()

def guest(self) -> Optional[GuestArtemis]:
""" Return the provisioned guest """
Expand Down
1 change: 1 addition & 0 deletions tmt/steps/provision/connect.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ def go(self) -> None:
data=data,
name=self.name,
parent=self.step)
self._guest.setup()

def guest(self) -> Optional[tmt.GuestSsh]:
""" Return the provisioned guest """
Expand Down
1 change: 1 addition & 0 deletions tmt/steps/provision/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ def go(self) -> None:
self.warn("The 'local' provision plugin does not support hardware requirements.")

self._guest = GuestLocal(logger=self._logger, data=data, name=self.name, parent=self.step)
self._guest.setup()

def guest(self) -> Optional[GuestLocal]:
""" Return the provisioned guest """
Expand Down
1 change: 1 addition & 0 deletions tmt/steps/provision/mrack.py
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,7 @@ def go(self) -> None:
logger=self._logger,
)
self._guest.start()
self._guest.setup()

def guest(self) -> Optional[GuestBeaker]:
""" Return the provisioned guest """
Expand Down
1 change: 1 addition & 0 deletions tmt/steps/provision/podman.py
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@ def go(self) -> None:
name=self.name,
parent=self.step)
self._guest.start()
self._guest.setup()

# TODO: this might be allowed with `--privileged`...
self._guest.facts.capabilities[GuestCapability.SYSLOG_ACTION_READ_ALL] = False
Expand Down
1 change: 1 addition & 0 deletions tmt/steps/provision/testcloud.py
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,7 @@ def _report_support(constraint: tmt.hardware.Constraint[Any]) -> bool:
name=self.name,
parent=self.step)
self._guest.start()
self._guest.setup()

def guest(self) -> Optional[tmt.Guest]:
""" Return the provisioned guest """
Expand Down

0 comments on commit 981dd52

Please sign in to comment.