[refpolicy patch, second try] samba policy updates

[refpolicy patch, second try] samba policy updates

[Mike Edenfield]

[-- Attachment #1: Type: text/plain, Size: 245 bytes --]


* Allow winbindd to manage its own sockets
* Allow nbmd to rename log files
* Add new interface to encapsulate home directory creation.
* Provide tunable policy to allow samba to create home directories on 
system without oddjobs PAM module.



[-- Attachment #2: samba.patch --]
[-- Type: text/plain, Size: 3321 bytes --]

Index: modules/services/samba.if
===================================================================
--- modules/services/samba.if	(revision 2767)
+++ modules/services/samba.if	(working copy)
@@ -484,17 +484,17 @@
 ## </param>
 #
 interface(`samba_stream_connect_winbind',`
-	ifdef(`distro_redhat',`
-		gen_require(`
-			type samba_var_t, winbind_t, winbind_var_run_t;
-		')
+	gen_require(`
+		type samba_var_t, winbind_t, winbind_var_run_t;
+	')
 
-		files_search_pids($1)
-		allow $1 samba_var_t:dir search_dir_perms;
-		stream_connect_pattern($1, winbind_var_run_t, winbind_var_run_t, winbind_t)
-	',`
+	files_search_pids($1)
+	allow $1 samba_var_t:dir search_dir_perms;
+	stream_connect_pattern($1, winbind_var_run_t, winbind_var_run_t, winbind_t)
+	
+	ifndef(`distro_redhat', `
 		gen_require(`
-			type winbind_t, winbind_tmp_t;
+		    type winbind_tmp_t;
 		')
 
 		# the default for the socket is (poorly named):
Index: modules/services/samba.te
===================================================================
--- modules/services/samba.te	(revision 2767)
+++ modules/services/samba.te	(working copy)
@@ -59,6 +59,13 @@
 ## </desc>
 gen_tunable(samba_share_nfs, false)
 
+## <desc>
+## <p>
+## Allow samba to create new home directories (e.g. via PAM) 
+## </p>
+## </desc>
+gen_tunable(samba_create_home_dirs, false)
+
 type nmbd_t;
 type nmbd_exec_t;
 init_daemon_domain(nmbd_t, nmbd_exec_t)
@@ -379,6 +386,14 @@
 	unprivuser_home_dir_filetrans_home_content(nmbd_t, { file dir })
 ')
 
+tunable_policy(`samba_create_home_dirs',`
+	ifdef(`distro_redhat', `
+		refpolicywarn(`Use of samba_create_home_dirs is discouraged.  Please use pam_oddjob_mkhomedir instead.')
+	', `
+		unprivuser_create_home_dirs(smbd_t)
+		allow smbd_t self:capability chown;
+	')
+')
 ########################################
 #
 # nmbd Local policy
@@ -404,8 +419,7 @@
 read_files_pattern(nmbd_t, samba_etc_t, samba_etc_t)
 
 manage_dirs_pattern(nmbd_t, samba_log_t, samba_log_t)
-append_files_pattern(nmbd_t, samba_log_t, samba_log_t)
-allow nmbd_t samba_log_t:file unlink;
+manage_files_pattern(nmbd_t, samba_log_t, samba_log_t)
 
 read_files_pattern(nmbd_t, samba_log_t, samba_log_t)
 create_files_pattern(nmbd_t, samba_log_t, samba_log_t)
@@ -675,6 +689,7 @@
 
 manage_dirs_pattern(winbind_t, winbind_tmp_t, winbind_tmp_t)
 manage_files_pattern(winbind_t, winbind_tmp_t, winbind_tmp_t)
+manage_sock_files_pattern(winbind_t, winbind_tmp_t, winbind_tmp_t)
 files_tmp_filetrans(winbind_t, winbind_tmp_t, { file dir })
 
 manage_files_pattern(winbind_t, winbind_var_run_t, winbind_var_run_t)
Index: modules/roles/unprivuser.if
===================================================================
--- modules/roles/unprivuser.if	(revision 2767)
+++ modules/roles/unprivuser.if	(working copy)
@@ -146,6 +146,22 @@
 
 ########################################
 ## <summary>
+## Create new home directories with the proper
+## home directory label.
+## </summary>
+## <param name="domain">
+##  <summary>
+##  Domain allowed access.
+##  </summary>
+## </param>
+#
+interface(`unprivuser_create_home_dirs',`
+	unprivuser_home_filetrans_home_dir($1)
+	unprivuser_manage_home_dirs($1)
+')
+
+########################################
+## <summary>
 ##	Create, read, write, and delete
 ##	subdirectories of generic user
 ##	home directories.

Re: [refpolicy patch, second try] samba policy updates

[Chris PeBenito]

[-- Attachment #1: Type: text/plain, Size: 1122 bytes --]

On Mon, 2008-07-28 at 12:00 -0400, Mike Edenfield wrote:
> +tunable_policy(`samba_create_home_dirs',`
> +	ifdef(`distro_redhat', `
> +		refpolicywarn(`Use of samba_create_home_dirs is discouraged.  Please use pam_oddjob_mkhomedir instead.')
> +	', `
> +		unprivuser_create_home_dirs(smbd_t)
> +		allow smbd_t self:capability chown;
> +	')
> +')

I don't think this has the effect that you're trying to get.  The ifdef
is resolved during build time, and the tunable is resolved at runtime.
So the warning message won't do any good for most redhat users (and it
leaves them with a noop samba_create_home_dirs tunable).

> +interface(`unprivuser_create_home_dirs',`
> +       unprivuser_home_filetrans_home_dir($1)
> +       unprivuser_manage_home_dirs($1)
> +')

"Create" just means directory create, but you have the full manage
permission set, in addition to a filetrans.

-- 
Chris PeBenito
<pebenito@gentoo.org>
Developer,
Hardened Gentoo Linux
 
Public Key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0xE6AF9243
Key fingerprint = B0E6 877A 883F A57A 8E6A  CB00 BC8E E42D E6AF 9243

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [refpolicy patch, second try] samba policy updates

[Mike Edenfield]

Chris PeBenito wrote:
> On Mon, 2008-07-28 at 12:00 -0400, Mike Edenfield wrote:
>> +tunable_policy(`samba_create_home_dirs',`
>> +	ifdef(`distro_redhat', `
>> +		refpolicywarn(`Use of samba_create_home_dirs is discouraged.  Please use pam_oddjob_mkhomedir instead.')
>> +	', `
>> +		unprivuser_create_home_dirs(smbd_t)
>> +		allow smbd_t self:capability chown;
>> +	')
>> +')
> 
> I don't think this has the effect that you're trying to get.  The ifdef
> is resolved during build time, and the tunable is resolved at runtime.
> So the warning message won't do any good for most redhat users (and it
> leaves them with a noop samba_create_home_dirs tunable).

I wasn't sure if that would work; I know Dan has expressed a preference 
for RH users to use oddjob for this, but I didn't know if there was any 
way to enforce that.  I certainly didn't want it to silently do nothing. 
  Is there a correct place to document this preference?

>> +interface(`unprivuser_create_home_dirs',`
>> +       unprivuser_home_filetrans_home_dir($1)
>> +       unprivuser_manage_home_dirs($1)
>> +')
> 
> "Create" just means directory create, but you have the full manage
> permission set, in addition to a filetrans.

I think I did go a bit overboard with the manage permissions.  I'd 
copied these from the oddjob's policy, but I notice that oddjobs also 
permits deleting home directories, which probably isn't needed here.

The filetrans is needed, though, because without it, the created home 
directories were getting labeled home_root_t instead of user_home_dir_t. 
  It also needs to copy the template files in and label them correctly, 
like the user_home_ssh_t on .ssh, etc.  I'll narrow this down.


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [refpolicy patch, second try] samba policy updates

[Chris PeBenito]

[-- Attachment #1: Type: text/plain, Size: 2785 bytes --]

On Tue, 2008-07-29 at 13:38 -0400, Mike Edenfield wrote:
> Chris PeBenito wrote:
> > On Mon, 2008-07-28 at 12:00 -0400, Mike Edenfield wrote:
> >> +tunable_policy(`samba_create_home_dirs',`
> >> +	ifdef(`distro_redhat', `
> >> +		refpolicywarn(`Use of samba_create_home_dirs is discouraged.  Please use pam_oddjob_mkhomedir instead.')
> >> +	', `
> >> +		unprivuser_create_home_dirs(smbd_t)
> >> +		allow smbd_t self:capability chown;
> >> +	')
> >> +')
> > 
> > I don't think this has the effect that you're trying to get.  The ifdef
> > is resolved during build time, and the tunable is resolved at runtime.
> > So the warning message won't do any good for most redhat users (and it
> > leaves them with a noop samba_create_home_dirs tunable).
> 
> I wasn't sure if that would work; I know Dan has expressed a preference 
> for RH users to use oddjob for this, but I didn't know if there was any 
> way to enforce that.  I certainly didn't want it to silently do nothing. 
>   Is there a correct place to document this preference?

I would like to just put

optional_policy(
  tunable_policy(samba_
    oddjob_domtrans_
  )
',`
  tunable_policy(samba_
    unprivuser_create_
  ')
')

but that doesn't work in the compiler; it doesn't allow require blocks
in the else of an optional block.  So to work around it I guess we can
just put an optional oddjob_domtrans_mkhomedir()

ifdef(`distro_redhat
  optional_policy(`
    tunable_policy(`samba_
      oddjob_domtrans_
    ')
  ')
',`
  tunable_policy(`samba_
    unprivuser_create_
    unprivuser_home_filetrans_
  ')
')

Hopefully my pseudopolicy makes sense.

> >> +interface(`unprivuser_create_home_dirs',`
> >> +       unprivuser_home_filetrans_home_dir($1)
> >> +       unprivuser_manage_home_dirs($1)
> >> +')
> > 
> > "Create" just means directory create, but you have the full manage
> > permission set, in addition to a filetrans.
> 
> I think I did go a bit overboard with the manage permissions.  I'd 
> copied these from the oddjob's policy, but I notice that oddjobs also 
> permits deleting home directories, which probably isn't needed here.
> 
> The filetrans is needed, though, because without it, the created home 
> directories were getting labeled home_root_t instead of user_home_dir_t. 
>   It also needs to copy the template files in and label them correctly, 
> like the user_home_ssh_t on .ssh, etc.  I'll narrow this down.

Thats fine, but the filetrans shouldn't be included in the create
interface itself.

-- 
Chris PeBenito
<pebenito@gentoo.org>
Developer,
Hardened Gentoo Linux
 
Public Key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0xE6AF9243
Key fingerprint = B0E6 877A 883F A57A 8E6A  CB00 BC8E E42D E6AF 9243

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [refpolicy patch, second try] samba policy updates

[Mike Edenfield]

Chris PeBenito wrote:

> ifdef(`distro_redhat
>   optional_policy(`
>     tunable_policy(`samba_
>       oddjob_domtrans_
>     ')
>   ')
> ',`
>   tunable_policy(`samba_
>     unprivuser_create_
>     unprivuser_home_filetrans_
>   ')
> ')
> 
> Hopefully my pseudopolicy makes sense.

It does, but I wonder if it's making the policy more complex than it 
needs.  Right now, in the RedHat case, samba doesn't know nor care that 
oddjob is being used to make home directories, since the PAM module runs 
an external binary that transitions into its own domain, controlled by 
the oddjob module.  I'm not sure what actual policy rules could go into 
the samba_oddjob case that would do anything useful, and I don't yet 
have a RH or FC system up and running to test any changes on (thats in 
the works, tho).

IMHO I think the two best alternatives are: skip the distro-specific 
rules and do the same thing in all cases, or just do nothing at all in 
the RedHat case.  RedHat users really should never have to enable this 
option anyway, but if they do I don't think it opens any major new 
security risks: samba already needs to have full read/write permission 
to home directory *content* (based on the existing enable_home_dirs 
tunable) for this to make any sense, and I think I've fixed the policy 
so it can only create new home directories, not delete or rename them.

Would it be sufficient warning to add this to the .te file so it shows 
up in the booleans.conf file as such:

## <desc>
## <p>
## Allow samba to create new home directories.  (Not recommended
## for RedHat-based systems where oddjob is available.)
## </p>
## </desc>
gen_tunable(samba_create_home_dirs, false)


>>>> +interface(`unprivuser_create_home_dirs',`
>>>> +       unprivuser_home_filetrans_home_dir($1)
>>>> +       unprivuser_manage_home_dirs($1)
>>>> +')
>>> "Create" just means directory create, but you have the full manage
>>> permission set, in addition to a filetrans.
>> I think I did go a bit overboard with the manage permissions.  I'd 
>> copied these from the oddjob's policy, but I notice that oddjobs also 
>> permits deleting home directories, which probably isn't needed here.
>>
>> The filetrans is needed, though, because without it, the created home 
>> directories were getting labeled home_root_t instead of user_home_dir_t. 
>>   It also needs to copy the template files in and label them correctly, 
>> like the user_home_ssh_t on .ssh, etc.  I'll narrow this down.
> 
> Thats fine, but the filetrans shouldn't be included in the create
> interface itself.

Ok, but I'm a little confused then... if I move the filetrans call out 
of the interface, then it becomes just a call into an existing 
interface.  Am I misunderstanding something?  I think I'm having trouble 
determining how granular I should make interfaces like this.

After your previous response, I updated the interface to use more 
specific rules, but it still does the same basic stuff.  For this, I'm 
using "create" not in the low-level "make directory entry in /home" 
sense, but in the higher level "do what needs to be done to end up with 
a fully functional home directory" sense.  Is that where I'm getting 
confused?  Would it help to name the interface something different, like:

interface(`unprivuser_generate_full_home_dir',`
   gen_require(`
     type user_home_t, user_home_dir_t;
   ')

   # Create home directory itself, label as user_home_dir_t
   files_search_home($1)
   files_home_filetrans($1, user_home_dir_t, dir)

   # Copy /etc/skel tree into new home directory, label as
   # user_home_t
   filetrans_pattern($1, user_home_dir_t, user_home_t,
     notdevfile_class_set)
   rw_files_pattern($1, user_home_t, user_home_t)
   add_entry_dirs_pattern($1, user_home_t, user_home_t)
')


--Mike

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [refpolicy patch, second try] samba policy updates

[Christopher J. PeBenito]

On Tue, 2008-07-29 at 17:22 -0400, Mike Edenfield wrote:
> Chris PeBenito wrote:
> 
> > ifdef(`distro_redhat
> >   optional_policy(`
> >     tunable_policy(`samba_
> >       oddjob_domtrans_
> >     ')
> >   ')
> > ',`
> >   tunable_policy(`samba_
> >     unprivuser_create_
> >     unprivuser_home_filetrans_
> >   ')
> > ')
> > 
> > Hopefully my pseudopolicy makes sense.
> 
> It does, but I wonder if it's making the policy more complex than it 
> needs.  Right now, in the RedHat case, samba doesn't know nor care that 
> oddjob is being used to make home directories, since the PAM module runs 
> an external binary that transitions into its own domain, controlled by 
> the oddjob module.  I'm not sure what actual policy rules could go into 
> the samba_oddjob case that would do anything useful, and I don't yet 
> have a RH or FC system up and running to test any changes on (thats in 
> the works, tho).
> 
> IMHO I think the two best alternatives are: skip the distro-specific 
> rules and do the same thing in all cases

I'd prefer this.

> , or just do nothing at all in 
> the RedHat case.  RedHat users really should never have to enable this 
> option anyway, but if they do I don't think it opens any major new 
> security risks: samba already needs to have full read/write permission 
> to home directory *content* (based on the existing enable_home_dirs 
> tunable) for this to make any sense, and I think I've fixed the policy 
> so it can only create new home directories, not delete or rename them.
> 
> Would it be sufficient warning to add this to the .te file so it shows 
> up in the booleans.conf file as such:
> 
> ## <desc>
> ## <p>
> ## Allow samba to create new home directories.  (Not recommended
> ## for RedHat-based systems where oddjob is available.)
> ## </p>
> ## </desc>
> gen_tunable(samba_create_home_dirs, false)

I don't think its necessary.  Mentioning oddjob doesn't do anything
since if they enable oddjob in samba, with the current policy, it will
fail, regardless if this tunable is on or off.

> >>>> +interface(`unprivuser_create_home_dirs',`
> >>>> +       unprivuser_home_filetrans_home_dir($1)
> >>>> +       unprivuser_manage_home_dirs($1)
> >>>> +')
> >>> "Create" just means directory create, but you have the full manage
> >>> permission set, in addition to a filetrans.
> >> I think I did go a bit overboard with the manage permissions.  I'd 
> >> copied these from the oddjob's policy, but I notice that oddjobs also 
> >> permits deleting home directories, which probably isn't needed here.
> >>
> >> The filetrans is needed, though, because without it, the created home 
> >> directories were getting labeled home_root_t instead of user_home_dir_t. 
> >>   It also needs to copy the template files in and label them correctly, 
> >> like the user_home_ssh_t on .ssh, etc.  I'll narrow this down.
> > 
> > Thats fine, but the filetrans shouldn't be included in the create
> > interface itself.
> 
> Ok, but I'm a little confused then... if I move the filetrans call out 
> of the interface, then it becomes just a call into an existing 
> interface.  Am I misunderstanding something?  I think I'm having trouble 
> determining how granular I should make interfaces like this.

Interfaces should not have side effects.  If you have a create interface
and it does something else like filetrans, filetrans is a side effect.
The tunable should be:

tunable_policy(
  unprivuser_create_home_dirs()
  unprivuser_home_filetrans_home_dir()
)

> After your previous response, I updated the interface to use more 
> specific rules, but it still does the same basic stuff.  For this, I'm 
> using "create" not in the low-level "make directory entry in /home" 
> sense, but in the higher level "do what needs to be done to end up with 
> a fully functional home directory" sense.  Is that where I'm getting 
> confused?  Would it help to name the interface something different, like:
> 
> interface(`unprivuser_generate_full_home_dir',`
>    gen_require(`
>      type user_home_t, user_home_dir_t;
>    ')
> 
>    # Create home directory itself, label as user_home_dir_t
>    files_search_home($1)
>    files_home_filetrans($1, user_home_dir_t, dir)
> 
>    # Copy /etc/skel tree into new home directory, label as
>    # user_home_t
>    filetrans_pattern($1, user_home_dir_t, user_home_t,
>      notdevfile_class_set)
>    rw_files_pattern($1, user_home_t, user_home_t)
>    add_entry_dirs_pattern($1, user_home_t, user_home_t)
> ')

Perhaps.  But I think the name needs work.  You'd also want to provide
rules for reading /etc/skel too.

-- 
Chris PeBenito
Tresys Technology, LLC
(410) 290-1411 x150


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [refpolicy patch, second try] samba policy updates

[Mike Edenfield]

Christopher J. PeBenito wrote:

> Interfaces should not have side effects.  If you have a create interface
> and it does something else like filetrans, filetrans is a side effect.
> The tunable should be:
> 
> tunable_policy(
>   unprivuser_create_home_dirs()
>   unprivuser_home_filetrans_home_dir()
> )

Ok, I think I get it now.  Sorry for being kinda dense :)

So, what I should have dome is something like this:

tunable_policy (
   unprivuser_home_filetrans_home_dir()
   unprivuser_create_home_dir()
   unprivuser_add_entry_home_dir()
)


with two new interfaces:

unprivuser_create_home_dir (
   allow user_home_dir_t:dir create_dir_perms
)

unprivuser_add_entry_home_dir (
   allow user_home_dir_t:dir add_entry_dir_perms
)

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [refpolicy patch, second try] samba policy updates

[Christopher J. PeBenito]

On Thu, 2008-07-31 at 12:27 -0400, Mike Edenfield wrote:
> Christopher J. PeBenito wrote:
> 
> > Interfaces should not have side effects.  If you have a create interface
> > and it does something else like filetrans, filetrans is a side effect.
> > The tunable should be:
> > 
> > tunable_policy(
> >   unprivuser_create_home_dirs()
> >   unprivuser_home_filetrans_home_dir()
> > )
> 
> Ok, I think I get it now.  Sorry for being kinda dense :)
> 
> So, what I should have dome is something like this:
> 
> tunable_policy (
>    unprivuser_home_filetrans_home_dir()
>    unprivuser_create_home_dir()
>    unprivuser_add_entry_home_dir()
> )
> 
> 
> with two new interfaces:
> 
> unprivuser_create_home_dir (
>    allow user_home_dir_t:dir create_dir_perms
> )
> 
> unprivuser_add_entry_home_dir (
>    allow user_home_dir_t:dir add_entry_dir_perms
> )

Why do you need the add_entry?  You just want to create
the /home/foouser directory right?  Add_entry is for when you want to
create an object in that type of directory.

-- 
Chris PeBenito
Tresys Technology, LLC
(410) 290-1411 x150


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [refpolicy patch, second try] samba policy updates

[Mike Edenfield]

Christopher J. PeBenito wrote:
> On Thu, 2008-07-31 at 12:27 -0400, Mike Edenfield wrote:
>> Christopher J. PeBenito wrote:
>>
>>> Interfaces should not have side effects.  If you have a create interface
>>> and it does something else like filetrans, filetrans is a side effect.
>>> The tunable should be:
>>>
>>> tunable_policy(
>>>   unprivuser_create_home_dirs()
>>>   unprivuser_home_filetrans_home_dir()
>>> )
>> Ok, I think I get it now.  Sorry for being kinda dense :)
>>
>> So, what I should have dome is something like this:
>>
>> tunable_policy (
>>    unprivuser_home_filetrans_home_dir()
>>    unprivuser_create_home_dir()
>>    unprivuser_add_entry_home_dir()
>> )
>>
>>
>> with two new interfaces:
>>
>> unprivuser_create_home_dir (
>>    allow user_home_dir_t:dir create_dir_perms
>> )
>>
>> unprivuser_add_entry_home_dir (
>>    allow user_home_dir_t:dir add_entry_dir_perms
>> )
> 
> Why do you need the add_entry?  You just want to create
> the /home/foouser directory right?  Add_entry is for when you want to
> create an object in that type of directory.

Well, I do actually want to create objects in the new home directory. 
It needs to be populated from /etc/skel (which the PAM module does 
automatically).

But it turns out that the existing samba policy already takes care of 
this, *if* you also turn on "enable_home_dirs", so its not needed here. 
  It does mean that the "create" tunable, without the "enable" tunable, 
creates an empty home directory, which I was trying to minimize.  But as 
I think about it, it probably doesn't make sense to have Samba create 
user home directories if you're not going to allow Samba to share them 
out, so it's probably ok.

New patch coming up.

--Mike


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.