On Fri, Jul 26, 2024 at 1:54 PM Stephen Smalley <stephen.smalley.work@xxxxxxxxx> wrote: > > On Fri, Jul 26, 2024 at 1:50 PM Stephen Smalley > <stephen.smalley.work@xxxxxxxxx> wrote: > > > > On Thu, Jul 25, 2024 at 12:11 PM Vit Mojzis <vmojzis@xxxxxxxxxx> wrote: > > > > > > Make sure that file context (all parts) and ownership of > > > files/directories in policy store does not change no matter which user > > > and under which context executes policy rebuild. > > > > > > Fixes: > > > # semodule -B > > > # ls -lZ /etc/selinux/targeted/contexts/files > > > > > > -rw-r--r--. 1 root root unconfined_u:object_r:file_context_t:s0 421397 Jul 11 09:57 file_contexts > > > -rw-r--r--. 1 root root unconfined_u:object_r:file_context_t:s0 593470 Jul 11 09:57 file_contexts.bin > > > -rw-r--r--. 1 root root unconfined_u:object_r:file_context_t:s0 14704 Jul 11 09:57 file_contexts.homedirs > > > -rw-r--r--. 1 root root unconfined_u:object_r:file_context_t:s0 20289 Jul 11 09:57 file_contexts.homedirs.bin > > > > > > SELinux user changed from system_u to the user used to execute semodule > > > > > > # capsh --user=testuser --caps="cap_dac_override,cap_chown+eip" --addamb=cap_dac_override,cap_chown -- -c "semodule -B" > > > # ls -lZ /etc/selinux/targeted/contexts/files > > > > > > -rw-r--r--. 1 testuser testuser unconfined_u:object_r:file_context_t:s0 421397 Jul 19 09:10 file_contexts > > > -rw-r--r--. 1 testuser testuser unconfined_u:object_r:file_context_t:s0 593470 Jul 19 09:10 file_contexts.bin > > > -rw-r--r--. 1 testuser testuser unconfined_u:object_r:file_context_t:s0 14704 Jul 19 09:10 file_contexts.homedirs > > > -rw-r--r--. 1 testuser testuser unconfined_u:object_r:file_context_t:s0 20289 Jul 19 09:10 file_contexts.homedirs.bin > > > > > > Both file context and ownership changed -- causes remote login > > > failures and other issues in some scenarios. > > > > > > Signed-off-by: Vit Mojzis <vmojzis@xxxxxxxxxx> > > > > With the selinux/restorecon.h fix applied first, > > > > Acked-by: Stephen Smalley <stephen.smalley.work@xxxxxxxxx> > > Ah, spoke too soon. The GitHub CI testing failed with this: > semanage_store.c: In function ‘semanage_setfiles’: > 520 semanage_store.c:3047:25: error: ignoring return value of ‘fchown’ > declared with attribute ‘warn_unused_result’ [-Werror=unused-result] > 521 3047 | fchown(fd, 0, 0); > 522 | ^~~~~~~~~~~~~~~~ Unfortunately, you can't just mark this as intentional, as per https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25509. Casting to void doesn't eliminate the warning. Saving the return value but not using it somewhere triggers another warning. Calling assert() if rc != 0 breaks make test in libsemanage. We don't have a handle here or in the immediate callers so can't just call ERR(). Changing it to: rc = fchown(fd, 0, 0); if (rc) fprintf(stderr, "Warning! Could not set ownership of %s to root\n",path); seems to solve the warning problem but is a bit ugly.
