Skip to content
Open

qemu #1169

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions policy/modules/apps/qemu.if
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,25 @@ interface(`qemu_run',`
roleattribute $2 qemu_roles;
')

########################################
## <summary>
## Allow a domain to be in the qemu role
## </summary>
## <param name="domain">
## <summary>
## Domain allowed in the role
## </summary>
## </param>
## <rolecap/>
#
interface(`qemu_domain',`
gen_require(`
attribute_role qemu_roles;
')

role qemu_roles types $1;
')

########################################
## <summary>
## Read qemu process state files.
Expand Down
6 changes: 6 additions & 0 deletions policy/modules/apps/qemu.te
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,15 @@ init_unit_file(qemu_unit_t)

dev_read_sysfs(qemu_t)

allow qemu_t self:anon_inode { create map read write };

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably can fully upgrade to mmap_manage_file_perms.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not use "self" for anon_inode because then you lose control over the various kinds like uffd, io_uring, perf_event, secretmem etc.

In the case of qemu you would allow excessive permissions because afaik qemu only leverages io_uring and uffd anon_inodes

Here is how i deal with anon_inodes:

https://git.defensec.nl/forge/defensec/dssp5/src/branch/dssp5-debian/src/anoninode.cil
https://git.defensec.nl/forge/defensec/dssp5/src/branch/dssp5-debian/src/anoninode

https://git.defensec.nl/forge/defensec/dssp5/src/branch/dssp5-debian/src/anoninode/iouringanoninode.cil#L32

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

root@nimbus:~# sesearch -A -s qemu.guest.subj -c anon_inode
allow qemu.guest.subj qemu.guest.iouring.anon_inode:anon_inode { append create getattr ioctl link lock map open read rename setattr unlink write };
allow qemu.guest.subj qemu.guest.uffd.anon_inode:anon_inode { append create getattr ioctl link lock open read rename setattr unlink write };

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

root@nimbus:~# sesearch -T -s qemu.guest.subj -c anon_inode
type_transition qemu.guest.subj qemu.guest.subj:anon_inode qemu.guest.iouring.anon_inode [io_uring];
type_transition qemu.guest.subj qemu.guest.subj:anon_inode qemu.guest.uffd.anon_inode [userfaultfd];

allow qemu_t self:io_uring allowed;
allow qemu_t qemu_runtime_t:sock_file create_sock_file_perms;
files_runtime_filetrans(qemu_t, qemu_runtime_t, sock_file)

kernel_read_vm_sysctls(qemu_t)

files_mmap_read_boot_files(qemu_t)

tunable_policy(`qemu_full_network',`
corenet_udp_sendrecv_generic_if(qemu_t)
corenet_udp_sendrecv_generic_node(qemu_t)
Expand Down
4 changes: 4 additions & 0 deletions policy/modules/roles/sysadm.te
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,10 @@ optional_policy(`
pyzor_role(sysadm, sysadm_t, sysadm_application_exec_domain, sysadm_r)
')

optional_policy(`
qemu_role(sysadm_r, sysadm_t)
')
Comment on lines +906 to +908

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the use case here?


optional_policy(`
qpidd_admin(sysadm_t, sysadm_r)
')
Expand Down
13 changes: 12 additions & 1 deletion policy/modules/services/virt.te
Original file line number Diff line number Diff line change
Expand Up @@ -1125,7 +1125,7 @@ optional_policy(`
# Bridgehelper local policy
#

allow virt_bridgehelper_t self:process { getcap setcap };
allow virt_bridgehelper_t self:process { getcap setcap getsched };
allow virt_bridgehelper_t self:capability { net_admin setgid setpcap setuid };
allow virt_bridgehelper_t self:tcp_socket create_stream_socket_perms;
allow virt_bridgehelper_t self:tun_socket create_socket_perms;
Expand All @@ -1136,13 +1136,24 @@ allow virt_bridgehelper_t virt_etc_t:file read_file_perms;

manage_files_pattern(virt_bridgehelper_t, svirt_home_t, svirt_home_t)

kernel_getattr_proc(virt_bridgehelper_t)
kernel_read_kernel_sysctls(virt_bridgehelper_t)
kernel_read_network_state(virt_bridgehelper_t)
kernel_read_system_state(virt_bridgehelper_t)

dev_read_sysfs(virt_bridgehelper_t)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What entries are read? Please add a comment in the policy with that info.


corenet_rw_tun_tap_dev(virt_bridgehelper_t)

domain_use_interactive_fds(virt_bridgehelper_t)

userdom_search_user_home_dirs(virt_bridgehelper_t)
userdom_use_user_ptys(virt_bridgehelper_t)

optional_policy(`
qemu_domain(virt_bridgehelper_t)
')
Comment on lines +1153 to +1155

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should instead be a run interface provided by the virt module.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the role matters. If a role is not authorized to associate with qemu_t then it should also not be authorized to associate with virt_bridgehelper_t because let's face it: it only makes sense for qemu to ever run that helper.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand this comment. It sounds like you're agreeing with me?. The point of the run interface is to pass the qemu roles down to virt_bridgehelper_t.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand this comment. It sounds like you're agreeing with me?. The point of the run interface is to pass the qemu roles down to virt_bridgehelper_t.

Yes. Sorry.


########################################
#
# Leaseshelper local policy
Expand Down
Loading