* [PATCH] libsemanage: do not change file mode of seusers and users_extra @ 2018-04-12 10:26 Vit Mojzis 2018-04-12 15:07 ` Stephen Smalley 0 siblings, 1 reply; 5+ messages in thread From: Vit Mojzis @ 2018-04-12 10:26 UTC (permalink / raw) To: selinux Commit 8702a865e08b5660561e194a83e4a363061edc03 causes file mode of seusers and users_extra to change based on the value defined in config file whenever direct_commit is called and policy is not rebuilt. (e.g. when setting a boolean). Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1512639 $ ll /var/lib/selinux/targeted/active/users_extra -rw-------. 1 root root 101 11. dub 17.31 /var/lib/selinux/targeted/active/users_extra $ ll /var/lib/selinux/targeted/active/seusers -rw-------. 1 root root 73 11. dub 17.31 /var/lib/selinux/targeted/active/seusers $ semanage boolean -m --on httpd_can_network_connect $ ll /var/lib/selinux/targeted/active/seusers -rw-r--r--. 1 root root 73 23. bře 16.59 /var/lib/selinux/targeted/active/seusers $ ll /var/lib/selinux/targeted/active/users_extra -rw-r--r--. 1 root root 101 23. bře 16.59 /var/lib/selinux/targeted/active/users_extra $ rpm -Vq selinux-policy-targeted .M.....T. /var/lib/selinux/targeted/active/seusers .M.....T. /var/lib/selinux/targeted/active/users_extra Signed-off-by: Vit Mojzis <vmojzis@redhat.com> --- libsemanage/src/direct_api.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c index e7ec952f..c58961be 100644 --- a/libsemanage/src/direct_api.c +++ b/libsemanage/src/direct_api.c @@ -1481,7 +1481,7 @@ rebuild: retval = semanage_copy_file(path, semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS), - sh->conf->file_mode); + 0); if (retval < 0) goto cleanup; pseusers->dtable->drop_cache(pseusers->dbase); @@ -1499,7 +1499,7 @@ rebuild: retval = semanage_copy_file(path, semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA), - sh->conf->file_mode); + 0); if (retval < 0) goto cleanup; pusers_extra->dtable->drop_cache(pusers_extra->dbase); -- 2.14.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] libsemanage: do not change file mode of seusers and users_extra 2018-04-12 10:26 [PATCH] libsemanage: do not change file mode of seusers and users_extra Vit Mojzis @ 2018-04-12 15:07 ` Stephen Smalley 2018-04-12 17:22 ` Stephen Smalley 0 siblings, 1 reply; 5+ messages in thread From: Stephen Smalley @ 2018-04-12 15:07 UTC (permalink / raw) To: Vit Mojzis, selinux On 04/12/2018 06:26 AM, Vit Mojzis wrote: > Commit 8702a865e08b5660561e194a83e4a363061edc03 causes file mode of > seusers and users_extra to change based on the value defined in config > file whenever direct_commit is called and policy is not rebuilt. > (e.g. when setting a boolean). > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1512639 I think this patch is correct and expect to apply it, but am left wondering about the permissions on /var/lib/selinux/targeted in general. It appears that we are inconsistent in our file modes on files under /var/lib/selinux/targeted/active, e.g. file_contexts.homedirs, *.local, and modules/*/* are 0644, whereas other files are 0600. Of course, given that the directories are 0600, only root can even lookup files under these directories regardless of their individual file modes so it isn't as though those files are truly accessible. Looks like there are other uses of sh->conf->file_mode that are suspect in semanage_direct_commit() for files in the store, whereas I think it should only be used for installed files (i.e. /etc/selinux/targeted/*). > > $ ll /var/lib/selinux/targeted/active/users_extra > -rw-------. 1 root root 101 11. dub 17.31 /var/lib/selinux/targeted/active/users_extra > $ ll /var/lib/selinux/targeted/active/seusers > -rw-------. 1 root root 73 11. dub 17.31 /var/lib/selinux/targeted/active/seusers > $ semanage boolean -m --on httpd_can_network_connect > $ ll /var/lib/selinux/targeted/active/seusers > -rw-r--r--. 1 root root 73 23. bře 16.59 /var/lib/selinux/targeted/active/seusers > $ ll /var/lib/selinux/targeted/active/users_extra > -rw-r--r--. 1 root root 101 23. bře 16.59 /var/lib/selinux/targeted/active/users_extra > $ rpm -Vq selinux-policy-targeted > .M.....T. /var/lib/selinux/targeted/active/seusers > .M.....T. /var/lib/selinux/targeted/active/users_extra > > Signed-off-by: Vit Mojzis <vmojzis@redhat.com> > --- > libsemanage/src/direct_api.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c > index e7ec952f..c58961be 100644 > --- a/libsemanage/src/direct_api.c > +++ b/libsemanage/src/direct_api.c > @@ -1481,7 +1481,7 @@ rebuild: > retval = semanage_copy_file(path, > semanage_path(SEMANAGE_TMP, > SEMANAGE_STORE_SEUSERS), > - sh->conf->file_mode); > + 0); > if (retval < 0) > goto cleanup; > pseusers->dtable->drop_cache(pseusers->dbase); > @@ -1499,7 +1499,7 @@ rebuild: > retval = semanage_copy_file(path, > semanage_path(SEMANAGE_TMP, > SEMANAGE_USERS_EXTRA), > - sh->conf->file_mode); > + 0); > if (retval < 0) > goto cleanup; > pusers_extra->dtable->drop_cache(pusers_extra->dbase); > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libsemanage: do not change file mode of seusers and users_extra 2018-04-12 15:07 ` Stephen Smalley @ 2018-04-12 17:22 ` Stephen Smalley 2018-04-12 20:03 ` Petr Lautrbach 0 siblings, 1 reply; 5+ messages in thread From: Stephen Smalley @ 2018-04-12 17:22 UTC (permalink / raw) To: Vit Mojzis, selinux On 04/12/2018 11:07 AM, Stephen Smalley wrote: > On 04/12/2018 06:26 AM, Vit Mojzis wrote: >> Commit 8702a865e08b5660561e194a83e4a363061edc03 causes file mode of >> seusers and users_extra to change based on the value defined in config >> file whenever direct_commit is called and policy is not rebuilt. >> (e.g. when setting a boolean). >> >> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1512639 > > I think this patch is correct and expect to apply it, but am left wondering about the permissions > on /var/lib/selinux/targeted in general. It appears that we are inconsistent in our file modes > on files under /var/lib/selinux/targeted/active, e.g. file_contexts.homedirs, *.local, and modules/*/* are 0644, > whereas other files are 0600. Of course, given that the directories are 0600, only root can even lookup files under > these directories regardless of their individual file modes so it isn't as though those files are truly accessible. > Looks like there are other uses of sh->conf->file_mode that are suspect in semanage_direct_commit() for files > in the store, whereas I think it should only be used for installed files (i.e. /etc/selinux/targeted/*). Actually, we seem to be inconsistent even among different modules; some seem to be 0600 and others 0644, likely due to some being prebuilt/prepackaged that way and others installed via semodule -i. Also, policy.kern and policy.linked are presently 0644. On a separate but related note, rpm -V selinux-policy-targeted output seems somewhat surprising, e.g. wouldn't expect file_contexts.local, commit_num, etc to be managed by rpm itself. Not sure it should be managing /var/lib/selinux at all. > >> >> $ ll /var/lib/selinux/targeted/active/users_extra >> -rw-------. 1 root root 101 11. dub 17.31 /var/lib/selinux/targeted/active/users_extra >> $ ll /var/lib/selinux/targeted/active/seusers >> -rw-------. 1 root root 73 11. dub 17.31 /var/lib/selinux/targeted/active/seusers >> $ semanage boolean -m --on httpd_can_network_connect >> $ ll /var/lib/selinux/targeted/active/seusers >> -rw-r--r--. 1 root root 73 23. bře 16.59 /var/lib/selinux/targeted/active/seusers >> $ ll /var/lib/selinux/targeted/active/users_extra >> -rw-r--r--. 1 root root 101 23. bře 16.59 /var/lib/selinux/targeted/active/users_extra >> $ rpm -Vq selinux-policy-targeted >> .M.....T. /var/lib/selinux/targeted/active/seusers >> .M.....T. /var/lib/selinux/targeted/active/users_extra >> >> Signed-off-by: Vit Mojzis <vmojzis@redhat.com> >> --- >> libsemanage/src/direct_api.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c >> index e7ec952f..c58961be 100644 >> --- a/libsemanage/src/direct_api.c >> +++ b/libsemanage/src/direct_api.c >> @@ -1481,7 +1481,7 @@ rebuild: >> retval = semanage_copy_file(path, >> semanage_path(SEMANAGE_TMP, >> SEMANAGE_STORE_SEUSERS), >> - sh->conf->file_mode); >> + 0); >> if (retval < 0) >> goto cleanup; >> pseusers->dtable->drop_cache(pseusers->dbase); >> @@ -1499,7 +1499,7 @@ rebuild: >> retval = semanage_copy_file(path, >> semanage_path(SEMANAGE_TMP, >> SEMANAGE_USERS_EXTRA), >> - sh->conf->file_mode); >> + 0); >> if (retval < 0) >> goto cleanup; >> pusers_extra->dtable->drop_cache(pusers_extra->dbase); >> > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libsemanage: do not change file mode of seusers and users_extra 2018-04-12 17:22 ` Stephen Smalley @ 2018-04-12 20:03 ` Petr Lautrbach 2018-04-12 20:31 ` Stephen Smalley 0 siblings, 1 reply; 5+ messages in thread From: Petr Lautrbach @ 2018-04-12 20:03 UTC (permalink / raw) To: Stephen Smalley; +Cc: Vit Mojzis, selinux, lvrabec [-- Attachment #1: Type: text/plain, Size: 4346 bytes --] On Thu, Apr 12, 2018 at 01:22:40PM -0400, Stephen Smalley wrote: > On 04/12/2018 11:07 AM, Stephen Smalley wrote: > > On 04/12/2018 06:26 AM, Vit Mojzis wrote: > >> Commit 8702a865e08b5660561e194a83e4a363061edc03 causes file mode of > >> seusers and users_extra to change based on the value defined in config > >> file whenever direct_commit is called and policy is not rebuilt. > >> (e.g. when setting a boolean). > >> > >> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1512639 > > > > I think this patch is correct and expect to apply it, but am left wondering about the permissions > > on /var/lib/selinux/targeted in general. It appears that we are inconsistent in our file modes > > on files under /var/lib/selinux/targeted/active, e.g. file_contexts.homedirs, *.local, and modules/*/* are 0644, > > whereas other files are 0600. Of course, given that the directories are 0600, only root can even lookup files under > > these directories regardless of their individual file modes so it isn't as though those files are truly accessible. > > Looks like there are other uses of sh->conf->file_mode that are suspect in semanage_direct_commit() for files > > in the store, whereas I think it should only be used for installed files (i.e. /etc/selinux/targeted/*). > > Actually, we seem to be inconsistent even among different modules; some seem to be 0600 and others 0644, likely due > to some being prebuilt/prepackaged that way and others installed via semodule -i. Also, policy.kern and policy.linked are presently 0644. > > On a separate but related note, rpm -V selinux-policy-targeted output seems somewhat surprising, e.g. wouldn't expect file_contexts.local, commit_num, etc to be managed by rpm itself. Not sure it should be managing /var/lib/selinux at all. Note that /etc/selinux/targeted/modules/active was part of selinux-policy-targeted since 2011. file_contexts.local is in /etc/selinux and is shipped with %config(noreplace). It means it's preserved during updates and `rpm -qf /etc/selinux/targeted/contexts/files/file_contexts.local` shows the relevant package. The other files showed by `rpm -V` are probably not necessary to be included in the package. As far as I know we need to ship the SELinux store in /var/lib/selinux as whole for systems using OSTree where packages are not installed, i.e. post installation scripts are not run, but they are just extracted to a filesystem. > > > >> > >> $ ll /var/lib/selinux/targeted/active/users_extra > >> -rw-------. 1 root root 101 11. dub 17.31 /var/lib/selinux/targeted/active/users_extra > >> $ ll /var/lib/selinux/targeted/active/seusers > >> -rw-------. 1 root root 73 11. dub 17.31 /var/lib/selinux/targeted/active/seusers > >> $ semanage boolean -m --on httpd_can_network_connect > >> $ ll /var/lib/selinux/targeted/active/seusers > >> -rw-r--r--. 1 root root 73 23. bře 16.59 /var/lib/selinux/targeted/active/seusers > >> $ ll /var/lib/selinux/targeted/active/users_extra > >> -rw-r--r--. 1 root root 101 23. bře 16.59 /var/lib/selinux/targeted/active/users_extra > >> $ rpm -Vq selinux-policy-targeted > >> .M.....T. /var/lib/selinux/targeted/active/seusers > >> .M.....T. /var/lib/selinux/targeted/active/users_extra > >> > >> Signed-off-by: Vit Mojzis <vmojzis@redhat.com> > >> --- > >> libsemanage/src/direct_api.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c > >> index e7ec952f..c58961be 100644 > >> --- a/libsemanage/src/direct_api.c > >> +++ b/libsemanage/src/direct_api.c > >> @@ -1481,7 +1481,7 @@ rebuild: > >> retval = semanage_copy_file(path, > >> semanage_path(SEMANAGE_TMP, > >> SEMANAGE_STORE_SEUSERS), > >> - sh->conf->file_mode); > >> + 0); > >> if (retval < 0) > >> goto cleanup; > >> pseusers->dtable->drop_cache(pseusers->dbase); > >> @@ -1499,7 +1499,7 @@ rebuild: > >> retval = semanage_copy_file(path, > >> semanage_path(SEMANAGE_TMP, > >> SEMANAGE_USERS_EXTRA), > >> - sh->conf->file_mode); > >> + 0); > >> if (retval < 0) > >> goto cleanup; > >> pusers_extra->dtable->drop_cache(pusers_extra->dbase); > >> > > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libsemanage: do not change file mode of seusers and users_extra 2018-04-12 20:03 ` Petr Lautrbach @ 2018-04-12 20:31 ` Stephen Smalley 0 siblings, 0 replies; 5+ messages in thread From: Stephen Smalley @ 2018-04-12 20:31 UTC (permalink / raw) To: Petr Lautrbach; +Cc: Vit Mojzis, selinux, lvrabec On 04/12/2018 04:03 PM, Petr Lautrbach wrote: > On Thu, Apr 12, 2018 at 01:22:40PM -0400, Stephen Smalley wrote: >> On 04/12/2018 11:07 AM, Stephen Smalley wrote: >>> On 04/12/2018 06:26 AM, Vit Mojzis wrote: >>>> Commit 8702a865e08b5660561e194a83e4a363061edc03 causes file mode of >>>> seusers and users_extra to change based on the value defined in config >>>> file whenever direct_commit is called and policy is not rebuilt. >>>> (e.g. when setting a boolean). >>>> >>>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1512639 >>> >>> I think this patch is correct and expect to apply it, but am left wondering about the permissions >>> on /var/lib/selinux/targeted in general. It appears that we are inconsistent in our file modes >>> on files under /var/lib/selinux/targeted/active, e.g. file_contexts.homedirs, *.local, and modules/*/* are 0644, >>> whereas other files are 0600. Of course, given that the directories are 0600, only root can even lookup files under >>> these directories regardless of their individual file modes so it isn't as though those files are truly accessible. >>> Looks like there are other uses of sh->conf->file_mode that are suspect in semanage_direct_commit() for files >>> in the store, whereas I think it should only be used for installed files (i.e. /etc/selinux/targeted/*). >> >> Actually, we seem to be inconsistent even among different modules; some seem to be 0600 and others 0644, likely due >> to some being prebuilt/prepackaged that way and others installed via semodule -i. Also, policy.kern and policy.linked are presently 0644. >> >> On a separate but related note, rpm -V selinux-policy-targeted output seems somewhat surprising, e.g. wouldn't expect file_contexts.local, commit_num, etc to be managed by rpm itself. Not sure it should be managing /var/lib/selinux at all. > > Note that /etc/selinux/targeted/modules/active was part of selinux-policy-targeted since 2011. > > file_contexts.local is in /etc/selinux and is shipped with %config(noreplace). It means it's preserved during updates and > `rpm -qf /etc/selinux/targeted/contexts/files/file_contexts.local` shows the relevant package. > > The other files showed by `rpm -V` are probably not necessary to be included in the package. > > As far as I know we need to ship the SELinux store in /var/lib/selinux as whole for systems using OSTree where packages > are not installed, i.e. post installation scripts are not run, but they are just extracted to a filesystem. Don't quite understand why you'd ship a file_contexts.local at all; any file contexts you want to ship in the base policy can just go in a policy module. Also not sure why setrans.conf is part of selinux-policy-targeted instead of mcstrans. Understand why you are packaging /var/lib/selinux, just wondering if they should be verified or not as part of rpm -V. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-04-12 20:31 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-04-12 10:26 [PATCH] libsemanage: do not change file mode of seusers and users_extra Vit Mojzis 2018-04-12 15:07 ` Stephen Smalley 2018-04-12 17:22 ` Stephen Smalley 2018-04-12 20:03 ` Petr Lautrbach 2018-04-12 20:31 ` Stephen Smalley
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.