Fix SELinux permission denied error for swtpm in migration test #6704
Conversation
When running migration tests with shared TPM, the VM fails to start with "cannot execute binary /usr/bin/swtpm: Permission denied" error. This occurs because SELinux context for swtpm binary is incorrect or the required SELinux policies are not properly configured. Changes: - Add prepare_swtpm_selinux() function to automatically configure SELinux environment for swtpm on both local and remote hosts - Install swtpm-selinux package if missing - Restore SELinux contexts for swtpm binaries - Enable required SELinux booleans (swtpm_use_tpm_device, virt_use_swtpm) - Fix typo in check_tpm_security_context(): 'remote' -> 'remote_host' Committer: Bolatbek Issakh <bissakh@redhat.com>
WalkthroughThis change introduces a new helper function Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
Before Fix After Fix |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libvirt/tests/src/migration/migration_with_vtpm/migration_with_shared_tpm.py(4 hunks)
🧰 Additional context used
🪛 Ruff (0.14.7)
libvirt/tests/src/migration/migration_with_vtpm/migration_with_shared_tpm.py
19-19: Unused function argument: params
(ARG001)
47-47: Function call with shell=True parameter identified, security issue
(S604)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Python 3.11
- GitHub Check: Python 3.12
- GitHub Check: Python 3.9
- GitHub Check: Python 3.8
🔇 Additional comments (3)
libvirt/tests/src/migration/migration_with_vtpm/migration_with_shared_tpm.py (3)
222-236: LGTM! SELinux preparation properly integrated.The SELinux preparation is correctly called on both local and remote hosts before the mount operations, and the session lifecycle is properly managed.
258-275: Verify restorecon consistency after mount operations.After calling
prepare_swtpm_selinuxon both hosts and mounting on the remote, line 274 runsrestorecon -Rv /var/lib/libvirt/swtpmonly on the local host. Consider whether:
- This restorecon is still necessary after the
prepare_swtpm_selinuxcall, or- The same restorecon should also be applied on the remote host after its mount operation (line 273).
62-62: Good catch fixing the variable name!Correctly changed the log statement to use
remote_host(the actual parameter name) instead of the undefinedremotevariable.
| def prepare_swtpm_selinux(params, test, session=None): | ||
| """ | ||
| Prepare SELinux environment for swtpm to work properly. | ||
| This fixes "Permission denied" errors when libvirt tries to execute swtpm. | ||
| :param params: dict, test parameters | ||
| :param test: test object | ||
| :param session: remote session object (None for local execution) | ||
| """ | ||
| test.log.info("Preparing SELinux environment for swtpm%s.", | ||
| " on remote host" if session else "") | ||
|
|
||
| commands = [ | ||
| "rpm -q swtpm-selinux || yum install -y swtpm-selinux", | ||
| "restorecon -v /usr/bin/swtpm", | ||
| "restorecon -v /usr/bin/swtpm_setup", | ||
| "restorecon -Rv /var/lib/swtpm-localca 2>/dev/null || true", | ||
| "getsebool swtpm_use_tpm_device &>/dev/null && setsebool -P swtpm_use_tpm_device on || true", | ||
| "getsebool virt_use_swtpm &>/dev/null && setsebool -P virt_use_swtpm on || true", | ||
| ] | ||
|
|
||
| for cmd in commands: | ||
| test.log.debug("Running: %s", cmd) | ||
| if session: | ||
| status, output = session.cmd_status_output(cmd, timeout=120) | ||
| if status and "rpm -q" not in cmd: | ||
| test.log.warning("Command '%s' failed on remote: %s", cmd, output) | ||
| else: | ||
| result = process.run(cmd, ignore_status=True, shell=True, timeout=120) | ||
| if result.exit_status and "rpm -q" not in cmd: | ||
| test.log.warning("Command '%s' failed: %s", cmd, result.stderr_text) | ||
|
|
There was a problem hiding this comment.
Remove unused params parameter.
The params parameter is declared but never used in the function body. Consider removing it to clean up the function signature.
Apply this diff:
-def prepare_swtpm_selinux(params, test, session=None):
+def prepare_swtpm_selinux(test, session=None):
"""
Prepare SELinux environment for swtpm to work properly.
This fixes "Permission denied" errors when libvirt tries to execute swtpm.
- :param params: dict, test parameters
:param test: test object
:param session: remote session object (None for local execution)
"""And update the function calls at lines 224, 233, 260, and 271:
- prepare_swtpm_selinux(params, test)
+ prepare_swtpm_selinux(test)- prepare_swtpm_selinux(params, test, session=server_session)
+ prepare_swtpm_selinux(test, session=server_session)Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.7)
19-19: Unused function argument: params
(ARG001)
47-47: Function call with shell=True parameter identified, security issue
(S604)
🤖 Prompt for AI Agents
In libvirt/tests/src/migration/migration_with_vtpm/migration_with_shared_tpm.py
around lines 19 to 50 the function prepare_swtpm_selinux(params, test,
session=None) declares an unused params parameter; remove params from the
signature so it becomes prepare_swtpm_selinux(test, session=None) and update all
internal uses accordingly (none needed), then update every call site referenced
in the review at lines 224, 233, 260, and 271 to stop passing the params
argument (call prepare_swtpm_selinux(test, session) or
prepare_swtpm_selinux(test) as appropriate).
| " on remote host" if session else "") | ||
|
|
||
| commands = [ | ||
| "rpm -q swtpm-selinux || yum install -y swtpm-selinux", |
There was a problem hiding this comment.
We do not install any package on host, instead we should check if the package is installed. If it does not exist, let's skip the case if this package is required.
| "restorecon -v /usr/bin/swtpm", | ||
| "restorecon -v /usr/bin/swtpm_setup", | ||
| "restorecon -Rv /var/lib/swtpm-localca 2>/dev/null || true", | ||
| "getsebool swtpm_use_tpm_device &>/dev/null && setsebool -P swtpm_use_tpm_device on || true", |
There was a problem hiding this comment.
Can we simply run setsebool command?
| "getsebool virt_use_swtpm &>/dev/null && setsebool -P virt_use_swtpm on || true", | ||
| ] | ||
|
|
||
| for cmd in commands: |
There was a problem hiding this comment.
To execute a command, you can use utils_misc.cmd_status_output()
| from provider.migration import base_steps | ||
|
|
||
|
|
||
| def prepare_swtpm_selinux(params, test, session=None): |
2 participants
When running migration tests with shared TPM, the VM fails to start with "cannot execute binary /usr/bin/swtpm: Permission denied" error. This occurs because SELinux context for swtpm binary is incorrect or the required SELinux policies are not properly configured.
Changes:
Committer: Bolatbek Issakh bissakh@redhat.com
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.