refpolicy is missing on lots of hits with audit2allow -R.

refpolicy is missing on lots of hits with audit2allow -R.

[Daniel J Walsh]

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

The reason for this is threshold, setting.  I think the interfaces are
getting more complicated and one AVC that is looking for read ends up
being two far different from the threshold, so audit2allow does not
report it.

For example.

node=(removed) type=AVC msg=audit(1271587735.632:422): avc:  denied  {
getattr } for  pid=13239 comm="openvpn"
path="/home/bbaetz/.pki/vpn01_cacert.pem" dev=dm-3 ino=565334
scontext=system_u:system_r:openvpn_t:s0
tcontext=unconfined_u:object_r:home_cert_t:s0 tclass=file

node=(removed) type=SYSCALL msg=audit(1271587735.632:422): arch=c000003e
syscall=5 success=yes exit=0 a0=6 a1=7fffd84f4a20 a2=7fffd84f4a20 a3=18
items=0 ppid=13235 pid=13239 auid=4294967295 uid=0 gid=0 euid=0 suid=0
fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="openvpn"
exe="/usr/sbin/openvpn" subj=system_u:system_r:openvpn_t:s0 key=(null)

Finds userdom_read_home_certs but it's threshold is two far off, so it
reports nothing.

I think we should either eliminate the threshold and report the best
interface that we have, and let the policy writer decide if he wants the
match.

If you look at the interface userdom_read_home_certs.

[InterfaceVector userdom_read_home_certs $1:source ]
$1,home_cert_t,file,read,lock,getattr,open,ioctl
$1,home_cert_t,dir,ioctl,search,read,lock,open,getattr
$1,home_cert_t,lnk_file,read,getattr
$1,home_root_t,dir,getattr,open,search
$1,home_root_t,lnk_file,read,getattr
$1,user_home_dir_t,dir,getattr,open,search
$1,user_home_dir_t,lnk_file,read,getattr

A domain that is allowed to search the homedir is always going to
generate an AVC that is a long way off.

I thing we should either remove the bastards and just add all as childs,
or recode it like the attachment.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkvMacAACgkQrlYvE4MpobNeAgCfcoVssEQJ8mfZT/aBvAt0z7+3
CoMAnR1bOcXk7x/jIZ+0i2Kc/faUJAVk
=Uuf7
-----END PGP SIGNATURE-----

[-- Attachment #2: diff --]
[-- Type: text/plain, Size: 650 bytes --]

--- matching.py~	2010-03-30 11:10:18.000000000 -0400
+++ matching.py	2010-04-19 10:31:51.000000000 -0400
@@ -63,14 +63,15 @@
     def best(self):
         if len(self.children):
             return self.children[0]
-        else:
-            return None
+        if len(self.bastards):
+            return self.bastards[0]
+        return None
 
     def __len__(self):
         # Only return the length of the matches so
         # that this can be used to test if there is
         # a match.
-        return len(self.children)
+        return len(self.children) + len(self.bastards)
 
     def __iter__(self):
         return iter(self.children)

[-- Attachment #3: diff.sig --]
[-- Type: application/pgp-signature, Size: 72 bytes --]

Re: refpolicy is missing on lots of hits with audit2allow -R.

[Karl MacMillan]

On Mon, Apr 19, 2010 at 10:33 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> The reason for this is threshold, setting.  I think the interfaces are
> getting more complicated and one AVC that is looking for read ends up
> being two far different from the threshold, so audit2allow does not
> report it.
>

Is it really that the interfaces have more access in them or is it
that our measure of difference is off? See below.

[snip]

>
> If you look at the interface userdom_read_home_certs.
>
> [InterfaceVector userdom_read_home_certs $1:source ]
> $1,home_cert_t,file,read,lock,getattr,open,ioctl
> $1,home_cert_t,dir,ioctl,search,read,lock,open,getattr
> $1,home_cert_t,lnk_file,read,getattr
> $1,home_root_t,dir,getattr,open,search
> $1,home_root_t,lnk_file,read,getattr
> $1,user_home_dir_t,dir,getattr,open,search
> $1,user_home_dir_t,lnk_file,read,getattr
>
> A domain that is allowed to search the homedir is always going to
> generate an AVC that is a long way off.
>

Seems to me that the problem is that the read / getattr on
user_home_dir_t directories and files is adding too much distance.

> I thing we should either remove the bastards and just add all as childs,
> or recode it like the attachment.
>

I'm against removing the threshold altogether - if we do that then
we'll get a match for almost everything including completely wrong
interfaces. Can we start with tweaking either the perm weights or the
distance calculation? For example, what happens when you drop the
weight for dir read down to 5 or 1 and similar for lnk_file (they are
both 10 right now)? After that we might need to tweak the threshold.

Also - I've been hacking on a patch to add in attribute access to the
interface vectors. Any idea how much help we would get from that?

Karl

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.14 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkvMacAACgkQrlYvE4MpobNeAgCfcoVssEQJ8mfZT/aBvAt0z7+3
> CoMAnR1bOcXk7x/jIZ+0i2Kc/faUJAVk
> =Uuf7
> -----END PGP SIGNATURE-----
>


--
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 is missing on lots of hits with audit2allow -R.

[Karl MacMillan]

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

On Mon, Apr 19, 2010 at 11:53 AM, Karl MacMillan
<karlwmacmillan@gmail.com> wrote:
> On Mon, Apr 19, 2010 at 10:33 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
>>
>> If you look at the interface userdom_read_home_certs.
>>
>> [InterfaceVector userdom_read_home_certs $1:source ]
>> $1,home_cert_t,file,read,lock,getattr,open,ioctl
>> $1,home_cert_t,dir,ioctl,search,read,lock,open,getattr
>> $1,home_cert_t,lnk_file,read,getattr
>> $1,home_root_t,dir,getattr,open,search
>> $1,home_root_t,lnk_file,read,getattr
>> $1,user_home_dir_t,dir,getattr,open,search
>> $1,user_home_dir_t,lnk_file,read,getattr
>>
>> A domain that is allowed to search the homedir is always going to
>> generate an AVC that is a long way off.
>>
>
> Seems to me that the problem is that the read / getattr on
> user_home_dir_t directories and files is adding too much distance.
>

I looked at this a bit more - there are a few interesting issues:

1. The open permissions have not been added to the perm_map file
(patch attached to fix that). When there is no perm map then we weight
the permission at 5 and assume read and write. Since we heavily
penalize providing a write interface for a read access request, this
causes the return of a large distance (as I believe that it should).
I'd like to find a long term home for the perm map file that increases
it's likelihood of being updated with new permissions (Chris - what do
you think of including this with reference policy?).

2. As I mentioned, the weighting on things like reading a symlink or
directory contents is fairly high. For this use I don't think that's
appropriate - the attached perm map patch also reduces these pretty
drastically.

3. The bulk of the rest of the distance comes from the unrelated
type/object access. We get access to 4 type/object class pairs that
aren't requested. Unfortunately there is no tracking of the notion of
related access or scaling of the type penalty based upon how minor the
related access is. Perhaps we should scale the type penalty with the
permission weighting?

4. The final issue is that this interface is a bit broad for the
requested access - requesting getattr for the cert files and getting
read is a pretty significant bump. Once you resolve the perm_map issue
then the distance is just over the threshold - which is about where I
was aiming with this algorithm (go me). Now, the reality is likely
that a read would come right after this getattr - if that had have
been in the audit log all would be fine. Having said that, I'm not
against making the threshold a little higher to return these (another
patch attached).

Long term, I wonder if we should return interfaces above the threshold
but commented out with a note explaining that the interface, while
providing the requested access, is potentially dangerously broad. Dan,
any chance you want to add that feature?

Karl

>> I thing we should either remove the bastards and just add all as childs,
>> or recode it like the attachment.
>>
>
> I'm against removing the threshold altogether - if we do that then
> we'll get a match for almost everything including completely wrong
> interfaces. Can we start with tweaking either the perm weights or the
> distance calculation? For example, what happens when you drop the
> weight for dir read down to 5 or 1 and similar for lnk_file (they are
> both 10 right now)? After that we might need to tweak the threshold.
>
> Also - I've been hacking on a patch to add in attribute access to the
> interface vectors. Any idea how much help we would get from that?
>
> Karl
>
>> -----BEGIN PGP SIGNATURE-----
>> Version: GnuPG v2.0.14 (GNU/Linux)
>> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>>
>> iEYEARECAAYFAkvMacAACgkQrlYvE4MpobNeAgCfcoVssEQJ8mfZT/aBvAt0z7+3
>> CoMAnR1bOcXk7x/jIZ+0i2Kc/faUJAVk
>> =Uuf7
>> -----END PGP SIGNATURE-----
>>
>

[-- Attachment #2: 0001-Update-perm-map-with-open-permissions.patch --]
[-- Type: application/octet-stream, Size: 4787 bytes --]

From 83b22cfdc01073a378fc9d50875575d252b95003 Mon Sep 17 00:00:00 2001
Message-Id: <83b22cfdc01073a378fc9d50875575d252b95003.1271724209.git.kmacmillan@tresys.com>
From: Karl MacMillan <kmacmillan@tresys.com>
Date: Mon, 19 Apr 2010 20:09:41 -0400
Subject: [PATCH 1/2] Update perm map with open permissions.

---
 sepolgen/src/share/perm_map |   51 ++++++++++++++++++++++++------------------
 1 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/sepolgen/src/share/perm_map b/sepolgen/src/share/perm_map
index eb2e23b..ca4fa4d 100644
--- a/sepolgen/src/share/perm_map
+++ b/sepolgen/src/share/perm_map
@@ -124,7 +124,7 @@ class filesystem 10
           quotamod     w           1
           quotaget     r           1
 
-class file 20
+class file 21
   execute_no_trans     r           1
         entrypoint     r           1
            execmod     n           1
@@ -141,48 +141,50 @@ class file 20
             unlink     w           1
               link     w           1
             rename     w           5
-           execute     r           100
+           execute     r           10
             swapon     b           1
            quotaon     b           1
            mounton     b           1
+	      open     r	   1
 
-class dir 22
-          add_name     w           5
+class dir 23
+          add_name     w           1
        remove_name     w           1
           reparent     w           1
             search     r           1
              rmdir     b           1
              ioctl     n           1
-              read     r          10
-             write     w          10
+              read     r           1
+             write     w           1
             create     w           1
-           getattr     r           7
-           setattr     w           7
+           getattr     r           1
+           setattr     w           1
               lock     n           1
-       relabelfrom     r           10
-         relabelto     w           10
+       relabelfrom     r           1
+         relabelto     w           1
             append     w           1
             unlink     w           1
               link     w           1
-            rename     w           5
+            rename     w           1
            execute     r           1
             swapon     b           1
            quotaon     b           1
            mounton     b           1
+	      open     r	   1
 
 class fd 1
                use     b           1
 
-class lnk_file 17
+class lnk_file 18
              ioctl     n           1
-              read     r          10
-             write     w          10
+              read     r           1
+             write     w           1
             create     w           1
-           getattr     r           7
-           setattr     w           7
+           getattr     r           1
+           setattr     w           1
               lock     n           1
-       relabelfrom     r           10
-         relabelto     w           10
+       relabelfrom     r           1
+         relabelto     w           1
             append     w           1
             unlink     w           1
               link     w           1
@@ -191,8 +193,9 @@ class lnk_file 17
             swapon     b           1
            quotaon     b           1
            mounton     b           1
+	      open     r	   1
 
-class chr_file 20
+class chr_file 21
   execute_no_trans     r           1
         entrypoint     r           1
            execmod     n           1
@@ -213,8 +216,9 @@ class chr_file 20
             swapon     b           1
            quotaon     b           1
            mounton     b           1
+	      open     r	   1
 
-class blk_file 17
+class blk_file 18
              ioctl     n           1
               read     r          10
              write     w          10
@@ -232,8 +236,9 @@ class blk_file 17
             swapon     b           1
            quotaon     b           1
            mounton     b           1
+	      open     r	   1
 
-class sock_file 17
+class sock_file 18
              ioctl     n           1
               read     r          10
              write     w          10
@@ -251,8 +256,9 @@ class sock_file 17
             swapon     b           1
            quotaon     b           1
            mounton     b           1
+	      open     r	   1
 
-class fifo_file 17
+class fifo_file 18
              ioctl     n           1
               read     r          10
              write     w          10
@@ -270,6 +276,7 @@ class fifo_file 17
             swapon     b           1
            quotaon     b           1
            mounton     b           1
+	      open     r	   1
 
 class socket 22
              ioctl     n           1
-- 
1.6.6.1


[-- Attachment #3: 0002-Increase-the-matching-threshold.patch --]
[-- Type: application/octet-stream, Size: 1029 bytes --]

From f08915971be3f54acb94c931db33dc64f0c3d435 Mon Sep 17 00:00:00 2001
Message-Id: <f08915971be3f54acb94c931db33dc64f0c3d435.1271724209.git.kmacmillan@tresys.com>
In-Reply-To: <83b22cfdc01073a378fc9d50875575d252b95003.1271724209.git.kmacmillan@tresys.com>
References: <83b22cfdc01073a378fc9d50875575d252b95003.1271724209.git.kmacmillan@tresys.com>
From: Karl MacMillan <kmacmillan@tresys.com>
Date: Mon, 19 Apr 2010 20:40:48 -0400
Subject: [PATCH 2/2] Increase the matching threshold.

---
 sepolgen/src/sepolgen/matching.py |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sepolgen/src/sepolgen/matching.py b/sepolgen/src/sepolgen/matching.py
index 1a9a3e5..492694f 100644
--- a/sepolgen/src/sepolgen/matching.py
+++ b/sepolgen/src/sepolgen/matching.py
@@ -50,7 +50,7 @@ class Match:
                 return 1
 
 class MatchList:
-    DEFAULT_THRESHOLD = 120
+    DEFAULT_THRESHOLD = 150
     def __init__(self):
         # Match objects that pass the threshold
         self.children = []
-- 
1.6.6.1

Re: refpolicy is missing on lots of hits with audit2allow -R.

[Daniel J Walsh]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/19/2010 10:33 AM, Daniel J Walsh wrote:
> The reason for this is threshold, setting.  I think the interfaces are
> getting more complicated and one AVC that is looking for read ends up
> being two far different from the threshold, so audit2allow does not
> report it.
> 
> For example.
> 
> node=(removed) type=AVC msg=audit(1271587735.632:422): avc:  denied  {
> getattr } for  pid=13239 comm="openvpn"
> path="/home/bbaetz/.pki/vpn01_cacert.pem" dev=dm-3 ino=565334
> scontext=system_u:system_r:openvpn_t:s0
> tcontext=unconfined_u:object_r:home_cert_t:s0 tclass=file
> 
> node=(removed) type=SYSCALL msg=audit(1271587735.632:422): arch=c000003e
> syscall=5 success=yes exit=0 a0=6 a1=7fffd84f4a20 a2=7fffd84f4a20 a3=18
> items=0 ppid=13235 pid=13239 auid=4294967295 uid=0 gid=0 euid=0 suid=0
> fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="openvpn"
> exe="/usr/sbin/openvpn" subj=system_u:system_r:openvpn_t:s0 key=(null)
> 
> Finds userdom_read_home_certs but it's threshold is two far off, so it
> reports nothing.
> 
> I think we should either eliminate the threshold and report the best
> interface that we have, and let the policy writer decide if he wants the
> match.
> 
> If you look at the interface userdom_read_home_certs.
> 
> [InterfaceVector userdom_read_home_certs $1:source ]
> $1,home_cert_t,file,read,lock,getattr,open,ioctl
> $1,home_cert_t,dir,ioctl,search,read,lock,open,getattr
> $1,home_cert_t,lnk_file,read,getattr
> $1,home_root_t,dir,getattr,open,search
> $1,home_root_t,lnk_file,read,getattr
> $1,user_home_dir_t,dir,getattr,open,search
> $1,user_home_dir_t,lnk_file,read,getattr
> 
> A domain that is allowed to search the homedir is always going to
> generate an AVC that is a long way off.
> 
> I thing we should either remove the bastards and just add all as childs,
> or recode it like the attachment.
> 
I tried that but we got down to,

#   userdom_read_home_certs(openvpn_t) # [283]

Which is still way higher then 120.

Isn't the problem that openvpn_t has

1# sesearch -A -s openvpn_t -t home_root_t
Found 2 semantic av rules:
   allow openvpn_t home_root_t : dir { getattr search open } ;
   allow openvpn_t home_root_t : lnk_file { read getattr } ;

- -bash-4.1# sesearch -A -s openvpn_t -t user_home_dir_t
Found 1 semantic av rules:
   allow openvpn_t user_home_dir_t : dir { ioctl read getattr lock
search open } ;


But these count towards the total?

Should we be checking in the policy what access is already present in
the domain, to make a better decision.  Or would this end up taking to long.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkvNvkYACgkQrlYvE4MpobPVGQCfZEeAd1f7424uWXrJGbiUDyPZ
Z5UAn1k60lKjKJxZtq5d6ON51iKMvda2
=2SYZ
-----END PGP SIGNATURE-----

--
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 is missing on lots of hits with audit2allow -R.

[Christopher J. PeBenito]

On Tue, 2010-04-20 at 10:37 -0400, Karl MacMillan wrote:
> On Mon, Apr 19, 2010 at 11:53 AM, Karl MacMillan
> <karlwmacmillan@gmail.com> wrote:
> > On Mon, Apr 19, 2010 at 10:33 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
> >>
> >> If you look at the interface userdom_read_home_certs.
> >>
> >> [InterfaceVector userdom_read_home_certs $1:source ]
> >> $1,home_cert_t,file,read,lock,getattr,open,ioctl
> >> $1,home_cert_t,dir,ioctl,search,read,lock,open,getattr
> >> $1,home_cert_t,lnk_file,read,getattr
> >> $1,home_root_t,dir,getattr,open,search
> >> $1,home_root_t,lnk_file,read,getattr
> >> $1,user_home_dir_t,dir,getattr,open,search
> >> $1,user_home_dir_t,lnk_file,read,getattr
> >>
> >> A domain that is allowed to search the homedir is always going to
> >> generate an AVC that is a long way off.
> >>
> >
> > Seems to me that the problem is that the read / getattr on
> > user_home_dir_t directories and files is adding too much distance.
> >
> 
> I looked at this a bit more - there are a few interesting issues:
> 
> 1. The open permissions have not been added to the perm_map file
> (patch attached to fix that). When there is no perm map then we weight
> the permission at 5 and assume read and write. Since we heavily
> penalize providing a write interface for a read access request, this
> causes the return of a large distance (as I believe that it should).
> I'd like to find a long term home for the perm map file that increases
> it's likelihood of being updated with new permissions (Chris - what do
> you think of including this with reference policy?).

I'm fine with it, just as long as the output perm map file has a
agreed-upon standard format.  It looks like sepolgen has the same format
as setools, so that probably won't be a problem (unless there are other
tools with perm maps that I am unaware of).

-- 
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 is missing on lots of hits with audit2allow -R.

[Daniel J Walsh]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/20/2010 10:37 AM, Karl MacMillan wrote:
> On Mon, Apr 19, 2010 at 11:53 AM, Karl MacMillan
> <karlwmacmillan@gmail.com> wrote:
>> On Mon, Apr 19, 2010 at 10:33 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
>>>
>>> If you look at the interface userdom_read_home_certs.
>>>
>>> [InterfaceVector userdom_read_home_certs $1:source ]
>>> $1,home_cert_t,file,read,lock,getattr,open,ioctl
>>> $1,home_cert_t,dir,ioctl,search,read,lock,open,getattr
>>> $1,home_cert_t,lnk_file,read,getattr
>>> $1,home_root_t,dir,getattr,open,search
>>> $1,home_root_t,lnk_file,read,getattr
>>> $1,user_home_dir_t,dir,getattr,open,search
>>> $1,user_home_dir_t,lnk_file,read,getattr
>>>
>>> A domain that is allowed to search the homedir is always going to
>>> generate an AVC that is a long way off.
>>>
>>
>> Seems to me that the problem is that the read / getattr on
>> user_home_dir_t directories and files is adding too much distance.
>>
> 
> I looked at this a bit more - there are a few interesting issues:
> 
> 1. The open permissions have not been added to the perm_map file
> (patch attached to fix that). When there is no perm map then we weight
> the permission at 5 and assume read and write. Since we heavily
> penalize providing a write interface for a read access request, this
> causes the return of a large distance (as I believe that it should).
> I'd like to find a long term home for the perm map file that increases
> it's likelihood of being updated with new permissions (Chris - what do
> you think of including this with reference policy?).
> 
> 2. As I mentioned, the weighting on things like reading a symlink or
> directory contents is fairly high. For this use I don't think that's
> appropriate - the attached perm map patch also reduces these pretty
> drastically.
> 
> 3. The bulk of the rest of the distance comes from the unrelated
> type/object access. We get access to 4 type/object class pairs that
> aren't requested. Unfortunately there is no tracking of the notion of
> related access or scaling of the type penalty based upon how minor the
> related access is. Perhaps we should scale the type penalty with the
> permission weighting?
> 
> 4. The final issue is that this interface is a bit broad for the
> requested access - requesting getattr for the cert files and getting
> read is a pretty significant bump. Once you resolve the perm_map issue
> then the distance is just over the threshold - which is about where I
> was aiming with this algorithm (go me). Now, the reality is likely
> that a read would come right after this getattr - if that had have
> been in the audit log all would be fine. Having said that, I'm not
> against making the threshold a little higher to return these (another
> patch attached).
> 
> Long term, I wonder if we should return interfaces above the threshold
> but commented out with a note explaining that the interface, while
> providing the requested access, is potentially dangerously broad. Dan,
> any chance you want to add that feature?
> 
> Karl
> 
>>> I thing we should either remove the bastards and just add all as childs,
>>> or recode it like the attachment.
>>>
>>
>> I'm against removing the threshold altogether - if we do that then
>> we'll get a match for almost everything including completely wrong
>> interfaces. Can we start with tweaking either the perm weights or the
>> distance calculation? For example, what happens when you drop the
>> weight for dir read down to 5 or 1 and similar for lnk_file (they are
>> both 10 right now)? After that we might need to tweak the threshold.
>>
>> Also - I've been hacking on a patch to add in attribute access to the
>> interface vectors. Any idea how much help we would get from that?
>>
>> Karl
>>
>>> -----BEGIN PGP SIGNATURE-----
>>> Version: GnuPG v2.0.14 (GNU/Linux)
>>> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>>>
>>> iEYEARECAAYFAkvMacAACgkQrlYvE4MpobNeAgCfcoVssEQJ8mfZT/aBvAt0z7+3
>>> CoMAnR1bOcXk7x/jIZ+0i2Kc/faUJAVk
>>> =Uuf7
>>> -----END PGP SIGNATURE-----
>>>
>>
Ok that works, but If we move to a more general case. or openvn_t
getattr on etc_t

#============= openvpn_t ==============
# src="openvpn_t" tgt="etc_t" class="file", perms="getattr"
# comm="openvpn" exe="" path=""
# Interface options:
#   automount_exec_config(openvpn_t) # [51]
#   files_exec_etc_files(openvpn_t) # [51]
#   files_delete_etc_files(openvpn_t) # [118]
#   files_relabel_etc_files(openvpn_t) # [136]
#   files_rw_etc_files(openvpn_t) # [161]
#   files_read_etc_files(openvpn_t) # [171]
#   files_manage_etc_files(openvpn_t) # [179]
#   auth_use_nsswitch(openvpn_t) # [1342]
#   seutil_semanage_policy(openvpn_t) # [3489]
#   auth_login_pgm_domain(openvpn_t) # [3717]
#   portage_compile_domain(openvpn_t) # [4004]

I would have expected files_read_etc_files(openvpn_t)  to be the
closest/best match.

The tool is getting confused by attributes.  Since attributes are not
currently interpretable, they should be eliminated from the calculation.
Best way to do this is just eliminate any types that don't end in a _t.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkvPBdMACgkQrlYvE4MpobP9IQCePlmwSbiO94NTCiu1mHwUzdkI
8YsAn3tlgDQljeLLLhJmMaUGRHFkrBVp
=8OfI
-----END PGP SIGNATURE-----

--
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 is missing on lots of hits with audit2allow -R.

[Karl MacMillan]

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

On Wed, Apr 21, 2010 at 10:04 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
> Ok that works, but If we move to a more general case. or openvn_t
> getattr on etc_t
>
> #============= openvpn_t ==============
> # src="openvpn_t" tgt="etc_t" class="file", perms="getattr"
> # comm="openvpn" exe="" path=""
> # Interface options:
> #   automount_exec_config(openvpn_t) # [51]
> #   files_exec_etc_files(openvpn_t) # [51]
> #   files_delete_etc_files(openvpn_t) # [118]
> #   files_relabel_etc_files(openvpn_t) # [136]
> #   files_rw_etc_files(openvpn_t) # [161]
> #   files_read_etc_files(openvpn_t) # [171]
> #   files_manage_etc_files(openvpn_t) # [179]
> #   auth_use_nsswitch(openvpn_t) # [1342]
> #   seutil_semanage_policy(openvpn_t) # [3489]
> #   auth_login_pgm_domain(openvpn_t) # [3717]
> #   portage_compile_domain(openvpn_t) # [4004]
>
> I would have expected files_read_etc_files(openvpn_t)  to be the
> closest/best match.
>

Can you send me the audit messages for this?

> The tool is getting confused by attributes.  Since attributes are not
> currently interpretable, they should be eliminated from the calculation.
> Best way to do this is just eliminate any types that don't end in a _t.

I'm not certain what you mean by this - confused in what way? The only
thing I know about is the lack of typattribute statements. The
attached patch adds attribute handling to sepolgen. It's only lightly
tested but I wanted you to get it sooner rather than later.

Karl

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.14 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkvPBdMACgkQrlYvE4MpobP9IQCePlmwSbiO94NTCiu1mHwUzdkI
> 8YsAn3tlgDQljeLLLhJmMaUGRHFkrBVp
> =8OfI
> -----END PGP SIGNATURE-----
>
> --
> 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.
>

[-- Attachment #2: 0001-Add-attribute-handling-to-sepolgen.patch --]
[-- Type: application/octet-stream, Size: 20819 bytes --]

From 6192cbf8ea0e7bf7e7fb9e1857f2da781fb378fc Mon Sep 17 00:00:00 2001
From: Karl MacMillan <kmacmillan@tresys.com>
Date: Wed, 21 Apr 2010 21:16:26 -0400
Subject: [PATCH]  Add attribute handling to sepolgen.

Sepolgen currently cannot resolve typeattribute statements in interfaces,
meaning that much of the access allowed by some attributes is missed. This
patch adds a helper application to extract attribute access from the current
binary policy file, changes sepolgen-ifgen to use that helper, and changes
the core code to add in the access from those tools.

I also move sepolgen-ifgen to its own directory under policycoreutils.
---
 policycoreutils/Makefile                           |    2 +-
 policycoreutils/audit2allow/Makefile               |    1 -
 policycoreutils/audit2allow/sepolgen-ifgen         |   89 --------
 policycoreutils/sepolgen-ifgen/Makefile            |   26 +++
 policycoreutils/sepolgen-ifgen/sepolgen-ifgen      |  126 +++++++++++
 .../sepolgen-ifgen/sepolgen-ifgen-attr-helper.c    |  230 ++++++++++++++++++++
 sepolgen/src/sepolgen/defaults.py                  |    3 +
 sepolgen/src/sepolgen/interfaces.py                |   72 ++++++-
 8 files changed, 450 insertions(+), 99 deletions(-)
 delete mode 100644 policycoreutils/audit2allow/sepolgen-ifgen
 create mode 100644 policycoreutils/sepolgen-ifgen/Makefile
 create mode 100644 policycoreutils/sepolgen-ifgen/sepolgen-ifgen
 create mode 100644 policycoreutils/sepolgen-ifgen/sepolgen-ifgen-attr-helper.c

diff --git a/policycoreutils/Makefile b/policycoreutils/Makefile
index 538302b..cbfb0e3 100644
--- a/policycoreutils/Makefile
+++ b/policycoreutils/Makefile
@@ -1,4 +1,4 @@
-SUBDIRS = setfiles semanage load_policy newrole run_init secon audit2allow audit2why scripts sestatus semodule_package semodule semodule_link semodule_expand semodule_deps setsebool po
+SUBDIRS = setfiles semanage load_policy newrole run_init secon audit2allow audit2why scripts sestatus semodule_package semodule semodule_link semodule_expand semodule_deps setsebool sepolgen-ifgen po
 
 INOTIFYH = $(shell ls /usr/include/sys/inotify.h 2>/dev/null)
 
diff --git a/policycoreutils/audit2allow/Makefile b/policycoreutils/audit2allow/Makefile
index 144f10f..999c422 100644
--- a/policycoreutils/audit2allow/Makefile
+++ b/policycoreutils/audit2allow/Makefile
@@ -10,7 +10,6 @@ all: ;
 install: all
 	-mkdir -p $(BINDIR)
 	install -m 755 audit2allow $(BINDIR)
-	install -m 755 sepolgen-ifgen $(BINDIR)
 	-mkdir -p $(MANDIR)/man1
 	install -m 644 audit2allow.1 $(MANDIR)/man1/
 
diff --git a/policycoreutils/audit2allow/sepolgen-ifgen b/policycoreutils/audit2allow/sepolgen-ifgen
deleted file mode 100644
index 03f95a1..0000000
--- a/policycoreutils/audit2allow/sepolgen-ifgen
+++ /dev/null
@@ -1,89 +0,0 @@
-#! /usr/bin/python -E
-#
-# Authors: Karl MacMillan <kmacmillan@mentalrootkit.com>
-#
-# Copyright (C) 2006 Red Hat 
-# see file 'COPYING' for use and warranty information
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License as
-# published by the Free Software Foundation; version 2 only
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
-#
-
-# Parse interfaces and output extracted information about them
-# suitable for policy generation. By default writes the output
-# to the default location (obtained from sepolgen.defaults), but
-# will output to another file provided as an argument:
-#   sepolgen-ifgen [headers] [output-filename]
-
-
-import sys
-import os
-
-import sepolgen.refparser as refparser
-import sepolgen.defaults as defaults
-import sepolgen.interfaces as interfaces
-
-
-VERSION = "%prog .1"
-
-def parse_options():
-    from optparse import OptionParser
-
-    parser = OptionParser(version=VERSION)
-    parser.add_option("-o", "--output", dest="output", default=defaults.interface_info(),
-                      help="filename to store output")
-    parser.add_option("-i", "--interfaces", dest="headers", default=defaults.headers(),
-                      help="location of the interface header files")
-    parser.add_option("-v", "--verbose", action="store_true", default=False,
-                      help="print debuging output")
-    parser.add_option("-d", "--debug", action="store_true", default=False,
-                     help="extra debugging output")
-    options, args = parser.parse_args()
-    
-    return options
-
-
-def main():
-    options = parse_options()
-
-    # Open the output first to generate errors before parsing
-    try:
-        f = open(options.output, "w")
-    except IOError, e:
-        sys.stderr.write("could not open output file [%s]\n" % options.output)
-        return 1
-
-    if options.verbose:
-        log = sys.stdout
-    else:
-        log = None
-
-    try:
-        headers = refparser.parse_headers(options.headers, output=log, debug=options.debug)
-    except ValueError, e:
-        print "error parsing headers"
-        print str(e)
-        return 1
-
-    if_set = interfaces.InterfaceSet(output=log)
-    if_set.add_headers(headers)
-    if_set.to_file(f)
-    f.close()
-
-    if refparser.success:
-        return 0
-    else:
-        return 1
-    
-if __name__ == "__main__":
-    sys.exit(main())
diff --git a/policycoreutils/sepolgen-ifgen/Makefile b/policycoreutils/sepolgen-ifgen/Makefile
new file mode 100644
index 0000000..370a4c3
--- /dev/null
+++ b/policycoreutils/sepolgen-ifgen/Makefile
@@ -0,0 +1,26 @@
+# Installation directories.
+PREFIX ?= ${DESTDIR}/usr
+BINDIR ?= $(PREFIX)/bin
+LIBDIR ?= ${PREFIX}/lib
+INCLUDEDIR ?= $(PREFIX)/include
+
+CFLAGS ?= -Wall -W
+override CFLAGS += -I$(INCLUDEDIR)
+LDLIBS = $(LIBDIR)/libsepol.a
+
+all: sepolgen-ifgen-attr-helper
+
+sepolgen-ifgen-attr-helper: sepolgen-ifgen-attr-helper.o
+
+install: all
+	-mkdir -p $(BINDIR)
+	install -m 755 sepolgen-ifgen $(BINDIR)
+	install -m 755 sepolgen-ifgen-attr-helper $(BINDIR)
+
+clean:
+	rm -f *~ *.o
+
+indent:
+	../../scripts/Lindent $(wildcard *.[ch])
+
+relabel: ;
diff --git a/policycoreutils/sepolgen-ifgen/sepolgen-ifgen b/policycoreutils/sepolgen-ifgen/sepolgen-ifgen
new file mode 100644
index 0000000..0994c81
--- /dev/null
+++ b/policycoreutils/sepolgen-ifgen/sepolgen-ifgen
@@ -0,0 +1,126 @@
+#! /usr/bin/python -E
+#
+# Authors: Karl MacMillan <kmacmillan@mentalrootkit.com>
+#
+# Copyright (C) 2006 Red Hat 
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; version 2 only
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+#
+
+# Parse interfaces and output extracted information about them
+# suitable for policy generation. By default writes the output
+# to the default location (obtained from sepolgen.defaults), but
+# will output to another file provided as an argument:
+#   sepolgen-ifgen [headers] [output-filename]
+
+
+import sys
+import os
+import tempfile
+import subprocess
+
+import selinux
+
+import sepolgen.refparser as refparser
+import sepolgen.defaults as defaults
+import sepolgen.interfaces as interfaces
+
+
+VERSION = "%prog .1"
+ATTR_HELPER = "/usr/bin/sepolgen-ifgen-attr-helper"
+
+def parse_options():
+    from optparse import OptionParser
+
+    parser = OptionParser(version=VERSION)
+    parser.add_option("-o", "--output", dest="output", default=defaults.interface_info(),
+                      help="filename to store output")
+    parser.add_option("-i", "--interfaces", dest="headers", default=defaults.headers(),
+                      help="location of the interface header files")
+    parser.add_option("-a", "--attribute_info", dest="attribute_info")
+    parser.add_option("-v", "--verbose", action="store_true", default=False,
+                      help="print debuging output")
+    parser.add_option("-d", "--debug", action="store_true", default=False,
+                     help="extra debugging output")
+    parser.add_option("--no_attrs", action="store_true", default=False,
+                      help="do not retrieve attribute access from kernel policy")
+    options, args = parser.parse_args()
+    
+    return options
+
+def get_attrs():
+    policy_path = selinux.selinux_binary_policy_path() + "." + str(selinux.security_policyvers())
+    try:
+        outfile = tempfile.NamedTemporaryFile()
+    except IOError, e:
+        sys.stderr.write("could not open attribute output file\n")
+        return None
+
+    ret = subprocess.call([ATTR_HELPER, policy_path, outfile.name])
+    if ret != 0:
+        sys.stderr.write("could not run attribute helper")
+        return None
+
+    attrs = interfaces.AttributeSet()
+    try:
+        attrs.from_file(outfile)
+    except:
+        print "error parsing attribute info"
+        return None
+
+    return attrs
+
+def main():
+    options = parse_options()
+
+    # Open the output first to generate errors before parsing
+    try:
+        f = open(options.output, "w")
+    except IOError, e:
+        sys.stderr.write("could not open output file [%s]\n" % options.output)
+        return 1
+
+    if options.verbose:
+        log = sys.stdout
+    else:
+        log = None
+
+    # Get the attibutes from the binary
+    attrs = None
+    if not options.no_attrs:
+        attrs = get_attrs()
+        if attrs is None:
+            return 1
+
+    # Parse the headers
+    try:
+        headers = refparser.parse_headers(options.headers, output=log, debug=options.debug)
+    except ValueError, e:
+        print "error parsing headers"
+        print str(e)
+        return 1
+
+    if_set = interfaces.InterfaceSet(output=log)
+    if_set.add_headers(headers, attributes=attrs)
+    if_set.to_file(f)
+    f.close()
+
+    if refparser.success:
+        return 0
+    else:
+        return 1
+    
+if __name__ == "__main__":
+    sys.exit(main())
diff --git a/policycoreutils/sepolgen-ifgen/sepolgen-ifgen-attr-helper.c b/policycoreutils/sepolgen-ifgen/sepolgen-ifgen-attr-helper.c
new file mode 100644
index 0000000..acecd41
--- /dev/null
+++ b/policycoreutils/sepolgen-ifgen/sepolgen-ifgen-attr-helper.c
@@ -0,0 +1,230 @@
+/* Authors: Frank Mayer <mayerf@tresys.com>
+ *   and Karl MacMillan <kmacmillan@tresys.com>
+ *
+ * Copyright (C) 2003,2010 Tresys Technology, LLC
+ * 
+ *	This program is free software; you can redistribute it and/or
+ *  	modify it under the terms of the GNU General Public License as
+ *  	published by the Free Software Foundation, version 2.
+ *
+ * Adapted from dispol.c.
+ *
+ * This program is used by sepolgen-ifgen to get the access for all of
+ * the attributes in the policy so that it can resolve the
+ * typeattribute statements in the interfaces.
+ *
+ * It outputs the attribute access in a similar format to what sepolgen
+ * uses to store interface vectors:
+ *   [Attribute sandbox_x_domain]
+ *   sandbox_x_domain,samba_var_t,file,ioctl,read,getattr,lock,open
+ *   sandbox_x_domain,samba_var_t,dir,getattr,search,open
+ *   sandbox_x_domain,initrc_var_run_t,file,ioctl,read,getattr,lock,open
+ *
+ */
+
+#include <sepol/policydb/policydb.h>
+#include <sepol/policydb/avtab.h>
+#include <sepol/policydb/util.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <sys/mman.h>
+#include <unistd.h>
+
+struct val_to_name {
+	unsigned int val;
+	char *name;
+};
+
+static int perm_name(hashtab_key_t key, hashtab_datum_t datum, void *data)
+{
+	struct val_to_name *v = data;
+	perm_datum_t *perdatum;
+
+	perdatum = (perm_datum_t *) datum;
+
+	if (v->val == perdatum->s.value) {
+		v->name = key;
+		return 1;
+	}
+
+	return 0;
+}
+
+int render_access_mask(uint32_t av, avtab_key_t *key, policydb_t *policydbp,
+		       FILE *fp)
+{
+	struct val_to_name v;
+	class_datum_t *cladatum;
+	char *perm = NULL;
+	unsigned int i;
+	int rc;
+	uint32_t tclass = key->target_class;
+
+	cladatum = policydbp->class_val_to_struct[tclass - 1];
+	for (i = 0; i < cladatum->permissions.nprim; i++) {
+		if (av & (1 << i)) {
+			v.val = i + 1;
+			rc = hashtab_map(cladatum->permissions.table,
+					 perm_name, &v);
+			if (!rc && cladatum->comdatum) {
+				rc = hashtab_map(cladatum->comdatum->
+						 permissions.table, perm_name,
+						 &v);
+			}
+			if (rc)
+				perm = v.name;
+			if (perm) {
+				fprintf(fp, ",%s", perm);
+			}
+		}
+	}
+
+	return 0;
+}
+
+static int render_key(avtab_key_t *key, policydb_t *p, FILE *fp)
+{
+	char *stype, *ttype, *tclass;
+	stype = p->p_type_val_to_name[key->source_type - 1];
+	ttype = p->p_type_val_to_name[key->target_type - 1];
+	tclass = p->p_class_val_to_name[key->target_class - 1];
+	if (stype && ttype) {
+		fprintf(fp, "%s,%s,%s", stype, ttype, tclass);
+	} else {
+		fprintf(stderr, "error rendering key\n");
+		exit(1);
+	}
+
+	return 0;
+}
+
+struct callback_data
+{
+	uint32_t attr;
+	policydb_t *policy;
+	FILE *fp;
+};
+
+int output_avrule(avtab_key_t *key, avtab_datum_t *datum, void *args)
+{
+	struct callback_data *cb_data = (struct callback_data *)args;
+
+	if (key->source_type != cb_data->attr)
+		return 0;
+
+	if (!(key->specified & AVTAB_AV && key->specified & AVTAB_ALLOWED))
+		return 0;
+
+	render_key(key, cb_data->policy, cb_data->fp);
+	render_access_mask(datum->data, key, cb_data->policy, cb_data->fp);
+	fprintf(cb_data->fp, "\n");
+
+	return 0;
+}
+
+static int attribute_callback(hashtab_key_t key, hashtab_datum_t datum, void *datap)
+{
+	struct callback_data *cb_data = (struct callback_data *)datap;
+	type_datum_t *t = (type_datum_t *)datum;
+
+	if (t->flavor == TYPE_ATTRIB) {
+		fprintf(cb_data->fp, "[Attribute %s]\n", key);
+		cb_data->attr = t->s.value;
+		if (avtab_map(&cb_data->policy->te_avtab, output_avrule, cb_data) < 0)
+			return -1;
+		if (avtab_map(&cb_data->policy->te_cond_avtab, output_avrule, cb_data) < 0)
+			return -1;
+	}
+
+	return 0;
+}
+
+static policydb_t *load_policy(const char *filename)
+{
+	policydb_t *policydb;
+	struct policy_file pf;
+	FILE *fp;
+	int ret;
+
+	fp = fopen(filename, "r");
+	if (fp == NULL) {
+		fprintf(stderr, "Can't open '%s':  %s\n",
+			filename, strerror(errno));
+		return NULL;
+	}
+
+	policy_file_init(&pf);
+	pf.type = PF_USE_STDIO;
+	pf.fp = fp;
+
+	policydb = malloc(sizeof(policydb_t));
+	if (policydb == NULL) {
+		fprintf(stderr, "Out of memory!\n");
+		return NULL;
+	}
+
+	if (policydb_init(policydb)) {
+		fprintf(stderr, "Out of memory!\n");
+		return NULL;
+	}
+
+	ret = policydb_read(policydb, &pf, 1);
+	if (ret) {
+		fprintf(stderr,
+			"error(s) encountered while parsing configuration\n");
+		return NULL;
+	}
+
+	fclose(fp);
+	
+	return policydb;
+
+}
+
+void usage(char *progname)
+{
+	printf("usage: %s policy_file out_file\n", progname);
+}
+
+int main(int argc, char **argv)
+{
+	policydb_t *p;
+	struct callback_data cb_data;
+	FILE *fp;
+
+	if (argc != 3) {
+		usage(argv[0]);
+		exit(1);
+	}
+
+	/* Open the policy. */
+	p = load_policy(argv[1]);
+	if (p == NULL) {
+		exit(1);
+	}
+
+	/* Open the output policy. */
+	fp = fopen(argv[2], "w");
+	if (fp == NULL) {
+		fprintf(stderr, "error opening output file\n");
+		policydb_destroy(p);
+		free(p);
+	}
+
+	/* Find all of the attributes and output their access. */
+	cb_data.policy = p;
+	cb_data.fp = fp;
+
+	if (hashtab_map(p->p_types.table, attribute_callback, &cb_data)) {
+		printf("error finding attributes\n");
+	}
+
+	policydb_destroy(p);
+	free(p);
+	fclose(fp);
+
+	return 0;
+}
diff --git a/sepolgen/src/sepolgen/defaults.py b/sepolgen/src/sepolgen/defaults.py
index 45ce61a..6d511c3 100644
--- a/sepolgen/src/sepolgen/defaults.py
+++ b/sepolgen/src/sepolgen/defaults.py
@@ -30,6 +30,9 @@ def perm_map():
 def interface_info():
     return data_dir() + "/interface_info"
 
+def attribute_info():
+    return data_dir() + "/attribute_info"
+
 def refpolicy_devel():
     return "/usr/share/selinux/devel"
 
diff --git a/sepolgen/src/sepolgen/interfaces.py b/sepolgen/src/sepolgen/interfaces.py
index d8b3e34..bb2223a 100644
--- a/sepolgen/src/sepolgen/interfaces.py
+++ b/sepolgen/src/sepolgen/interfaces.py
@@ -29,6 +29,8 @@ import matching
 
 from sepolgeni18n import _
 
+import copy
+
 class Param:
     """
     Object representing a paramater for an interface.
@@ -197,10 +199,48 @@ def ifcall_extract_params(ifcall, params):
                 ret = 1
 
     return ret
-            
+
+class AttributeVector:
+    def __init__(self):
+        self.name = ""
+        self.access = access.AccessVectorSet()
+
+    def add_av(self, av):
+        self.access.add_av(av)
+
+class AttributeSet:
+    def __init__(self):
+        self.attributes = { }
+
+    def add_attr(self, attr):
+        self.attributes[attr.name] = attr
+
+    def from_file(self, fd):
+        def parse_attr(line):
+            fields = line[1:-1].split()
+            if len(fields) != 2 or fields[0] != "Attribute":
+                raise SyntaxError("Syntax error Attribute statement %s" % line)
+            a = AttributeVector()
+            a.name = fields[1]
+
+            return a
+
+        a = None
+        for line in fd:
+            line = line[:-1]
+            if line[0] == "[":
+                if a:
+                    self.add_attr(a)
+                a = parse_attr(line)
+            elif a:
+                l = line.split(",")
+                av = access.AccessVector(l)
+                a.add_av(av)
+        if a:
+            self.add_attr(a)
 
 class InterfaceVector:
-    def __init__(self, interface=None):
+    def __init__(self, interface=None, attributes={}):
         # Enabled is a loose concept currently - we are essentially
         # not enabling interfaces that we can't handle currently.
         # See InterfaceVector.add_ifv for more information.
@@ -214,10 +254,10 @@ class InterfaceVector:
         # value: Param object).
         self.params = { }
         if interface:
-            self.from_interface(interface)
+            self.from_interface(interface, attributes)
         self.expanded = False
 
-    def from_interface(self, interface):
+    def from_interface(self, interface, attributes={}):
         self.name = interface.name
 
         # Add allow rules
@@ -232,6 +272,22 @@ class InterfaceVector:
             for av in avs:
                 self.add_av(av)
 
+        # Add typeattribute access
+        for typeattribute in interface.typeattributes():
+            for attr in typeattribute.attributes:
+                if not attributes.attributes.has_key(attr):
+                    print "missing attribute " + attr
+                    continue
+                attr_vec = attributes.attributes[attr]
+                for a in attr_vec.access:
+                    av = copy.copy(a)
+                    if av.src_type == attr_vec.name:
+                        av.src_type = typeattribute.type
+                    if av.tgt_type == attr_vec.name:
+                        av.tgt_type = typeattribute.type
+                    self.add_av(av)
+
+
         # Extract paramaters from roles
         for role in interface.roles():
             if role_extract_params(role, self.params):
@@ -346,13 +402,13 @@ class InterfaceSet:
                 l = self.tgt_type_map.setdefault(type, [])
                 l.append(ifv)
 
-    def add(self, interface):
-        ifv = InterfaceVector(interface)
+    def add(self, interface, attributes={}):
+        ifv = InterfaceVector(interface, attributes)
         self.add_ifv(ifv)
 
-    def add_headers(self, headers, output=None):
+    def add_headers(self, headers, output=None, attributes={}):
         for i in itertools.chain(headers.interfaces(), headers.templates()):
-            self.add(i)
+            self.add(i, attributes)
 
         self.expand_ifcalls(headers)
         self.index()
-- 
1.6.6.1

Re: refpolicy is missing on lots of hits with audit2allow -R.

[Daniel J Walsh]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/21/2010 09:53 PM, Karl MacMillan wrote:
> On Wed, Apr 21, 2010 at 10:04 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
>> Ok that works, but If we move to a more general case. or openvn_t
>> getattr on etc_t
>>
>> #============= openvpn_t ==============
>> # src="openvpn_t" tgt="etc_t" class="file", perms="getattr"
>> # comm="openvpn" exe="" path=""
>> # Interface options:
>> #   automount_exec_config(openvpn_t) # [51]
>> #   files_exec_etc_files(openvpn_t) # [51]
>> #   files_delete_etc_files(openvpn_t) # [118]
>> #   files_relabel_etc_files(openvpn_t) # [136]
>> #   files_rw_etc_files(openvpn_t) # [161]
>> #   files_read_etc_files(openvpn_t) # [171]
>> #   files_manage_etc_files(openvpn_t) # [179]
>> #   auth_use_nsswitch(openvpn_t) # [1342]
>> #   seutil_semanage_policy(openvpn_t) # [3489]
>> #   auth_login_pgm_domain(openvpn_t) # [3717]
>> #   portage_compile_domain(openvpn_t) # [4004]
>>
>> I would have expected files_read_etc_files(openvpn_t)  to be the
>> closest/best match.
>>
> 
> Can you send me the audit messages for this?
> 
>> The tool is getting confused by attributes.  Since attributes are not
>> currently interpretable, they should be eliminated from the calculation.
>> Best way to do this is just eliminate any types that don't end in a _t.
> 
> I'm not certain what you mean by this - confused in what way? The only
> thing I know about is the lack of typattribute statements. The
> attached patch adds attribute handling to sepolgen. It's only lightly
> tested but I wanted you to get it sooner rather than later.
> 
> Karl
> 
>> -----BEGIN PGP SIGNATURE-----
>> Version: GnuPG v2.0.14 (GNU/Linux)
>> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>>
>> iEYEARECAAYFAkvPBdMACgkQrlYvE4MpobP9IQCePlmwSbiO94NTCiu1mHwUzdkI
>> 8YsAn3tlgDQljeLLLhJmMaUGRHFkrBVp
>> =8OfI
>> -----END PGP SIGNATURE-----
>>
>> --
>> 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.
>>
First let me get rid of these ^M all over the patch.pwd

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkvQSWwACgkQrlYvE4MpobPjqQCgkQAXldyncbGkD5KgOI49vVRQ
b0sAoJ2wSfzsPELFd9efh4XRtKdBACR1
=Pkv1
-----END PGP SIGNATURE-----

--
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 is missing on lots of hits with audit2allow -R.

[Daniel J Walsh]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


I have not fully examined this patch but I don't believe it will do what
I need.

If I define an interface that ends up looking like this:


[InterfaceVector files_read_etc_files $1:source ]
$1,configfile,dir,ioctl,search,read,lock,open,getattr
$1,configfile,file,read,lock,getattr,open,ioctl
$1,configfile,lnk_file,read,getattr


And have and avc that looks like this:

node=(removed) type=AVC msg=audit(1271587735.632:422): avc:  denied  {
getattr } for  pid=13239 comm="openvpn"
path="/home/bbaetz/.pki/vpn01_cacert.pem" dev=dm-3 ino=565334
scontext=system_u:system_r:openvpn_t:s0
tcontext=unconfined_u:object_r:etc_t:s0 tclass=file


This will never match.

What we need is a way to expand out configfile into etc_t, or to realize
that etc_t is a configfile.

The tool you added does not do the equivalent of

seinfo -aconfigfile -x
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkvQUTwACgkQrlYvE4MpobMfWwCfbg6t/396jHWHCpasqosqf8Mw
7qkAoNiXMi8+RK5/4mBu8WbnGLvxlb7+
=EDEY
-----END PGP SIGNATURE-----

--
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 is missing on lots of hits with audit2allow -R.

[Karl MacMillan]

On Thu, Apr 22, 2010 at 9:04 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
> First let me get rid of these ^M all over the patch.pwd
>

Oops - I'll take care of that in the next patch.

Karl

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.14 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkvQSWwACgkQrlYvE4MpobPjqQCgkQAXldyncbGkD5KgOI49vVRQ
> b0sAoJ2wSfzsPELFd9efh4XRtKdBACR1
> =Pkv1
> -----END PGP SIGNATURE-----
>
> --
> 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.
>

--
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 is missing on lots of hits with audit2allow -R.

[Karl MacMillan]

On Thu, Apr 22, 2010 at 9:38 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>
> I have not fully examined this patch but I don't believe it will do what
> I need.
>
> If I define an interface that ends up looking like this:
>
>
> [InterfaceVector files_read_etc_files $1:source ]
> $1,configfile,dir,ioctl,search,read,lock,open,getattr
> $1,configfile,file,read,lock,getattr,open,ioctl
> $1,configfile,lnk_file,read,getattr
>
>
> And have and avc that looks like this:
>
> node=(removed) type=AVC msg=audit(1271587735.632:422): avc:  denied  {
> getattr } for  pid=13239 comm="openvpn"
> path="/home/bbaetz/.pki/vpn01_cacert.pem" dev=dm-3 ino=565334
> scontext=system_u:system_r:openvpn_t:s0
> tcontext=unconfined_u:object_r:etc_t:s0 tclass=file
>
>
> This will never match.
>
> What we need is a way to expand out configfile into etc_t, or to realize
> that etc_t is a configfile.
>
> The tool you added does not do the equivalent of
>
> seinfo -aconfigfile -x

No, it doesn't do that yet, but I'll get to that soon. Now that I have
the infrastructure for digging through the binary policy for attribute
information it won't be too hard.

Karl

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.14 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkvQUTwACgkQrlYvE4MpobMfWwCfbg6t/396jHWHCpasqosqf8Mw
> 7qkAoNiXMi8+RK5/4mBu8WbnGLvxlb7+
> =EDEY
> -----END PGP SIGNATURE-----
>
> --
> 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.
>


--
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 is missing on lots of hits with audit2allow -R.

[Karl MacMillan]

On Thu, Apr 22, 2010 at 10:29 AM, Karl MacMillan <kmacmillan@tresys.com> wrote:
> On Thu, Apr 22, 2010 at 9:38 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>>
>> I have not fully examined this patch but I don't believe it will do what
>> I need.
>>
>> If I define an interface that ends up looking like this:
>>
>>
>> [InterfaceVector files_read_etc_files $1:source ]
>> $1,configfile,dir,ioctl,search,read,lock,open,getattr
>> $1,configfile,file,read,lock,getattr,open,ioctl
>> $1,configfile,lnk_file,read,getattr
>>
>>
>> And have and avc that looks like this:
>>
>> node=(removed) type=AVC msg=audit(1271587735.632:422): avc:  denied  {
>> getattr } for  pid=13239 comm="openvpn"
>> path="/home/bbaetz/.pki/vpn01_cacert.pem" dev=dm-3 ino=565334
>> scontext=system_u:system_r:openvpn_t:s0
>> tcontext=unconfined_u:object_r:etc_t:s0 tclass=file
>>
>>
>> This will never match.
>>
>> What we need is a way to expand out configfile into etc_t, or to realize
>> that etc_t is a configfile.
>>
>> The tool you added does not do the equivalent of
>>
>> seinfo -aconfigfile -x
>
> No, it doesn't do that yet, but I'll get to that soon. Now that I have
> the infrastructure for digging through the binary policy for attribute
> information it won't be too hard.
>

So it wasn't hard - I built a version that gets all of the attributes
out of the binary policy and expands them in the interfaces. Running
your denial now gives:

#============= openvpn_t ==============
# audit(1271587735.632:422):
#  scontext="system_u:system_r:openvpn_t:s0"
tcontext="unconfined_u:object_r:etc_t:s0"
#  class="file" perms="getattr"
#  comm="openvpn" exe="" path=""
#  message=" node=(removed) type=AVC msg=audit(1271587735.632:422): avc:  denied
#   { getattr } for  pid=13239 comm="openvpn"
#   path="/home/bbaetz/.pki/vpn01_cacert.pem" dev=dm-3 ino=565334
#   scontext=system_u:system_r:openvpn_t:s0
#   tcontext=unconfined_u:object_r:etc_t:s0 tclass=file  "
# Interface options:
#   automount_exec_config(openvpn_t) # [51]
#   files_exec_etc_files(openvpn_t) # [51]
#   files_delete_etc_files(openvpn_t) # [118]
#   files_relabel_etc_files(openvpn_t) # [136]
#   files_rw_etc_files(openvpn_t) # [161]
#   files_manage_etc_files(openvpn_t) # [179]
#   files_read_etc_files(openvpn_t) # [1702]
#   auth_use_nsswitch(openvpn_t) # [2893]
#   auth_login_pgm_domain(openvpn_t) # [32016]
#   seutil_semanage_policy(openvpn_t) # [41089]
#   portage_compile_domain(openvpn_t) # [59755]
automount_exec_config(openvpn_t)

I was surprised by the 1702 distance for files_read_etc_files, but
because of calling files_read_config_files and the configfile
attribute this interface now allows access to ~60 additional types. So
the distance calculation here is right I think (Chris mentioned that
the upstream refpol does not have this interface call in
files_read_config_files).

So I'm at a loss for what to do here. Adding the attribute expansion
for rules makes audit2allow _very_ slow and it seems that it will only
allow us to match very broad interfaces. I'm not convinced we really
want that. Eventually it might be nice to notice that we got denials
for many types and coalesce those into one broad interface call, but
that will be hard (I do some of that already but this case would be
harder) and doesn't seem worth it.

What do you think? Any chance you can just revert your change to
files_read_etc_files?

Karl

> Karl
>
>> -----BEGIN PGP SIGNATURE-----
>> Version: GnuPG v2.0.14 (GNU/Linux)
>> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>>
>> iEYEARECAAYFAkvQUTwACgkQrlYvE4MpobMfWwCfbg6t/396jHWHCpasqosqf8Mw
>> 7qkAoNiXMi8+RK5/4mBu8WbnGLvxlb7+
>> =EDEY
>> -----END PGP SIGNATURE-----
>>
>> --
>> 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.
>>
>


--
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 is missing on lots of hits with audit2allow -R.

[Daniel J Walsh]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I do not totally understand your matching, but I thought if you looked for

allow TYPE etc_t:file getattr;

You could get extra matches.

I was thinking in terms of sepolgen-ifgen would take every type and
expand the attributes for the type then if you find attribute that
matches, not add weight.

seinfo -tetc_t -x
   etc_t
      file_type
      non_security_file_type
      configfile

If my target was etc_t then I would get the same weight as if I
substituted the attrbute with etc_t.



[InterfaceVector files_read_etc_files $1:source ]
$1,etc_t,file,read,lock,getattr,open,ioctl
$1,etc_t,dir,ioctl,search,read,lock,open,getattr
$1,etc_t,lnk_file,read,getattr
$1,configfile,dir,ioctl,search,read,lock,open,getattr
$1,configfile,file,read,lock,getattr,open,ioctl
$1,configfile,lnk_file,read,getattr

Would get translated at

[InterfaceVector files_read_etc_files $1:source ]
$1,etc_t,file,read,lock,getattr,open,ioctl
$1,etc_t,dir,ioctl,search,read,lock,open,getattr
$1,etc_t,lnk_file,read,getattr
$1,etc_t,dir,ioctl,search,read,lock,open,getattr
$1,etc_t,file,read,lock,getattr,open,ioctl
$1,etc_t,lnk_file,read,getattr

If I am looking for a target of etc_t

Which would then get boiled down to.

[InterfaceVector files_read_etc_files $1:source ]
$1,etc_t,file,read,lock,getattr,open,ioctl
$1,etc_t,dir,ioctl,search,read,lock,open,getattr
$1,etc_t,lnk_file,read,getattr


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkvRuEMACgkQrlYvE4MpobPKzwCg3T0NUD5u1dQV6DmFHmPd22V1
uqYAnj/ytX750LXS6Um5izsloK4jhO7w
=oSpR
-----END PGP SIGNATURE-----

--
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 is missing on lots of hits with audit2allow -R.

[Karl MacMillan]

On Fri, Apr 23, 2010 at 11:09 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> I do not totally understand your matching, but I thought if you looked for
>
> allow TYPE etc_t:file getattr;
>
> You could get extra matches.
>

That's essentially what I do.

> I was thinking in terms of sepolgen-ifgen would take every type and
> expand the attributes for the type then if you find attribute that
> matches, not add weight.
>

I think you are missing what the core problem is - it's easy to find
interfaces that match this access. Many interfaces (including
files_read_etc_files) include a rule that directly matches getattr on
etc_t files. So I don't really need to do attribute expansion to get
enough matches.

The key problem is finding the _best_ match, which I define as the
interface that allows the requested access with the least amount of
"extra" access. That's what the distance measure is for - it is a
measure of extra access. So I add extra distance for access to
unrelated types, extra permissions, returning a write interface when a
read was requested, etc. So when you add access to a broad attribute
in files_read_etc_files the distance measure - as currently shipped -
adds more distance for 1 type (the attribute). However, when I expand
the attribute the distance goes up because it becomes clear that the
interface is in fact allowing a _lot_ of access.

This seems correct to me, hence my request to return what was intended
to be a narrow interface back to that narrow definition.

However, there are some things that I can do to make this distance
algorithm better:

1. Don't penalize as much for access that involves container object
classes, such as directory, in an attempt to not penalize for access
that is actually related.

2. Detect execute and add a big penalty for interfaces that allow
execute when it wasn't requested (similar to what I do with write).

I'm also open to other suggestions. I don't think that what I have is
perfect. However, none of this will fix the fact that the
files_read_etc_files shipped in Fedora grants broad access. If
everyone agrees that we want that interface to match regardless then I
think we are going to have to add some other mechanism to the matching
algorithm. Perhaps some "hinting" mechanism to let the policy author
to tell sepolgen to prefer certain interfaces. I might even be able to
automatically derive this from how popular the interface is in current
policies.

Karl

--
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 is missing on lots of hits with audit2allow -R.

[Daniel J Walsh]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/28/2010 10:25 AM, Karl MacMillan wrote:
> On Fri, Apr 23, 2010 at 11:09 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> I do not totally understand your matching, but I thought if you looked for
>>
>> allow TYPE etc_t:file getattr;
>>
>> You could get extra matches.
>>
> 
> That's essentially what I do.
> 
>> I was thinking in terms of sepolgen-ifgen would take every type and
>> expand the attributes for the type then if you find attribute that
>> matches, not add weight.
>>
> 
> I think you are missing what the core problem is - it's easy to find
> interfaces that match this access. Many interfaces (including
> files_read_etc_files) include a rule that directly matches getattr on
> etc_t files. So I don't really need to do attribute expansion to get
> enough matches.
> 
> The key problem is finding the _best_ match, which I define as the
> interface that allows the requested access with the least amount of
> "extra" access. That's what the distance measure is for - it is a
> measure of extra access. So I add extra distance for access to
> unrelated types, extra permissions, returning a write interface when a
> read was requested, etc. So when you add access to a broad attribute
> in files_read_etc_files the distance measure - as currently shipped -
> adds more distance for 1 type (the attribute). However, when I expand
> the attribute the distance goes up because it becomes clear that the
> interface is in fact allowing a _lot_ of access.
> 
> This seems correct to me, hence my request to return what was intended
> to be a narrow interface back to that narrow definition.
> 
> However, there are some things that I can do to make this distance
> algorithm better:
> 
> 1. Don't penalize as much for access that involves container object
> classes, such as directory, in an attempt to not penalize for access
> that is actually related.
> 
> 2. Detect execute and add a big penalty for interfaces that allow
> execute when it wasn't requested (similar to what I do with write).
> 
> I'm also open to other suggestions. I don't think that what I have is
> perfect. However, none of this will fix the fact that the
> files_read_etc_files shipped in Fedora grants broad access. If
> everyone agrees that we want that interface to match regardless then I
> think we are going to have to add some other mechanism to the matching
> algorithm. Perhaps some "hinting" mechanism to let the policy author
> to tell sepolgen to prefer certain interfaces. I might even be able to
> automatically derive this from how popular the interface is in current
> policies.
> 
> Karl

I understand, although I don't agree with penalizying because you match
on an attribute,   Currently if I wrote this interface the way I want,
with just the attribute your algorithm would not find it at all.

files_read_etc_files to me means match all config files in the /etc
directory, just because some random application decides to change the
context of /etc/hostname to etc_runtime_t or net_conf_t, we should not
need to change all domains that are supposed to read generic files in
/etc.  Especially when I don't even need to read them.


I would argue that

allow X etc_t:file read;
allow X configfile:file read;

Should be weighted equivalently if etc_t is a configfile or only
slightly heavier, and just because etc_runtime_t or some other random
types are configfile does not mean we need to add weight.

But when the attribute adds more weight then "write" does, I think the
algorithm is broken.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkvYVZEACgkQrlYvE4MpobPf6QCfY6wAi3VqF9s3do7QwAoQ8UrA
PZwAoN5L0WvquabGFHNfINoJq4MkOy8S
=l2u0
-----END PGP SIGNATURE-----

--
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 is missing on lots of hits with audit2allow -R.

[Daniel J Walsh]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/28/2010 10:25 AM, Karl MacMillan wrote:


I understand, although I don't agree with penalizying because you match
on an attribute,   Currently if I wrote this interface the way I want,
with just the attribute your algorithm would not find it at all.

files_read_etc_files to me means match all config files in the /etc
directory, just because some random application decides to change the
context of /etc/hostname to etc_runtime_t or net_conf_t, we should not
need to change all domains that are supposed to read generic files in
/etc.  Especially when I don't even need to read them.


I would argue that

allow X etc_t:file read;
allow X configfile:file read;

Should be weighted equivalently if etc_t is a configfile or only
slightly heavier, and just because etc_runtime_t or some other random
types are configfile does not mean we need to add weight.

But when the attribute adds more weight then "write" does, I think the
algorithm is broken.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkvYVY4ACgkQrlYvE4MpobOKSgCgguxc5xhYFpW1GGSnN4N+6v/Z
qUsAoOjz7qSp8kiDHBUN2SYoKWsZfTNK
=Ubgb
-----END PGP SIGNATURE-----

--
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 is missing on lots of hits with audit2allow -R.

[Karl MacMillan]

On Wed, Apr 28, 2010 at 11:34 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 04/28/2010 10:25 AM, Karl MacMillan wrote:
>> On Fri, Apr 23, 2010 at 11:09 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
>>> -----BEGIN PGP SIGNED MESSAGE-----
>>> Hash: SHA1
>>>
>>> I do not totally understand your matching, but I thought if you looked for
>>>
>>> allow TYPE etc_t:file getattr;
>>>
>>> You could get extra matches.
>>>
>>
>> That's essentially what I do.
>>
>>> I was thinking in terms of sepolgen-ifgen would take every type and
>>> expand the attributes for the type then if you find attribute that
>>> matches, not add weight.
>>>
>>
>> I think you are missing what the core problem is - it's easy to find
>> interfaces that match this access. Many interfaces (including
>> files_read_etc_files) include a rule that directly matches getattr on
>> etc_t files. So I don't really need to do attribute expansion to get
>> enough matches.
>>
>> The key problem is finding the _best_ match, which I define as the
>> interface that allows the requested access with the least amount of
>> "extra" access. That's what the distance measure is for - it is a
>> measure of extra access. So I add extra distance for access to
>> unrelated types, extra permissions, returning a write interface when a
>> read was requested, etc. So when you add access to a broad attribute
>> in files_read_etc_files the distance measure - as currently shipped -
>> adds more distance for 1 type (the attribute). However, when I expand
>> the attribute the distance goes up because it becomes clear that the
>> interface is in fact allowing a _lot_ of access.
>>
>> This seems correct to me, hence my request to return what was intended
>> to be a narrow interface back to that narrow definition.
>>
>> However, there are some things that I can do to make this distance
>> algorithm better:
>>
>> 1. Don't penalize as much for access that involves container object
>> classes, such as directory, in an attempt to not penalize for access
>> that is actually related.
>>
>> 2. Detect execute and add a big penalty for interfaces that allow
>> execute when it wasn't requested (similar to what I do with write).
>>
>> I'm also open to other suggestions. I don't think that what I have is
>> perfect. However, none of this will fix the fact that the
>> files_read_etc_files shipped in Fedora grants broad access. If
>> everyone agrees that we want that interface to match regardless then I
>> think we are going to have to add some other mechanism to the matching
>> algorithm. Perhaps some "hinting" mechanism to let the policy author
>> to tell sepolgen to prefer certain interfaces. I might even be able to
>> automatically derive this from how popular the interface is in current
>> policies.
>>
>> Karl
>
> I understand, although I don't agree with penalizying because you match
> on an attribute,   Currently if I wrote this interface the way I want,
> with just the attribute your algorithm would not find it at all.
>

I'm not penalizing because it's an attribute, it's just that the
penalty is for every extra type so attributes are going to tend to add
a lot of weight.

> files_read_etc_files to me means match all config files in the /etc
> directory, just because some random application decides to change the
> context of /etc/hostname to etc_runtime_t or net_conf_t, we should not
> need to change all domains that are supposed to read generic files in
> /etc.  Especially when I don't even need to read them.
>
>
> I would argue that
>
> allow X etc_t:file read;
> allow X configfile:file read;
>
> Should be weighted equivalently if etc_t is a configfile or only
> slightly heavier, and just because etc_runtime_t or some other random
> types are configfile does not mean we need to add weight.
>

This seems like mostly a policy design question and I'd be curious to
hear what others think as well. I agree that broad read access to
configuration files is not much of a risk for general Linux systems.
However, if I change the weighting algorithm to not worry about broad
read access my concern is for matching interfaces that allow broad
read access to files when the concern is confidentiality. Currently
there is no clear way to infer security goals beyond read, write, and
maybe exec so there is no way to categorize types as mainly for
integrity vs. confidentiality.

Thoughts? What about creating some hints based upon attributes? Or
what about favoring the most commonly used interfaces?

> But when the attribute adds more weight then "write" does, I think the
> algorithm is broken.

I can certainly try to greatly reduce the weighting from extraneous
types and increase the weighting for write or execute vs. read. I'll
try to experiment a bit and see what I can come up with.

What would really help me the most is if you could start collecting
audit messages that give both good and bad results from audit2allow. I
need some well known test cases to tune the weighting algorithm.

Karl

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.14 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkvYVZEACgkQrlYvE4MpobPf6QCfY6wAi3VqF9s3do7QwAoQ8UrA
> PZwAoN5L0WvquabGFHNfINoJq4MkOy8S
> =l2u0
> -----END PGP SIGNATURE-----
>
> --
> 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.
>


--
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 is missing on lots of hits with audit2allow -R.

[Joshua Brindle]

Karl MacMillan wrote:
> On Wed, Apr 28, 2010 at 11:34 AM, Daniel J Walsh<dwalsh@redhat.com>  wrote:
<snip>>
>> I would argue that
>>
>> allow X etc_t:file read;
>> allow X configfile:file read;
>>
>> Should be weighted equivalently if etc_t is a configfile or only
>> slightly heavier, and just because etc_runtime_t or some other random
>> types are configfile does not mean we need to add weight.
>>
>

I'm going to weigh in here even though policy isn't normally my thing.

I am very against reducing distance based on attribute match over 
individual unrelated types. allow X configfile:file read should be the 
exact same distance as having an allow rule for every type in configfile 
in the interface, otherwise you have inconsistent behavior and are 
rewarding interfaces that are overly broad.

The reason for using sepolgen is to find the best match, not the most 
broad, if we wanted that we could make sepolgen 1000% less complicated 
and just return allow domain filetype:file *;

I suppose there is a fundamental difference in the use of the tool, the 
people I know that use it use it to find the best match, the way you 
want to use it is to fix the denial any way possible. These 2 usages 
conflict and we can't have people thinking they are making secure policy 
when in fact they aren't.


--
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 is missing on lots of hits with audit2allow -R.

[Daniel J Walsh]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/28/2010 02:12 PM, Joshua Brindle wrote:
> Karl MacMillan wrote:
>> On Wed, Apr 28, 2010 at 11:34 AM, Daniel J Walsh<dwalsh@redhat.com> 
>> wrote:
> <snip>>
>>> I would argue that
>>>
>>> allow X etc_t:file read;
>>> allow X configfile:file read;
>>>
>>> Should be weighted equivalently if etc_t is a configfile or only
>>> slightly heavier, and just because etc_runtime_t or some other random
>>> types are configfile does not mean we need to add weight.
>>>
>>
> 
> I'm going to weigh in here even though policy isn't normally my thing.
> 
> I am very against reducing distance based on attribute match over
> individual unrelated types. allow X configfile:file read should be the
> exact same distance as having an allow rule for every type in configfile
> in the interface, otherwise you have inconsistent behavior and are
> rewarding interfaces that are overly broad.
> 
I agree but the question is on an AVC that needs

allow $1 etc_t:file read;


Which is a better match

allow $1 configfile:file read;

Or

allow $1 etc_t:file { read write };

> The reason for using sepolgen is to find the best match, not the most
> broad, if we wanted that we could make sepolgen 1000% less complicated
> and just return allow domain filetype:file *;
> 
> I suppose there is a fundamental difference in the use of the tool, the
> people I know that use it use it to find the best match, the way you
> want to use it is to fix the denial any way possible. These 2 usages
> conflict and we can't have people thinking they are making secure policy
> when in fact they aren't.
> 

Thats Bullshit.  I am trying to get the best match.  The example I
showed earlier the tool was getting way off, because it did not take
into effect attributes.  The access returned for a getattr was to allow
the domain to delete the file.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkvYgBAACgkQrlYvE4MpobNo3ACeIkGbGm9mWWUaAVPkA8B64YTS
zMkAn3PhjVrB1cw0jSIZo7bUmmcK58e+
=A6kc
-----END PGP SIGNATURE-----

--
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 is missing on lots of hits with audit2allow -R.

[Karl MacMillan]

On Wed, Apr 28, 2010 at 2:12 PM, Joshua Brindle <jbrindle@tresys.com> wrote:
> Karl MacMillan wrote:
>>
>> On Wed, Apr 28, 2010 at 11:34 AM, Daniel J Walsh<dwalsh@redhat.com>
>>  wrote:
>
> <snip>>
>>>
>>> I would argue that
>>>
>>> allow X etc_t:file read;
>>> allow X configfile:file read;
>>>
>>> Should be weighted equivalently if etc_t is a configfile or only
>>> slightly heavier, and just because etc_runtime_t or some other random
>>> types are configfile does not mean we need to add weight.
>>>
>>
>
> I'm going to weigh in here even though policy isn't normally my thing.
>
> I am very against reducing distance based on attribute match over individual
> unrelated types. allow X configfile:file read should be the exact same
> distance as having an allow rule for every type in configfile in the
> interface, otherwise you have inconsistent behavior and are rewarding
> interfaces that are overly broad.
>
> The reason for using sepolgen is to find the best match, not the most broad,
> if we wanted that we could make sepolgen 1000% less complicated and just
> return allow domain filetype:file *;
>
> I suppose there is a fundamental difference in the use of the tool, the
> people I know that use it use it to find the best match, the way you want to
> use it is to fix the denial any way possible. These 2 usages conflict and we
> can't have people thinking they are making secure policy when in fact they
> aren't.
>

While I'm concerned about providing secure options as well, to
characterize Dan's current position as fixing the denial in any way
possible is not fair either. Here are the current matches in order:

# Interface options:
#   automount_exec_config(openvpn_t) # [51]
#   files_exec_etc_files(openvpn_t) # [51]
#   files_delete_etc_files(openvpn_t) # [118]
#   files_relabel_etc_files(openvpn_t) # [136]
#   files_rw_etc_files(openvpn_t) # [161]
#   files_read_etc_files(openvpn_t) # [171]
#   files_manage_etc_files(openvpn_t) # [179]
#   auth_use_nsswitch(openvpn_t) # [1362]
#   seutil_semanage_policy(openvpn_t) # [3532]
#   auth_login_pgm_domain(openvpn_t) # [3736]
#   portage_compile_domain(openvpn_t) # [3902]

Something is clearly wrong - largely because I'm not distinguishing
between exec, delete, relabel, and read/write.

Karl

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


--
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 is missing on lots of hits with audit2allow -R.

[Joshua Brindle]

Daniel J Walsh wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 04/28/2010 02:12 PM, Joshua Brindle wrote:
>> Karl MacMillan wrote:
>>> On Wed, Apr 28, 2010 at 11:34 AM, Daniel J Walsh<dwalsh@redhat.com>
>>> wrote:
>> <snip>>
>>>> I would argue that
>>>>
>>>> allow X etc_t:file read;
>>>> allow X configfile:file read;
>>>>
>>>> Should be weighted equivalently if etc_t is a configfile or only
>>>> slightly heavier, and just because etc_runtime_t or some other random
>>>> types are configfile does not mean we need to add weight.
>>>>
>> I'm going to weigh in here even though policy isn't normally my thing.
>>
>> I am very against reducing distance based on attribute match over
>> individual unrelated types. allow X configfile:file read should be the
>> exact same distance as having an allow rule for every type in configfile
>> in the interface, otherwise you have inconsistent behavior and are
>> rewarding interfaces that are overly broad.
>>
> I agree but the question is on an AVC that needs
>
> allow $1 etc_t:file read;
>
>
> Which is a better match
>
> allow $1 configfile:file read;
>
> Or
>
> allow $1 etc_t:file { read write };
>
>> The reason for using sepolgen is to find the best match, not the most
>> broad, if we wanted that we could make sepolgen 1000% less complicated
>> and just return allow domain filetype:file *;
>>
>> I suppose there is a fundamental difference in the use of the tool, the
>> people I know that use it use it to find the best match, the way you
>> want to use it is to fix the denial any way possible. These 2 usages
>> conflict and we can't have people thinking they are making secure policy
>> when in fact they aren't.
>>
>
> Thats Bullshit.  I am trying to get the best match.  The example I
> showed earlier the tool was getting way off, because it did not take
> into effect attributes.  The access returned for a getattr was to allow
> the domain to delete the file.
>

Fair enough. It seems like it would work as expected if interfaces were 
relatively side effect free though (eg., files_read_etc_files shouldn't 
have the side effect of reading all config files, perhaps a new 
interface called files_read_config_files should be used instead).

I guess the big problem is that without looking at the interface (and 
unraveling all the interface calls within) the user can't actually 
determine what the matched interfaces do.

I agree with Karl about penalizing exec and delete though, even though 
exec really isn't necessary to execute something, all you need is read 
access.

--
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 is missing on lots of hits with audit2allow -R.

[Daniel J Walsh]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/28/2010 02:47 PM, Joshua Brindle wrote:
> Daniel J Walsh wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> On 04/28/2010 02:12 PM, Joshua Brindle wrote:
>>> Karl MacMillan wrote:
>>>> On Wed, Apr 28, 2010 at 11:34 AM, Daniel J Walsh<dwalsh@redhat.com>
>>>> wrote:
>>> <snip>>
>>>>> I would argue that
>>>>>
>>>>> allow X etc_t:file read;
>>>>> allow X configfile:file read;
>>>>>
>>>>> Should be weighted equivalently if etc_t is a configfile or only
>>>>> slightly heavier, and just because etc_runtime_t or some other random
>>>>> types are configfile does not mean we need to add weight.
>>>>>
>>> I'm going to weigh in here even though policy isn't normally my thing.
>>>
>>> I am very against reducing distance based on attribute match over
>>> individual unrelated types. allow X configfile:file read should be the
>>> exact same distance as having an allow rule for every type in configfile
>>> in the interface, otherwise you have inconsistent behavior and are
>>> rewarding interfaces that are overly broad.
>>>
>> I agree but the question is on an AVC that needs
>>
>> allow $1 etc_t:file read;
>>
>>
>> Which is a better match
>>
>> allow $1 configfile:file read;
>>
>> Or
>>
>> allow $1 etc_t:file { read write };
>>
>>> The reason for using sepolgen is to find the best match, not the most
>>> broad, if we wanted that we could make sepolgen 1000% less complicated
>>> and just return allow domain filetype:file *;
>>>
>>> I suppose there is a fundamental difference in the use of the tool, the
>>> people I know that use it use it to find the best match, the way you
>>> want to use it is to fix the denial any way possible. These 2 usages
>>> conflict and we can't have people thinking they are making secure policy
>>> when in fact they aren't.
>>>
>>
>> Thats Bullshit.  I am trying to get the best match.  The example I
>> showed earlier the tool was getting way off, because it did not take
>> into effect attributes.  The access returned for a getattr was to allow
>> the domain to delete the file.
>>
> 
> Fair enough. It seems like it would work as expected if interfaces were
> relatively side effect free though (eg., files_read_etc_files shouldn't
> have the side effect of reading all config files, perhaps a new
> interface called files_read_config_files should be used instead).
> 
> I guess the big problem is that without looking at the interface (and
> unraveling all the interface calls within) the user can't actually
> determine what the matched interfaces do.
> 
> I agree with Karl about penalizing exec and delete though, even though
> exec really isn't necessary to execute something, all you need is read
> access.
> 
> -- 
> 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.


I think the best solution is to start expanding attributes so we at
least have a chance at a match, without changing the weighting.

Secondly would be to start collecting oddball returns by the tool and
sending them to Karl.

I just tried a few and they came up with the correct interface.
Lets ask dgrift, mgrepl, Chris and other prolific policy writers to use
this tool to see if the interface matches are working for them.  The
problem is people who write enough policy, tend to know which interface
to use, so they do not necessarily use audit2allow -R.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkvYhhcACgkQrlYvE4MpobMYjgCffoR0aerRBPvKu7BsvD1cZRxi
k30AoOqKOQsiZH6BPBY77Oxq9wEY6PRC
=dTwd
-----END PGP SIGNATURE-----

--
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 is missing on lots of hits with audit2allow -R.

[Stephen Smalley]

On Wed, 2010-04-21 at 21:53 -0400, Karl MacMillan wrote:
> The
> attached patch adds attribute handling to sepolgen. It's only lightly
> tested but I wanted you to get it sooner rather than later.

Evidently this patch has made its way into F-13 and RHEL-6, although it
is still not upstream.

Some concerns/questions:
- This creates a dependency of sepolgen-ifgen on a specific policy.
Previously it was only dependent on the headers.  But this will mean
that the data generated by sepolgen-ifgen and used by sepolgen could
easily differ for targeted vs mls even if they use the same headers.  Do
we need per-policy data directories for sepolgen?

- It should be possible to load a specified policy file rather than
always using the active one.

- You are using the latest policy version supported by the kernel rather
than the one supported by libsepol.  See audit2why.c or load_policy.c in
libselinux to see how they determine the right policy version to use by
default.  Otherwise this will break when we have divergence between the
libsepol and kernel supported policy versions (i.e. whenever we next
introduce a new policy version).

- This creates another user of the static libsepol.  

-- 
Stephen Smalley
National Security Agency


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