* [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.