On Tue, Nov 14, 2023 at 9:14 AM Chris PeBenito <pebenito@xxxxxxxx> wrote: > > On 11/13/2023 10:35 AM, Stephen Smalley wrote: > > On Sun, Nov 12, 2023 at 11:52 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > >> On Thu, Nov 9, 2023 at 1:26 PM Chris PeBenito > >> <chpebeni@xxxxxxxxxxxxxxxxxxx> wrote: > >>> systemd is increasing usage of memfds, pidfds, etc. This is resulting > >>> in a need for wide inheritance of fds across the system. For example in > >>> a lot of systemd interfaces that have a pid field now have a comparable > >>> pidfd interface. dbus-broker and polkit are similarly updated. > >>> > >>> Some references from an All Systems Go! talk: > >>> https://cfp.all-systems-go.io/media/all-systems-go-2023/submissions/T3LJAM/resources/ASG_2023_PID_FD-ize_all_the_things_E98Zw9Q.pdf > >>> This is from a few months ago; the switch to PIDFDs is nearly > >>> complete, and we're already seeing denials for this usage. > >>> > >>> > >>> Since file descriptors are increasing use as references for various > >>> operations, I think it would be useful to have a finer-grained fd class, > >>> so we can limit file descriptor inheritance, particularly as it looks > >>> like systemd/pid1 will need to inherit pidfd file descriptors from > >>> possibly all domains. Specifically, I propose adding new permissions to > >>> the fd class, such as use_pidfd and use_memfd. Then systemd can use > >>> pidfds from any domain, but only use regular fds from trusted domains. > >>> > >>> Thoughts? > >> > >> I think adding some granularity to the fd:use permission makes sense, > >> although I'm wondering if we are better served by creating new object > >> classes for these new types of reference fds, e.g. pidfd:use, > >> memfd:user, etc.? When I read "use_pidfd" my first thought is that we > >> are encoding an object class in the permission. > >> > >> Have you looked at the associated kernel code yet? I suspect we might > >> need to augment the existing memfd/pidfd/etc. code paths with an > >> additional LSM hook to be able to mark the fd's LSM/SELinux state with > >> class info, but I'm not sure off the top of my head. > > > > We don't actually store SECCLASS_FD in any security blobs currently > > (i.e. there is no sclass field in the file_security_struct); we just > > always check against SECCLASS_FD in file_has_perm(), > > selinux_binder_transfer_file(), ioctl_has_perm(), > > selinux_kernel_module_from_file(). As you note, we don't have a way of > > knowing what kind of fd it is at those points so we would need to > > somehow pass that information to selinux_file_alloc_security() and > > save the class at that time, or otherwise introduce new hooks. > > > > There are three approaches that could be taken here: > > 1. Introduce new permissions on the existing class, as proposed by Chris, > > 2. Introduce new classes, as you proposed, > > 3. Label different kinds of fds via type_transitions or similar so > > that we can distinguish them by type in policy rather than needing > > separate permissions or classes. > > > > We've used all three approaches in the past for different kinds of > > checks so it is more a question of what is optimal for this use case. > > The last one is the heaviest approach since it imposes extra overhead > > upon allocation/labeling that doesn't currently exist for fds. > > Either new perms or new class works for me. I like the new classes idea > more than my original proposal; however, I'm unclear how a > type_transition would work in this scenario. What would the target type be? > > type_transition foo_t ?:fd foo_pidfd_t; > type_transition foo_t ?:fd foo_memfd_t; Sorry, perhaps I was unclear. The new classes proposal is just to use a different security class i.e. fd, pidfd, memfd classes, such that allow rules would be written accordingly, e.g. allow foo_t self:fd use; allow foo_t domain:pidfd use; That was Paul's proposal. Using a derived type via type_transition is the 3rd option, and that one would require introducing logic into selinux_file_alloc_security() to call security_transition_sid() and impose that overhead on all fd creation, which is likely a non-starter.
