seandroid-list is shutting down

[Stephen Smalley]

Hi,

As originally noted back in May [1], we plan to shut down the 
seandroid-list mailing list in the near future.


SELinux questions can still be brought to the selinux mailing list, 
which is moving to vger.kernel.org [2], or you can take Android security 
questions to the android-security-discuss Google group.


Be advised that vger.kernel.org does not accept HTML email, so configure 
your mail clients accordingly.


Thanks.

[1] 
https://seandroid-list.tycho.nsa.narkive.com/apZCNhBr/shutting-down-the-seandroid-bitbucket-site-and-possibly-this-list


[2] 
https://lore.kernel.org/selinux/8263f9fa-16f1-1d0a-e391-61d609e50...@tycho.nsa.gov/T/#u

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: Regarding tmpfs selinux denials

[Stephen Smalley]

On 09/14/2018 04:31 AM, Sameer Joshi wrote:

Hi ,

We are trying to have our own way of generating debug report and for 
that we are running a script from settings app , after "Generate report" 
is pressed. We are using Android 8.1 for this scenario.


In this case , we are trying to create new files in "/tmp" directory.


Use of a global /tmp is insecure.  Create files in the app's data 
directory instead.




We get the following denials:

09-14 12:24:01.174  4592  4592 W media_report.sh: type=1400 
audit(0.0:53): avc: denied { create } for name="audio_result" 
scontext=u:r:system_app:s0 tcontext=u:object_r:system_app_tmpfs:s0 
tclass=file permissive=0
09-14 12:24:01.174  4592  4592 W media_report.sh: type=1400 
audit(0.0:54): avc: denied { create } for name="audio_params" 
scontext=u:r:system_app:s0 tcontext=u:object_r:system_app_tmpfs:s0 
tclass=file permissive=0


Using audit2allow , gives the following rules:
#= system_app ==
allow system_app system_app_tmpfs:file create;

Adding this rule into system_app.te gives following error during 
compilation.


FAILED: 
out/target/product/brio/obj/ETC/nonplat_sepolicy.cil_intermediates/nonplat_policy_raw.cil 

/bin/bash -c "(ASAN_OPTIONS=detect_leaks=0 
out/host/linux-x86/bin/checkpolicy -C -M -c 30 -o 
out/target/product/brio/obj/ETC/nonplat_sepolicy.cil_intermediates/nonplat_policy_raw.cil.tmp 
out/target/product/brio/obj/ETC/nonplat_sepolicy.cil_intermediates/nonplat_policy.conf 
) && (grep -Fxv -f 
out/target/product/brio/obj/FAKE/selinux_policy_intermediates/reqd_policy_mask.cil 
out/target/product/brio/obj/ETC/nonplat_sepolicy.cil_intermediates/nonplat_policy_raw.cil.tmp 
 > 
out/target/product/brio/obj/ETC/nonplat_sepolicy.cil_intermediates/nonplat_policy_raw.cil 
)"
*device/avaya/brio/sepolicy/system_app.te:38:ERROR 'unknown type 
system_app_tmpfs' at token ';' on line 26169:*

*#= system_app ==*
*allow system_app system_app_tmpfs:file create;*
checkpolicy:  error(s) encountered while parsing configuration

Can anyone help to know how to solve this problem?


I think this is due to system_app_tmpfs being defined as part of the 
platform private sepolicy, and thus not being exported to the 
non-platform policy.  Regardless, you shouldn't be allowing this.




Regards,

Sameer Joshi




___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.



___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: Failed to perform read/write operation from /hardware/qcom/audio/post_proc/volume_listener.c

[Stephen Smalley]

On 05/27/2018 09:08 AM, Mantesh Eksambe wrote:
> Hi,
> 
> I want to perform file read write operation from /hardware 
> /qcom 
> /audio 
> /post_proc 
> /volume_listener.c
>  
> 
>  effect file. I have created directory at /data/vendor/misc/my_dir.
> So i want to write effect data from volume_listener.c to my directory.
> 
> As per my understanding post_proc effect comes under hal_audio_default 
> domain. Then i have added "allow hal_audio_default system_data_file:file { 
> write create };" in hal_audio.te file. But after adding I'm facing following 
> issue while building AOSP
> 
> NOTE - I'm working on Android Oreo.
> 
> Error -
> 
> libsepol.report_failure: neverallow on line 856 of 
> system/sepolicy/public/domain.te (or line 9111 of policy.conf) violated by 
> allow hal_audio_default system_data_file:file { write create };
> 
> I hope you understand my issue. please help me to solve this issue.
> Please find attached build log for more clarity.

You need to define a type other than system_data_file and assign it to your 
directory via file_contexts so that your process only needs create/write to 
your own type and not arbitrary system data files.

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: mkfs.ext2 /dev/block/loop7 got avc denials with 4.14 kernel but not with 4.9 kernel

[Stephen Smalley]

On 05/07/2018 01:17 PM, Yongqin Liu wrote:
> 
> 
> On 8 May 2018 at 00:55, Stephen Smalley <s...@tycho.nsa.gov 
> <mailto:s...@tycho.nsa.gov>> wrote:
> 
> On 05/07/2018 12:51 PM, Stephen Smalley wrote:
> > On 05/07/2018 12:30 PM, Yongqin Liu wrote:
> >>     I run the commands as root with userdebug build, after run su 
> command.
> > 
> > Can you run id -Z before and after running su?  I'm trying to 
> understand why the scontext is u:r:kernel:s0 instead of e.g. u:r:shell:s0 
> (regular shell) or u:r:su:s0 (su shell). 
> 
> h01:04:28 liuyq: ~$ adb shell 
> hikey:/ $ id
> uid=2000(shell) gid=2000(shell) 
> groups=2000(shell),1004(input),1007(log),1011(adb),1015(sdcard_rw),1028(sdcard_r),3001(net_bt_admin),3002(net_bt),3003(inet),3006(net_bw_stats),3009(readproc),3011(uhid)
>  context=u:r:shell:s0
> hikey:/ $ su
> hikey:/ # id
> uid=0(root) gid=0(root) 
> groups=0(root),1004(input),1007(log),1011(adb),1015(sdcard_rw),1028(sdcard_r),3001(net_bt_admin),3002(net_bt),3003(inet),3006(net_bw_stats),3009(readproc),3011(uhid)
>  context=u:r:su:s0
> hikey:/ # ^D
> hikey:/ $ ^D
> 01:05:52 liuyq: ~$ adb shell 
> hikey:/ $ 
> hikey:/ $ id -Z
> context=u:r:shell:s0
> hikey:/ $ su
> hikey:/ # id -Z
> context=u:r:su:s0
> hikey:/ # 
> 
>  
> 
> Is it because it is a console rather than adb and there is no domain 
> transition defined for shell execution from the console?  Should there be a 
> domain_auto_trans(kernel, shell_exec, shell) rule in policy?
> 
> Both running it from the serial console after su, or the via vts with adb 
> shell(after adb root), will report kernel scontext domain.
> 
> 
> Actually, we don't allow kernel domain to execute anything other than 
> init, so I don't understand how you got a shell running in kernel domain (if 
> that is in fact what you did).
> 
> 
> it's what i want to be clear too.

Ok, the implication is that the actual write request is happening from a kernel 
thread, not in the context of the process that is performing the mkfs command, 
e.g. the write is deferred to a work queue or similar mechanism. If so, then 
there isn't much point in performing the check at all, because it will always 
be from the kernel domain regardless of the userspace originator.

I'm not sure that moving the security_file_permission() calls into 
rw_verify_area() was a good idea since a userspace permissions check is 
logically different than the other kinds of validation being performed there.  
However, I think it
was motivated by the fact that originally all callers of rw_verify_area() were 
also performing a security_file_permission() call, and to help ensure that no 
future read/write interfaces bypassed the security check.

The underlying hook function, selinux_file_permission(), only performs a 
permission check if something has changed since the checks performed at open 
time, e.g. the current process' sid differs from that of the opener, the inode 
SID has changed, or the policy has changed.  In this case, I assume it is 
because the writer is running in the kernel domain whereas the
opener was in the domain of the process that invoked mkfs, e.g. su.

The near term fix is to simply allow it for the kernel domain under 
userdebug_or_eng().  A longer term fix might be
to rework where security_file_permission() is called so that it only occurs 
when operating in the context of a userspace
process, not a kernel thread.  The latter would be taken to the 
linux-security-module and linux-fsdevel mailing lists for consideration.

Re: mkfs.ext2 /dev/block/loop7 got avc denials with 4.14 kernel but not with 4.9 kernel

[Stephen Smalley]

On 05/07/2018 12:51 PM, Stephen Smalley wrote:
> On 05/07/2018 12:30 PM, Yongqin Liu wrote:
>> I run the commands as root with userdebug build, after run su command.
> 
> Can you run id -Z before and after running su?  I'm trying to understand why 
> the scontext is u:r:kernel:s0 instead of e.g. u:r:shell:s0 (regular shell) or 
> u:r:su:s0 (su shell).  Is it because it is a console rather than adb and 
> there is no domain transition defined for shell execution from the console?  
> Should there be a domain_auto_trans(kernel, shell_exec, shell) rule in policy?

Actually, we don't allow kernel domain to execute anything other than init, so 
I don't understand how you got a shell running in kernel domain (if that is in 
fact what you did).

> 
>>  
>>
>> It makes sense that you would need read and write permissions to the 
>> underlying storage.  I am a little puzzled
>> as to why it is showing up as a denial on a scontext of 
>> u:r:kernel:s0 unless your console shell is running in
>> the kernel's context.
>>
>> I don't know what changed in the kernel but it seems correct that it 
>> is now making these checks.  Possibly
>> this was part of the changes to support mounting of filesystems from 
>> user namespaces, to ensure that the
>> process was truly authorized to read/write the underlying storage.
>>
>>
>> I think I found the change, it the change here:
>> 
>> https://android.googlesource.com/kernel/hikey-linaro/+/abbb65899aecfc97bda64b6816d1e501754cfe1f%5E%21/#F3
>>  
>> <https://android.googlesource.com/kernel/hikey-linaro/+/abbb65899aecfc97bda64b6816d1e501754cfe1f%5E%21/#F3>
>>
>> In the change, it calls do_iter_write in vfs_iter_write, and that makes 
>> the vfs_iter_write call rw_verify_area in directly, 
>> 
>> https://android.googlesource.com/kernel/hikey-linaro/+/android-hikey-linaro-4.14/fs/read_write.c#938
>>  
>> <https://android.googlesource.com/kernel/hikey-linaro/+/android-hikey-linaro-4.14/fs/read_write.c#938>
>>
>> which calls security_file_permission for permission check.
>>
>> While the 4.9 vfs_iter_write does not security_file_permission in it's 
>> implementation here:
>> 
>> https://android.googlesource.com/kernel/hikey-linaro/+/android-hikey-linaro-4.9/fs/read_write.c
>>  
>> <https://android.googlesource.com/kernel/hikey-linaro/+/android-hikey-linaro-4.9/fs/read_write.c>
>>
>> I do not verify my thought with any build yet, but I think if I reverted 
>> the above change for 4.14 kernel, then the denials will go.
>>
>> Verified with the change 
>> https://android.googlesource.com/kernel/hikey-linaro/+/abbb65899aecfc97bda64b6816d1e501754cfe1f%5E%21/#F3
>>  reverted,
>> and no similar avc denials reported again. And the original failed VTS test 
>> cases passed now.
>>
>> Need to check on how to update the sepolicy rules on userspace side.
> 
> Just make sure you wrap it with userdebug_or_eng() so it doesn't get included 
> in user builds.
> 

Re: mkfs.ext2 /dev/block/loop7 got avc denials with 4.14 kernel but not with 4.9 kernel

[Stephen Smalley]

On 05/07/2018 12:30 PM, Yongqin Liu wrote:
> I run the commands as root with userdebug build, after run su command.

Can you run id -Z before and after running su?  I'm trying to understand why 
the scontext is u:r:kernel:s0 instead of e.g. u:r:shell:s0 (regular shell) or 
u:r:su:s0 (su shell).  Is it because it is a console rather than adb and there 
is no domain transition defined for shell execution from the console?  Should 
there be a domain_auto_trans(kernel, shell_exec, shell) rule in policy?

>  
> 
> It makes sense that you would need read and write permissions to the 
> underlying storage.  I am a little puzzled
> as to why it is showing up as a denial on a scontext of u:r:kernel:s0 
> unless your console shell is running in
> the kernel's context.
> 
> I don't know what changed in the kernel but it seems correct that it 
> is now making these checks.  Possibly
> this was part of the changes to support mounting of filesystems from 
> user namespaces, to ensure that the
> process was truly authorized to read/write the underlying storage.
> 
> 
> I think I found the change, it the change here:
> 
> https://android.googlesource.com/kernel/hikey-linaro/+/abbb65899aecfc97bda64b6816d1e501754cfe1f%5E%21/#F3
>  
> 
> 
> In the change, it calls do_iter_write in vfs_iter_write, and that makes 
> the vfs_iter_write call rw_verify_area in directly, 
> 
> https://android.googlesource.com/kernel/hikey-linaro/+/android-hikey-linaro-4.14/fs/read_write.c#938
>  
> 
> 
> which calls security_file_permission for permission check.
> 
> While the 4.9 vfs_iter_write does not security_file_permission in it's 
> implementation here:
> 
> https://android.googlesource.com/kernel/hikey-linaro/+/android-hikey-linaro-4.9/fs/read_write.c
>  
> 
> 
> I do not verify my thought with any build yet, but I think if I reverted 
> the above change for 4.14 kernel, then the denials will go.
> 
> Verified with the change 
> https://android.googlesource.com/kernel/hikey-linaro/+/abbb65899aecfc97bda64b6816d1e501754cfe1f%5E%21/#F3
>  reverted,
> and no similar avc denials reported again. And the original failed VTS test 
> cases passed now.
> 
> Need to check on how to update the sepolicy rules on userspace side.

Just make sure you wrap it with userdebug_or_eng() so it doesn't get included 
in user builds.

Re: mkfs.ext2 /dev/block/loop7 got avc denials with 4.14 kernel but not with 4.9 kernel

[Stephen Smalley]

On 05/07/2018 01:29 AM, Yongqin Liu wrote:
> @Sandeep,
> I see you submitted the change "Add label for kernel test files and 
> executables" here:
> https://android.googlesource.com/platform/system/sepolicy/+/34e35e9e9500608409920471dc05f12b9317338e
> 
> So looped you here, maybe you have some suggestion on this problem.
> 
> 
> On 5 May 2018 at 01:02, Stephen Smalley <s...@tycho.nsa.gov 
> <mailto:s...@tycho.nsa.gov>> wrote:
> 
> On 05/04/2018 09:56 AM, Yongqin Liu wrote:
> > Hi, All
> > 
> > When I run "mkfs.ext2 /dev/block/loop7" with 4.14 kernel on AOSP master 
> build, i got the following  denials:
> > 
> > [ 3004.028178] type=1400 audit(1525358655.127:5624): avc: denied { read 
> } for pid=2868 comm="loop7" path="/data/local/tmp/fstest/fstest.img" 
> dev="mmcblk0p10" ino=130561 scontext=u:r:kernel:s0 
> tcontext=u:object_r:shell_data_file:s0
> > tclass=file permissive=0 
> > 
> > 
> > but not get such denials with 4.9 kernel.
> > 
> > The only change is the kernel version, the userspace of Android is the 
> same.
> > 
> > For details, please check the links here:
> > 
> > 4.14-mkfs.ext2 https://pastebin.ubuntu.com/p/yBzz7TXjGy/ 
> <https://pastebin.ubuntu.com/p/yBzz7TXjGy/>
> > 4.9-mkfs.ext2   https://pastebin.ubuntu.com/p/JCHYznxHww/ 
> <https://pastebin.ubuntu.com/p/JCHYznxHww/>
> > 
> > 
> > I guess there is more strict check related to the mkfs operation in 
> kernel side,
> > but I could not find out which operation yet.
> > not sure if anyone knows any clues about this problem.
> > 
> > Thanks in advance!
> > 
> > BTW, mkfs.vfat does not have this problem with 4.14, mkfs.ext4 has the 
> same problem.
> 
> I see the following in system/sepolicy/public/kernel.te:
> # Allow reading loop device in update_engine_unittests. (b/28319454)
> # and for LTP kernel tests (b/73220071)
> userdebug_or_eng(`
>   allow kernel update_engine_data_file:file read;
>   allow kernel nativetest_data_file:file read;
> ')
> 
> It seems like you could add another rule there for shell_data_file, as 
> long as it remains bracketed
> by userdebug_or_eng().  This obviously is not something that should 
> happen on user builds.
> 
> 
> After changed to label the img file with nativetest_data_file, the mkfs.ext2 
> command exit with 0, but still could see avc denials related to write 
> permission.
> and it caused the mount command next failed.
> When change to permissive mode, do not see the IO message in kernel log for 
> mkfs.ext2 command, and the mount command next could be run successfully.
> seems the mkfs.ext2 command will write something to the local .img file.
> 
> I am thinking if we should allow write permission(like read) for kernel on 
> nativetest_data_file under userdebug_or_eng,
> but not sure if it's the right solution or there is any other better solution.
> 
> but considering this only happens with 4.14, but not with 4.9 kernel, it 
> might be better to understand what changed in the kernel side.
> 
> Background:
> I am testing the VtsKernelLtp with android build, and found there are 
> failures passed when run under permissive mode.
> the instructions I run here are similar to the steps run by the VtsKernelLtp 
> failed tests cases.
> 
> Following is the output for the commands and kernel message from the serial 
> console.
>  commands under enforce mode ##
> console:/data/local/tmp/ltp/tmp/tmpdir #dd if=/dev/zero of=fstest.img bs=1M 
> count=100                                  <
> 100+0 records in
> 100+0 records out
> 104857600 bytes (100 M) copied, 0.502641 s, 199 M/s
> console:/data/local/tmp/ltp/tmp/tmpdir # ls -Z fstest.img                     
>  
> u:object_r:nativetest_data_file:s0 fstest.img
> console:/data/local/tmp/ltp/tmp/tmpdir # losetup /dev/block/loop0 fstest.img
> console:/data/local/tmp/ltp/tmp/tmpdir # mkfs.ext2 /dev/block/loop0
> mke2fs 1.43.3 (04-Sep-2016)
> Discarding device blocks: done                            
> Creating filesystem with 102400 1k blocks and 25688 inodes
> Filesystem UUID: 7d0a8476-7beb-4423-af6d-63dc4f3fc5f4
> Superblock backups stored on blocks: 
>         8193, 24577, 40961, 57345, 73729
> 
> Allocating group tables: done                            
> Writing inode tables: done                            
> Writing superblocks and filesystem accounting information: [ 1902.539349] 
> lo_write_bvec: 899 callbacks suppressed
> [ 1902.539355] loop

Re: mkfs.ext2 /dev/block/loop7 got avc denials with 4.14 kernel but not with 4.9 kernel

[Stephen Smalley]

On 05/04/2018 09:56 AM, Yongqin Liu wrote:
> Hi, All
> 
> When I run "mkfs.ext2 /dev/block/loop7" with 4.14 kernel on AOSP master 
> build, i got the following  denials:
> 
> [ 3004.028178] type=1400 audit(1525358655.127:5624): avc: denied { read } for 
> pid=2868 comm="loop7" path="/data/local/tmp/fstest/fstest.img" 
> dev="mmcblk0p10" ino=130561 scontext=u:r:kernel:s0 
> tcontext=u:object_r:shell_data_file:s0 
> tclass=file permissive=0 
> 
> 
> but not get such denials with 4.9 kernel.
> 
> The only change is the kernel version, the userspace of Android is the same.
> 
> For details, please check the links here:
> 
> 4.14-mkfs.ext2 https://pastebin.ubuntu.com/p/yBzz7TXjGy/
> 4.9-mkfs.ext2   https://pastebin.ubuntu.com/p/JCHYznxHww/
> 
> 
> I guess there is more strict check related to the mkfs operation in kernel 
> side,
> but I could not find out which operation yet.
> not sure if anyone knows any clues about this problem.
> 
> Thanks in advance!
> 
> BTW, mkfs.vfat does not have this problem with 4.14, mkfs.ext4 has the same 
> problem.

I see the following in system/sepolicy/public/kernel.te:
# Allow reading loop device in update_engine_unittests. (b/28319454)
# and for LTP kernel tests (b/73220071)
userdebug_or_eng(`
  allow kernel update_engine_data_file:file read;
  allow kernel nativetest_data_file:file read;
')

It seems like you could add another rule there for shell_data_file, as long as 
it remains bracketed
by userdebug_or_eng().  This obviously is not something that should happen on 
user builds.

As to why the kernel changed, I would speculate that some refactoring of the 
vfs code has caused
this check to be triggered (via the security_file_permission hook).  We didn't 
specifically change
anything in SELinux in this area as far as I recall.

Shutting down the seandroid bitbucket site and possibly this list

[Stephen Smalley]

Hi,

The seandroid bitbucket site stopped being updated a couple of years ago with 
the completion
of merging SE for Android's SELinux support into mainline Android and both the 
repositories
and the web content had become very stale (with some broken links on the latter 
due to other
sites shuffling their content).  Most existing links to our web pages were 
broken anyway when bitbucket
stopped redirecting the bitbucket.org links to bitbucket.io after they 
relocated all project pages.
We have now removed those repositories and web pages entirely.

If you want a quick reference to the historical presentations and papers 
relevant to SE for
Android, you can find an updated list at 
http://selinuxproject.org/page/SEforAndroid; feel free to
let us know of others that should be added in the future.

We are considering shutting down this list (seandroid-list) since there isn't 
significant
activity on it any more and all development activity moved to AOSP.  People can 
still bring SELinux questions
to the selinux mailing list, and there are the Android discussion groups for 
more Android-specific topics.
Would anyone have any concerns with decommissioning this list?

Re: init: SELinux: Could not load policy

[Stephen Smalley]

On 04/12/2018 07:59 AM, kiran mardi wrote:
> Hi Stephen,
> 
> it is a field issue and we see only on 1 set. it is seen on android N set.
> it is strange that it is seen on only 1 set. can it be hardware issue? DDR 
> issue since we see ebitmap is giving different data?.

Yes, the fact that you are getting different values from what should be the 
same policy on recovery is suspect.

> 
> On Wed, Apr 11, 2018 at 7:08 PM, Stephen Smalley <s...@tycho.nsa.gov 
> <mailto:s...@tycho.nsa.gov>> wrote:
> 
> On 04/11/2018 06:12 AM, kiran mardi wrote:
> > Hi All,
> >
> > I see in one of my device getting the selinux policy loading error 
> during init first stage.
> > however the logs give every boot different error w.r.t selinux policy 
> loading.
> >
> > 1st bootup of set:
> >
> > [    7.933699] init: SELinux:  Could not load policy:  Out of memory
> > [    7.938900] init: failed to load policy: Out of memory
> > [    7.943884] init: Security failure; rebooting into recovery mode...
> >
> >
> > 2nd bootup[to recovery]:
> >
> > [ 7.028166] SELinux: ebitmap start bit (*400*) is not a multiple of the 
> map unit size (64)
> > [    7.035557] init: SELinux:  Could not load policy:  Invalid argument
> > [    7.041652] init: failed to load policy: Invalid argument
> > [    7.047031] init: Security failure; rebooting into recovery mode...
> >
> >
> > 3rd bootup[to recovery]:
> >
> > [ 7.622606] SELinux: ebitmap: map size *1048640 *does not match my size 
> 64 (high bit was 0)
> >  [    7.630081] init: SELinux:  Could not load policy:  Invalid argument
> >  [    7.636214] init: failed to load policy: Invalid argument
> > [    7.641447] init: Security failure; rebooting into recovery mode...
> >
> >
> > is it problem with my kernel allocating memory for selinux sys/fs?
> > can i suspect RAM not working properly?
> 
> Sounds like the policy is corrupted.  Can you confirm that the policy 
> file itself is valid, e.g. on the build host, run seinfo on the policy file?
> 
> Does your kernel match your policy?  There was an incompatible change in 
> policy format between Android 6 Marshmallow and Android 7 Nougat; Google 
> provided a backward compatibility patch in their common kernels so that 
> Android 7 kernels could still load older policies.
> 
> 
> 
> 
> -- 
> regards,
> kiran mardi

Re: init: SELinux: Could not load policy

[Stephen Smalley]

On 04/11/2018 06:12 AM, kiran mardi wrote:
> Hi All,
> 
> I see in one of my device getting the selinux policy loading error during 
> init first stage.
> however the logs give every boot different error w.r.t selinux policy loading.
> 
> 1st bootup of set:
> 
> [7.933699] init: SELinux:  Could not load policy:  Out of memory
> [7.938900] init: failed to load policy: Out of memory
> [7.943884] init: Security failure; rebooting into recovery mode...
> 
> 
> 2nd bootup[to recovery]:
> 
> [ 7.028166] SELinux: ebitmap start bit (*400*) is not a multiple of the map 
> unit size (64)
> [7.035557] init: SELinux:  Could not load policy:  Invalid argument
> [7.041652] init: failed to load policy: Invalid argument
> [7.047031] init: Security failure; rebooting into recovery mode...
> 
> 
> 3rd bootup[to recovery]:
> 
> [ 7.622606] SELinux: ebitmap: map size *1048640 *does not match my size 64 
> (high bit was 0)
>  [7.630081] init: SELinux:  Could not load policy:  Invalid argument
>  [7.636214] init: failed to load policy: Invalid argument
> [7.641447] init: Security failure; rebooting into recovery mode...
> 
> 
> is it problem with my kernel allocating memory for selinux sys/fs?
> can i suspect RAM not working properly?

Sounds like the policy is corrupted.  Can you confirm that the policy file 
itself is valid, e.g. on the build host, run seinfo on the policy file?

Does your kernel match your policy?  There was an incompatible change in policy 
format between Android 6 Marshmallow and Android 7 Nougat; Google provided a 
backward compatibility patch in their common kernels so that Android 7 kernels 
could still load older policies.

Re: /data/misc contents are unlabeled

[Stephen Smalley]

On 03/09/2018 08:13 AM, Stephen Smalley wrote:
> On 03/09/2018 02:55 AM, kiran mardi wrote:
>>     >>>>>>>>sh-3.2# toybox restorecon -nv /data/misc/dhcp
>>
>> [  158.754324] type=1400 audit(946742542.500:16): avc: denied { search } for 
>> pid=983 comm="toybox" name="security" dev="mmcblk0p7" ino=186945 
>> scontext=u:r:shell:s0 tcontext=u:object_r:security_file:s0 tclass=dir 
>> permissive=1
>>
>> SELinux: Loaded file_contexts contexts from /file_contexts.bin.[  
>> 158.776446] type=1400 audit(946742542.520:17): avc: denied { getattr } for 
>> pid=983 comm="toybox" path="/data/misc/dhcp" dev="mmcblk0p7" ino=406419 
>> scontext=u:r:shell:s0 tcontext=u:object_r:unlabeled:s0 tclass=dir 
>> permissive=1
>>
>>  
>>
>> SELinux:  Relabeling /data/misc/dhcp from u:object_r:unlabeled:s0 to 
>> u:object_r:dhcp_data_file:s0.
> 
> Ok, so you have a valid context for /data/misc/dhcp in your file_contexts, 
> which should have been used if the restorecon_recursive /data executed.
> 
> Did your file_contexts configuration change between the old and new versions? 
>  restorecon_recursive /data will skip the tree walk if file_contexts has not 
> changed since the last time it was run; this is based on a separate 
> security.restorecon_last xattr set on the /data directory with the SHA1 hash 
> of the /file_contexts.bin file.
> 
> Also, what was the context on /data/misc/dhcp in the prior version from which 
> you are upgrading?  Was it the same or different?  If different, what was it?

Also, were there any interesting log messages on the first boot after the 
upgrade (i.e. when we would expect the restorecon_recursive to execute)?  Look 
for any logcat or dmesg messages with SELinux: or avc: prefixes.

Re: Re: /data/misc contents are unlabeled

[Stephen Smalley]

On 03/08/2018 12:05 PM, kiran mardi wrote:
> Hi Stephen,
> 
> Please find my response inline.
> 
> Regards,
> Kiran Mardi
> 
> On Thu, Mar 8, 2018 at 8:56 PM, Stephen Smalley <s...@tycho.nsa.gov 
> <mailto:s...@tycho.nsa.gov>> wrote:
> 
> Is this on AOSP master or a particular release branch?
> [Kiran]: it is particular release branch Android N.

Which one?

> 
>  
> 
> Is this occurring on a clean install or on an upgrade from a previous 
> version?
> 
>     [Kiran]: It is upgrade from previous version.

Which previous version, and was SELinux enabled in that version?
 
> There should be a recursive restorecon by init.rc of /data already from 
> post-fs-data which should label everything.
> [Kiran]: yes, from my archive the restorecon is part of init.rc but it is 
> not recursive. is this the root cause?
> 
> 349 
> <http://2k16-xref.tpvision.com:8080/source/xref/MTK_N_NEWSTRUCT/android/n-base/system/core/rootdir/init.rc#349>
>  # We restorecon /data in case the userdata partition has been reset. 350 
> <http://2k16-xref.tpvision.com:8080/source/xref/MTK_N_NEWSTRUCT/android/n-base/system/core/rootdir/init.rc#350>
>  restorecon /data

That one is just to fix the label on the top directory upon a reset of 
userdata; if you look further down in the init.rc file you should find this 
line:
  # Set SELinux security contexts on upgrade or policy update.
restorecon_recursive /data

It is there in AOSP's nougat-mr2.3-release branch, for example.

> 
>  
> 
> Is there perhaps an invalid context in your file_contexts configuration 
> for these directories?
> Does it pass checkfc validation against your policy file?
> [Kiran]: no there are no invalid file_contexts and never got any 
> compilation error.
> 
>  
> 
> What does a restorecon -nv /data/misc/dhcp report from an adb shell?
> [Kiran]: will get back on the result.
> 
>  
> 
> On 03/08/2018 10:14 AM, kiran mardi wrote:
> > some more data to the below issue. below are the folders which have 
> become unlabeled dont know the relation though.
> >
> > drwxrwxr-x  2 dhcp         dhcp         u:object_r:unlabeled:s0         
>       24576 2013-03-30 16:44 dhcp
> > drwxrwx---  2 system       system       u:object_r:unlabeled:s0         
>       24576 1995-09-25 03:39 ethernet
> > drwx--  2 media        media        u:object_r:unlabeled:s0         
>       24576 1995-09-25 03:39 media
> > drwxrwxr-x  2 system       cache        u:object_r:unlabeled:s0         
>       24576 2013-10-19 10:47 onehelp
> > drwxrwxr-x  2 root         root         u:object_r:unlabeled:s0         
>       24576 1995-09-25 03:39 perfprofd
> > drwxrwx--x  3 root         root         u:object_r:unlabeled:s0         
>       24576 2000-01-01 16:05 user
> > drwx--  2 root         root         u:object_r:unlabeled:s0         
>       24576 1995-09-25 03:39 vold
> >
> >
> > On Thu, Mar 8, 2018 at 7:13 PM, kiran mardi <mardiki...@gmail.com 
> <mailto:mardiki...@gmail.com> <mailto:mardiki...@gmail.com 
> <mailto:mardiki...@gmail.com>>> wrote:
> >
> >     Hi All,
> >
> >     In one of our set we are seeing /data/misc/netd, keystore, user as 
> unlabeled (dont know why it has become unlabeled). Since it is google AOSP 
> module I am expecting restorecon should be part of AOSP code on these folder.
> >
> >     ===
> >     [Wed Mar 07 20:20:23.103 2018] [   19.018081] type=1400 
> audit(946742413.155:43): avc: denied { unlink } for pid=1146 comm="netd" 
> name="netd_pid" dev="mmcblk0p7" ino=406433 scontext=u:r:netd:s0 
> tcontext=u:object_r:unlabeled:s0 tclass=file permissive=0
> >
> >     [Wed Mar 07 20:20:23.103 2018] [   19.034844] type=1400 
> audit(946742413.160:44): avc: denied { setattr } for pid=1161 comm="installd" 
> name="0" dev="mmcblk0p7" ino=407130 scontext=u:r:installd:s0 
> tcontext=u:object_r:unlabeled:s0 tclass=dir permissive=0
> >
> >     [Wed Mar 07 20:20:29.397 2018] [   25.229311] type=1400 
> audit(946742419.495:47): avc: denied { write } for pid=1146 comm="netd" 
> name="*rt_tables*" dev="mmcblk0p7" ino=406432 scontext=u:r:netd:s0 
> tcontext=u:object_r:unlabeled:s0 tclass=file permissive=0
> >
> >     [Wed Mar 07 20:20:29.425 2018] [   25.256023] type=1400 
> audit(946742419.520:48): avc: denied { write } for pid=1146 
> comm="Binder:1146_2" name="*netd_pid*" dev="mmcblk0p7" ino=406433 
> sco

Re: /data/misc contents are unlabeled

[Stephen Smalley]

Is this on AOSP master or a particular release branch?

Is this occurring on a clean install or on an upgrade from a previous version?

There should be a recursive restorecon by init.rc of /data already from 
post-fs-data which should label everything.

Is there perhaps an invalid context in your file_contexts configuration for 
these directories?
Does it pass checkfc validation against your policy file?

What does a restorecon -nv /data/misc/dhcp report from an adb shell?

On 03/08/2018 10:14 AM, kiran mardi wrote:
> some more data to the below issue. below are the folders which have become 
> unlabeled dont know the relation though.
> 
> drwxrwxr-x  2 dhcp         dhcp         u:object_r:unlabeled:s0               
> 24576 2013-03-30 16:44 dhcp
> drwxrwx---  2 system       system       u:object_r:unlabeled:s0               
> 24576 1995-09-25 03:39 ethernet
> drwx--  2 media        media        u:object_r:unlabeled:s0               
> 24576 1995-09-25 03:39 media
> drwxrwxr-x  2 system       cache        u:object_r:unlabeled:s0               
> 24576 2013-10-19 10:47 onehelp
> drwxrwxr-x  2 root         root         u:object_r:unlabeled:s0               
> 24576 1995-09-25 03:39 perfprofd
> drwxrwx--x  3 root         root         u:object_r:unlabeled:s0               
> 24576 2000-01-01 16:05 user
> drwx--  2 root         root         u:object_r:unlabeled:s0               
> 24576 1995-09-25 03:39 vold
> 
> 
> On Thu, Mar 8, 2018 at 7:13 PM, kiran mardi  > wrote:
> 
> Hi All,
> 
> In one of our set we are seeing /data/misc/netd, keystore, user as 
> unlabeled (dont know why it has become unlabeled). Since it is google AOSP 
> module I am expecting restorecon should be part of AOSP code on these folder.
> 
> ===
> [Wed Mar 07 20:20:23.103 2018] [   19.018081] type=1400 
> audit(946742413.155:43): avc: denied { unlink } for pid=1146 comm="netd" 
> name="netd_pid" dev="mmcblk0p7" ino=406433 scontext=u:r:netd:s0 
> tcontext=u:object_r:unlabeled:s0 tclass=file permissive=0
> 
> [Wed Mar 07 20:20:23.103 2018] [   19.034844] type=1400 
> audit(946742413.160:44): avc: denied { setattr } for pid=1161 comm="installd" 
> name="0" dev="mmcblk0p7" ino=407130 scontext=u:r:installd:s0 
> tcontext=u:object_r:unlabeled:s0 tclass=dir permissive=0
> 
> [Wed Mar 07 20:20:29.397 2018] [   25.229311] type=1400 
> audit(946742419.495:47): avc: denied { write } for pid=1146 comm="netd" 
> name="*rt_tables*" dev="mmcblk0p7" ino=406432 scontext=u:r:netd:s0 
> tcontext=u:object_r:unlabeled:s0 tclass=file permissive=0
> 
> [Wed Mar 07 20:20:29.425 2018] [   25.256023] type=1400 
> audit(946742419.520:48): avc: denied { write } for pid=1146 
> comm="Binder:1146_2" name="*netd_pid*" dev="mmcblk0p7" ino=406433 
> scontext=u:r:*netd*:s0 tcontext=u:object_r:unlabeled:s0 tclass=file 
> permissive=0
> =
> below folders are getting unlabeled.
> 
> /data/misc/keystore
> data/misc/netd
> data/misc/user
> 
> what may be the reason for this unlabeled?
> 
> Is restorecon -R in init.rc file for these folder is the solution? want 
> to know the real reason.
> 
> Please help
> 
> -- 
> regards,
> kiran mardi
> 
> 
> 
> 
> -- 
> regards,
> kiran mardi

Re: Help: selinux training on android Platform

[Stephen Smalley]

On Wed, 2017-12-06 at 14:52 +0530, kiran mardi wrote:
> Hi All,
> 
> May I know if there are any Selinux training conducted in Bangalore
> India for android Platform?
> 
> I am interested in understanding the Architecture of selinux.
> 
> If any good material also be a help.

I don't know of any training in Bangalore, but here are some online
resources that may be helpful to you:

Google's Android SELinux docs (note that there is more than one page,
see the menu on the left):
https://source.android.com/security/selinux/

Our SE for Android site (historical at this point, but may still be helpful):
https://seandroid.bitbucket.io/

The SELinux Notebook,
http://freecomputerbooks.com/The-SELinux-Notebook-The-Foundations.html

Re: SElinux Category levels relabelfrom issue

[Stephen Smalley]

On Wed, 2017-12-06 at 09:11 -0500, Stephen Smalley wrote:
> On Thu, 2017-11-30 at 11:21 +0530, Iranna Badiger wrote:
> > Hi,
> > 
> > I am getting some denials at the bootup which is mentioned below, 
> > 
> >  type=1400 audit(1219320528.560:4): avc:  denied  { relabelfrom }
> > for  pid=1 comm="init" name="Mypath.bin" dev="mmcblk0p16" ino=24922
> > scontext=u:r:init:s0 tcontext=u:object_r:mypath:s0:c512,c768
> > tclass=file permissive=0
> > 
> > 1. /Mypath/Mypath.bin is created by Platform_app process which has
> > label as below,
> > 
> > MyService u:r:platform_app:s0:c512,c768
> > 
> > 2. On every boot in init i am doing restorecon_recursive on Mypath/
> > dir.
> > 
> > above denial is seen only some times, not every boot up. i am
> > worried
> > whether to allow relabelfrom permission for init.
> > 
> > 1. hoping to know why these denials are printed only sometimes, why
> > not every bootup.
> > 
> > Can you please suggest how to go with this kind of denials.
> 
> Since you are performing a restorecon_recursive of this directory
> from
> init.rc, you need to allow init to relabel it.  restorecon_recursive
> however only performs the file tree walk if file_contexts has changed
> since the last time, which is why you only see the denial some
> times. 
> Normally, init is allowed relabelfrom to all file types with a few
> exceptions through a rule in init.te.  If you assigned the file_type
> attribute to your mypath type, then this rule would allow relabeling.
> 
> I think the larger issue here is that you say that a platform app
> process is creating a file outside of its own app data
> directory.  That
> seems like a violation of Android's model.

I guess the other question is whether you added an entry for
/Mypath(/.*)? to file_contexts so that restorecon_recursive sets the
context correctly.

Re: map on _tmpfs

[Stephen Smalley]

On Wed, 2017-11-01 at 11:06 -0700, William Roberts wrote:
> We're using a new kernel that has the map permission
> 
> We're seeing denials on apps in/using the tmpfs_domain()
> macro.
> 
> I *think* that this was just missed in:
> https://android-review.googlesource.com/#/c/platform/system/sepolicy/
> +/432339/
> 
> I have an RFC patch here:
> https://android-review.googlesource.com/526419

Yes, that makes sense to me.

Re: Question about lnk_file

[Stephen Smalley]

On Wed, 2017-06-14 at 11:24 +0800, peng fei wrote:
> In the app data file, there is lib direction:  lib -> /data/app-
> lib/com.android.providers.settings. 
> For example if a process has domain  A_domain.
> There is a requirement that allow A_domian  open lib and create file
> inside it.
> 
> To implement the requirement , we need two part of policy.
> One part is :      allow A_domian system_app_data_file :lnk_file
> {open, read,getattr}
> another part is: allow A_domian system_data_file: file
> create_file_perms
>                            allow A_domian system_data_file: dir
> {write, add_name}
> 
> requirement- I want to open link and create file inside
> it.
> policy--I should have                       allow
> rule to open and read link. 
> --And I also should have       allow
> rules to open and write dir which the link pointed to.
> two part policy is needed to implement the requirement 
> Is that right?
> 
> I am looking  forward to your answer. Thanks advance.

I doubt you want to have your process opening these directories through
the symlink in the app data directory (consider what happens if the app
replaces the symlink with one pointing somewhere else).  Why not just
directly access /data/app-lib/?  And why would anything need
to be created there after package installation?

Re: APP data protection

[Stephen Smalley]

On Wed, 2017-06-14 at 11:03 +0800, peng fei wrote:
> I want to add a context '/data/data/com.UCMobile.intl/databases(/.*)?
> u:object_r:ub_data_file :s0 ' in the file_context.
> And only allow UCMobile read and write
> /data/data/com.UCMobile.intl/databases.
> 
> Can it take effect?
> 
> Please help me. Thanks advance.

App data directories are labeled based on seapp_contexts, not
file_contexts.

You would likely need/want to add an entry to mac_permissions.xml for
UCMobile to map its certificate and package name to a unique seinfo tag
and add the path to the certificate to keys.conf, and add an entry to
seapp_contexts to assign a distinct domain and type for apps with that
seinfo tag.  seapp_contexts also has a name= specifier that can be used
to identify the package name, but that always needs to be paired with a
seinfo= tag to ensure that it was signed with an appropriate
certificate.

Generally a single type is assigned to the entire app data directory,
not just to a particular subdirectory like databases.  You could
perhaps use the path= specifier in seapp_contexts to limit the type to
the databases subdirectory, but I doubt that is really what you want
(and it requires your app or whatever creates the directory to call
setfscreatecon() before mkdir() or restorecon() after mkdir() to assign
that type to the directory).  Much easier to just apply ub_data_file or
whatever type it is to all files under /data/data/com.UCMobile.intl.

An alternative to introducing a distinct domain/type would just be to
enable levelFrom=all, at least for this particular app, which would
enable a unique category set for the app and thereby prevent any other
app from opening its files.

Re: How to check a neverallow for a single allow rule?

[Stephen Smalley]

On Fri, 2017-05-19 at 16:52 +0900, HAN wrote:
> Dear All,
> 
> I'm doing a SEAndroid in my company and have one question.
> Our developers add SEAndroid policies for their own function oftenly.
> 
> However, they don't know whether the policies are violated neverallow
> or not.
> Since our environment is slows to build kernel, I  want to suggest a
> check their policies before pushing to our repository.
> 
> So I want to apply a system which verifies entered policies and
> return the neverallow checking result.
> 
> Is there any tool for this?
> 
> I've checked a "sepolicy-analyze" tool, but looks like it checks a
> sepolicy binary
> for checking neverallow, not raw allow rules.
> 
> 
> Any response will be greatly appreciated and hope you have a great
> day.

Sorry, I don't follow.  All they have to do is test building the
policy; any neverallow failures will be caught at build time.

mmm -B system/sepolicy

Re: Questions about disable SECURITY_SELINUX_DEVELOP

[Stephen Smalley]

On Mon, 2017-03-13 at 08:40 +, Weiyuan (David, Euler) wrote:
> Hi
>  
> Currently I’m thinking about disable SECURITY_SELINUX_DEVELOP
> by default to enhance security,  So hacker can not easily turn off
> selinux
> by modify the global variable “selinux_enforing”.
>  
> Question:
> If SECURITY_SELINUX_DEVELOP is disabled, the kernel will run in
> enforcing mode from start,
> but there is no policy before init process load sepolicy into kernel.
> In this no policy but enforcing stage,  what will kernel behave?  
> Will there be avc denied before loading sepolicy?

Until the policy is loaded, the security server allows all permissions.
See security/selinux/ss/services.c:security_compute_av(), the
!ss_initialized case.  So, no, you won't get any avc denials before
loading policy.  The security server resets the AVC upon policy loads,
so any permissions cached before loading policy will be flushed upon
the first policy load and rechecked on subsequent operations.
___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: Permission denied error when opening /proc//stat file

[Stephen Smalley]

On Dec 21, 2016 9:35 PM, "Helen Chiang" <chiangh...@gmail.com> wrote:

In another custom ROM build, I'm seeing problem again where the data file
for my system app is not getting created with the context
I specified, attiqi_app_data_file. I'm seeing errors in logcat (see below).
Am I missing allow rules for "installd" ? This is what I have:

 seapp_context ***
# process name is diagandroid.iqd
# package name is com.att.iqi

user=system seinfo=platform domain=attiqi_app name=diagandroid.iqd

user=system seinfo=platform name=com.att.iqi type=attiqi_app_data_file


 snippet from my te file 

allow installd {
attiqi_app_data_file
}:dir { create_dir_perms relabelfrom relabelto };

allow installd {
attiqi_app_data_file
}:notdevfile_class_set { create_file_perms relabelfrom relabelto };


 Errors from logcat 

12-21 18:05:55.569   905   905 E SELinux : selinux_android_setfilecon:
Error setting context for pkgdir /data/data/com.att.iqi, uid 1000:
Permission denied

12-21 18:05:55.569   905   905 E installd: Failed to setfilecon
/data/data/com.att.iqi: Permission denied


Did you get any avc: denied messages?



On Tue, Dec 6, 2016 at 12:10 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:

> On 12/06/2016 03:01 PM, Helen Chiang wrote:
> > The name of the process is "diagandroid.iqd", package name is
> "com.att.iqi".
> > Is this a problem?
>
> Ok, so add two entries to seapp_contexts, one to assign the domain by
> process name and the other to assign the type by package name, ala:
> user=system seinfo=platform name=diagandroid.iqd domain=attiqi_app
> user=system seinfo=platform name=com.att.iqi type=attiqi_app_data_file
>
> >
> > On Tue, Dec 6, 2016 at 11:34 AM, Stephen Smalley <s...@tycho.nsa.gov
> > <mailto:s...@tycho.nsa.gov>> wrote:
> >
> > On 12/06/2016 02:00 PM, Helen Chiang wrote:
> > > I declared a new file type and it seems to work but when I use ls
> -Z
> > >  /data/data/, it still shows
> > >  "u:object_r:system_app_data_file:s0". Its proc file
> /proc//stat
> > > shows the new file label, "u:r:attiqi_app:s0"
> > >
> > > bullhead:/data/data # ls -Z /proc/6795/stat
> > > u:r:attiqi_app:s0 /proc/6795/stat
> > >
> > >
> > > bullhead:/data/data # ls -Zl /data/data/com.att.iqi
> > > total 24
> > >
> > > drwxrwx--x 2 system system u:object_r:system_app_data_file:s0 4096
> > > 2016-12-06 10:38 app_iq_archive
> > > drwxrwx--x 2 system system u:object_r:system_app_data_file:s0 4096
> > > 2016-12-06 10:35 cache
> > > drwxrwx--x 2 system system u:object_r:system_app_data_file:s0 4096
> > > 2016-12-06 10:35 files
> > >
> > >
> > > What am I missing? Is this what you'd expect?
> > >
> > > From seapp_contexts
> > >
> > > user=system seinfo=platform domain=attiqi_app name=diagandroid.iqd
> > > type=attiqi_app_data_file
> >
> > The name doesn't match the /data/data name (diagandroid.iqd vs
> > com.att.iqi)?
> > Did you mean to omit the list from your reply?
> >
> > >
> > >
> > > This is what I have in the TE file for my new domain:
> > > type attiqi_app_data_file, file_type, data_file_type;
> > >
> > > allow installd {
> > > attiqi_app_data_file
> > > }:dir { create_dir_perms relabelfrom relabelto };
> > >
> > > allow installd {
> > > attiqi_app_data_file
> > > }:notdevfile_class_set { create_file_perms relabelfrom relabelto };
> > >
> > > allow system_server { attiqi_app_data_file }:dir { getattr read
> > search };
> > > allow system_server { attiqi_app_data_file }:file { getattr read
> > write };
> > > allow system_server attiqi_app_data_file:dir create_dir_perms;
> > > allow system_server attiqi_app_data_file:file create_file_perms;
> > >
> > >
> > > type attiqi_app, domain, domain_deprecated;
>
> > >
> > > # Include all appdomain rules
> > > app_domain(attiqi_app)
> > > # Access the network.
> > > net_domain(attiqi_app)
> > > # Access bluetooth.
> > > bluetooth_domain(attiqi_app)
>
> > >
> > >
> > >
> > >
> > >
> > > > Also, what you're saying seems to imply that if I run as
> >   

Re: Permission denied error when opening /proc//stat file

[Stephen Smalley]

On 12/06/2016 03:17 PM, Nys S wrote:
> hi,
> 
> I tried to do as Halen suggested, but now my java app (which run as
> service) isn't starting.
> in the log I have this error:
> installd: invalid apk path '*/path/to/my/apk*' (bad prefix)
> while I see the apk in that path.
> and then I get:
> ActivityManager: Process ProcessRecord{dd649ae 3859:*my.service.name
> */1000} failed to attach
> 
> What am I doing wrong?
> What I changed from the previous version is:
> - adding seapp_contexts
> - adding mac_permissions.xml
> - adding new_domain.te
> 
> I suspect my error is with the mac_permissions.xml file.
> Where do I put it? under the same folder as the rest sepolicy settings
> of my app? (I have a whole stack, built of native service and native
> libraries).
> what exactly is its content?
> this is what I have now:
> 
> 
> 
> 
> http://my.service.name>*">
> 
> 
> 
> 
> 
> Thanks!!

The AOSP policy lives in external/sepolicy (in M and earlier) or
system/sepolicy (in N and later).  Device-specific policy files can be
added in a device/ or vendor/ subdirectory and merged with the AOSP
policy by specifying a BOARD_SEPOLICY_DIRS variable with the location of
your subdirectory in your BoardConfig.mk or equivalent.
___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: restorecon_recursive /cache in recovery mode

[Stephen Smalley]

On 12/05/2016 05:25 PM, Wei Liu wrote:
> Thanks. My understanding is that init.rc is for normal mode. Shall we
> also need the change under /bootable/recovery/etc?

Sorry, I misunderstood your question.  I don't know whether it is needed
for recovery.

> 
> 
> 
> ----
> *From:* Stephen Smalley <s...@tycho.nsa.gov>
> *Sent:* Monday, December 5, 2016 2:16 PM
> *To:* Wei Liu; seandroid-list@tycho.nsa.gov
> *Subject:* Re: restorecon_recursive /cache in recovery mode
>  
> On 12/05/2016 02:08 PM, Wei Liu wrote:
>> Dear all,
>> 
>> 
>> Just wondering whether we need to do the "restorecon_recursive /cache",
>> in case the recovery image is updated in normal mode then we actually
>> boot up from updated recovery image with possible modified policy/contexts?
> 
> It appears to have been added to ensure proper labeling of
> /cache/lost+found, see:
> https://android-review.googlesource.com/#/c/100675/
> 
> Although I would expect this other change to ensure that:
> https://android-review.googlesource.com/#/c/92290/

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: restorecon_recursive /cache in recovery mode

[Stephen Smalley]

On 12/05/2016 02:08 PM, Wei Liu wrote:
> Dear all,
> 
> 
> Just wondering whether we need to do the "restorecon_recursive /cache",
> in case the recovery image is updated in normal mode then we actually
> boot up from updated recovery image with possible modified policy/contexts?

It appears to have been added to ensure proper labeling of
/cache/lost+found, see:
https://android-review.googlesource.com/#/c/100675/

Although I would expect this other change to ensure that:
https://android-review.googlesource.com/#/c/92290/
___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: Permission denied error when opening /proc//stat file

[Stephen Smalley]

On 12/02/2016 01:59 PM, Helen Chiang wrote:
> On Fri, Dec 2, 2016 at 10:40 AM, Stephen Smalley <s...@tycho.nsa.gov
> <mailto:s...@tycho.nsa.gov>> wrote:
> You removed the offending allow rules from your domain, or the
> neverallow rules that triggered the failure?  CTS has a collection of
> SELinux tests, including checking all neverallows from the AOSP policy
> for that version of Android.  So you can't just remove neverallows from
> your policy to escape them.  But if you removed the offending allow
> rules and your app still works, that is ok.  seandroid.bitbucket.org
> <http://seandroid.bitbucket.org> has
> a list of CTS tests related to SELinux, although it may not be entirely
> up-to-date.
> 
>  
> I removed the offending rules from my domain

Good, that's the right thing to do (assuming your app didn't need those
permissions, of course).

> > It also needs access to files for system_app, and radio. Would still
> > need mlstrustedsubject then?
> 
> What kind of access?  Read or write?
> Also, you probably ought to use your own type (e.g.
> type=mydiag_app_data_file) for your data files rather than just leaving
> them in app_data_file, since that is accessible to untrusted apps
> (modulo DAC).
> 
> 
> Read access.

No, you shouldn't need mlstrustedsubject just for read access.

> If I use my own type, do I have to declare this somewhere
> else? Do I have to grant permission to this file type to other system
> domains, like vold, etc? In general, how do I create a new type? Sorry,
> I'm really new at this.

You need to declare the type, but you can declare it in your .te file.
The system ones are in file.te, e.g. see system_app_data_file there.
But they can go in any .te file.  You'll also need to allow installd and
system_server to have the appropriate permissions to your new type;
again, see the existing rules for system_app_data_file, but you can put
your new ones in your own .te file.  Note that untrusted_app is allowed
{ read write getattr } to system_app_data_file but not open, so it can
use open files passed to it over binder or local socket IPC but cannot
directly open them.  You likely want the same for your new type.

> Also, what you're saying seems to imply that if I run as untrusted_app,
> I can actually access /proc//stat of any untrusted app? This also
> means any 3rd party app can read /proc//stat of another.

On a conventional Linux system, /proc/pid/stat is world-readable, so
apps used to be able to access any /proc/pid/stat at all.  When SELinux
went enforcing for all apps in Android (5.0), we could at least limit
them to only being able to do it for other third party apps, and when
levelFrom=user was enabled (6.0), we could limit them to doing it for
other third party apps running for the same user.  Then, in some version
(don't remember which one), they also starting mounting /proc with
hidepid=2, so apps cannot see other's /proc/pid at all (unless they have
AID_READPROC in their group set).  Hopefully they'll eventually turn on
levelFrom=all at some point too, at which point SELinux will further
isolate all apps with unique category sets (not just per-user).

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: Permission denied error when opening /proc//stat file

[Stephen Smalley]

On 12/02/2016 01:31 PM, Helen Chiang wrote:
> On Fri, Dec 2, 2016 at 10:09 AM, Stephen Smalley <s...@tycho.nsa.gov
> <mailto:s...@tycho.nsa.gov>> wrote:
> 
> On 12/02/2016 12:28 PM, Helen Chiang wrote:
> > I have a similar situation. We're customizing AOSP (Marshmallow and
> > Nougat) and developing a device diagnostic service that runs as "system"
> > user and installed as a privileged app. The use cases of this app is to
> > read /proc//stat periodically for overall OS process information.
> 
> Generally I would expect such functionality to go into the
> system_server, as part of the AMS, or another system service.  An app
> could certainly present the UI for viewing that information, but
> wouldn't directly need to access the /proc/pid files.
> 
> 
> I can't find any APIs from AMS that will return CPU elapsed time.
> SystemHealthManager is only on Nougat and the stats it returns is
> dependent on battery status so it wouldn't satisfy our use case.
> Ultimately, we're planning to create a new system service that will
> provide the info but for our immediate needs, we have to customize the
> policy files.

Yes, I would extend AMS I think.

> 
> 
> > In principal, this app should be able to access everything that a
> > system_app would, in addition to /proc//stat. It also creates and
> > access shared memory and unix sockets. Your previous answer implied that
> > it would be insecure to make "system_app" a mlstrustedsubject. So, I
> > want to create a new domain that has all the access as system_app and
> > priv_app + mlstrustedsubject. Is there a way to do this without copying
> > and pasting the rules directly from system_app and priv_app te files?
> > Also, what allow rules are needed for shared memory and  socket r/w ?
> 
> No, you'd have to copy and modify the rules at present (if this became
> common, then one would perhaps move the rules to a macro in te_macros or
> elsewhere, and then call the macro from both files).  However, you have
> to be careful because there may be neverallow rules on system_app or
> priv_app that will prevent compiling your policy if you copy certain
> rules to your new domain that are supposed to be restricted to only
> system_app or priv_app, and CTS also checks those neverallows.
> 
> 
> I did run into and removed the offending allow rules. Are you saying
> that if everything compiles with AOSP rules, CTS won't be an issue or
> does CTS have additional checks?

You removed the offending allow rules from your domain, or the
neverallow rules that triggered the failure?  CTS has a collection of
SELinux tests, including checking all neverallows from the AOSP policy
for that version of Android.  So you can't just remove neverallows from
your policy to escape them.  But if you removed the offending allow
rules and your app still works, that is ok.  seandroid.bitbucket.org has
a list of CTS tests related to SELinux, although it may not be entirely
up-to-date.

> > 2. seapp_contexts
> >
> > user=system seinfo=mydiag domain=trusted_diag type=app_data_file
> > levelFrom=user
> 
> Why levelFrom=user?  Does your app run per-user?  If so, then it would
> already run with the same category set as the user's apps and therefore
> wouldn't need mlstrustedsubject.
> 
> 
> It also needs access to files for system_app, and radio. Would still
> need mlstrustedsubject then?

What kind of access?  Read or write?
Also, you probably ought to use your own type (e.g.
type=mydiag_app_data_file) for your data files rather than just leaving
them in app_data_file, since that is accessible to untrusted apps
(modulo DAC).
___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: Permission denied error when opening /proc//stat file

[Stephen Smalley]

On 12/02/2016 12:28 PM, Helen Chiang wrote:
> I have a similar situation. We're customizing AOSP (Marshmallow and
> Nougat) and developing a device diagnostic service that runs as "system"
> user and installed as a privileged app. The use cases of this app is to
> read /proc//stat periodically for overall OS process information.

Generally I would expect such functionality to go into the
system_server, as part of the AMS, or another system service.  An app
could certainly present the UI for viewing that information, but
wouldn't directly need to access the /proc/pid files.

> In principal, this app should be able to access everything that a
> system_app would, in addition to /proc//stat. It also creates and
> access shared memory and unix sockets. Your previous answer implied that
> it would be insecure to make "system_app" a mlstrustedsubject. So, I
> want to create a new domain that has all the access as system_app and
> priv_app + mlstrustedsubject. Is there a way to do this without copying
> and pasting the rules directly from system_app and priv_app te files?
> Also, what allow rules are needed for shared memory and  socket r/w ?

No, you'd have to copy and modify the rules at present (if this became
common, then one would perhaps move the rules to a macro in te_macros or
elsewhere, and then call the macro from both files).  However, you have
to be careful because there may be neverallow rules on system_app or
priv_app that will prevent compiling your policy if you copy certain
rules to your new domain that are supposed to be restricted to only
system_app or priv_app, and CTS also checks those neverallows.

If by shared memory you mean System V shared memory (i.e. shmget(2)),
that is not supported in Android and prohibited in policy for all
domains.  If you just mean tmpfs/ashmem access, then that will get
picked up automatically via app_domain().  Creation and use of Unix
sockets is allowed to all domains in domain.te; you just need to
authorize the appropriate connections by labeling the Unix socket file
and adding unix_socket_connect() calls as appropriate.  Plenty of
examples in other .te files.

> For DAC, I'm able to add AID_READPROC group to the app via a new
> permission in platform.xml.

Not sure that is recommended; again, you wouldn't need to do so if you
put the logic in system_server/AMS or another system service rather than
in an app.

> For access to /proc//stat, I created a new domain as follows:
> 
> 1. mac_permissions.xml
> 
> 
> 
> 
> 
> 
> 
> 2. seapp_contexts
> 
> user=system seinfo=mydiag domain=trusted_diag type=app_data_file
> levelFrom=user

Why levelFrom=user?  Does your app run per-user?  If so, then it would
already run with the same category set as the user's apps and therefore
wouldn't need mlstrustedsubject.

> 3. trusted_diag.te
> 
> type trusted_diag, domain, domain_deprecated;
> 
> # Include all appdomain rules
> app_domain(trusted_diag)
> # Access the network.
> net_domain(trusted_diag)
> # Access bluetooth.
> bluetooth_domain(trusted_diag)
> 
> # Access /proc//stat files for metrics
> typeattribute trusted_diag mlstrustedsubject;
> r_dir_file(trusted_diag, system_app)
> r_dir_file(trusted_diag, platform_app)
> r_dir_file(trusted_diag, priv_app)
> r_dir_file(trusted_diag, untrusted_app)
> r_dir_file(trusted_diag, radio)
> 
> [+ COPY LINES FROM system_app.te]

Only copy the lines you need, and make sure your policy compiles with
all AOSP neverallows present, or you'll fail CTS.

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: model SEAndroid capability

[Stephen Smalley]

On 10/20/2016 08:02 AM, peng fei wrote:
> I want to model the security protection ability of  SEAndroid.
> I suppose to establish the model by extracting subjects which perform
> action on the same object.
> OR extracting object which can be performed action by the same subject.
> For example, /data/anr(/.*)?u:object_r:anr_data_file:s0
> 
> allow system_server anr_data_file:dir create_dir_perms;
> allow shell anr_data_file:dir r_dir_perms;
> allow dumpstate anr_data_file:dir { rw_dir_perms relabelto };
> toward the anr_data_file:, the subject perform action on it is shell,
> system_server, dumpstate and so on.
> 
> Is it a good method to model  SEAndroid  security capability?
> If I use the method to extract the subject and object, I also confuse
> how to analyse the extracting result.
> Please give me some suggesstion about how to model the security
> capability, and If I use the method above, how to analyse  the
> extracting result.
> Thanks advance.

That's already done for you in the SELinux policy; domain types are
subjects and all types can be objects; each type is a security
equivalence class (i.e. all processes with the same domain type have the
same permissions to the same objects; all objects with the same type can
be accessed by the same subjects in the same way).

setools already provides SELinux policy analysis tools, and the Android
tree includes prebuilt versions of sesearch and friends that will work
with that Android version's policy format.

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: can't load policy

[Stephen Smalley]

On 10/20/2016 05:06 AM, peng fei wrote:
> I have cm12.1 for Nexus 7(grouper) in my computer.
> 
> I modify file.te and file_context in external/sepolicy, add a new type
> sec_file.
> #/data/audit
> type sec_file, file_type, data_file_type;
> /data/audit(/.*)?   u:object_r:sec_file:s0
> 
> Then, I build the system again, and the new sepolicy and file_contexts
> is in out/target/product/grouper/root.
> [pengfei@pengfei system]$ cd out/target/product/grouper/root/
> [pengfei@pengfei root]$ adb push file_contexts /data/security/current
> [pengfei@pengfei root]$ adb push sepolicy/data/security/current
> root@grouper: # cp -f /selinux_version /data/security/current
> root@grouper:/data/security/current # setprop sys.init_log_level 6  
> root@grouper:/data/security/current # setprop selinux.reload_policy 1
> root@grouper:/data/security/current # dmesg | grep 'policy'
>
> <3>[84141.809238] init: sys_prop: permission denied uid:0
>  name:selinux.reload_policy
> <3>[84189.547889] init: sys_prop: permission denied uid:0
>  name:selinux.reload_policy
> <3>[84341.150198] init: sys_prop: permission denied uid:0
>  name:selinux.reload_policy
> <3>[84371.244797] init: sys_prop: permission denied uid:0
>  name:selinux.reload_policy
> <3>[84402.695624] init: sys_prop: permission denied uid:0
>  name:selinux.reload_policy

What else did you get in dmesg, e.g. avc: denied messages?

Why do you need to push your policy files to /data/security/current if
you modified them in your source tree and rebuilt the images?  Why can't
you just flash the rebuilt images and be done with it?


___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH 8/8] libselinux: add booleans.c to ANDROID_HOST=y recipe

[Stephen Smalley]

On 10/17/2016 04:24 PM, william.c.robe...@intel.com wrote:
> From: William Roberts 
> 
> We build booleans.c with DISABLE_BOOL set on Android host
> and target. Add that file to the upstream Makefile.
> 
> Signed-off-by: William Roberts 

Thanks, applied the series.

> ---
>  libselinux/src/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
> index 7a1ae05..ccd8442 100644
> --- a/libselinux/src/Makefile
> +++ b/libselinux/src/Makefile
> @@ -100,7 +100,7 @@ DISABLE_FLAGS+= -DNO_MEDIA_BACKEND -DNO_DB_BACKEND 
> -DNO_X_BACKEND \
>   -DBUILD_HOST
>  SRCS= callbacks.c freecon.c label.c label_file.c \
>   label_backends_android.c regex.c label_support.c \
> - matchpathcon.c setrans_client.c sha1.c
> + matchpathcon.c setrans_client.c sha1.c booleans.c
>  else
>  DISABLE_FLAGS+= -DNO_ANDROID_BACKEND
>  SRCS:= $(filter-out label_backends_android.c, $(SRCS))
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: Extending file_contexts

[Stephen Smalley]

On 10/18/2016 01:01 PM, Sava Mikalački wrote:
> And if I label it in init.rc (I have my custom one), would I need to
> call restorecon() anyways?

No, if you add a mkdir /data/system/ifw to your init.rc post-fs-data
section, then init will create it with whatever label you specify in
file_contexts, and then it should be fine without needing to modify
IntentFirewall code (assuming nothing ever deletes the directory once
created).

Conceivably you could also achieve the same effect through a name-based
type transition rule in policy, but probably not worth it:
type_transition system_server system_data_file:dir system_data_ifw "ifw";

> 
> 
> On Oct 18, 2016 18:41, "Stephen Smalley" <s...@tycho.nsa.gov
> <mailto:s...@tycho.nsa.gov>> wrote:
> 
> On 10/18/2016 12:26 PM, Stephen Smalley wrote:
> > On 10/18/2016 11:43 AM, Sava Mikalački wrote:
> >> Yup, exactly as Stephen said. When I set my app to share the
> system uid,
> >> I do get the following denial:
> >> type=1400 audit(0.0:15): avc: denied { write } for name="ifw"
> dev="dm-0"
> >> ino=678613 scontext=u:r:system_app:s0
> >> tcontext=u:object_r:system_data_file:s0 tclass=dir permissive=0
> >>
> >> Here is the output of the commands Stephen pointed out:
> >> $ ls -lZd /data/system/ifw
> >> drwx-- 2 system system u:object_r:system_data_file:s0 4096
> >> 1971-01-02 12:23 /data/system/ifw
> >>
> >> $ ps -eZ | grep com.ariel.guardian
> >> system4017  503   1588756 68980 SyS_epoll_ 768b37aa74 S
> >> com.ariel.guardian
> >>
> >> So, if I create a new file type label and assign allow rule to the
> >> system_app for this file type, would that (at least in theory) work?
> >
> > You'll need to allow access to the directory as well (your earlier
> > policy only had an allow rule for class file; you'll need rw_dir_perms
> > to the :dir as well).
> >
> > The other issue is getting /data/system/ifw labeled correctly. 
> Doesn't
> > look like it is created by the init.rc file, so it won't automatically
> > be labeled based on file_contexts (init does that for anything it
> > creates).  Probably created by the IntentFirewall code, and may
> need you
> > to call SELinux.restorecon() on it after creating it.  You'll see some
> > examples in other code, e.g.
> >
> 
> frameworks/base/services/core/java/com/android/server/pm/PackageInstallerService.java
> > or wallpaper/WallpaperManagerService.java.
> 
> Yes, looks like you would need to add a SELinux.restorecon() call right
> after the rulesDir.mkdirs() call in IntentFirewall().
> 
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: Extending file_contexts

[Stephen Smalley]

On 10/18/2016 12:26 PM, Stephen Smalley wrote:
> On 10/18/2016 11:43 AM, Sava Mikalački wrote:
>> Yup, exactly as Stephen said. When I set my app to share the system uid,
>> I do get the following denial:
>> type=1400 audit(0.0:15): avc: denied { write } for name="ifw" dev="dm-0"
>> ino=678613 scontext=u:r:system_app:s0
>> tcontext=u:object_r:system_data_file:s0 tclass=dir permissive=0
>>
>> Here is the output of the commands Stephen pointed out:
>> $ ls -lZd /data/system/ifw
>> drwx-- 2 system system u:object_r:system_data_file:s0 4096
>> 1971-01-02 12:23 /data/system/ifw
>>
>> $ ps -eZ | grep com.ariel.guardian
>> system4017  503   1588756 68980 SyS_epoll_ 768b37aa74 S
>> com.ariel.guardian
>>
>> So, if I create a new file type label and assign allow rule to the
>> system_app for this file type, would that (at least in theory) work?
> 
> You'll need to allow access to the directory as well (your earlier
> policy only had an allow rule for class file; you'll need rw_dir_perms
> to the :dir as well).
> 
> The other issue is getting /data/system/ifw labeled correctly.  Doesn't
> look like it is created by the init.rc file, so it won't automatically
> be labeled based on file_contexts (init does that for anything it
> creates).  Probably created by the IntentFirewall code, and may need you
> to call SELinux.restorecon() on it after creating it.  You'll see some
> examples in other code, e.g.
> frameworks/base/services/core/java/com/android/server/pm/PackageInstallerService.java
> or wallpaper/WallpaperManagerService.java.

Yes, looks like you would need to add a SELinux.restorecon() call right
after the rulesDir.mkdirs() call in IntentFirewall().


___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: Extending file_contexts

[Stephen Smalley]

On 10/18/2016 11:43 AM, Sava Mikalački wrote:
> Yup, exactly as Stephen said. When I set my app to share the system uid,
> I do get the following denial:
> type=1400 audit(0.0:15): avc: denied { write } for name="ifw" dev="dm-0"
> ino=678613 scontext=u:r:system_app:s0
> tcontext=u:object_r:system_data_file:s0 tclass=dir permissive=0
> 
> Here is the output of the commands Stephen pointed out:
> $ ls -lZd /data/system/ifw
> drwx-- 2 system system u:object_r:system_data_file:s0 4096
> 1971-01-02 12:23 /data/system/ifw
> 
> $ ps -eZ | grep com.ariel.guardian
> system4017  503   1588756 68980 SyS_epoll_ 768b37aa74 S
> com.ariel.guardian
> 
> So, if I create a new file type label and assign allow rule to the
> system_app for this file type, would that (at least in theory) work?

You'll need to allow access to the directory as well (your earlier
policy only had an allow rule for class file; you'll need rw_dir_perms
to the :dir as well).

The other issue is getting /data/system/ifw labeled correctly.  Doesn't
look like it is created by the init.rc file, so it won't automatically
be labeled based on file_contexts (init does that for anything it
creates).  Probably created by the IntentFirewall code, and may need you
to call SELinux.restorecon() on it after creating it.  You'll see some
examples in other code, e.g.
frameworks/base/services/core/java/com/android/server/pm/PackageInstallerService.java
or wallpaper/WallpaperManagerService.java.




___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: Extending file_contexts

[Stephen Smalley]

On 10/18/2016 10:56 AM, Stephen Smalley wrote:
> On 10/18/2016 10:49 AM, Sava Mikalački wrote:
>> I'm not sure how to answer the ownership question. I'm trying to allow
>> my application to write files in data/system/ifw which would be picked
>> up by the IntentFilter and then block certain application components
>> from executing. I have existing code that does that and it worked on
>> Marshmallow but its not working on Nougat because of that permission
>> denied exception when creating a file in data/system/ifw folder. Does
>> that help out in your question?
> 
> On a device running 7.0, ls -ld /data/system/ifw shows that it is owned
> by the system UID and is only writable by owner.  So your app has to run
> with the system UID (and thus would be system_app) in order to write
> there.  I don't really think that's new to 7.0 though.

What is new to 7.0 is that system_app is no longer allowed to
create/write to system_data_file, which is the default type on
/data/system/ifw.  So SELinux would deny those attempts (but you should
get avc messages in logcat / dmesg).

ls -lZd /data/system/ifw and ps -eZ | grep  might be
interesting.

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: Extending file_contexts

[Stephen Smalley]

On 10/18/2016 10:49 AM, Sava Mikalački wrote:
> I'm not sure how to answer the ownership question. I'm trying to allow
> my application to write files in data/system/ifw which would be picked
> up by the IntentFilter and then block certain application components
> from executing. I have existing code that does that and it worked on
> Marshmallow but its not working on Nougat because of that permission
> denied exception when creating a file in data/system/ifw folder. Does
> that help out in your question?

On a device running 7.0, ls -ld /data/system/ifw shows that it is owned
by the system UID and is only writable by owner.  So your app has to run
with the system UID (and thus would be system_app) in order to write
there.  I don't really think that's new to 7.0 though.

> 
> 2016-10-18 16:47 GMT+02:00 Stephen Smalley <s...@tycho.nsa.gov
> <mailto:s...@tycho.nsa.gov>>:
> 
> On 10/18/2016 10:41 AM, Sava Mikalački wrote:
> > Thanks everyone for your quick answers. Yes, compilation worked once I
> > defined the type in file.te. I will try this out and also will try with
> > system_app, probably thats simpler as you said. Whats confusing me is
> > that I get Permission denied exception when I try to create a file in
> > that directory with a system app but there is no selinux avc denial
> > before the exception, it just fires the exception and thats it, so I'm
> > afraid if changing the sepolicy would even work.
> 
>     What's the ownership and mode of the directory?
> 
> >
> > 2016-10-18 16:35 GMT+02:00 Stephen Smalley <s...@tycho.nsa.gov 
> <mailto:s...@tycho.nsa.gov>
> > <mailto:s...@tycho.nsa.gov <mailto:s...@tycho.nsa.gov>>>:
> >
> > On 10/18/2016 10:23 AM, William Roberts wrote:
> > > On Oct 18, 2016 9:34 AM, "Sava Mikalački" <mikalac...@gmail.com 
> <mailto:mikalac...@gmail.com>
> <mailto:mikalac...@gmail.com <mailto:mikalac...@gmail.com>>
> > > <mailto:mikalac...@gmail.com <mailto:mikalac...@gmail.com>
> <mailto:mikalac...@gmail.com <mailto:mikalac...@gmail.com>>>> wrote:
> > >>
> > >> I'm trying to extend aosp file_contexts by adding a new
> entry for
> > > /data/system/ifw. I've created a file_contexts under my
> vendor directory
> > > structure but if I try to use the new label, build crashes
> with unknown
> > > type. I'm
> > >
> > > You need to declare the type with the type keyword:
> > >
> > > type system_data_ifw, file_type;
> > >
> > > trying to enable a platform_app to write to data/system/ifw
> and here is
> > > what I have so far:
> > >> file_contexts:
> > >> /data/system/ifw(/.*)? 
>  u:object_r:system_data_ifw:s0
> > >> platform_app.te:
> > >> allow platform_app system_data_ifw:file create_file_perms;
> > >
> > > Platform applications shouldn't be creating stuff around the
> system,
> > > they should stick to their sandbox. I cant recall offhand,
> but a never
> > > allow I wrote might assert itself on that allow rule.
> >
> > Probably not since it is a new type he just defined.  However,
> it occurs
> > to me that DAC will be a problem for this use case, since
> platform apps
> > can be assigned arbitrary UIDs and thus won't be able to pass
> the DAC
> > checks on writing to /data/system/ifw unless you set up a
> group, map a
> > permission to that group, assign that group to
> /data/system/ifw, and
> > make it group-writable.  Simpler if you use a system_app or
> some other
> > fixed UID app instead, although that carries its own set of
> issues.
> >
> >
> >
> >
> >
> > --
> > I have only two questions: How much and give it to me.
> 
> 
> 
> 
> -- 
> I have only two questions: How much and give it to me.

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: Extending file_contexts

[Stephen Smalley]

On 10/18/2016 10:23 AM, William Roberts wrote:
> On Oct 18, 2016 9:34 AM, "Sava Mikalački"  > wrote:
>>
>> I'm trying to extend aosp file_contexts by adding a new entry for
> /data/system/ifw. I've created a file_contexts under my vendor directory
> structure but if I try to use the new label, build crashes with unknown
> type. I'm
> 
> You need to declare the type with the type keyword:
> 
> type system_data_ifw, file_type;

Just to be clear, you also want at least the data_file_type attribute
here (for all types on files under /data) and possibly the
mlstrustedobject attribute (if it needs to be writable by any app using
levelFrom= in seapp_contexts).  The latter is not necessary for system_app.

> 
> trying to enable a platform_app to write to data/system/ifw and here is
> what I have so far:
>> file_contexts:
>> /data/system/ifw(/.*)?   u:object_r:system_data_ifw:s0
>> platform_app.te:
>> allow platform_app system_data_ifw:file create_file_perms;
> 
> Platform applications shouldn't be creating stuff around the system,
> they should stick to their sandbox. I cant recall offhand, but a never
> allow I wrote might assert itself on that allow rule.
> 
>>
>> I also tried adding:
>> /data/system/ifw(/.*)?   u:object_r:system_data_ifw:s0
>> to my device specific sepolicy but it still doesnt get picked up.
>>
>> Am I taking the right approach? 
> 
> You extend policy in your own specific location set by
> BOARD_SEPOlICY_DIRS = path/to/directory
> 
> Then just add files to that directory. No need to ever touch
> system/sepolicy or on older versions of Android external/sepolicy.
> 
>>
>> --
>> I have only two questions: How much and give it to me.
>>
>> ___
>> Seandroid-list mailing list
>> Seandroid-list@tycho.nsa.gov 
>> To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov
> .
>> To get help, send an email containing "help" to
> seandroid-list-requ...@tycho.nsa.gov
> .
> 
> 
> 
> ___
> Seandroid-list mailing list
> Seandroid-list@tycho.nsa.gov
> To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
> To get help, send an email containing "help" to 
> seandroid-list-requ...@tycho.nsa.gov.
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: Extending file_contexts

[Stephen Smalley]

On 10/18/2016 10:23 AM, William Roberts wrote:
> On Oct 18, 2016 9:34 AM, "Sava Mikalački"  > wrote:
>>
>> I'm trying to extend aosp file_contexts by adding a new entry for
> /data/system/ifw. I've created a file_contexts under my vendor directory
> structure but if I try to use the new label, build crashes with unknown
> type. I'm
> 
> You need to declare the type with the type keyword:
> 
> type system_data_ifw, file_type;
> 
> trying to enable a platform_app to write to data/system/ifw and here is
> what I have so far:
>> file_contexts:
>> /data/system/ifw(/.*)?   u:object_r:system_data_ifw:s0
>> platform_app.te:
>> allow platform_app system_data_ifw:file create_file_perms;
> 
> Platform applications shouldn't be creating stuff around the system,
> they should stick to their sandbox. I cant recall offhand, but a never
> allow I wrote might assert itself on that allow rule.

Probably not since it is a new type he just defined.  However, it occurs
to me that DAC will be a problem for this use case, since platform apps
can be assigned arbitrary UIDs and thus won't be able to pass the DAC
checks on writing to /data/system/ifw unless you set up a group, map a
permission to that group, assign that group to /data/system/ifw, and
make it group-writable.  Simpler if you use a system_app or some other
fixed UID app instead, although that carries its own set of issues.


___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: Confidentiality and privacy

[Stephen Smalley]

On 10/13/2016 04:53 PM, Eduardo Aguirre wrote:
> Thank you so much for all your help!
> 
> Any recommended documentation about SE for Android, LSMs implemented in
> Android and maybe an in-depth view of Android security?
> I have already read the official documentation and the "Android security
> internals book" but I was wondering if there is another good source of
> information.

The list of links from
https://source.android.com/security/selinux/#supporting_documentation
is a good starting point.

> 
> El jue., 13 oct. 2016 a las 11:25, Stephen Smalley (<s...@tycho.nsa.gov
> <mailto:s...@tycho.nsa.gov>>) escribió:
> 
> On 10/13/2016 11:20 AM, Eduardo Aguirre wrote:
> > Do you know why the MMAC mechanisms proposed in SEAndroid weren't
> > adopted?  I have also heard of something called "Intent firewall" that
> > has not been integrated to Android(as far as I know).
> 
> Not entirely sure why (we didn't get feedback), but recent versions of
> Android do incorporate a runtime permissions model (built on top of
> AppOps) and also include various enterprise-focused features.
> 
> Last I looked, Intent Firewall was still part of Android, but not
> something that can be configured by anyone other than the OEM (aside
> from using custom ROMs).  Some information about Intent Firewall is
> available here:
>     http://www.cis.syr.edu/~wedu/android/IntentFirewall/
> 
> > El jue., 13 oct. 2016 a las 10:00, Stephen Smalley
> (<s...@tycho.nsa.gov <mailto:s...@tycho.nsa.gov>
> > <mailto:s...@tycho.nsa.gov <mailto:s...@tycho.nsa.gov>>>) escribió:
> >
> > On 10/13/2016 10:33 AM, Eduardo Aguirre wrote:
> > > Could a policy in SEAndroid ensure confidentality and privacy?:
> > >
> > > Restrict emails to some domains, restrict messages from some
> contacts,
> > > or even modify some rules when location changes?
> > >
> > > I think nothing like this has been implemented, but I also
> think that
> > > SEAndroid could be used to do something like that (maybe some
> > > modifications are needed?)
> >
> > The concepts you are describing would be implemented at the
> middleware
> > or, in some cases, even the application layer.  While the SE
> for Android
> > project did experiment with several middleware mandatory
> access control
> > mechanisms (MMAC), none of those were ever adopted into the
> Android Open
> > Source Project; only the SELinux support was.
> >
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: Confidentiality and privacy

[Stephen Smalley]

On 10/13/2016 11:20 AM, Eduardo Aguirre wrote:
> Do you know why the MMAC mechanisms proposed in SEAndroid weren't
> adopted?  I have also heard of something called "Intent firewall" that
> has not been integrated to Android(as far as I know).

Not entirely sure why (we didn't get feedback), but recent versions of
Android do incorporate a runtime permissions model (built on top of
AppOps) and also include various enterprise-focused features.

Last I looked, Intent Firewall was still part of Android, but not
something that can be configured by anyone other than the OEM (aside
from using custom ROMs).  Some information about Intent Firewall is
available here:
http://www.cis.syr.edu/~wedu/android/IntentFirewall/

> El jue., 13 oct. 2016 a las 10:00, Stephen Smalley (<s...@tycho.nsa.gov
> <mailto:s...@tycho.nsa.gov>>) escribió:
> 
> On 10/13/2016 10:33 AM, Eduardo Aguirre wrote:
> > Could a policy in SEAndroid ensure confidentality and privacy?:
> >
> > Restrict emails to some domains, restrict messages from some contacts,
> > or even modify some rules when location changes?
> >
> > I think nothing like this has been implemented, but I also think that
> > SEAndroid could be used to do something like that (maybe some
> > modifications are needed?)
> 
> The concepts you are describing would be implemented at the middleware
> or, in some cases, even the application layer.  While the SE for Android
> project did experiment with several middleware mandatory access control
> mechanisms (MMAC), none of those were ever adopted into the Android Open
> Source Project; only the SELinux support was.
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: Confidentiality and privacy

[Stephen Smalley]

On 10/13/2016 10:33 AM, Eduardo Aguirre wrote:
> Could a policy in SEAndroid ensure confidentality and privacy?:
> 
> Restrict emails to some domains, restrict messages from some contacts,
> or even modify some rules when location changes?
> 
> I think nothing like this has been implemented, but I also think that
> SEAndroid could be used to do something like that (maybe some
> modifications are needed?)

The concepts you are describing would be implemented at the middleware
or, in some cases, even the application layer.  While the SE for Android
project did experiment with several middleware mandatory access control
mechanisms (MMAC), none of those were ever adopted into the Android Open
Source Project; only the SELinux support was.
___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: labelling /sys/kernel/debug aka debugfs

[Stephen Smalley]

On 10/12/2016 11:51 AM, Roberts, William C wrote:
> If Bin is using our N tree, then all the stuff for debugfs are exact
> matches:
> 
>  
> 
> /sys/kernel/debug/sync  u:object_r:debugfs_graphics_sync:s0
> 
> /sys/kernel/debug/dri/0/i915_frequency_info u:object_r:debugfs_graphics:s0
> 
> /sys/kernel/debug/pstate_snb/setpoint u:object_r:debugfs_pstate:s0
> 
>  
> 
> Bin, can you confirm the state of your tree? You can run this command to
> find the entries:
> 
>  
> 
> find . -name file_contexts | xargs grep 'kernel/debug'

It could be any /sys entry that breaks the partial match pruning logic
(by enabling a possible match of any subdir), not just ones under
/sys/kernel/debug.

> 
>  
> 
> Thanks,
> 
> Bill
> 
>  
> 
>  
> 
> *From:*Nick Kralevich [mailto:n...@google.com]
> *Sent:* Wednesday, October 12, 2016 11:42 AM
> *To:* Roberts, William C <william.c.robe...@intel.com>
> *Cc:* Stephen Smalley <s...@tycho.nsa.gov>; seandroid-list@tycho.nsa.gov;
> Yang, Bin Y <bin.y.y...@intel.com>
> *Subject:* Re: labelling /sys/kernel/debug aka debugfs
> 
>  
> 
> Can you attach the file_contexts file you're using? Or at least all the
> entries starting with /sys? In the past, when I've seen this kind of
> slow restorecon on boot, it's been due to a regex which defeats the
> restorecon directory tree walking optimizations.
> 
>  
> 
> -- Nick
> 
>  
> 
> On Wed, Oct 12, 2016 at 6:42 AM, Roberts, William C
> <william.c.robe...@intel.com <mailto:william.c.robe...@intel.com>> wrote:
> 
> 
> 
> > -Original Message-
> > From: Stephen Smalley [mailto:s...@tycho.nsa.gov
> <mailto:s...@tycho.nsa.gov>]
> > Sent: Wednesday, October 12, 2016 9:37 AM
> > To: Roberts, William C <william.c.robe...@intel.com
> <mailto:william.c.robe...@intel.com>>; 'seandroid-
> > l...@tycho.nsa.gov <mailto:l...@tycho.nsa.gov>'
> <seandroid-list@tycho.nsa.gov <mailto:seandroid-list@tycho.nsa.gov>>
> > Cc: Yang, Bin Y <bin.y.y...@intel.com <mailto:bin.y.y...@intel.com>>
> > Subject: Re: labelling /sys/kernel/debug aka debugfs
> >
> > On 10/12/2016 09:24 AM, Roberts, William C wrote:
> > > It’s been reported that labelling via restorecon_recursive
> > > /sys/kernel/debug is taking 0.25s on a device. I wanted to verify a
> > > thought:
> > >
> > >
> > >
> > > It looks like genfscon per file labeling is supported by selinux
> (like
> > > procfs), on linux master branch, I see:
> > >
> > >
> > >
> > > selinux_set_mnt_opts():
> > >
> > > 
> > >
> > > 815 if (!strcmp(sb->s_type->name, "debugfs") ||
> > >
> > > 816 !strcmp(sb->s_type->name, "sysfs") ||
> > >
> > > 817 !strcmp(sb->s_type->name, "pstore"))
> > >
> > > 818 sbsec->flags |= SE_SBGENFS;
> > >
> > > 
> > >
> > >
> > >
> > > Would using genfscon statements and removing the
> restorecon_recursive
> > > be faster since it avoids the tree walk? Any caveats, issues one
> can think of?
> >
> > First, I'd be interested in understanding why that is taking so
> long, and compare
> > with time on restorecon_recursive /sys (performed directly by init).
> >
> > The SE for Android todo list does suggest investigating this for
> replacing the
> > restorecon_recursive /sys, so it would make sense to investigate
> it for both.  It
> > does require that the device kernel include the necessary support.
> As noted in
> > https://android-review.googlesource.com/#/c/151776/, you are also
> limited in
> > that genfscon only supports pathname prefix matching, not regexes.
> 
> I don't see any uses where the lack of regex support would be a problem.
> I'll see if Bin, the reporter, can collect more stats for us, Yang
> could you collect
> those?
> 
> 
> ___
> Seandroid-list mailing list
> Seandroid-list@tycho.nsa.gov <mailto:Seandroid-list@tycho.nsa.gov>
> To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov
> <mailto:seandroid-list-le...@tycho.nsa.gov>.
> To get help, send an email containing &q

Re: labelling /sys/kernel/debug aka debugfs

[Stephen Smalley]

On 10/12/2016 09:36 AM, Stephen Smalley wrote:
> On 10/12/2016 09:24 AM, Roberts, William C wrote:
>> It’s been reported that labelling via restorecon_recursive
>>  /sys/kernel/debug is taking 0.25s on a device. I wanted to verify a
>> thought:
>>
>>  
>>
>> It looks like genfscon per file labeling is supported by selinux (like
>> procfs), on linux master branch, I see:
>>
>>  
>>
>> selinux_set_mnt_opts():
>>
>> 
>>
>> 815 if (!strcmp(sb->s_type->name, "debugfs") ||
>>
>> 816 !strcmp(sb->s_type->name, "sysfs") ||
>>
>> 817 !strcmp(sb->s_type->name, "pstore"))
>>
>> 818 sbsec->flags |= SE_SBGENFS;
>>
>> 
>>
>>  
>>
>> Would using genfscon statements and removing the restorecon_recursive be
>> faster since it avoids the tree walk? Any caveats, issues one can think of?
> 
> First, I'd be interested in understanding why that is taking so long,
> and compare with time on restorecon_recursive /sys (performed directly
> by init).
> 
> The SE for Android todo list does suggest investigating this for
> replacing the restorecon_recursive /sys, so it would make sense to
> investigate it for both.  It does require that the device kernel include
> the necessary support. As noted in
> https://android-review.googlesource.com/#/c/151776/, you are also
> limited in that genfscon only supports pathname prefix matching, not
> regexes.

The corresponding change for debugfs was:
https://android-review.googlesource.com/#/q/I6460fbed6bb6bd36eb8554ac8c4fdd574edf3b07

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: labelling /sys/kernel/debug aka debugfs

[Stephen Smalley]

On 10/12/2016 09:24 AM, Roberts, William C wrote:
> It’s been reported that labelling via restorecon_recursive
>  /sys/kernel/debug is taking 0.25s on a device. I wanted to verify a
> thought:
> 
>  
> 
> It looks like genfscon per file labeling is supported by selinux (like
> procfs), on linux master branch, I see:
> 
>  
> 
> selinux_set_mnt_opts():
> 
> 
> 
> 815 if (!strcmp(sb->s_type->name, "debugfs") ||
> 
> 816 !strcmp(sb->s_type->name, "sysfs") ||
> 
> 817 !strcmp(sb->s_type->name, "pstore"))
> 
> 818 sbsec->flags |= SE_SBGENFS;
> 
> 
> 
>  
> 
> Would using genfscon statements and removing the restorecon_recursive be
> faster since it avoids the tree walk? Any caveats, issues one can think of?

First, I'd be interested in understanding why that is taking so long,
and compare with time on restorecon_recursive /sys (performed directly
by init).

The SE for Android todo list does suggest investigating this for
replacing the restorecon_recursive /sys, so it would make sense to
investigate it for both.  It does require that the device kernel include
the necessary support. As noted in
https://android-review.googlesource.com/#/c/151776/, you are also
limited in that genfscon only supports pathname prefix matching, not
regexes.
___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: can't reload sepolicy

[Stephen Smalley]

On 10/12/2016 05:57 AM, peng fei wrote:
> I want to modify sepolicy and verify it.
> 
> First,
> 
> I download the android4.4.4 sepolicy, and modify file.te and
> file_context, add a new type sec_file.
> #/data/audit
> type sec_file, file_type, data_file_type;
> /data/audit(/.*)?   u:object_r:sec_file:s0
> 
> --
> Second,compile policy.
> 
> m4 -D mls_num_sens=1 -D mls_num_cats=1024 -D target_build_variant=user  
>-s security_classes initial_sids access_vectors global_macros
>  mls_macros mls policy_capabilities te_macros attributes *.te
> roles users initial_sid_contexts fs_use genfs_contexts
> port_contexts > policy.conf
> [pengfei@pengfei seandroid-4.4.4-external-sepolicy]$ checkpolicy -M -c
> 26 -o sepolicy policy.conf
> checkpolicy:  loading policy configuration from policy.conf
> checkpolicy:  policy configuration loaded
> checkpolicy:  writing binary representation (version 26) to sepolicy
> [pengfei@pengfei seandroid-4.4.4-external-sepolicy]$ file
> sepolicysepolicy: SE Linux policy v26 MLS 8 symbols 7 ocons
> --
> Then, I connected with N958St, which is also android4.4.4 .
> I adb push sepolicy to and file_contexts to /data/security/current.
> 
> root@N958St:/data/security/current # setprop sys.init_log_level 6
> root@N958St:/data/security/current # setprop selinux.reload_policy 1
> root@N958St:/data/security/current # dmesg | grep 'SELinux'
> <7>[ 3802.717538] SELinux: 512 avtab hash slots, 1346 rules.
> <7>[ 3802.718476] SELinux: 512 avtab hash slots, 1346 rules.
> <7>[ 3802.718497] SELinux:  1 users, 2 roles, 293 types, 1 bools, 1
> sens, 1024 cats
> <7>[ 3802.718513] SELinux:  84 classes, 1346 rules
> <6>[ 3802.719963] SELinux:  Permission attach_queue in class tun_socket
> not defined in policy.
> <6>[ 3802.719978] SELinux: the above unknown classes and permissions
> will be denied
> <14>[ 3803.548149] SELinux: Loaded policy from /sepolicy
> <7>[ 4479.980176] SELinux: 512 avtab hash slots, 1346 rules.
> <7>[ 4479.981074] SELinux: 512 avtab hash slots, 1346 rules.
> <7>[ 4479.981095] SELinux:  1 users, 2 roles, 293 types, 1 bools, 1
> sens, 1024 cats
> <7>[ 4479.981107] SELinux:  84 classes, 1346 rules
> <6>[ 4479.982588] SELinux:  Permission attach_queue in class tun_socket
> not defined in policy.
> <6>[ 4479.982603] SELinux: the above unknown classes and permissions
> will be denied
> <14>[ 4480.187929] SELinux: Loaded policy from /sepolicy
> <7>[ 4503.340545] SELinux: 512 avtab hash slots, 1346 rules.
> <7>[ 4503.341450] SELinux: 512 avtab hash slots, 1346 rules.
> <7>[ 4503.341467] SELinux:  1 users, 2 roles, 293 types, 1 bools, 1
> sens, 1024 cats
> <7>[ 4503.341479] SELinux:  84 classes, 1346 rules
> <6>[ 4503.342907] SELinux:  Permission attach_queue in class tun_socket
> not defined in policy.
> <6>[ 4503.342921] SELinux: the above unknown classes and permissions
> will be denied
> <14>[ 4504.009018] SELinux: Loaded policy from /sepolicy
> <7>[ 4524.927888] SELinux: 512 avtab hash slots, 1346 rules.
> <7>[ 4524.928835] SELinux: 512 avtab hash slots, 1346 rules.
> <7>[ 4524.928854] SELinux:  1 users, 2 roles, 293 types, 1 bools, 1
> sens, 1024 cats
> <7>[ 4524.928865] SELinux:  84 classes, 1346 rules
> <6>[ 4524.930319] SELinux:  Permission attach_queue in class tun_socket
> not defined in policy.
> <6>[ 4524.930333] SELinux: the above unknown classes and permissions
> will be denied
> <14>[ 4525.218099] SELinux: Loaded policy from /sepolicy
> <7>[ 6609.537301] SELinux: 512 avtab hash slots, 1346 rules.
> <7>[ 6609.538209] SELinux: 512 avtab hash slots, 1346 rules.
> <7>[ 6609.538231] SELinux:  1 users, 2 roles, 293 types, 1 bools, 1
> sens, 1024 cats
> <7>[ 6609.538243] SELinux:  84 classes, 1346 rules
> <6>[ 6609.539703] SELinux:  Permission attach_queue in class tun_socket
> not defined in policy.
> <6>[ 6609.539717] SELinux: the above unknown classes and permissions
> will be denied
> <14>[ 6609.828011] SELinux: Loaded policy from /sepolicy
> 
> This is the result. 
> root@N958St:/data # ls -Z 
> drwx-- root root  u:object_r:system_data_file:s0 audit
> 
> ---
> But I want to get the 
> drwx-- root root  u:object_r:sec_file:s0 audit
> 
> ---please help me . Thanks advance.

Android 4.4 disabled reading of selinux policies from /data in commit
72888bfad80641537c8ab2dda2b22779284682d9 to external/libselinux
(included in Android 4.4.3 and 4.4.4).  IIRC, there were problems with
devices being bricked upon an OTA with an old, incompatible /data
policy.  That problem was resolved in Android 5.0 by adding a comparison
of a new /selinux_version file to /data/security/current/selinux_version
and ignoring the /data policy if they did not match, so Android 5.0 and
5.1 once again included the support for loading /data policies. Android
6.0 removed the permissions for loading /data policies in the default
SELinux policy due to concerns about misuse of it, but the code remained
and could be enabled through device-specific policy. 

Re: kernel access device while enabling CONFIG_DEVTMPFS

[Stephen Smalley]

On 10/06/2016 09:20 AM, Stephen Smalley wrote:
> On 10/06/2016 04:52 AM, Inamdar Sharif wrote:
>> Hi,
>>
>>  
>>
>> I am getting the following denial when I enable CONFIG_DEVTMPFS
>>
>> avc: denied { write } for pid=37 comm="kdevtmpfs" dev="devtmpfs" ino=122
>> scontext=u:r:kernel:s0 tcontext=u:object_r:device:s0 tclass=dir permissive=0
>>
>>  
>>
>> What would be the best solution to solve this??
> 
> See:
> https://www.mail-archive.com/seandroid-list@tycho.nsa.gov/msg02389.html
> 
> The best answer is to not enable CONFIG_DEVTMPFS.

Also see the rest of that thread, e.g.
https://www.mail-archive.com/seandroid-list@tycho.nsa.gov/msg02393.html

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: kernel access device while enabling CONFIG_DEVTMPFS

[Stephen Smalley]

On 10/06/2016 04:52 AM, Inamdar Sharif wrote:
> Hi,
> 
>  
> 
> I am getting the following denial when I enable CONFIG_DEVTMPFS
> 
> avc: denied { write } for pid=37 comm="kdevtmpfs" dev="devtmpfs" ino=122
> scontext=u:r:kernel:s0 tcontext=u:object_r:device:s0 tclass=dir permissive=0
> 
>  
> 
> What would be the best solution to solve this??

See:
https://www.mail-archive.com/seandroid-list@tycho.nsa.gov/msg02389.html

The best answer is to not enable CONFIG_DEVTMPFS.

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH] libselinux: re-introduce DISABLE_BOOL=y

[Stephen Smalley]

On 09/29/2016 03:27 PM, William Roberts wrote:
> On Thu, Sep 29, 2016 at 3:15 PM, William Roberts
> <bill.c.robe...@gmail.com> wrote:
>> On Thu, Sep 29, 2016 at 2:54 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
>>> On 09/29/2016 02:46 PM, William Roberts wrote:
>>>> On Thu, Sep 29, 2016 at 2:44 PM, Stephen Smalley <s...@tycho.nsa.gov> 
>>>> wrote:
>>>>> On 09/29/2016 02:15 PM, William Roberts wrote:
>>>>>> On Thu, Sep 29, 2016 at 2:08 PM, Stephen Smalley <s...@tycho.nsa.gov> 
>>>>>> wrote:
>>>>>>> On 09/29/2016 02:02 PM, william.c.robe...@intel.com wrote:
>>>>>>>> From: William Roberts <william.c.robe...@intel.com>
>>>>>>>>
>>>>>>>> Provide stubs to the public boolean API that always returns -1.
>>>>>>>>
>>>>>>>> On Android, boolean symbols are needed for:
>>>>>>>> external/ltrace/sysdeps/linux-gnu/trace.c
>>>>>>>
>>>>>>> Is this really worth doing?
>>>>>>
>>>>>> It's this or disabling that selinux via #define, which that source has
>>>>>> HAVE_LIBSELINUX.
>>>>>>
>>>>>> But it would seem confusing IMHO to have a libselinux.so, so one would
>>>>>> set HAVE_LIBSELINUX=1,
>>>>>> and you're getting link errors.
>>>>>
>>>>> Maybe I don't understand.  Obviously it builds today with
>>>>> external/libselinux without requiring this change.  Why do we need this 
>>>>> now?
>>>>>
>>>>
>>>> Richard Haines was doing further testing, and was building a different
>>>> lunch target for the
>>>> arm emulator and hit this issue. I have only tested x86_64 emulator.
>>>
>>> No, I mean that this is not required in external/libselinux (the Android
>>> fork) today.  So why is it needed here?  The Android fork builds
>>> src/booleans.c for the target.  It doesn't hurt anything to leave the
>>> code there.  The underlying kernel interface via selinuxfs still exists.
>>>  There just won't be any booleans in the policy.
>>>
>>
>> The target builds a modified booleans, if use booleans as is, we start
>> down the config c file
>> rabbit hole...
>>
>> external/selinux/libselinux/src/booleans.c:100: error: undefined
>> reference to 'selinux_booleans_subs_path'
>> external/selinux/libselinux/src/booleans.c:388: error: undefined
>> reference to 'selinux_booleans_path'
>> external/selinux/libselinux/src/booleans.c:529: error: undefined
>> reference to 'selinux_booleans_path'
>> external/selinux/libselinux/src/booleans.c:545: error: undefined
>> reference to 'selinux_booleans_path'
>> clang++.real: error: linker command failed with exit code 1 (use -v to
>> see invocation)
>>
>> I can take a look at that and see how much of a PITA it would be to
>> pull that in.
> 
> external/selinux/libselinux/src/selinux_config.c:100: error: undefined
> reference to 'fgets_unlocked'
> external/selinux/libselinux/src/selinux_config.c:100: error: undefined
> reference to 'fgets_unlocked'
> external/selinux/libselinux/src/selinux_config.c:231: error: undefined
> reference to 'require_seusers'
> external/selinux/libselinux/src/selinux_config.c:231: error: undefined
> reference to 'load_setlocaldefs'
> 
> fgets should be easy enough
> load_setlocaldefs is an exported integer value used in init_selinux_config()
> require_seusers is another exported int form seusers.c
> 
> I was figuring since we don't use any bools, to keep the size down,
> just stubbing dummies is the
> easiest route.
> 
> We could do something like STATIC_CONFIG and just stub in what things
> need and return the explicit paths.

Never mind, I'll take your original patch.


___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH 2/2] libselinux: set DISABLE_RPM default to y.

[Stephen Smalley]

On 09/28/2016 12:00 PM, william.c.robe...@intel.com wrote:
> From: William Roberts 
> 
> Change the default build behavior to always use DISABLE_RPM.
> To get the old behavior call make with DISABLE_RPM=n.
> 
> eg.)
> make DISABLE_RPM=n

I reverted this change.  It would break rpm on RHEL 7 and earlier, so it
is still too soon to make this the default ;(

> 
> Signed-off-by: William Roberts 
> ---
>  libselinux/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libselinux/Makefile b/libselinux/Makefile
> index 41d836c..cec2943 100644
> --- a/libselinux/Makefile
> +++ b/libselinux/Makefile
> @@ -1,7 +1,7 @@
>  SUBDIRS = src include utils man
>  
>  DISABLE_SETRANS ?= n
> -DISABLE_RPM ?= n
> +DISABLE_RPM ?= y
>  ifeq ($(DISABLE_RPM),y)
>   DISABLE_FLAGS+= -DDISABLE_RPM
>  endif
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH] libselinux: re-introduce DISABLE_BOOL=y

[Stephen Smalley]

On 09/29/2016 02:02 PM, william.c.robe...@intel.com wrote:
> From: William Roberts 
> 
> Provide stubs to the public boolean API that always returns -1.
> 
> On Android, boolean symbols are needed for:
> external/ltrace/sysdeps/linux-gnu/trace.c

Thanks, applied.

> 
> Signed-off-by: William Roberts 
> ---
>  libselinux/Makefile   |  4 +++
>  libselinux/src/booleans.c | 64 
> +++
>  2 files changed, 58 insertions(+), 10 deletions(-)
> 
> diff --git a/libselinux/Makefile b/libselinux/Makefile
> index f607115..b5f32bb 100644
> --- a/libselinux/Makefile
> +++ b/libselinux/Makefile
> @@ -5,6 +5,7 @@ DISABLE_RPM ?= y
>  ANDROID_HOST ?= n
>  ifeq ($(ANDROID_HOST),y)
>   override DISABLE_SETRANS=y
> + override DISABLE_BOOL=y
>  endif
>  ifeq ($(DISABLE_RPM),y)
>   DISABLE_FLAGS+= -DDISABLE_RPM
> @@ -12,6 +13,9 @@ endif
>  ifeq ($(DISABLE_SETRANS),y)
>   DISABLE_FLAGS+= -DDISABLE_SETRANS
>  endif
> +ifeq ($(DISABLE_BOOL),y)
> + DISABLE_FLAGS+= -DDISABLE_BOOL
> +endif
>  export DISABLE_SETRANS DISABLE_RPM DISABLE_FLAGS ANDROID_HOST
>  
>  USE_PCRE2 ?= n
> diff --git a/libselinux/src/booleans.c b/libselinux/src/booleans.c
> index c438af1..cbb0610 100644
> --- a/libselinux/src/booleans.c
> +++ b/libselinux/src/booleans.c
> @@ -25,6 +25,8 @@
>  
>  #define SELINUX_BOOL_DIR "/booleans/"
>  
> +#ifndef DISABLE_BOOL
> +
>  static int filename_select(const struct dirent *d)
>  {
>   if (d->d_name[0] == '.'
> @@ -85,8 +87,6 @@ int security_get_boolean_names(char ***names, int *len)
>   goto out;
>  }
>  
> -hidden_def(security_get_boolean_names)
> -
>  char *selinux_boolean_sub(const char *name)
>  {
>   char *sub = NULL;
> @@ -141,8 +141,6 @@ out:
>   return sub;
>  }
>  
> -hidden_def(selinux_boolean_sub)
> -
>  static int bool_open(const char *name, int flag) {
>   char *fname = NULL;
>   char *alt_name = NULL;
> @@ -262,8 +260,6 @@ int security_get_boolean_active(const char *name)
>   return val;
>  }
>  
> -hidden_def(security_get_boolean_active)
> -
>  int security_set_boolean(const char *name, int value)
>  {
>   int fd, ret;
> @@ -297,8 +293,6 @@ int security_set_boolean(const char *name, int value)
>   return -1;
>  }
>  
> -hidden_def(security_set_boolean)
> -
>  int security_commit_booleans(void)
>  {
>   int fd, ret;
> @@ -327,8 +321,6 @@ int security_commit_booleans(void)
>   return -1;
>  }
>  
> -hidden_def(security_commit_booleans)
> -
>  static char *strtrim(char *dest, char *source, int size)
>  {
>   int i = 0;
> @@ -567,3 +559,55 @@ int security_load_booleans(char *path)
>   errno = EINVAL;
>   return errors ? -1 : 0;
>  }
> +
> +#else
> +int security_set_boolean_list(size_t boolcnt __attribute__((unused)),
> + SELboolean * boollist __attribute__((unused)),
> + int permanent __attribute__((unused)))
> +{
> + return -1;
> +}
> +
> +int security_load_booleans(char *path __attribute__((unused)))
> +{
> + return -1;
> +}
> +
> +int security_get_boolean_names(char ***names __attribute__((unused)),
> + int *len __attribute__((unused)))
> +{
> + return -1;
> +}
> +
> +int security_get_boolean_pending(const char *name __attribute__((unused)))
> +{
> + return -1;
> +}
> +
> +int security_get_boolean_active(const char *name __attribute__((unused)))
> +{
> + return -1;
> +}
> +
> +int security_set_boolean(const char *name __attribute__((unused)),
> + int value __attribute__((unused)))
> +{
> + return -1;
> +}
> +
> +int security_commit_booleans(void)
> +{
> + return -1;
> +}
> +
> +char *selinux_boolean_sub(const char *name __attribute__((unused)))
> +{
> + return NULL;
> +}
> +#endif
> +
> +hidden_def(security_get_boolean_names)
> +hidden_def(selinux_boolean_sub)
> +hidden_def(security_get_boolean_active)
> +hidden_def(security_set_boolean)
> +hidden_def(security_commit_booleans)
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH] libselinux: re-introduce DISABLE_BOOL=y

[Stephen Smalley]

On 09/29/2016 02:46 PM, William Roberts wrote:
> On Thu, Sep 29, 2016 at 2:44 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
>> On 09/29/2016 02:15 PM, William Roberts wrote:
>>> On Thu, Sep 29, 2016 at 2:08 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
>>>> On 09/29/2016 02:02 PM, william.c.robe...@intel.com wrote:
>>>>> From: William Roberts <william.c.robe...@intel.com>
>>>>>
>>>>> Provide stubs to the public boolean API that always returns -1.
>>>>>
>>>>> On Android, boolean symbols are needed for:
>>>>> external/ltrace/sysdeps/linux-gnu/trace.c
>>>>
>>>> Is this really worth doing?
>>>
>>> It's this or disabling that selinux via #define, which that source has
>>> HAVE_LIBSELINUX.
>>>
>>> But it would seem confusing IMHO to have a libselinux.so, so one would
>>> set HAVE_LIBSELINUX=1,
>>> and you're getting link errors.
>>
>> Maybe I don't understand.  Obviously it builds today with
>> external/libselinux without requiring this change.  Why do we need this now?
>>
> 
> Richard Haines was doing further testing, and was building a different
> lunch target for the
> arm emulator and hit this issue. I have only tested x86_64 emulator.

No, I mean that this is not required in external/libselinux (the Android
fork) today.  So why is it needed here?  The Android fork builds
src/booleans.c for the target.  It doesn't hurt anything to leave the
code there.  The underlying kernel interface via selinuxfs still exists.
 There just won't be any booleans in the policy.


___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH] libselinux: re-introduce DISABLE_BOOL=y

[Stephen Smalley]

On 09/29/2016 02:15 PM, William Roberts wrote:
> On Thu, Sep 29, 2016 at 2:08 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
>> On 09/29/2016 02:02 PM, william.c.robe...@intel.com wrote:
>>> From: William Roberts <william.c.robe...@intel.com>
>>>
>>> Provide stubs to the public boolean API that always returns -1.
>>>
>>> On Android, boolean symbols are needed for:
>>> external/ltrace/sysdeps/linux-gnu/trace.c
>>
>> Is this really worth doing?
> 
> It's this or disabling that selinux via #define, which that source has
> HAVE_LIBSELINUX.
> 
> But it would seem confusing IMHO to have a libselinux.so, so one would
> set HAVE_LIBSELINUX=1,
> and you're getting link errors.

Maybe I don't understand.  Obviously it builds today with
external/libselinux without requiring this change.  Why do we need this now?


___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH] libselinux: re-introduce DISABLE_BOOL=y

[Stephen Smalley]

On 09/29/2016 02:02 PM, william.c.robe...@intel.com wrote:
> From: William Roberts 
> 
> Provide stubs to the public boolean API that always returns -1.
> 
> On Android, boolean symbols are needed for:
> external/ltrace/sysdeps/linux-gnu/trace.c

Is this really worth doing?

> 
> Signed-off-by: William Roberts 
> ---
>  libselinux/Makefile   |  4 +++
>  libselinux/src/booleans.c | 64 
> +++
>  2 files changed, 58 insertions(+), 10 deletions(-)
> 
> diff --git a/libselinux/Makefile b/libselinux/Makefile
> index f607115..b5f32bb 100644
> --- a/libselinux/Makefile
> +++ b/libselinux/Makefile
> @@ -5,6 +5,7 @@ DISABLE_RPM ?= y
>  ANDROID_HOST ?= n
>  ifeq ($(ANDROID_HOST),y)
>   override DISABLE_SETRANS=y
> + override DISABLE_BOOL=y
>  endif
>  ifeq ($(DISABLE_RPM),y)
>   DISABLE_FLAGS+= -DDISABLE_RPM
> @@ -12,6 +13,9 @@ endif
>  ifeq ($(DISABLE_SETRANS),y)
>   DISABLE_FLAGS+= -DDISABLE_SETRANS
>  endif
> +ifeq ($(DISABLE_BOOL),y)
> + DISABLE_FLAGS+= -DDISABLE_BOOL
> +endif
>  export DISABLE_SETRANS DISABLE_RPM DISABLE_FLAGS ANDROID_HOST
>  
>  USE_PCRE2 ?= n
> diff --git a/libselinux/src/booleans.c b/libselinux/src/booleans.c
> index c438af1..cbb0610 100644
> --- a/libselinux/src/booleans.c
> +++ b/libselinux/src/booleans.c
> @@ -25,6 +25,8 @@
>  
>  #define SELINUX_BOOL_DIR "/booleans/"
>  
> +#ifndef DISABLE_BOOL
> +
>  static int filename_select(const struct dirent *d)
>  {
>   if (d->d_name[0] == '.'
> @@ -85,8 +87,6 @@ int security_get_boolean_names(char ***names, int *len)
>   goto out;
>  }
>  
> -hidden_def(security_get_boolean_names)
> -
>  char *selinux_boolean_sub(const char *name)
>  {
>   char *sub = NULL;
> @@ -141,8 +141,6 @@ out:
>   return sub;
>  }
>  
> -hidden_def(selinux_boolean_sub)
> -
>  static int bool_open(const char *name, int flag) {
>   char *fname = NULL;
>   char *alt_name = NULL;
> @@ -262,8 +260,6 @@ int security_get_boolean_active(const char *name)
>   return val;
>  }
>  
> -hidden_def(security_get_boolean_active)
> -
>  int security_set_boolean(const char *name, int value)
>  {
>   int fd, ret;
> @@ -297,8 +293,6 @@ int security_set_boolean(const char *name, int value)
>   return -1;
>  }
>  
> -hidden_def(security_set_boolean)
> -
>  int security_commit_booleans(void)
>  {
>   int fd, ret;
> @@ -327,8 +321,6 @@ int security_commit_booleans(void)
>   return -1;
>  }
>  
> -hidden_def(security_commit_booleans)
> -
>  static char *strtrim(char *dest, char *source, int size)
>  {
>   int i = 0;
> @@ -567,3 +559,55 @@ int security_load_booleans(char *path)
>   errno = EINVAL;
>   return errors ? -1 : 0;
>  }
> +
> +#else
> +int security_set_boolean_list(size_t boolcnt __attribute__((unused)),
> + SELboolean * boollist __attribute__((unused)),
> + int permanent __attribute__((unused)))
> +{
> + return -1;
> +}
> +
> +int security_load_booleans(char *path __attribute__((unused)))
> +{
> + return -1;
> +}
> +
> +int security_get_boolean_names(char ***names __attribute__((unused)),
> + int *len __attribute__((unused)))
> +{
> + return -1;
> +}
> +
> +int security_get_boolean_pending(const char *name __attribute__((unused)))
> +{
> + return -1;
> +}
> +
> +int security_get_boolean_active(const char *name __attribute__((unused)))
> +{
> + return -1;
> +}
> +
> +int security_set_boolean(const char *name __attribute__((unused)),
> + int value __attribute__((unused)))
> +{
> + return -1;
> +}
> +
> +int security_commit_booleans(void)
> +{
> + return -1;
> +}
> +
> +char *selinux_boolean_sub(const char *name __attribute__((unused)))
> +{
> + return NULL;
> +}
> +#endif
> +
> +hidden_def(security_get_boolean_names)
> +hidden_def(selinux_boolean_sub)
> +hidden_def(security_get_boolean_active)
> +hidden_def(security_set_boolean)
> +hidden_def(security_commit_booleans)
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH] libselinux: android: fix lax service context lookup

[Stephen Smalley]

On 09/28/2016 12:35 PM, Janis Danisevskis wrote:
> 
> 
> On Wed, Sep 28, 2016 at 5:17 PM, Stephen Smalley <s...@tycho.nsa.gov
> <mailto:s...@tycho.nsa.gov>> wrote:
> 
> On 09/28/2016 12:04 PM, Janis Danisevskis wrote:
> > We use the same lookup function for service contexts
> > that we use for property contexts. However, property
> > contexts are namespace based and only compare the
> > prefix. This may lead to service associations with
> > a wrong label.
> >
> > This patch introduces a stricter lookup function for
> > services contexts. Now the service name must match
> > the key of the service label exactly.
> >
> > Signed-off-by: Janis Danisevskis <jda...@android.com 
> <mailto:jda...@android.com>>
> > ---
> >  libselinux/include/selinux/label.h  |  2 ++
> >  libselinux/src/label.c  |  1 +
> >  libselinux/src/label_android_property.c | 50 
> +
> >  libselinux/src/label_internal.h |  3 ++
> >  4 files changed, 56 insertions(+)
> 
> Normally each backend would go into its own file, so service would get
> its own.  Alternatively, since we are unlikely to ever support one
> without the other, perhaps label_android_property.c should be renamed to
> label_android.c and contain all of the Android-unique backends.
> 
> I was thinking along the same lines, was just eager to get home... I'll send
> a refactored patch by tomorrow.

Also, please update libselinux/utils/selabel_lookup.c and
selabel_digest.c to know about your new backend.
>  
> 
> >
> > diff --git a/libselinux/include/selinux/label.h 
> b/libselinux/include/selinux/label.h
> > index f0b1e10..277287e 100644
> > --- a/libselinux/include/selinux/label.h
> > +++ b/libselinux/include/selinux/label.h
> > @@ -34,6 +34,8 @@ struct selabel_handle;
> >  #define SELABEL_CTX_DB   3
> >  /* Android property service contexts */
> >  #define SELABEL_CTX_ANDROID_PROP 4
> > +/* Android service contexts */
> > +#define SELABEL_CTX_ANDROID_SERVICE 5
> >
> >  /*
> >   * Available options
> > diff --git a/libselinux/src/label.c b/libselinux/src/label.c
> > index 96a4ff1..eb0e766 100644
> > --- a/libselinux/src/label.c
> > +++ b/libselinux/src/label.c
> > @@ -45,6 +45,7 @@ static selabel_initfunc initfuncs[] = {
> >   CONFIG_X_BACKEND(selabel_x_init),
> >   CONFIG_DB_BACKEND(selabel_db_init),
> >   _property_init,
> > + _service_init,
> 
> Wondering if we should support selective enablement of the property and
> service backends too, similar to what William introduced for media, x,
> and db so that he could disable them on Android (in our case, so we can
> disable property and service backends on Linux distributions).
> 
> 
> Oh, that is what these wrappers are for :-) . Sure.
>  
> 
> 
> >  };
> >
> >  static void selabel_subs_fini(struct selabel_sub *ptr)
> > diff --git a/libselinux/src/label_android_property.c
> b/libselinux/src/label_android_property.c
> > index 290b438..69d6afd 100644
> > --- a/libselinux/src/label_android_property.c
> > +++ b/libselinux/src/label_android_property.c
> > @@ -279,6 +279,38 @@ finish:
> >   return ret;
> >  }
> >
> > +static struct selabel_lookup_rec *service_lookup(struct
> selabel_handle *rec,
> > + const char *key, int __attribute__((unused)) type)
> > +{
> > + struct saved_data *data = (struct saved_data *)rec->data;
> > + spec_t *spec_arr = data->spec_arr;
> > + unsigned int i;
> > + struct selabel_lookup_rec *ret = NULL;
> > +
> > + if (!data->nspec) {
> > + errno = ENOENT;
> > + goto finish;
> > + }
> > +
> > + for (i = 0; i < data->nspec; i++) {
> > + if (strcmp(spec_arr[i].property_key, key) == 0)
> > + break;
> > + if (strcmp(spec_arr[i].property_key, "*") == 0)
> > + break;
> > + }
> > +
> > + if (i >= data->nspec) {
> > + /* No matching specification. */
> > + errno = ENOENT;
> > + goto finish;
> > + }
> > +
>

Re: [PATCH] libselinux: android: fix lax service context lookup

[Stephen Smalley]

On 09/28/2016 12:43 PM, William Roberts wrote:
> On Wed, Sep 28, 2016 at 12:42 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
>> On 09/28/2016 12:25 PM, William Roberts wrote:
>>> On Wed, Sep 28, 2016 at 12:17 PM, Stephen Smalley <s...@tycho.nsa.gov> 
>>> wrote:
>>>> On 09/28/2016 12:04 PM, Janis Danisevskis wrote:
>>>>> We use the same lookup function for service contexts
>>>>> that we use for property contexts. However, property
>>>>> contexts are namespace based and only compare the
>>>>> prefix. This may lead to service associations with
>>>>> a wrong label.
>>>>>
>>>>> This patch introduces a stricter lookup function for
>>>>> services contexts. Now the service name must match
>>>>> the key of the service label exactly.
>>>>>
>>>>> Signed-off-by: Janis Danisevskis <jda...@android.com>
>>>>> ---
>>>>>  libselinux/include/selinux/label.h  |  2 ++
>>>>>  libselinux/src/label.c  |  1 +
>>>>>  libselinux/src/label_android_property.c | 50 
>>>>> +
>>>>>  libselinux/src/label_internal.h |  3 ++
>>>>>  4 files changed, 56 insertions(+)
>>>>
>>>> Normally each backend would go into its own file, so service would get
>>>> its own.  Alternatively, since we are unlikely to ever support one
>>>> without the other, perhaps label_android_property.c should be renamed to
>>>> label_android.c and contain all of the Android-unique backends.
>>>>
>>>>>
>>>>> diff --git a/libselinux/include/selinux/label.h 
>>>>> b/libselinux/include/selinux/label.h
>>>>> index f0b1e10..277287e 100644
>>>>> --- a/libselinux/include/selinux/label.h
>>>>> +++ b/libselinux/include/selinux/label.h
>>>>> @@ -34,6 +34,8 @@ struct selabel_handle;
>>>>>  #define SELABEL_CTX_DB   3
>>>>>  /* Android property service contexts */
>>>>>  #define SELABEL_CTX_ANDROID_PROP 4
>>>>> +/* Android service contexts */
>>>>> +#define SELABEL_CTX_ANDROID_SERVICE 5
>>>>>
>>>>>  /*
>>>>>   * Available options
>>>>> diff --git a/libselinux/src/label.c b/libselinux/src/label.c
>>>>> index 96a4ff1..eb0e766 100644
>>>>> --- a/libselinux/src/label.c
>>>>> +++ b/libselinux/src/label.c
>>>>> @@ -45,6 +45,7 @@ static selabel_initfunc initfuncs[] = {
>>>>>   CONFIG_X_BACKEND(selabel_x_init),
>>>>>   CONFIG_DB_BACKEND(selabel_db_init),
>>>>>   _property_init,
>>>>> + _service_init,
>>>>
>>>> Wondering if we should support selective enablement of the property and
>>>> service backends too, similar to what William introduced for media, x,
>>>> and db so that he could disable them on Android (in our case, so we can
>>>> disable property and service backends on Linux distributions).
>>>
>>> I was wondering that too, im for it. If ANDROID_HOST patch is applied, we
>>> should just set the defaults to strip them out and only on ANDROID_HOST=y
>>> add them in.
>>>
>>> We'd also need to coordinate with the AOSP patches, but I can
>>> routinely update those
>>> based on whats going on.
>>
>> I could be wrong, but I think we only need the property and service
>> backends on the target, not on the build host.  The file backend is
>> needed on the build host to label the filesystem images when they are
>> created.  We are likely only building the property backend on the host
>> because we don't allow conditionally excluding it presently.
> 
> checkfc I thought uses them for checking property and service backends?

Ah, you're right.  Never mind...

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH] libselinux: android: fix lax service context lookup

[Stephen Smalley]

On 09/28/2016 12:25 PM, William Roberts wrote:
> On Wed, Sep 28, 2016 at 12:17 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
>> On 09/28/2016 12:04 PM, Janis Danisevskis wrote:
>>> We use the same lookup function for service contexts
>>> that we use for property contexts. However, property
>>> contexts are namespace based and only compare the
>>> prefix. This may lead to service associations with
>>> a wrong label.
>>>
>>> This patch introduces a stricter lookup function for
>>> services contexts. Now the service name must match
>>> the key of the service label exactly.
>>>
>>> Signed-off-by: Janis Danisevskis <jda...@android.com>
>>> ---
>>>  libselinux/include/selinux/label.h  |  2 ++
>>>  libselinux/src/label.c  |  1 +
>>>  libselinux/src/label_android_property.c | 50 
>>> +
>>>  libselinux/src/label_internal.h |  3 ++
>>>  4 files changed, 56 insertions(+)
>>
>> Normally each backend would go into its own file, so service would get
>> its own.  Alternatively, since we are unlikely to ever support one
>> without the other, perhaps label_android_property.c should be renamed to
>> label_android.c and contain all of the Android-unique backends.
>>
>>>
>>> diff --git a/libselinux/include/selinux/label.h 
>>> b/libselinux/include/selinux/label.h
>>> index f0b1e10..277287e 100644
>>> --- a/libselinux/include/selinux/label.h
>>> +++ b/libselinux/include/selinux/label.h
>>> @@ -34,6 +34,8 @@ struct selabel_handle;
>>>  #define SELABEL_CTX_DB   3
>>>  /* Android property service contexts */
>>>  #define SELABEL_CTX_ANDROID_PROP 4
>>> +/* Android service contexts */
>>> +#define SELABEL_CTX_ANDROID_SERVICE 5
>>>
>>>  /*
>>>   * Available options
>>> diff --git a/libselinux/src/label.c b/libselinux/src/label.c
>>> index 96a4ff1..eb0e766 100644
>>> --- a/libselinux/src/label.c
>>> +++ b/libselinux/src/label.c
>>> @@ -45,6 +45,7 @@ static selabel_initfunc initfuncs[] = {
>>>   CONFIG_X_BACKEND(selabel_x_init),
>>>   CONFIG_DB_BACKEND(selabel_db_init),
>>>   _property_init,
>>> + _service_init,
>>
>> Wondering if we should support selective enablement of the property and
>> service backends too, similar to what William introduced for media, x,
>> and db so that he could disable them on Android (in our case, so we can
>> disable property and service backends on Linux distributions).
> 
> I was wondering that too, im for it. If ANDROID_HOST patch is applied, we
> should just set the defaults to strip them out and only on ANDROID_HOST=y
> add them in.
> 
> We'd also need to coordinate with the AOSP patches, but I can
> routinely update those
> based on whats going on.

I could be wrong, but I think we only need the property and service
backends on the target, not on the build host.  The file backend is
needed on the build host to label the filesystem images when they are
created.  We are likely only building the property backend on the host
because we don't allow conditionally excluding it presently.

OTOH, being able to look things up on the build host could be useful
from a development POV, e.g. if you were to add utils/selabel_lookup to
the build host targets and kept the property and service backends in the
host libselinux, then one could check what would match a given key.
___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH 2/2] libselinux: set DISABLE_RPM default to y.

[Stephen Smalley]

On 09/28/2016 12:00 PM, william.c.robe...@intel.com wrote:
> From: William Roberts 
> 
> Change the default build behavior to always use DISABLE_RPM.
> To get the old behavior call make with DISABLE_RPM=n.
> 
> eg.)
> make DISABLE_RPM=n

Thanks, applied both.  We'll see if anyone complains about this one and
revert if necessary.

> 
> Signed-off-by: William Roberts 
> ---
>  libselinux/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libselinux/Makefile b/libselinux/Makefile
> index 41d836c..cec2943 100644
> --- a/libselinux/Makefile
> +++ b/libselinux/Makefile
> @@ -1,7 +1,7 @@
>  SUBDIRS = src include utils man
>  
>  DISABLE_SETRANS ?= n
> -DISABLE_RPM ?= n
> +DISABLE_RPM ?= y
>  ifeq ($(DISABLE_RPM),y)
>   DISABLE_FLAGS+= -DDISABLE_RPM
>  endif
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH] libselinux: fix unused variable error

[Stephen Smalley]

On 09/28/2016 11:53 AM, william.c.robe...@intel.com wrote:
> From: William Roberts 
> 
> When building for Android, this error manifests itself:
> 
> label_file.c:570:7: error: unused variable ‘subs_file’ 
> [-Werror=unused-variable]
>   char subs_file[PATH_MAX + 1];
> 
> Fix it by moving the variable into the ifdef'd usage block.

Thanks, applied.

> 
> Signed-off-by: William Roberts 
> ---
>  libselinux/src/label_file.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
> index adf3dcc..a4dc3cd 100644
> --- a/libselinux/src/label_file.c
> +++ b/libselinux/src/label_file.c
> @@ -567,7 +567,6 @@ static int init(struct selabel_handle *rec, const struct 
> selinux_opt *opts,
>   struct saved_data *data = (struct saved_data *)rec->data;
>   const char *path = NULL;
>   const char *prefix = NULL;
> - char subs_file[PATH_MAX + 1];
>   int status = -1, baseonly = 0;
>  
>   /* Process arguments */
> @@ -585,6 +584,7 @@ static int init(struct selabel_handle *rec, const struct 
> selinux_opt *opts,
>   }
>  
>  #if !defined(BUILD_HOST) && !defined(ANDROID)
> + char subs_file[PATH_MAX + 1];
>   /* Process local and distribution substitution files */
>   if (!path) {
>   rec->dist_subs =
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH 3/3] libselinux: drop DISABLE_BOOL=y option

[Stephen Smalley]

On 09/28/2016 11:26 AM, william.c.robe...@intel.com wrote:
> From: William Roberts 
> 
> Build option DISABLE_BOOL=y is not being used, and is broken, drop it.
> 
> Signed-off-by: William Roberts 

Thanks, applied all three.  At some point we might want to rename
EMFLAGS (presumably short for embedded flags) to DISABLE_FLAGS or similar.

> ---
>  libselinux/Makefile  | 6 +-
>  libselinux/src/Makefile  | 6 +-
>  libselinux/src/load_policy.c | 2 --
>  libselinux/utils/Makefile| 5 -
>  4 files changed, 2 insertions(+), 17 deletions(-)
> 
> diff --git a/libselinux/Makefile b/libselinux/Makefile
> index c96695d..bfeb4b8 100644
> --- a/libselinux/Makefile
> +++ b/libselinux/Makefile
> @@ -2,17 +2,13 @@ SUBDIRS = src include utils man
>  
>  DISABLE_SETRANS ?= n
>  DISABLE_RPM ?= n
> -DISABLE_BOOL ?= n
> -ifeq ($(DISABLE_BOOL),y)
> - EMFLAGS+= -DDISABLE_BOOL
> -endif
>  ifeq ($(DISABLE_RPM),y)
>   EMFLAGS+= -DDISABLE_RPM
>  endif
>  ifeq ($(DISABLE_SETRANS),y)
>   EMFLAGS+= -DDISABLE_SETRANS
>  endif
> -export DISABLE_SETRANS DISABLE_RPM DISABLE_BOOL EMFLAGS
> +export DISABLE_SETRANS DISABLE_RPM EMFLAGS
>  
>  USE_PCRE2 ?= n
>  ifeq ($(USE_PCRE2),y)
> diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
> index 8cfdf34..d8a79b4 100644
> --- a/libselinux/src/Makefile
> +++ b/libselinux/src/Makefile
> @@ -41,12 +41,8 @@ LIBSO=$(TARGET).$(LIBVERSION)
>  AUDIT2WHYLOBJ=$(PYPREFIX)audit2why.lo
>  AUDIT2WHYSO=$(PYPREFIX)audit2why.so
>  
> -ifeq ($(DISABLE_BOOL),y)
> - UNUSED_SRCS+=booleans.c
> -endif
> -
>  GENERATED=$(SWIGCOUT) $(SWIGRUBYCOUT) selinuxswig_python_exception.i
> -SRCS= $(filter-out $(UNUSED_SRCS) $(GENERATED) audit2why.c, $(sort 
> $(wildcard *.c)))
> +SRCS= $(filter-out $(GENERATED) audit2why.c, $(sort $(wildcard *.c)))
>  
>  MAX_STACK_SIZE=32768
>  
> diff --git a/libselinux/src/load_policy.c b/libselinux/src/load_policy.c
> index 249f82f..b7e1a6f 100644
> --- a/libselinux/src/load_policy.c
> +++ b/libselinux/src/load_policy.c
> @@ -256,7 +256,6 @@ checkbool:
>   }
>   }
>   
> -#ifndef DISABLE_BOOL
>   if (preservebools) {
>   int *values, len, i;
>   char **names;
> @@ -279,7 +278,6 @@ checkbool:
>   (void)genbools(data, size,
>  (char *)selinux_booleans_path());
>   }
> -#endif
>   }
>  
>  
> diff --git a/libselinux/utils/Makefile b/libselinux/utils/Makefile
> index 9ec928b..0708d6d 100644
> --- a/libselinux/utils/Makefile
> +++ b/libselinux/utils/Makefile
> @@ -36,11 +36,6 @@ sefcontext_compile: sefcontext_compile.o ../src/regex.o
>  
>  selinux_restorecon: LDLIBS += -lsepol
>  
> -ifeq ($(DISABLE_BOOL),y)
> - UNUSED_TARGETS+=getsebool togglesebool
> -endif
> -TARGETS:= $(filter-out $(UNUSED_TARGETS), $(TARGETS))
> -
>  all: $(TARGETS)
>  
>  install: all
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[Stephen Smalley]

On 09/27/2016 03:09 PM, William Roberts wrote:
> On Tue, Sep 27, 2016 at 12:08 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
>> On 09/27/2016 03:03 PM, William Roberts wrote:
>>> On Tue, Sep 27, 2016 at 11:51 AM, Stephen Smalley <s...@tycho.nsa.gov> 
>>> wrote:
>>>> On 09/27/2016 02:43 PM, William Roberts wrote:
>>>>> On Sep 27, 2016 10:00, "Stephen Smalley" <s...@tycho.nsa.gov
>>>>> <mailto:s...@tycho.nsa.gov>> wrote:
>>>>>>
>>>>>> On 09/27/2016 11:08 AM, William Roberts wrote:
>>>>>>> On Tue, Sep 27, 2016 at 7:11 AM, Stephen Smalley <s...@tycho.nsa.gov
>>>>> <mailto:s...@tycho.nsa.gov>> wrote:
>>>>>>>> On 09/26/2016 04:53 PM, william.c.robe...@intel.com
>>>>> <mailto:william.c.robe...@intel.com> wrote:
>>>>>>>>> From: William Roberts <william.c.robe...@intel.com
>>>>> <mailto:william.c.robe...@intel.com>>
>>>>>>>>>
>>>>>>>>> To build the selinux host configuration, specify
>>>>>>>>> ANDROID_HOST=y on the Make command line.
>>>>>>>>>
>>>>>>>>> eg)
>>>>>>>>> make ANDROID_HOST=y
>>>>>>>>
>>>>>>>> Seems oddly named, neither corresponding to the #define it enables
>>>>>>>> (BUILD_HOST) nor to the target platform.
>>>>>>>
>>>>>>> We could change this to BUILD_HOST=y to enable all of it, but
>>>>> considering
>>>>>>> that this build is specific for Android, I thought the naming to be more
>>>>>>> appropriate.
>>>>>>>
>>>>>>> Additionally, EMBEDDED doesn't flip anything called EMBEDDED as well.
>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Signed-off-by: William Roberts <william.c.robe...@intel.com
>>>>> <mailto:william.c.robe...@intel.com>>
>>>>>>>>> ---
>>>>>>>>>  libselinux/Makefile |  8 +++-
>>>>>>>>>  libselinux/src/Makefile | 50
>>>>> +
>>>>>>>>>  2 files changed, 41 insertions(+), 17 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/libselinux/Makefile b/libselinux/Makefile
>>>>>>>>> index 5a8d42c..50ae009 100644
>>>>>>>>> --- a/libselinux/Makefile
>>>>>>>>> +++ b/libselinux/Makefile
>>>>>>>>> @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y)
>>>>>>>>>   override DISABLE_RPM=y
>>>>>>>>>   override DISABLE_BOOL=y
>>>>>>>>>  endif
>>>>>>>>> +ifeq ($(ANDROID_HOST),y)
>>>>>>>>> + override DISABLE_SETRANS=y
>>>>>>>>> + EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND
>>>>> -DNO_X_BACKEND \
>>>>>>>>> + -DBUILD_HOST
>>>>>>>>> + SUBDIRS = src
>>>>>>>>> +endif
>>>>>>
>>>>>> Also, this is redundant; you can handle it entirely within
>>>>>> libselinux/src/Makefile without anything here.
>>>>>
>>>>> You mean all the ANDROID _HOST stuff? I didn't want to depart from
>>>>> what's there, that seemed to be the spot for disabling things.
>>>>
>>>> You don't use the -DNO_*_BACKEND or -DBUILD_HOST flags anywhere except
>>>> in src/Makefile, so you don't need to set them here.
>>>>
>>>
>>> The same could be said about DISABLE_SETRANS
>>
>> It isn't set in both Makefiles.  Pick one.
> 
> Its not set in both, did you mean referenced/used? In fact I don't
> even set the default n, which I should be doing.

You set -DNO_*_BACKEND and -DBUILD_HOST in both Makefiles if ANDROID_HOST=y.


___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[Stephen Smalley]

On 09/27/2016 02:43 PM, William Roberts wrote:
> On Sep 27, 2016 10:00, "Stephen Smalley" <s...@tycho.nsa.gov
> <mailto:s...@tycho.nsa.gov>> wrote:
>>
>> On 09/27/2016 11:08 AM, William Roberts wrote:
>> > On Tue, Sep 27, 2016 at 7:11 AM, Stephen Smalley <s...@tycho.nsa.gov
> <mailto:s...@tycho.nsa.gov>> wrote:
>> >> On 09/26/2016 04:53 PM, william.c.robe...@intel.com
> <mailto:william.c.robe...@intel.com> wrote:
>> >>> From: William Roberts <william.c.robe...@intel.com
> <mailto:william.c.robe...@intel.com>>
>> >>>
>> >>> To build the selinux host configuration, specify
>> >>> ANDROID_HOST=y on the Make command line.
>> >>>
>> >>> eg)
>> >>> make ANDROID_HOST=y
>> >>
>> >> Seems oddly named, neither corresponding to the #define it enables
>> >> (BUILD_HOST) nor to the target platform.
>> >
>> > We could change this to BUILD_HOST=y to enable all of it, but
> considering
>> > that this build is specific for Android, I thought the naming to be more
>> > appropriate.
>> >
>> > Additionally, EMBEDDED doesn't flip anything called EMBEDDED as well.
>> >
>> >>
>> >>>
>> >>> Signed-off-by: William Roberts <william.c.robe...@intel.com
> <mailto:william.c.robe...@intel.com>>
>> >>> ---
>> >>>  libselinux/Makefile |  8 +++-
>> >>>  libselinux/src/Makefile | 50
> +
>> >>>  2 files changed, 41 insertions(+), 17 deletions(-)
>> >>>
>> >>> diff --git a/libselinux/Makefile b/libselinux/Makefile
>> >>> index 5a8d42c..50ae009 100644
>> >>> --- a/libselinux/Makefile
>> >>> +++ b/libselinux/Makefile
>> >>> @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y)
>> >>>   override DISABLE_RPM=y
>> >>>   override DISABLE_BOOL=y
>> >>>  endif
>> >>> +ifeq ($(ANDROID_HOST),y)
>> >>> + override DISABLE_SETRANS=y
>> >>> + EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND
> -DNO_X_BACKEND \
>> >>> + -DBUILD_HOST
>> >>> + SUBDIRS = src
>> >>> +endif
>>
>> Also, this is redundant; you can handle it entirely within
>> libselinux/src/Makefile without anything here.
> 
> You mean all the ANDROID _HOST stuff? I didn't want to depart from
> what's there, that seemed to be the spot for disabling things.

You don't use the -DNO_*_BACKEND or -DBUILD_HOST flags anywhere except
in src/Makefile, so you don't need to set them here.


___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[Stephen Smalley]

On 09/27/2016 11:08 AM, William Roberts wrote:
> On Tue, Sep 27, 2016 at 7:11 AM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
>> On 09/26/2016 04:53 PM, william.c.robe...@intel.com wrote:
>>> From: William Roberts <william.c.robe...@intel.com>
>>>
>>> To build the selinux host configuration, specify
>>> ANDROID_HOST=y on the Make command line.
>>>
>>> eg)
>>> make ANDROID_HOST=y
>>
>> Seems oddly named, neither corresponding to the #define it enables
>> (BUILD_HOST) nor to the target platform.
> 
> We could change this to BUILD_HOST=y to enable all of it, but considering
> that this build is specific for Android, I thought the naming to be more
> appropriate.
> 
> Additionally, EMBEDDED doesn't flip anything called EMBEDDED as well.

Ok, fair point.  Actually EMBEDDED=y is broken and no one is using it
AFAIK so we should probably kill it at some point separately.

> 
>>
>>>
>>> Signed-off-by: William Roberts <william.c.robe...@intel.com>
>>> ---
>>>  libselinux/Makefile |  8 +++-
>>>  libselinux/src/Makefile | 50 
>>> +
>>>  2 files changed, 41 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/libselinux/Makefile b/libselinux/Makefile
>>> index 5a8d42c..50ae009 100644
>>> --- a/libselinux/Makefile
>>> +++ b/libselinux/Makefile
>>> @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y)
>>>   override DISABLE_RPM=y
>>>   override DISABLE_BOOL=y
>>>  endif
>>> +ifeq ($(ANDROID_HOST),y)
>>> + override DISABLE_SETRANS=y
>>> + EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND 
>>> -DNO_X_BACKEND \
>>> + -DBUILD_HOST
>>> + SUBDIRS = src
>>> +endif
>>
>> Don't you actually want to also pick up utils/sefcontext_compile?
>> That is built and used on the build host.  And I'm not sure why we'd
>> drop the other SUBDIRS.
> 
> You'll start running into linking issues if things that use
> libselinux, use something not
> in the build host IIRC. Perhaps, if that is the issue, we just limit
> it to sefcontext_compile.

Sure, the utils/Makefile can remove entries the same way it already does
for DISABLE_*.
> 
>>
>>>  ifeq ($(DISABLE_AVC),y)
>>>   EMFLAGS+= -DDISABLE_AVC
>>>  endif
>>> @@ -22,7 +28,7 @@ endif
>>>  ifeq ($(DISABLE_SETRANS),y)
>>>   EMFLAGS+= -DDISABLE_SETRANS
>>>  endif
>>> -export DISABLE_AVC DISABLE_SETRANS DISABLE_RPM DISABLE_BOOL EMFLAGS
>>> +export DISABLE_AVC DISABLE_SETRANS DISABLE_RPM DISABLE_BOOL EMFLAGS 
>>> ANDROID_HOST
>>>
>>>  USE_PCRE2 ?= n
>>>  ifeq ($(USE_PCRE2),y)
>>> diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
>>> index 36e42b8..d841ef7 100644
>>> --- a/libselinux/src/Makefile
>>> +++ b/libselinux/src/Makefile
>>> @@ -47,9 +47,17 @@ endif
>>>  ifeq ($(DISABLE_BOOL),y)
>>>   UNUSED_SRCS+=booleans.c
>>>  endif
>>> +ifeq ($(ANDROID_HOST),y)
>>> + SRCS=callbacks.c freecon.c label.c label_file.c \
>>> + label_android_property.c regex.c label_support.c \
>>> + matchpathcon.c setrans_client.c sha1.c
>>> + override CFLAGS += -DNO_MEDIA_BACKEND -DNO_DB_BACKEND -DNO_X_BACKEND \
>>> + -DBUILD_HOST
>>> +else
>>> + SRCS= $(filter-out $(UNUSED_SRCS) $(GENERATED) audit2why.c, $(sort 
>>> $(wildcard *.c)))
>>> +endif
>>>
>>>  GENERATED=$(SWIGCOUT) $(SWIGRUBYCOUT) selinuxswig_python_exception.i
>>> -SRCS= $(filter-out $(UNUSED_SRCS) $(GENERATED) audit2why.c, $(sort 
>>> $(wildcard *.c)))
>>>
>>>  MAX_STACK_SIZE=32768
>>>
>>> @@ -92,6 +100,28 @@ SWIG = swig -Wall -python -o $(SWIGCOUT) -outdir ./ 
>>> $(EMFLAGS)
>>>
>>>  SWIGRUBY = swig -Wall -ruby -o $(SWIGRUBYCOUT) -outdir ./ $(EMFLAGS)
>>>
>>> +$(LIBA): $(OBJS)
>>> + $(AR) rcs $@ $^
>>> + $(RANLIB) $@
>>> +
>>> +$(LIBSO): $(LOBJS)
>>> + $(CC) $(CFLAGS) -shared -o $@ $^ $(PCRE_LDFLAGS) -ldl $(LDFLAGS) 
>>> -L$(LIBDIR) -Wl,-soname,$(LIBSO),-z,defs,-z,relro
>>> + ln -sf $@ $(TARGET)
>>> +
>>> +%.o:  %.c policy.h
>>> + $(CC) $(CFLAGS) $(TLSFLAGS) -c -o $@ $<
>>> +
>>> +%.lo:  %.c policy.h
>>> + $(CC) $(CFLAGS) -fPIC -DSHARED -c -o $@ $<
>>
>> Did the

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[Stephen Smalley]

On 09/26/2016 04:53 PM, william.c.robe...@intel.com wrote:
> From: William Roberts 
> 
> To build the selinux host configuration, specify
> ANDROID_HOST=y on the Make command line.
> 
> eg)
> make ANDROID_HOST=y

Seems oddly named, neither corresponding to the #define it enables
(BUILD_HOST) nor to the target platform.

> 
> Signed-off-by: William Roberts 
> ---
>  libselinux/Makefile |  8 +++-
>  libselinux/src/Makefile | 50 
> +
>  2 files changed, 41 insertions(+), 17 deletions(-)
> 
> diff --git a/libselinux/Makefile b/libselinux/Makefile
> index 5a8d42c..50ae009 100644
> --- a/libselinux/Makefile
> +++ b/libselinux/Makefile
> @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y)
>   override DISABLE_RPM=y
>   override DISABLE_BOOL=y
>  endif
> +ifeq ($(ANDROID_HOST),y)
> + override DISABLE_SETRANS=y
> + EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND 
> -DNO_X_BACKEND \
> + -DBUILD_HOST
> + SUBDIRS = src
> +endif

Don't you actually want to also pick up utils/sefcontext_compile?
That is built and used on the build host.  And I'm not sure why we'd
drop the other SUBDIRS.

>  ifeq ($(DISABLE_AVC),y)
>   EMFLAGS+= -DDISABLE_AVC
>  endif
> @@ -22,7 +28,7 @@ endif
>  ifeq ($(DISABLE_SETRANS),y)
>   EMFLAGS+= -DDISABLE_SETRANS
>  endif
> -export DISABLE_AVC DISABLE_SETRANS DISABLE_RPM DISABLE_BOOL EMFLAGS
> +export DISABLE_AVC DISABLE_SETRANS DISABLE_RPM DISABLE_BOOL EMFLAGS 
> ANDROID_HOST
>  
>  USE_PCRE2 ?= n
>  ifeq ($(USE_PCRE2),y)
> diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
> index 36e42b8..d841ef7 100644
> --- a/libselinux/src/Makefile
> +++ b/libselinux/src/Makefile
> @@ -47,9 +47,17 @@ endif
>  ifeq ($(DISABLE_BOOL),y)
>   UNUSED_SRCS+=booleans.c
>  endif
> +ifeq ($(ANDROID_HOST),y)
> + SRCS=callbacks.c freecon.c label.c label_file.c \
> + label_android_property.c regex.c label_support.c \
> + matchpathcon.c setrans_client.c sha1.c
> + override CFLAGS += -DNO_MEDIA_BACKEND -DNO_DB_BACKEND -DNO_X_BACKEND \
> + -DBUILD_HOST
> +else
> + SRCS= $(filter-out $(UNUSED_SRCS) $(GENERATED) audit2why.c, $(sort 
> $(wildcard *.c)))
> +endif
>  
>  GENERATED=$(SWIGCOUT) $(SWIGRUBYCOUT) selinuxswig_python_exception.i
> -SRCS= $(filter-out $(UNUSED_SRCS) $(GENERATED) audit2why.c, $(sort 
> $(wildcard *.c)))
>  
>  MAX_STACK_SIZE=32768
>  
> @@ -92,6 +100,28 @@ SWIG = swig -Wall -python -o $(SWIGCOUT) -outdir ./ 
> $(EMFLAGS)
>  
>  SWIGRUBY = swig -Wall -ruby -o $(SWIGRUBYCOUT) -outdir ./ $(EMFLAGS)
>  
> +$(LIBA): $(OBJS)
> + $(AR) rcs $@ $^
> + $(RANLIB) $@
> +
> +$(LIBSO): $(LOBJS)
> + $(CC) $(CFLAGS) -shared -o $@ $^ $(PCRE_LDFLAGS) -ldl $(LDFLAGS) 
> -L$(LIBDIR) -Wl,-soname,$(LIBSO),-z,defs,-z,relro
> + ln -sf $@ $(TARGET)
> +
> +%.o:  %.c policy.h
> + $(CC) $(CFLAGS) $(TLSFLAGS) -c -o $@ $<
> +
> +%.lo:  %.c policy.h
> + $(CC) $(CFLAGS) -fPIC -DSHARED -c -o $@ $<

Did these truly need to move?  I don't see why.

> +
> +# ANDROID_HOST Build option only builds the shared and static versions of
> +# libselinux.
> +ifeq ($(ANDROID_HOST),y)
> +
> +all: $(LIBA) $(LIBSO)
> +
> +else
> +
>  all: $(LIBA) $(LIBSO) $(LIBPC)

Is this worthwhile/necessary?  The point of the build option IIUC is
just to allow upstream testing that the Android build host version will
still build, it shouldn't matter if there are extras.

>  
>  pywrap: all $(SWIGFILES) $(AUDIT2WHYSO)
> @@ -110,14 +140,6 @@ $(SWIGSO): $(SWIGLOBJ)
>  $(SWIGRUBYSO): $(SWIGRUBYLOBJ)
>   $(CC) $(CFLAGS) -shared -o $@ $^ -L. -lselinux $(LDFLAGS) -L$(LIBDIR)
>  
> -$(LIBA): $(OBJS)
> - $(AR) rcs $@ $^
> - $(RANLIB) $@
> -
> -$(LIBSO): $(LOBJS)
> - $(CC) $(CFLAGS) -shared -o $@ $^ $(PCRE_LDFLAGS) -ldl $(LDFLAGS) 
> -L$(LIBDIR) -Wl,-soname,$(LIBSO),-z,defs,-z,relro
> - ln -sf $@ $(TARGET) 
> -
>  $(LIBPC): $(LIBPC).in ../VERSION
>   sed -e 's/@VERSION@/$(VERSION)/; s:@prefix@:$(PREFIX):; 
> s:@libdir@:$(LIBBASE):; s:@includedir@:$(INCLUDEDIR):' < $< > $@
>  
> @@ -130,12 +152,6 @@ $(AUDIT2WHYLOBJ): audit2why.c
>  $(AUDIT2WHYSO): $(AUDIT2WHYLOBJ)
>   $(CC) $(CFLAGS) -shared -o $@ $^ -L. $(LDFLAGS) -lselinux 
> $(LIBDIR)/libsepol.a -L$(LIBDIR)
>  
> -%.o:  %.c policy.h
> - $(CC) $(CFLAGS) $(TLSFLAGS) -c -o $@ $<
> -
> -%.lo:  %.c policy.h
> - $(CC) $(CFLAGS) -fPIC -DSHARED -c -o $@ $<
> -
>  $(SWIGCOUT): $(SWIGIF)
>   $(SWIG) $<
>  
> @@ -178,4 +194,6 @@ distclean: clean
>  indent:
>   ../../scripts/Lindent $(filter-out $(GENERATED),$(wildcard *.[ch]))
>  
> -.PHONY: all clean pywrap rubywrap swigify install install-pywrap 
> install-rubywrap distclean
> +.PHONY: clean pywrap rubywrap swigify install install-pywrap 
> install-rubywrap distclean
> +endif
> +.PHONY: all

Why is this needed?

BTW, this option can be dangerous.  Don't build with it and then 

Re: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[Stephen Smalley]

On 09/26/2016 04:55 PM, William Roberts wrote:
> On Mon, Sep 26, 2016 at 1:53 PM,   wrote:
>> From: William Roberts 
>>
>> To build the selinux host configuration, specify
>> ANDROID_HOST=y on the Make command line.
>>
>> eg)
>> make ANDROID_HOST=y
>>
>> Signed-off-by: William Roberts 
>> ---
>>  libselinux/Makefile |  8 +++-
>>  libselinux/src/Makefile | 50 
>> +
>>  2 files changed, 41 insertions(+), 17 deletions(-)
>>
>> diff --git a/libselinux/Makefile b/libselinux/Makefile
>> index 5a8d42c..50ae009 100644
>> --- a/libselinux/Makefile
>> +++ b/libselinux/Makefile
>> @@ -10,6 +10,12 @@ ifeq ($(EMBEDDED),y)
>> override DISABLE_RPM=y
>> override DISABLE_BOOL=y
>>  endif
>> +ifeq ($(ANDROID_HOST),y)
>> +   override DISABLE_SETRANS=y
>> +   EMFLAGS+= -DDISABLE_RPM -DNO_MEDIA_BACKEND -DNO_DB_BACKEND 
>> -DNO_X_BACKEND \
> 
> argghhh .. missing too much, drop DISABLE_RPM... copy+paste+edit fail.
> Ill let v2 sit for a day, and aggregate other
> feedback for v3. Sorry for the noise.

Sorry, why can't you override DISABLE_RPM=y?


___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: 答复: A question about booting process with SELinux.

[Stephen Smalley]

On 09/27/2016 03:00 AM, Weiyuan (David, Euler) wrote:
> "The rootfs is typically just unpacked from initramfs and all files within it 
> are assigned a default label based on the genfscon statement"
> 
> Do you mean Kernel lable rootfs with genfscon before init loading the 
> sepolicy into kernel？
> 
> 
> Could you please describe the details of the process that how does the rootfs 
> be labeled with u:object_r:rootfs:s0 during the booting of Android (Such as 
> Nexus) ?

If you really want to delve into that level of detail, then this is what
happens:

When the inodes are allocated, SELinux initializes them with the
unlabeled SID (inode_alloc_security).  When the corresponding dentry is
instantiated, SELinux adds them to a list associated with the superblock
because policy has not yet been loaded so we do not yet know how to
label them (inode_doinit_with_dentry, sbsec->flags does not yet have
SE_SBINITIALIZED set).  When init loads the /sepolicy file into the
kernel, security_load_policy() calls selinux_complete_init() after
loading the policy.  selinux_complete_init() iterates the superblocks
with delayed_superblock_init() as the callback, which calls
superblock_doinit().  superblock_doinit() calls selinux_set_mnt_opts(),
which calls security_fs_use().  security_fs_use() checks to see if the
filesystem type has a fs_use_* rule, and if not, checks for a genfscon
rule.  For the rootfs, we find the genfscon rule and return
SECURITY_FS_USE_GENFS along with the SID/context from the rule.  Then
selinux_set_mnt_opts() calls sb_finish_set_opts().  sb_finish_set_opts()
calls inode_doinit_with_dentry() on the root inode and inode_doinit() on
any inode in the list (which in turn calls inode_doinit_with_dentry()
with a NULL dentry). inode_doinit_with_dentry() falls through to the
default case of the switch statement.  inode_doinit_with_dentry() then
sets the inode SID to the superblock SID (which came from the genfscon
rule) and is done (rootfs does not enable SE_SBGENFS).  The inode is now
labeled with the context specified by the genfscon rule.
___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH 2/2] libselinux: add ifdef'ing for ANDROID and BUILD_HOST

[Stephen Smalley]

On 09/26/2016 01:33 PM, william.c.robe...@intel.com wrote:
> From: William Roberts 
> 
> On Android, certain discrepancies arise for unused functionality or
> for dealing with the differences in Bionic libc. This patch includes
> all the "ifdef'ing" required and introduces the BUILD_HOST define.
> 
> The BUILD_HOST define removes functionality not needed when building
> libselinux for the Android build host machine.
> 
> Note that not all the libselinux src files are used to build
> the host and target libraries on Android.
> 
> Change-Id: I7984e7b769c4dfa627d6cf311411fa2c93bb7ef7
> Signed-off-by: William Roberts 

Thanks, applied both.

> ---
>  libselinux/src/callbacks.c  |   5 ++
>  libselinux/src/label_file.c |   2 +
>  libselinux/src/label_internal.h |   5 ++
>  libselinux/src/load_policy.c|   4 ++
>  libselinux/src/matchpathcon.c   | 116 
> 
>  libselinux/src/procattr.c   |   3 ++
>  6 files changed, 78 insertions(+), 57 deletions(-)
> 
> diff --git a/libselinux/src/callbacks.c b/libselinux/src/callbacks.c
> index c3cf98b..c18ccc5 100644
> --- a/libselinux/src/callbacks.c
> +++ b/libselinux/src/callbacks.c
> @@ -34,7 +34,12 @@ default_selinux_audit(void *ptr __attribute__((unused)),
>  static int
>  default_selinux_validate(char **ctx)
>  {
> +#ifndef BUILD_HOST
>   return security_check_context(*ctx);
> +#else
> + (void) ctx;
> + return 0;
> +#endif
>  }
>  
>  static int
> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
> index 8ff1170..5ba6a22 100644
> --- a/libselinux/src/label_file.c
> +++ b/libselinux/src/label_file.c
> @@ -543,6 +543,7 @@ static int init(struct selabel_handle *rec, const struct 
> selinux_opt *opts,
>   break;
>   }
>  
> +#if !defined(BUILD_HOST) && !defined(ANDROID)
>   /* Process local and distribution substitution files */
>   if (!path) {
>   rec->dist_subs =
> @@ -560,6 +561,7 @@ static int init(struct selabel_handle *rec, const struct 
> selinux_opt *opts,
>   rec->digest);
>   }
>  
> +#endif
>   rec->spec_file = strdup(path);
>  
>   /*
> diff --git a/libselinux/src/label_internal.h b/libselinux/src/label_internal.h
> index 0827ef6..7c55531 100644
> --- a/libselinux/src/label_internal.h
> +++ b/libselinux/src/label_internal.h
> @@ -16,6 +16,11 @@
>  #include "dso.h"
>  #include "sha1.h"
>  
> +#ifdef ANDROID
> +// Android does not have fgets_unlocked()
> +#define fgets_unlocked(buf, size, fp) fgets(buf, size, fp)
> +#endif
> +
>  /*
>   * Installed backends
>   */
> diff --git a/libselinux/src/load_policy.c b/libselinux/src/load_policy.c
> index 4f39fc7..249f82f 100644
> --- a/libselinux/src/load_policy.c
> +++ b/libselinux/src/load_policy.c
> @@ -11,8 +11,10 @@
>  #include 
>  #include 
>  #include "selinux_internal.h"
> +#ifndef ANDROID
>  #include 
>  #include 
> +#endif
>  #include 
>  #include "policy.h"
>  #include 
> @@ -45,6 +47,7 @@ int security_load_policy(void *data, size_t len)
>  
>  hidden_def(security_load_policy)
>  
> +#ifndef ANDROID
>  int load_setlocaldefs hidden = 1;
>  
>  #undef max
> @@ -465,3 +468,4 @@ int selinux_init_load_policy(int *enforce)
>*/
>   return -1;
>  }
> +#endif
> diff --git a/libselinux/src/matchpathcon.c b/libselinux/src/matchpathcon.c
> index 4764ab7..724eb65 100644
> --- a/libselinux/src/matchpathcon.c
> +++ b/libselinux/src/matchpathcon.c
> @@ -7,6 +7,64 @@
>  #include "callbacks.h"
>  #include 
>  
> +static int (*myinvalidcon) (const char *p, unsigned l, char *c) = NULL;
> +static int (*mycanoncon) (const char *p, unsigned l, char **c) =  NULL;
> +
> +static void
> +#ifdef __GNUC__
> +__attribute__ ((format(printf, 1, 2)))
> +#endif
> +default_printf(const char *fmt, ...)
> +{
> + va_list ap;
> + va_start(ap, fmt);
> + vfprintf(stderr, fmt, ap);
> + va_end(ap);
> +}
> +
> +void
> +#ifdef __GNUC__
> +__attribute__ ((format(printf, 1, 2)))
> +#endif
> +(*myprintf) (const char *fmt,...) = _printf;
> +int myprintf_compat = 0;
> +
> +void set_matchpathcon_printf(void (*f) (const char *fmt, ...))
> +{
> + myprintf = f ? f : _printf;
> + myprintf_compat = 1;
> +}
> +
> +int compat_validate(struct selabel_handle *rec,
> + struct selabel_lookup_rec *contexts,
> + const char *path, unsigned lineno)
> +{
> + int rc;
> + char **ctx = >ctx_raw;
> +
> + if (myinvalidcon)
> + rc = myinvalidcon(path, lineno, *ctx);
> + else if (mycanoncon)
> + rc = mycanoncon(path, lineno, ctx);
> + else {
> + rc = selabel_validate(rec, contexts);
> + if (rc < 0) {
> + if (lineno) {
> + COMPAT_LOG(SELINUX_WARNING,
> + "%s: line %u has invalid context 
> 

Re: Android Fork

[Stephen Smalley]

On 09/26/2016 01:33 PM, william.c.robe...@intel.com wrote:
> Below, are the last two majore patches to close the Android fork.
> 
> Patch "libselinux: add ifdef'ing for ANDROID and BUILD_HOST" I
> combined into 1 patch since some ANDROID and BUILD_HOST defines
> are on the same line, I can split it appart if its really needed.
> 
> Note, that you need the Android make recipe for some of these
> configurations to work as Android doesn't use all the src files.
> 
> The Build files are left out, for now, so AOSP can pull from
> upstream without it breaking the AOSP build.
> 
> Once merged AOSP can take the build files, once they drop
> external/libselinux, and then the last patch u[stream will
> be the enabling patch with the build files.

I was wondering whether the build files belong in upstream.
I know we have them for libsepol and checkpolicy currently, but
essentially they are just always out of date, any changes will be fed
through Android first anyway, and they aren't getting any testing or
updates from upstream.  Might be simpler to just keep those in Android
and drop the ones we already have from upstream.

> 
> [PATCH 1/2] libselinux: introduce configurable backends
> [PATCH 2/2] libselinux: add ifdef'ing for ANDROID and BUILD_HOST
> ___
> Selinux mailing list
> seli...@tycho.nsa.gov
> To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
> To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH 3/3] libselinux: sefcontext_compile invert semantics of "-r" flag

[Stephen Smalley]

On 09/26/2016 01:48 PM, William Roberts wrote:
> On Mon, Sep 26, 2016 at 10:43 AM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
>> On 09/26/2016 10:22 AM, Janis Danisevskis wrote:
>>> The "-r" flag of sefcontext_compile now causes it to omit the
>>> precompiled regular expressions from the output.
>>
>> The code itself looks ok, aside from William's suggestion. Experimenting
>> with this a bit, I noticed the following difference in sizes among the
>> various options:
>>
>> 383165  file_contexts (text)
>> 1507941 file_contexts.bin (binary with pcre1 regexes)
>> 8304105 file_contexts.bin (binary with pcre2 regexes)
>> 540540  file_contexts.bin (binary omitting pcre2 regexes, via -r)
> 
> What's the size of the textual intermediate file?

This was just taking the
/etc/selinux/targeted/contexts/files/file_contexts file on Fedora and
running it through sefcontext_compile built with and without USE_PCRE2=y
and in the PCRE2 case, running it with and without -r.  So the text size
above is exactly what was fed into sefcontext_compile, no intermediates.

> 
>>
>> The increase in file_contexts.bin size from pcre1 to pcre2 (unless using
>> -r) is quite substantial.  Wondering how that affects the cost/benefit
>> tradeoff...
>>
>>>
>>> Signed-off-by: Janis Danisevskis <jda...@android.com>
>>> ---
>>>  libselinux/utils/sefcontext_compile.c | 12 +++-
>>>  1 file changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/libselinux/utils/sefcontext_compile.c 
>>> b/libselinux/utils/sefcontext_compile.c
>>> index 8c48d32..b2746c7 100644
>>> --- a/libselinux/utils/sefcontext_compile.c
>>> +++ b/libselinux/utils/sefcontext_compile.c
>>> @@ -276,10 +276,12 @@ static void usage(const char *progname)
>>>   " will be fc_file with the .bin suffix appended.\n\t"
>>>   "-p   Optional binary policy file that will be used to\n\t"
>>>   " validate contexts defined in the fc_file.\n\t"
>>> - "-r   Include precompiled regular expressions in the 
>>> output.\n\t"
>>> + "-r   Omit precompiled regular expressions from the 
>>> output.\n\t"
>>>   " (PCRE2 only. Compiled PCRE2 regular expressions are\n\t"
>>> - " not portable across architectures. When linked 
>>> against\n\t"
>>> - " PCRE this flag is ignored)\n\t"
>>> + " not portable across architectures. Use this flag\n\t"
>>> + " if you know that you build for an incompatible\n\t"
>>> + " architecture to save space. When linked against\n\t"
>>> + " PCRE1 this flag is ignored.)\n\t"
>>>   "-i   Print regular expression info end exit. That is, 
>>> back\n\t"
>>>   " end version and architecture identifier.\n\t"
>>>   " Arch identifier format (PCRE2):\n\t"
>>> @@ -294,7 +296,7 @@ int main(int argc, char *argv[])
>>>  {
>>>   const char *path = NULL;
>>>   const char *out_file = NULL;
>>> - int do_write_precompregex = 0;
>>> + int do_write_precompregex = 1;
>>>   char stack_path[PATH_MAX + 1];
>>>   char *tmp = NULL;
>>>   int fd, rc, opt;
>>> @@ -315,7 +317,7 @@ int main(int argc, char *argv[])
>>>   policy_file = optarg;
>>>   break;
>>>   case 'r':
>>> - do_write_precompregex = 1;
>>> + do_write_precompregex = 0;
>>>   break;
>>>   case 'i':
>>>   printf("%s (%s)\n", regex_version(),
>>>
>>
> 
> 
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH 3/3] libselinux: sefcontext_compile invert semantics of "-r" flag

[Stephen Smalley]

On 09/26/2016 10:22 AM, Janis Danisevskis wrote:
> The "-r" flag of sefcontext_compile now causes it to omit the
> precompiled regular expressions from the output.

The code itself looks ok, aside from William's suggestion. Experimenting
with this a bit, I noticed the following difference in sizes among the
various options:

383165  file_contexts (text)
1507941 file_contexts.bin (binary with pcre1 regexes)
8304105 file_contexts.bin (binary with pcre2 regexes)
540540  file_contexts.bin (binary omitting pcre2 regexes, via -r)

The increase in file_contexts.bin size from pcre1 to pcre2 (unless using
-r) is quite substantial.  Wondering how that affects the cost/benefit
tradeoff...

> 
> Signed-off-by: Janis Danisevskis 
> ---
>  libselinux/utils/sefcontext_compile.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/libselinux/utils/sefcontext_compile.c 
> b/libselinux/utils/sefcontext_compile.c
> index 8c48d32..b2746c7 100644
> --- a/libselinux/utils/sefcontext_compile.c
> +++ b/libselinux/utils/sefcontext_compile.c
> @@ -276,10 +276,12 @@ static void usage(const char *progname)
>   " will be fc_file with the .bin suffix appended.\n\t"
>   "-p   Optional binary policy file that will be used to\n\t"
>   " validate contexts defined in the fc_file.\n\t"
> - "-r   Include precompiled regular expressions in the 
> output.\n\t"
> + "-r   Omit precompiled regular expressions from the output.\n\t"
>   " (PCRE2 only. Compiled PCRE2 regular expressions are\n\t"
> - " not portable across architectures. When linked 
> against\n\t"
> - " PCRE this flag is ignored)\n\t"
> + " not portable across architectures. Use this flag\n\t"
> + " if you know that you build for an incompatible\n\t"
> + " architecture to save space. When linked against\n\t"
> + " PCRE1 this flag is ignored.)\n\t"
>   "-i   Print regular expression info end exit. That is, back\n\t"
>   " end version and architecture identifier.\n\t"
>   " Arch identifier format (PCRE2):\n\t"
> @@ -294,7 +296,7 @@ int main(int argc, char *argv[])
>  {
>   const char *path = NULL;
>   const char *out_file = NULL;
> - int do_write_precompregex = 0;
> + int do_write_precompregex = 1;
>   char stack_path[PATH_MAX + 1];
>   char *tmp = NULL;
>   int fd, rc, opt;
> @@ -315,7 +317,7 @@ int main(int argc, char *argv[])
>   policy_file = optarg;
>   break;
>   case 'r':
> - do_write_precompregex = 1;
> + do_write_precompregex = 0;
>   break;
>   case 'i':
>   printf("%s (%s)\n", regex_version(),
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: A question about booting process with SELinux.

[Stephen Smalley]

On 09/26/2016 12:23 PM, Weiyuan (David, Euler) wrote:
> Dear All:
> 
>I have a question that is when and how the root“/”and files in it
> are labeled?
> 
>  
> 
> There are  "/ u:object_r:rootfs:s0" in file_contexts,  and  "genfscon
> rootfs / u:object_r:rootfs:s0" in genfs_contexts.
> 
> My understanding is, First, kernel will load the initial_sid_contexts
> before init process do the selinux_initialize().
> 
> Then when rootfs is mounted to “/”, kernel will label it with
> “u:object_r:labeledfs.
> 
> And After init process do the selinux_initialize() to load sepolicy to
> kernel,  there will be a restorecon to “/”.
> 
>  
> 
> Am I right?   If I am right, then when do this restorecon happen?

restorecon is only needed for /data or other filesystems that are
updated at runtime.  The rootfs is typically just unpacked from
initramfs and all files within it are assigned a default label based on
the genfscon statement, unless using a real ext4 root filesystem
partition (in which case the inode xattrs would be set when the
filesystem image is generated, not when the system is booting).

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: Killing The Android libselinux Fork (available)

[Stephen Smalley]

On 09/23/2016 04:01 PM, Joshua Brindle wrote:
> William Roberts wrote:
>> On Fri, Sep 23, 2016 at 6:57 AM, Joshua Brindle
>>   wrote:
>>> William Roberts wrote:
 On Sep 22, 2016 9:18 PM, "Jeffrey Vander Stoep"  
 wrote:
> Remember to test on the Mac build. About a year ago I moved the
> host side
 tools over to upstream libselinux, but had to revert because it
 broke the
 Mac build in multiple places. Since then Richard Haines has done a
 lot of
 work to reduce the diff between upstream and the Android fork.
 Hopefully
 that will reduce your effort.

 Yeah I'm quite concerned about the Mac build, does anyone on here have
 access to a Mac for testing?
>>>
>>> I do, let me know when you have a branch you need looked at and I'll
>>> try to
>>> get to it.
>>
>> Feel free to test the fork-kill branch from my github, you should find
>> the details
>> below from a previous message in the thread.
>>
> 
> Sure,
> 
> Mac uses llvm which seems to catch much more than gcc, I have to get rid
> of Werror to even get partially through a build. First issues:
> 
> genbools.c:71:9: warning: unused variable 'size' [-Wunused-variable]
> size_t size = 0;
>^
> 1 warning generated.
> cc -Wall -W -Wundef -Wshadow -Wmissing-format-attribute -O2 -I.
> -I../include -D_GNU_SOURCE -I../cil/include -fPIC -c -o genusers.o
> genusers.c
> genusers.c:39:9: warning: unused variable 'len' [-Wunused-variable]
> size_t len = 0;
>^
> genusers.c:63:14: warning: variable 'nread' is uninitialized when used
> here [-Wuninitialized]
> if (buffer[nread - 1] == '\n')
>^
> genusers.c:40:15: note: initialize the variable 'nread' to silence this
> warning
> ssize_t nread;
>  ^
>   = 0
> 
> but the .symver actually kills it altogether (I get a lot of these):
> 
> :10:1: error: unknown directive
> .symver cil_filecons_to_string_nopdb, cil_filecons_to_string@@LIBSEPOL_1.1
> ^

Are you building this in the context of Android?  We disable the symver
stuff automatically if ANDROID is defined.  We only need to ensure that
the portion of selinux userspace that is compiled for Android builds on
MacOS X; it will never fully build there.



___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH] Fix redefinition of XATTR_NAME_SELINUX

[Stephen Smalley]

On 09/21/2016 07:59 PM, william.c.robe...@intel.com wrote:
> From: William Roberts 
> 
> When the Kernel UAPI header is present, this error occurs:
> 
> external/selinux/libselinux/src/policy.h:7:9: warning: 'XATTR_NAME_SELINUX' 
> macro redefined [-Wmacro-redefined]
> \#define XATTR_NAME_SELINUX "security.selinux"
> ^
> bionic/libc/kernel/uapi/linux/xattr.h:52:9: note: previous definition is here
> \#define XATTR_NAME_SELINUX XATTR_SECURITY_PREFIX XATTR_SELINUX_SUFFIX
> 
> Just use the kernel UAPI version on that case.
> 
> Change-Id: I1b2d34e463477adaec227ac8c3364f1b9d49e997
> Signed-off-by: William Roberts 

Thanks, applied.

> ---
>  libselinux/src/policy.h | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/libselinux/src/policy.h b/libselinux/src/policy.h
> index bf270b5..f6d7242 100644
> --- a/libselinux/src/policy.h
> +++ b/libselinux/src/policy.h
> @@ -3,8 +3,13 @@
>  
>  /* Private definitions used internally by libselinux. */
>  
> -/* xattr name for SELinux attributes. */
> +/*
> + * xattr name for SELinux attributes.
> + * This may have been exported via Kernel uapi header.
> + */
> +#ifndef XATTR_NAME_SELINUX
>  #define XATTR_NAME_SELINUX "security.selinux"
> +#endif
>  
>  /* Initial length guess for getting contexts. */
>  #define INITCONTEXTLEN 255
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: unlocked stdio

[Stephen Smalley]

On 09/21/2016 07:47 PM, William Roberts wrote:
> Another thing I noticed rectifying the Android tree is that the
> selinux/Android.mk upstream is empty, but the secondary levels are
> present, any reason that hasn't been pushed?

No, just that no one has submitted them upstream.


___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: unlocked stdio

[Stephen Smalley]

On 09/21/2016 04:11 PM, William Roberts wrote:
> On Sep 21, 2016 13:06, "Stephen Smalley" <s...@tycho.nsa.gov
> <mailto:s...@tycho.nsa.gov>> wrote:
>>
>> On 09/21/2016 03:57 PM, Roberts, William C wrote:
>> > Correction, it’s just fgets_unlocked, it appears to support the others.
>>
>> Seems like a bug in bionic, but we can work around it by:
>> #ifdef ANDROID
>> #define fgets_unlocked(x) fgets(x)
>> #endif
>>
>> in selinux_internal.h or some similar internal header.
>>
>> It avoids unnecessary locking overheads when dealing with FILE
>> descriptors that are only used locally and guaranteed to not be shared
>> by multiple threads.
> 
> I know what it does and why, but was it really that necessary?

The patch came from Red Hat.  Anyway, we use the _unlocked functions
throughout, and the fact that bionic supports all of them except that
one function suggests that we should just use a fix like the above
rather than dropping it.


___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: unlocked stdio

[Stephen Smalley]

On 09/21/2016 03:57 PM, Roberts, William C wrote:
> Correction, it’s just fgets_unlocked, it appears to support the others.

Seems like a bug in bionic, but we can work around it by:
#ifdef ANDROID
#define fgets_unlocked(x) fgets(x)
#endif

in selinux_internal.h or some similar internal header.

It avoids unnecessary locking overheads when dealing with FILE
descriptors that are only used locally and guaranteed to not be shared
by multiple threads.

> 
>  
> 
> *From:* Roberts, William C
> *Sent:* Wednesday, September 21, 2016 12:53 PM
> *To:* 'seandroid-list@tycho.nsa.gov' ;
> 'seli...@tycho.nsa.gov' ; 's...@tycho.nsa.gov'
> 
> *Subject:* unlocked stdio
> 
>  
> 
> What was the purpose of using unlocked stdio in libselinux? Bionic
> doesn’t support it, is it really necessary?
> 
>  
> 
> Bill
> 
>  
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [RFC] mmap file_contexts and property_contexts:

[Stephen Smalley]

On 09/20/2016 02:27 AM, William Roberts wrote:
> On Sep 19, 2016 22:25, "Jason Zaman"  wrote:
>>
>> On 20 Sep 2016 12:50 pm, "William Roberts" 
> wrote:
>>>
>>> On Sep 19, 2016 21:16, "Jason Zaman"  wrote:

 On 20 Sep 2016 5:47 am,  wrote:
>
> From: William Roberts 
>
> THIS IS WIP...
>
> Rather than using stdio and making copies, just mmap the files
> and use the pointers in place. The affect of this change, is that
> text file load time is now faster than binary load time by 4.7%
> when testing with a file_contexts file from the Android tree. Note
> that the Android doesn't use monstrous regexs.
>
> Times are the average of 3 runs.
>
> BEFORE:
> Text file allocs: 114803
> Text file load time: 0.266101
> Bin file allocs: 93073
> Bin file load time: 0.248757667
>
> AFTER:
> Text file allocs: 103933
> Text file load time: 0.236192667
> Bin file allocs: 87645
> Bin file load time: .247607333

 Do you have the scripts that generated these stats so I can play with
> it too? These stats are from android right? Do you also have a comparison
> for refpolicy too?
>>>
>>> For generating these I used checkfc.c from the Android tree. I used
> valgrind to measure allocations and clock to measure the time in
> selabel_open().
>>
>> Okay cool I'll fetch that and give it a whirl when I get time.
>>

 I haven't looked that closely yet but just realised, will this need
> new perms because of the mmap? If it does, can you send a patch to
> refpolicy?
>>>
>>> I'm confused, mmap is not a permission, even if it was the binary path
> already was doing an mmap, so the permission would have been there. We're
> just making it so it always mmaps.
>>
>> Yeah but mmap needs execute perms sometimes (always?). I am out so just
> wanted to send an email before I forgot. If it was mmaping already then
> there is nothing to worry about :).
> 
> Mmap would only need execute if you attempted to set the prot bits to
> execute it use mprotect to change the mapping. Then things like execmod
> might come I to play if the mapping was ever writable.

The only case where mmap without PROT_EXEC would require execute would
be if the process has READ_IMPLIES_EXEC set in its personality.
Typically only for programs with the executable stack flag set.

Anyway, it is already mmap'ing file_contexts.bin so there shouldn't be
an issue here.



___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH v5] libselinux: correct error path to always try text

[Stephen Smalley]

On 09/16/2016 03:37 PM, william.c.robe...@intel.com wrote:
> From: William Roberts 
> 
> patch 5e15a52aaa cleans up the process_file() routine,
> but introduced a bug. If the binary file cannot be
> opened, always attempt to fall back to the textual file,
> this was not occurring.
> 
> The logic should be:
> 1. Open the newest file between base path + suffix and
>base_path + suffix + ".bin"
> 2. If anything fails, attempt to load the oldest file.
> 
> The result, with a concrete example, would be:
> If file_contexts is the newest file, and it cannot be
> processed, the code will fall back to file_contexts.bin
> and vice versa.
> 
> Signed-off-by: William Roberts 

Thanks, applied.

> ---
>  libselinux/src/label_file.c | 47 
> ++---
>  1 file changed, 32 insertions(+), 15 deletions(-)
> 
> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
> index 9faecdb..ff6bc94 100644
> --- a/libselinux/src/label_file.c
> +++ b/libselinux/src/label_file.c
> @@ -447,7 +447,7 @@ static bool fcontext_is_binary(FILE *fp)
>  #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>  
>  static FILE *open_file(const char *path, const char *suffix,
> -char *save_path, size_t len, struct stat *sb)
> +char *save_path, size_t len, struct stat *sb, bool open_oldest)
>  {
>   unsigned int i;
>   int rc;
> @@ -493,9 +493,15 @@ static FILE *open_file(const char *path, const char 
> *suffix,
>* includes equality. This provides a precedence on
>* secondary suffixes even when the timestamp is the
>* same. Ie choose file_contexts.bin over file_contexts
> -  * even if the time stamp is the same.
> +  * even if the time stamp is the same. Invert this logic
> +  * on open_oldest set to true. The idea is that if the
> +  * newest file failed to process, we can attempt to
> +  * process the oldest. The logic here is subtle and depends
> +  * on the array ordering in fdetails for the case when time
> +  * stamps are the same.
>*/
> - if (fdetails[i].sb.st_mtime >= found->sb.st_mtime) {
> + if (open_oldest ^
> + (fdetails[i].sb.st_mtime >= found->sb.st_mtime)) {
>   found = [i];
>   strcpy(save_path, path);
>   }
> @@ -515,24 +521,35 @@ static int process_file(const char *path, const char 
> *suffix,
> const char *prefix, struct selabel_digest *digest)
>  {
>   int rc;
> + unsigned int i;
>   struct stat sb;
>   FILE *fp = NULL;
>   char found_path[PATH_MAX];
>  
> - fp = open_file(path, suffix, found_path, sizeof(found_path), );
> - if (fp == NULL)
> - return -1;
> + /*
> +  * On the first pass open the newest modified file. If it fails to
> +  * process, then the second pass shall open the oldest file. If both
> +  * passes fail, then it's a fatal error.
> +  */
> + for (i = 0; i < 2; i++) {
> + fp = open_file(path, suffix, found_path, sizeof(found_path),
> + , i > 0);
> + if (fp == NULL)
> + return -1;
>  
> - rc = fcontext_is_binary(fp) ?
> - load_mmap(fp, sb.st_size, rec, found_path) :
> - process_text_file(fp, prefix, rec, found_path);
> - if (rc < 0)
> - goto out;
> + rc = fcontext_is_binary(fp) ?
> + load_mmap(fp, sb.st_size, rec, found_path) :
> + process_text_file(fp, prefix, rec, found_path);
> + if (!rc)
> + rc = digest_add_specfile(digest, fp, NULL, sb.st_size,
> + found_path);
>  
> - rc = digest_add_specfile(digest, fp, NULL, sb.st_size, found_path);
> -out:
> - fclose(fp);
> - return rc;
> + fclose(fp);
> +
> + if (!rc)
> + return 0;
> + }
> + return -1;
>  }
>  
>  static void closef(struct selabel_handle *rec);
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH v3] libselinux: correct error path to always try text

[Stephen Smalley]

On 09/16/2016 01:06 PM, William Roberts wrote:
> On Fri, Sep 16, 2016 at 8:04 AM, William Roberts
> <bill.c.robe...@gmail.com> wrote:
>> On Fri, Sep 16, 2016 at 8:00 AM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
>>> On 09/16/2016 10:44 AM, William Roberts wrote:
>>>> On Fri, Sep 16, 2016 at 7:41 AM, William Roberts
>>>> <bill.c.robe...@gmail.com> wrote:
>>>>> On Fri, Sep 16, 2016 at 7:38 AM, Stephen Smalley <s...@tycho.nsa.gov> 
>>>>> wrote:
>>>>>> On 09/16/2016 10:30 AM, Stephen Smalley wrote:
>>>>>>> On 09/15/2016 07:13 PM, william.c.robe...@intel.com wrote:
>>>>>>>> From: William Roberts <william.c.robe...@intel.com>
>>>>>>>>
>>>>>>>> patch 5e15a52aaa cleans up the process_file() but introduced
>>>>>>>> a bug. If the binary file cannot be opened, always attempt
>>>>>>>> to fall back to the textual file, this was not occurring.
>>>>>>>>
>>>>>>>> The logic should be:
>>>>>>>> 1. Open the newest file based on base path + suffix vs
>>>>>>>>base_path + suffix + ".bin".
>>>>>>>> 2. If anything fails, attempt base_path + suffix.
>>>>>>>>
>>>>>>>> In the case that the file_contexts was the newest file and
>>>>>>>> used for processing fails, it will attempt the same failure
>>>>>>>> recovery, which will fail. It was decided to keep it this
>>>>>>>> was for simplicity.
>>>>>>>
>>>>>>> I don't like the approach.  What we want is:
>>>>>>> - if .bin file exists and is not older, try to load it,
>>>>>>> - if any of the above fails (i.e. .bin file does not exist, is older, or
>>>>>>> cannot be loaded for any reason), then load the text file.
>>>>>>>
>>>>>>> We shouldn't try loading the text file twice.
>>>>>>>
>>>>>>> Also, attached is checkpatch output for your patch.  Please fix.
>>>>>>
>>>>>> Also, there is a further wrinkle: Android passes in file_contexts.bin as
>>>>>> the SELABEL_OPT_PATH, so that is the base path.  Under the old logic
>>>>>> (before your original clean up patch), we would open that file, detect
>>>>>> that it is binary, and then load it.  Under the current logic, we'll
>>>>>> open file_contexts.bin, then try to open file_contexts.bin.bin (which
>>>>>> will fail), and then use the first one.
>>>>>
>>>>> Not true, I don't try to open it, I try to stat it.
>>>>
>>>> My code never assumes file suffix == type
>>>>
>>>>>
>>>>>>
>>>>>> Wondering if we just need to revert.
>>>>>
>>>>> If you want to revert I have no problem with that, but I provided IMO
>>>>> a valid fix.
>>>>> Since I won't likely have a next version patch out till after you go
>>>>> home today, I
>>>>> would support reverting.
>>>
>>> Unfortunately it is now entangled with Janis' patch.  Let's do this: fix
>>> the coding style issues I sent to you from checkpatch, and we'll take
>>> this one.  Then we'll look to avoid the extraneous load in a subsequent
>>> patch.
>>
>> Fine by me, i'm running to an appointment, I wont have that patch out to
>> probably 3-4pm your time.
> 
> BTW did you not get v3 of this patch?

I did - that's what I ran through checkpatch.pl and sent you the output.

> I also thought about the additional load attempt even on "textual"
> files, and I would
> argue we keep it with a slight modification. The boolean flag passed
> to open_file should really
> by called, choose oldest file and invert the time comparison logic.
> This way, if one file
> is corrupted, we always attempt to load the other file. Also, all of
> this code is agnostic to
> file extension determining content type. This code, with that change
> would work in the
> case file_contexts is newer and corrupted, it will try to fall back on
> binary file.

Ok.  Just remember that the timestamps might be the same.

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH] Change semantic of -r in sefcontext_compile

[Stephen Smalley]

On 09/16/2016 11:08 AM, William Roberts wrote:
> On Fri, Sep 16, 2016 at 7:41 AM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
>> On 09/16/2016 09:08 AM, Janis Danisevskis wrote:
>>> This patch reestablishes the default behavior of sefcontext_compile
>>> to include precompiled regular expressions in the output. If linked
>>> against PCRE2 the flag "-r" now causes the precompiled regular
>>> expressions to be omitted from the output.
>>
>> I thought your original rationale was more compelling.  If we add
>> detection of the relevant arch properties, then we can do this.
>> Otherwise, I don't think we should.
> 
> I was assuming based on the thread earlier that those patches would be coming.
> If we cant detect and compile on the current "undefined behavior"
> case, then this
> needs to stay as is.
> 
> But I thought someone had a list of PCRE things that can be checked for 
> "arch",
> so its just a matter of encoding those, assuming that list is correct.
> 
> Binary file_contexts only make sense if you compile in the regex info, else
> just use the textual representation.

That was my thought originally, but Janis did say that it was still
faster, and Android presently only ships file_contexts.bin, so we can't
just break that.


___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH] libselinux: add support for pcre2

[Stephen Smalley]

On 09/16/2016 09:31 AM, Jason Zaman wrote:
> On Fri, Sep 16, 2016 at 06:15:01AM -0700, William Roberts wrote:
>> On Fri, Sep 16, 2016 at 6:09 AM, Janis Danisevskis  wrote:
>>> I don't mind. Then before sefcontext_compile -r gets widely adapted we
>>> should change the semantic quickly. I'll prepare a patch.
>>
>> Did I miss something and this was merged? Iv'e been out recovering
>> from a surgery so I haven't been
>> following this as well as I normally would have,
>>
>> If its merged, just leave it.
> 
> Its the very latest thing in master yeah, but I do also agree with changing 
> it.
> 
> I just wanted to add that from a distro perspective, compiling things by
> default makes more sense. In gentoo, the package post_install runs
> sefcontext_compile. Using the fcontext files happens a lot more than any
> updates to libselinux (and thus potential format changes) so I'm pretty
> sure most people would prefer to have the speedup.
> 
> Gentoo does it on the machine itself, I am not sure about redhat or
> debian but I wouldnt be surprised if they do it per-arch at the very
> least so cross-arch probably isnt an issue.

In Red Hat, SELinux policy is noarch, and they switched to precompiling
both policy and file_contexts.bin at build time to minimize the cost at
package install time.  Otherwise, in small VMs, they had issues with
running out of memory during semodule -B.  So file_contexts.bin
presently has to be arch-independent, or we need the arch properties
detection logic and fallback.  That said, none of this matters unless
you build with USE_PCRE2=y, and no one outside of Android is doing that
today.

> Also, I think we should add the arch to the version string stored. I
> would rather have false negatives than positives especially since we are
> not 100% sure exactly what part of the arch is important. We can always
> loosen it up later if that gets locked down.

We don't want the arch string itself, because that would invalidate use
of file_contexts.bin entirely on typical Android use cases (build on
x86_64, install to ARM), but only the relevant properties.  And for
Android, that is fatal - there is no file_contexts text file on which to
fallback anymore.  They only ship file_contexts.bin.

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH] Change semantic of -r in sefcontext_compile

[Stephen Smalley]

On 09/16/2016 09:08 AM, Janis Danisevskis wrote:
> This patch reestablishes the default behavior of sefcontext_compile
> to include precompiled regular expressions in the output. If linked
> against PCRE2 the flag "-r" now causes the precompiled regular
> expressions to be omitted from the output.

I thought your original rationale was more compelling.  If we add
detection of the relevant arch properties, then we can do this.
Otherwise, I don't think we should.

> ---
>  libselinux/utils/sefcontext_compile.c | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/libselinux/utils/sefcontext_compile.c 
> b/libselinux/utils/sefcontext_compile.c
> index 770ec4c..c1284d5 100644
> --- a/libselinux/utils/sefcontext_compile.c
> +++ b/libselinux/utils/sefcontext_compile.c
> @@ -263,12 +263,10 @@ static void usage(const char *progname)
>   " will be fc_file with the .bin suffix appended.\n\t"
>   "-p   Optional binary policy file that will be used to\n\t"
>   " validate contexts defined in the fc_file.\n\t"
> - "-r   Include precompiled regular expressions in the 
> output.\n\t"
> + "-r   Omit precompiled regular expressions in the output.\n\t"
>   " (PCRE2 only. Compiled PCRE2 regular expressions are\n\t"
>   " not portable across architectures. When linked 
> against\n\t"
>   " PCRE this flag is ignored)\n\t"
> - " Omit precompiled regular expressions (only meaningful\n\t"
> - " when using PCRE2 regular expression back-end).\n\t"
>   "fc_file  The text based file contexts file to be processed.\n",
>   progname);
>   exit(EXIT_FAILURE);
> @@ -278,7 +276,7 @@ int main(int argc, char *argv[])
>  {
>   const char *path = NULL;
>   const char *out_file = NULL;
> - int do_write_precompregex = 0;
> + int do_write_precompregex = 1;
>   char stack_path[PATH_MAX + 1];
>   char *tmp = NULL;
>   int fd, rc, opt;
> @@ -299,7 +297,7 @@ int main(int argc, char *argv[])
>   policy_file = optarg;
>   break;
>   case 'r':
> - do_write_precompregex = 1;
> + do_write_precompregex = 0;
>   break;
>   default:
>   usage(argv[0]);
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH v3] libselinux: correct error path to always try text

[Stephen Smalley]

On 09/16/2016 10:30 AM, Stephen Smalley wrote:
> On 09/15/2016 07:13 PM, william.c.robe...@intel.com wrote:
>> From: William Roberts <william.c.robe...@intel.com>
>>
>> patch 5e15a52aaa cleans up the process_file() but introduced
>> a bug. If the binary file cannot be opened, always attempt
>> to fall back to the textual file, this was not occurring.
>>
>> The logic should be:
>> 1. Open the newest file based on base path + suffix vs
>>base_path + suffix + ".bin".
>> 2. If anything fails, attempt base_path + suffix.
>>
>> In the case that the file_contexts was the newest file and
>> used for processing fails, it will attempt the same failure
>> recovery, which will fail. It was decided to keep it this
>> was for simplicity.
> 
> I don't like the approach.  What we want is:
> - if .bin file exists and is not older, try to load it,
> - if any of the above fails (i.e. .bin file does not exist, is older, or
> cannot be loaded for any reason), then load the text file.
> 
> We shouldn't try loading the text file twice.
> 
> Also, attached is checkpatch output for your patch.  Please fix.

Also, there is a further wrinkle: Android passes in file_contexts.bin as
the SELABEL_OPT_PATH, so that is the base path.  Under the old logic
(before your original clean up patch), we would open that file, detect
that it is binary, and then load it.  Under the current logic, we'll
open file_contexts.bin, then try to open file_contexts.bin.bin (which
will fail), and then use the first one.

Wondering if we just need to revert.

> 
>>
>> Signed-off-by: William Roberts <william.c.robe...@intel.com>
>> ---
>>  libselinux/src/label_file.c | 41 +++--
>>  1 file changed, 27 insertions(+), 14 deletions(-)
>>
>> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
>> index 9faecdb..0dee059 100644
>> --- a/libselinux/src/label_file.c
>> +++ b/libselinux/src/label_file.c
>> @@ -447,7 +447,7 @@ static bool fcontext_is_binary(FILE *fp)
>>  #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>>  
>>  static FILE *open_file(const char *path, const char *suffix,
>> -   char *save_path, size_t len, struct stat *sb)
>> +   char *save_path, size_t len, struct stat *sb, bool 
>> force_text)
>>  {
>>  unsigned int i;
>>  int rc;
>> @@ -469,7 +469,11 @@ static FILE *open_file(const char *path, const char 
>> *suffix,
>>  return NULL;
>>  }
>>  
>> -for (i = 0; i < ARRAY_SIZE(fdetails); i++) {
>> +size_t array_size = ARRAY_SIZE(fdetails);
>> +if (force_text)
>> +array_size--;
>> +
>> +for (i = 0; i < array_size; i++) {
>>  
>>  /* This handles the case if suffix is null */
>>  path = rolling_append(stack_path, fdetails[i].suffix,
>> @@ -515,24 +519,33 @@ static int process_file(const char *path, const char 
>> *suffix,
>>const char *prefix, struct selabel_digest *digest)
>>  {
>>  int rc;
>> +unsigned int i;
>>  struct stat sb;
>>  FILE *fp = NULL;
>>  char found_path[PATH_MAX];
>>  
>> -fp = open_file(path, suffix, found_path, sizeof(found_path), );
>> -if (fp == NULL)
>> -return -1;
>> +/*
>> + * first path open the newest modified file, if it fails, the second
>> + * pass falls through to the plain text file.
>> + */
>> +for(i=0; i < 2; i++) {
>> +fp = open_file(path, suffix, found_path, sizeof(found_path), 
>> ,
>> +i > 0);
>> +if (fp == NULL)
>> +return -1;
>>  
>> -rc = fcontext_is_binary(fp) ?
>> -load_mmap(fp, sb.st_size, rec, found_path) :
>> -process_text_file(fp, prefix, rec, found_path);
>> -if (rc < 0)
>> -goto out;
>> +rc = fcontext_is_binary(fp) ?
>> +load_mmap(fp, sb.st_size, rec, found_path) :
>> +process_text_file(fp, prefix, rec, found_path);
>> +if (!rc)
>> +rc = digest_add_specfile(digest, fp, NULL, sb.st_size, 
>> found_path);
>>  
>> -rc = digest_add_specfile(digest, fp, NULL, sb.st_size, found_path);
>> -out:
>> -fclose(fp);
>> -return rc;
>> +fclose(fp);
>> +
>> +if(!rc)
>> +return 0;
>> +}
>> +return -1;
>>  }
>>  
>>  static void closef(struct selabel_handle *rec);
>>
> 
> 
> 
> ___
> Selinux mailing list
> seli...@tycho.nsa.gov
> To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
> To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH v3] libselinux: correct error path to always try text

[Stephen Smalley]

On 09/15/2016 07:13 PM, william.c.robe...@intel.com wrote:
> From: William Roberts 
> 
> patch 5e15a52aaa cleans up the process_file() but introduced
> a bug. If the binary file cannot be opened, always attempt
> to fall back to the textual file, this was not occurring.
> 
> The logic should be:
> 1. Open the newest file based on base path + suffix vs
>base_path + suffix + ".bin".
> 2. If anything fails, attempt base_path + suffix.
> 
> In the case that the file_contexts was the newest file and
> used for processing fails, it will attempt the same failure
> recovery, which will fail. It was decided to keep it this
> was for simplicity.

I don't like the approach.  What we want is:
- if .bin file exists and is not older, try to load it,
- if any of the above fails (i.e. .bin file does not exist, is older, or
cannot be loaded for any reason), then load the text file.

We shouldn't try loading the text file twice.

Also, attached is checkpatch output for your patch.  Please fix.

> 
> Signed-off-by: William Roberts 
> ---
>  libselinux/src/label_file.c | 41 +++--
>  1 file changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
> index 9faecdb..0dee059 100644
> --- a/libselinux/src/label_file.c
> +++ b/libselinux/src/label_file.c
> @@ -447,7 +447,7 @@ static bool fcontext_is_binary(FILE *fp)
>  #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>  
>  static FILE *open_file(const char *path, const char *suffix,
> -char *save_path, size_t len, struct stat *sb)
> +char *save_path, size_t len, struct stat *sb, bool 
> force_text)
>  {
>   unsigned int i;
>   int rc;
> @@ -469,7 +469,11 @@ static FILE *open_file(const char *path, const char 
> *suffix,
>   return NULL;
>   }
>  
> - for (i = 0; i < ARRAY_SIZE(fdetails); i++) {
> + size_t array_size = ARRAY_SIZE(fdetails);
> + if (force_text)
> + array_size--;
> +
> + for (i = 0; i < array_size; i++) {
>  
>   /* This handles the case if suffix is null */
>   path = rolling_append(stack_path, fdetails[i].suffix,
> @@ -515,24 +519,33 @@ static int process_file(const char *path, const char 
> *suffix,
> const char *prefix, struct selabel_digest *digest)
>  {
>   int rc;
> + unsigned int i;
>   struct stat sb;
>   FILE *fp = NULL;
>   char found_path[PATH_MAX];
>  
> - fp = open_file(path, suffix, found_path, sizeof(found_path), );
> - if (fp == NULL)
> - return -1;
> + /*
> +  * first path open the newest modified file, if it fails, the second
> +  * pass falls through to the plain text file.
> +  */
> + for(i=0; i < 2; i++) {
> + fp = open_file(path, suffix, found_path, sizeof(found_path), 
> ,
> + i > 0);
> + if (fp == NULL)
> + return -1;
>  
> - rc = fcontext_is_binary(fp) ?
> - load_mmap(fp, sb.st_size, rec, found_path) :
> - process_text_file(fp, prefix, rec, found_path);
> - if (rc < 0)
> - goto out;
> + rc = fcontext_is_binary(fp) ?
> + load_mmap(fp, sb.st_size, rec, found_path) :
> + process_text_file(fp, prefix, rec, found_path);
> + if (!rc)
> + rc = digest_add_specfile(digest, fp, NULL, sb.st_size, 
> found_path);
>  
> - rc = digest_add_specfile(digest, fp, NULL, sb.st_size, found_path);
> -out:
> - fclose(fp);
> - return rc;
> + fclose(fp);
> +
> + if(!rc)
> + return 0;
> + }
> + return -1;
>  }
>  
>  static void closef(struct selabel_handle *rec);
> 

WARNING: line over 80 characters
#84: FILE: libselinux/src/label_file.c:450:
+  char *save_path, size_t len, struct stat *sb, bool 
force_text)

WARNING: Missing a blank line after declarations
#94: FILE: libselinux/src/label_file.c:473:
+   size_t array_size = ARRAY_SIZE(fdetails);
+   if (force_text)

ERROR: spaces required around that '=' (ctx:VxV)
#117: FILE: libselinux/src/label_file.c:531:
+   for(i=0; i < 2; i++) {
 ^

ERROR: space required before the open parenthesis '('
#117: FILE: libselinux/src/label_file.c:531:
+   for(i=0; i < 2; i++) {

WARNING: line over 80 characters
#118: FILE: libselinux/src/label_file.c:532:
+   fp = open_file(path, suffix, found_path, sizeof(found_path), 
,

WARNING: line over 80 characters
#132: FILE: libselinux/src/label_file.c:541:
+   rc = digest_add_specfile(digest, fp, NULL, sb.st_size, 
found_path);

ERROR: space required before the open parenthesis '('
#140: FILE: libselinux/src/label_file.c:545:
+   if(!rc)

total: 3 

Re: [PATCH] libselinux: add support for pcre2

[Stephen Smalley]

On 09/15/2016 12:14 PM, Janis Danisevskis wrote:
> From: Janis Danisevskis <jda...@google.com>
> 
> This patch moves all pcre1/2 dependencies into the new files regex.h
> and regex.c implementing the common denominator of features needed
> by libselinux. The compiler flag -DUSE_PCRE2 toggles between the
> used implementations.
> 
> As of this patch libselinux supports either pcre or pcre2 but not
> both at the same time. The persistently stored file contexts
> information differs. This means libselinux can only load file
> context files generated by sefcontext_compile build with the
> same pcre variant.
> 
> Also, for pcre2 the persistent format is architecture dependent.
> Stored precompiled regular expressions can only be used on the
> same architecture they were generated on. If pcre2 is used,
> sefcontext_compile now respects the "-r". This flag makes
> sefcontext_compile include the precompiled regular expressions
> in the output file. The default is to omit them, so that the
> output remains portable at the cost of having to recompile
> the regular expressions at load time, or rather on first use.
> 
> Signed-off-by: Janis Danisevskis <jda...@google.com>

Thanks, applied, with the attached fix on top to allow building.
>From a9162c813adaadbdb632d1a71d7c6ffc3e43b1b0 Mon Sep 17 00:00:00 2001
From: Stephen Smalley <s...@tycho.nsa.gov>
Date: Thu, 15 Sep 2016 13:43:24 -0400
Subject: [PATCH] libselinux: regex_writef: Mark unused argument with
 __attribute__((unused)).

Signed-off-by: Stephen Smalley <s...@tycho.nsa.gov>
---
 libselinux/src/regex.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c
index 646351b..750088e 100644
--- a/libselinux/src/regex.c
+++ b/libselinux/src/regex.c
@@ -312,7 +312,8 @@ static inline pcre_extra *get_pcre_extra(struct regex_data *regex)
 	}
 }
 
-int regex_writef(struct regex_data *regex, FILE *fp, int unused)
+int regex_writef(struct regex_data *regex, FILE *fp,
+		 int unused __attribute__((unused)))
 {
 	int rc;
 	size_t len;
-- 
2.7.4

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH] libselinux: add support for pcre2

[Stephen Smalley]

On 09/15/2016 10:04 AM, Janis Danisevskis wrote:
> From: Janis Danisevskis 
> 
> This patch moves all pcre1/2 dependencies into the new files regex.h
> and regex.c implementing the common denominator of features needed
> by libselinux. The compiler flag -DUSE_PCRE2 toggles between the
> used implementations.
> 
> As of this patch libselinux supports either pcre or pcre2 but not
> both at the same time. The persistently stored file contexts
> information differs. This means libselinux can only load file
> context files generated by sefcontext_compile build with the
> same pcre variant.
> 
> Also, for pcre2 the persistent format is architecture dependent.
> Stored precompiled regular expressions can only be used on the
> same architecture they were generated on. If pcre2 is used,
> sefcontext_compile now respects the "-r". This flag makes
> sefcontext_compile include the precompiled regular expressions
> in the output file. The default is to omit them, so that the
> output remains portable at the cost of having to recompile
> the regular expressions at load time, or rather on first use.

Is that really the default behavior you want?

> Signed-off-by: Janis Danisevskis 
> ---

> diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h
> index 6d1e890..3df7972 100644
> --- a/libselinux/src/label_file.h
> +++ b/libselinux/src/label_file.h
> @@ -453,12 +429,14 @@ static inline int process_line(struct selabel_handle 
> *rec,
>*/
>   data->nspec++;
>  
> - if (rec->validating &&
> - compile_regex(data, _arr[nspec], )) {
> + if (rec->validating
> + && compile_regex(data, _arr[nspec], _data)) {
> + regex_format_error(_data, regex_error_format_buffer,
> + sizeof(regex_error_format_buffer));
>   COMPAT_LOG(SELINUX_ERROR,
>  "%s:  line %u has invalid regex %s:  %s\n",
>  path, lineno, regex,
> -(errbuf ? errbuf : "out of memory"));
> +regex_error_format_buffer);

compile_regex() may fail on an out of memory condition before
regex_error_format_buffer is initialized, which is why we previously
passed errbuf ?: "out of memory" above.  I suppose you could initialize
regex_error_format_buffer with "out of memory" prior to calling
compile_regex().

> diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c
> new file mode 100644
> index 000..1c4a84d
> --- /dev/null
> +++ b/libselinux/src/regex.c

> +int regex_writef(struct regex_data *regex, FILE *fp)

This needs to be updated with the new do_write_precompregex argument,
and either use the argument or mark it unused to permit compilation for
the USE_PCRE2=n.

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH v3] libselinux: clean up process file

[Stephen Smalley]

On 09/09/2016 02:27 PM, Stephen Smalley wrote:
> On 09/09/2016 01:44 PM, william.c.robe...@intel.com wrote:
>> From: William Roberts <william.c.robe...@intel.com>
>>
>> The current process_file() code will open the file
>> twice on the case of a binary file, correct this.
>>
>> The general flow through process_file() was a bit
>> difficult to read, streamline the routine to be
>> more readable.
>>
>> Detailed statistics of before and after:
>>
>> Source lines of code reported by cloc on modified files:
>> before: 735
>> after: 742
>>
>> Object size difference:
>> before: 195530 bytes
>> after:  195485 bytes
>>
>> Signed-off-by: William Roberts <william.c.robe...@intel.com>
> 
> Thanks, applied, with some whitespace and other fixes on top
> (scripts/checkpatch.pl from the kernel tree would have noted them).

Actually, there is a bug introduced by this patch: the old logic would
always fall back to loading the text file if anything goes wrong while
loading the binary file, and this is important for example if the binary
file format is not supported by libselinux.  Noticed this in testing the
pcre2 support patch; it was then failing on loading a file_contexts.bin
file compiled with pcre1 and then just failing completely rather than
falling back to file_contexts as before.  So we need to fix that.

> 
>> ---
>>  libselinux/src/label_file.c | 310 
>> 
>>  1 file changed, 166 insertions(+), 144 deletions(-)
>>
>> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
>> index c89bb35..94b7627 100644
>> --- a/libselinux/src/label_file.c
>> +++ b/libselinux/src/label_file.c
>> @@ -97,62 +97,40 @@ static int nodups_specs(struct saved_data *data, const 
>> char *path)
>>  return rc;
>>  }
>>  
>> -static int load_mmap(struct selabel_handle *rec, const char *path,
>> -struct stat *sb, bool isbinary,
>> -struct selabel_digest *digest)
>> +static int process_text_file(FILE *fp, const char *prefix, struct 
>> selabel_handle *rec, const char *path)
>> +{
>> +int rc;
>> +size_t line_len;
>> +unsigned lineno = 0;
>> +char *line_buf = NULL;
>> +
>> +while (getline(_buf, _len, fp) > 0) {
>> +rc = process_line(rec, path, prefix, line_buf, ++lineno);
>> +if (rc)
>> +goto out;
>> +}
>> +rc = 0;
>> +out:
>> +free(line_buf);
>> +return rc;
>> +}
>> +
>> +static int load_mmap(FILE *fp, size_t len, struct selabel_handle *rec, 
>> const char *path)
>>  {
>>  struct saved_data *data = (struct saved_data *)rec->data;
>> -char mmap_path[PATH_MAX + 1];
>> -int mmapfd;
>>  int rc;
>> -struct stat mmap_stat;
>>  char *addr, *str_buf;
>> -size_t len;
>>  int *stem_map;
>>  struct mmap_area *mmap_area;
>>  uint32_t i, magic, version;
>>  uint32_t entry_len, stem_map_len, regex_array_len;
>>  
>> -if (isbinary) {
>> -len = strlen(path);
>> -if (len >= sizeof(mmap_path))
>> -return -1;
>> -strcpy(mmap_path, path);
>> -} else {
>> -rc = snprintf(mmap_path, sizeof(mmap_path), "%s.bin", path);
>> -if (rc >= (int)sizeof(mmap_path))
>> -return -1;
>> -}
>> -
>> -mmapfd = open(mmap_path, O_RDONLY | O_CLOEXEC);
>> -if (mmapfd < 0)
>> -return -1;
>> -
>> -rc = fstat(mmapfd, _stat);
>> -if (rc < 0) {
>> -close(mmapfd);
>> -return -1;
>> -}
>> -
>> -/* if mmap is old, ignore it */
>> -if (mmap_stat.st_mtime < sb->st_mtime) {
>> -close(mmapfd);
>> -return -1;
>> -}
>> -
>> -/* ok, read it in... */
>> -len = mmap_stat.st_size;
>> -len += (sysconf(_SC_PAGE_SIZE) - 1);
>> -len &= ~(sysconf(_SC_PAGE_SIZE) - 1);
>> -
>>  mmap_area = malloc(sizeof(*mmap_area));
>>  if (!mmap_area) {
>> -close(mmapfd);
>>  return -1;
>>  }
>>  
>> -addr = mmap(NULL, len, PROT_READ, MAP_PRIVATE, mmapfd, 0);
>> -close(mmapfd);
>> +addr = mmap(NULL, len, PROT_READ, MAP_PRIVATE, fileno(fp), 0);
>>  if

Re: [PATCH v3] libselinux: clean up process file

[Stephen Smalley]

On 09/09/2016 01:44 PM, william.c.robe...@intel.com wrote:
> From: William Roberts 
> 
> The current process_file() code will open the file
> twice on the case of a binary file, correct this.
> 
> The general flow through process_file() was a bit
> difficult to read, streamline the routine to be
> more readable.
> 
> Detailed statistics of before and after:
> 
> Source lines of code reported by cloc on modified files:
> before: 735
> after: 742
> 
> Object size difference:
> before: 195530 bytes
> after:  195485 bytes
> 
> Signed-off-by: William Roberts 

Thanks, applied, with some whitespace and other fixes on top
(scripts/checkpatch.pl from the kernel tree would have noted them).

> ---
>  libselinux/src/label_file.c | 310 
> 
>  1 file changed, 166 insertions(+), 144 deletions(-)
> 
> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
> index c89bb35..94b7627 100644
> --- a/libselinux/src/label_file.c
> +++ b/libselinux/src/label_file.c
> @@ -97,62 +97,40 @@ static int nodups_specs(struct saved_data *data, const 
> char *path)
>   return rc;
>  }
>  
> -static int load_mmap(struct selabel_handle *rec, const char *path,
> - struct stat *sb, bool isbinary,
> - struct selabel_digest *digest)
> +static int process_text_file(FILE *fp, const char *prefix, struct 
> selabel_handle *rec, const char *path)
> +{
> + int rc;
> + size_t line_len;
> + unsigned lineno = 0;
> + char *line_buf = NULL;
> +
> + while (getline(_buf, _len, fp) > 0) {
> + rc = process_line(rec, path, prefix, line_buf, ++lineno);
> + if (rc)
> + goto out;
> + }
> + rc = 0;
> +out:
> + free(line_buf);
> + return rc;
> +}
> +
> +static int load_mmap(FILE *fp, size_t len, struct selabel_handle *rec, const 
> char *path)
>  {
>   struct saved_data *data = (struct saved_data *)rec->data;
> - char mmap_path[PATH_MAX + 1];
> - int mmapfd;
>   int rc;
> - struct stat mmap_stat;
>   char *addr, *str_buf;
> - size_t len;
>   int *stem_map;
>   struct mmap_area *mmap_area;
>   uint32_t i, magic, version;
>   uint32_t entry_len, stem_map_len, regex_array_len;
>  
> - if (isbinary) {
> - len = strlen(path);
> - if (len >= sizeof(mmap_path))
> - return -1;
> - strcpy(mmap_path, path);
> - } else {
> - rc = snprintf(mmap_path, sizeof(mmap_path), "%s.bin", path);
> - if (rc >= (int)sizeof(mmap_path))
> - return -1;
> - }
> -
> - mmapfd = open(mmap_path, O_RDONLY | O_CLOEXEC);
> - if (mmapfd < 0)
> - return -1;
> -
> - rc = fstat(mmapfd, _stat);
> - if (rc < 0) {
> - close(mmapfd);
> - return -1;
> - }
> -
> - /* if mmap is old, ignore it */
> - if (mmap_stat.st_mtime < sb->st_mtime) {
> - close(mmapfd);
> - return -1;
> - }
> -
> - /* ok, read it in... */
> - len = mmap_stat.st_size;
> - len += (sysconf(_SC_PAGE_SIZE) - 1);
> - len &= ~(sysconf(_SC_PAGE_SIZE) - 1);
> -
>   mmap_area = malloc(sizeof(*mmap_area));
>   if (!mmap_area) {
> - close(mmapfd);
>   return -1;
>   }
>  
> - addr = mmap(NULL, len, PROT_READ, MAP_PRIVATE, mmapfd, 0);
> - close(mmapfd);
> + addr = mmap(NULL, len, PROT_READ, MAP_PRIVATE, fileno(fp), 0);
>   if (addr == MAP_FAILED) {
>   free(mmap_area);
>   perror("mmap");
> @@ -227,7 +205,7 @@ static int load_mmap(struct selabel_handle *rec, const 
> char *path,
>   rc = next_entry(_len, mmap_area, sizeof(uint32_t));
>   if (rc < 0 || !stem_len) {
>   rc = -1;
> - goto err;
> + goto out;
>   }
>  
>   /* Check for stem_len wrap around. */
> @@ -236,15 +214,15 @@ static int load_mmap(struct selabel_handle *rec, const 
> char *path,
>   /* Check if over-run before null check. */
>   rc = next_entry(NULL, mmap_area, (stem_len + 1));
>   if (rc < 0)
> - goto err;
> + goto out;
>  
>   if (buf[stem_len] != '\0') {
>   rc = -1;
> - goto err;
> + goto out;
>   }
>   } else {
>   rc = -1;
> - goto err;
> + goto out;
>   }
>  
>   /* store the mapping between old and new */
> @@ -253,7 +231,7 @@ static int load_mmap(struct selabel_handle *rec, const 
> char *path,
>   newid = 

Re: [PATCH] libselinux: add support for pcre2

[Stephen Smalley]

On 09/08/2016 11:52 AM, Janis Danisevskis wrote:
> From: Janis Danisevskis 
> 
> This patch moves all pcre1/2 dependencies into the new files regex.h
> and regex.c implementing the common denominator of features needed
> by libselinux. The compiler flag -DUSE_PCRE2 toggles between the
> used implementations.
> 
> As of this patch libselinux supports either pcre or pcre2 but not
> both at the same time. The persistently stored file contexts
> information differs. This means libselinux can only load file
> context files generated by sefcontext_compile build with the
> same pcre variant.
> 
> Also, for pcre2 the persistent format is architecture dependent.
> Stored precompiled regular expressions can only be used on the
> same architecture they were generated on. If pcre2 is used and
> sefcontext_compile shall generate portable output, it and libselinux
> must be compiled with -DNO_PERSISTENTLY_STORED_PATTERNS, at the
> cost of having to recompile the regular expressions at load time.
> 
> Signed-off-by: Janis Danisevskis 
> 
> This patch includes includes:
> 
> libselinux: fix memory leak on pcre2
> 
> Introduced a malloc on pcre_version(). Libselinux
> expected this to be static, just use a static
> internal buffer.
> 
> Signed-off-by: William Roberts 
> ---
>  libselinux/Makefile   |  13 +
>  libselinux/src/Makefile   |   4 +-
>  libselinux/src/label_file.c   |  93 ++-
>  libselinux/src/label_file.h   |  59 ++---
>  libselinux/src/regex.c| 461 
> ++
>  libselinux/src/regex.h| 169 +
>  libselinux/utils/Makefile |   4 +-
>  libselinux/utils/sefcontext_compile.c |  55 +---
>  8 files changed, 697 insertions(+), 161 deletions(-)
>  create mode 100644 libselinux/src/regex.c
>  create mode 100644 libselinux/src/regex.h
> 
> diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
> index 37d01af..66687e6 100644
> --- a/libselinux/src/Makefile
> +++ b/libselinux/src/Makefile
> @@ -74,7 +74,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k 
> -Wformat-security -Winit-self -Wmissi
>-fipa-pure-const -Wno-suggest-attribute=pure 
> -Wno-suggest-attribute=const \
>-Werror -Wno-aggregate-return -Wno-redundant-decls
>  
> -override CFLAGS += -I../include -I$(INCLUDEDIR) -D_GNU_SOURCE $(EMFLAGS)
> +override CFLAGS += -I../include -I$(INCLUDEDIR) -D_GNU_SOURCE $(EMFLAGS) 
> $(PCRE_CFLAGS)
>  
>  SWIG_CFLAGS += -Wno-error -Wno-unused-variable -Wno-unused-but-set-variable 
> -Wno-unused-parameter \
>   -Wno-shadow -Wno-uninitialized -Wno-missing-prototypes 
> -Wno-missing-declarations
> @@ -113,7 +113,7 @@ $(LIBA): $(OBJS)
>   $(RANLIB) $@
>  
>  $(LIBSO): $(LOBJS)
> - $(CC) $(CFLAGS) -shared -o $@ $^ -lpcre -ldl $(LDFLAGS) -L$(LIBDIR) 
> -Wl,-soname,$(LIBSO),-z,defs,-z,relro
> + $(CC) $(CFLAGS) -shared -o $@ $^ $(PCRE_LDFLAGS) -ldl $(LDFLAGS) 
> -L$(LIBDIR) -Wl,-soname,$(LIBSO),-z,defs,-z,relro
>   ln -sf $@ $(TARGET) 
>  
>  $(LIBPC): $(LIBPC).in ../VERSION

We want make to still work in this subdirectory, so we need defaults
assigned to PCRE_CFLAGS and PCRE_LDFLAGS if not set by the caller.

> diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h
> index 6d1e890..24cb9e0 100644
> --- a/libselinux/src/label_file.h
> +++ b/libselinux/src/label_file.h
> @@ -394,7 +368,8 @@ static inline int process_line(struct selabel_handle *rec,
>   struct saved_data *data = (struct saved_data *)rec->data;
>   struct spec *spec_arr;
>   unsigned int nspec = data->nspec;
> - const char *errbuf = NULL;
> + char const *errbuf;

Unnecessary change?

> + struct regex_error_data error_data;
>  
>   items = read_spec_entries(line_buf, , 3, , , 
> );
>   if (items < 0) {
> @@ -454,7 +429,7 @@ static inline int process_line(struct selabel_handle *rec,
>   data->nspec++;
>  
>   if (rec->validating &&
> - compile_regex(data, _arr[nspec], )) {
> + compile_regex(data, _arr[nspec], _data)) 
> {

Same bug as before - errbuf was being used in the following log call, so
you need to change it to use regex_format_error().

>   COMPAT_LOG(SELINUX_ERROR,
>  "%s:  line %u has invalid regex %s:  %s\n",
>  path, lineno, regex,
> diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c
> new file mode 100644
> index 000..558a72a
> --- /dev/null
> +++ b/libselinux/src/regex.c
> @@ -0,0 +1,461 @@
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "regex.h"
> +#include "label_file.h"
> +
> +#ifdef USE_PCRE2
> +int regex_prepare_data(struct regex_data ** regex,
> +  char const * pattern_string,
> +  struct regex_error_data * errordata) {

All of the non-static functions in this 

Re: [PATCH v2] libselinux: clean up process file

[Stephen Smalley]

On 09/06/2016 08:07 PM, william.c.robe...@intel.com wrote:
> From: William Roberts 
> 
> The current process_file() code will open the file
> twice on the case of a binary file, correct this.
> 
> The general flow through process_file() was a bit
> difficult to read, streamline the routine to be
> more readable.
> 
> Detailed statistics of before and after:
> 
> Source lines of code reported by cloc on modified files:
> before: 735
> after: 740
> 
> Object size difference:
> before: 195530 bytes
> after:  195529 bytes
> 
> Signed-off-by: William Roberts 
> ---
>  libselinux/src/label_file.c | 263 
> 
>  1 file changed, 145 insertions(+), 118 deletions(-)
> 
> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
> index c89bb35..6839a77 100644
> --- a/libselinux/src/label_file.c
> +++ b/libselinux/src/label_file.c
> @@ -97,62 +97,43 @@ static int nodups_specs(struct saved_data *data, const 
> char *path)
>   return rc;
>  }
>  
> -static int load_mmap(struct selabel_handle *rec, const char *path,
> - struct stat *sb, bool isbinary,
> - struct selabel_digest *digest)
> +static int process_text_file(FILE *fp, const char *prefix, struct 
> selabel_handle *rec, const char *path)
> +{
> + /*
> +  * Then do detailed validation of the input and fill the spec array
> +  */

You can drop this comment; it is no longer helpful/relevant.

> + int rc;
> + size_t line_len;
> + unsigned lineno = 0;
> + char *line_buf = NULL;
> +
> + while (getline(_buf, _len, fp) > 0) {
> + rc = process_line(rec, path, prefix, line_buf, ++lineno);
> + if (rc)
> + goto out;
> + }
> + rc = 0;
> +out:
> + free(line_buf);
> + return rc;
> +}
> +
> +static int load_mmap(FILE *fp, size_t len, struct selabel_handle *rec, const 
> char *path)
>  {
>   struct saved_data *data = (struct saved_data *)rec->data;
> - char mmap_path[PATH_MAX + 1];
> - int mmapfd;
>   int rc;
> - struct stat mmap_stat;
>   char *addr, *str_buf;
> - size_t len;
>   int *stem_map;
>   struct mmap_area *mmap_area;
>   uint32_t i, magic, version;
>   uint32_t entry_len, stem_map_len, regex_array_len;
>  
> - if (isbinary) {
> - len = strlen(path);
> - if (len >= sizeof(mmap_path))
> - return -1;
> - strcpy(mmap_path, path);
> - } else {
> - rc = snprintf(mmap_path, sizeof(mmap_path), "%s.bin", path);
> - if (rc >= (int)sizeof(mmap_path))
> - return -1;
> - }
> -
> - mmapfd = open(mmap_path, O_RDONLY | O_CLOEXEC);
> - if (mmapfd < 0)
> - return -1;
> -
> - rc = fstat(mmapfd, _stat);
> - if (rc < 0) {
> - close(mmapfd);
> - return -1;
> - }
> -
> - /* if mmap is old, ignore it */
> - if (mmap_stat.st_mtime < sb->st_mtime) {
> - close(mmapfd);
> - return -1;
> - }
> -
> - /* ok, read it in... */
> - len = mmap_stat.st_size;
> - len += (sysconf(_SC_PAGE_SIZE) - 1);
> - len &= ~(sysconf(_SC_PAGE_SIZE) - 1);
> -
>   mmap_area = malloc(sizeof(*mmap_area));
>   if (!mmap_area) {
> - close(mmapfd);
>   return -1;
>   }
>  
> - addr = mmap(NULL, len, PROT_READ, MAP_PRIVATE, mmapfd, 0);
> - close(mmapfd);
> + addr = mmap(NULL, len, PROT_READ, MAP_PRIVATE, fileno(fp), 0);
>   if (addr == MAP_FAILED) {
>   free(mmap_area);
>   perror("mmap");
> @@ -306,7 +287,7 @@ static int load_mmap(struct selabel_handle *rec, const 
> char *path,
>   if (strcmp(spec->lr.ctx_raw, "<>") && rec->validating) {
>   if (selabel_validate(rec, >lr) < 0) {
>   selinux_log(SELINUX_ERROR,
> - "%s: context %s is invalid\n", 
> mmap_path, spec->lr.ctx_raw);
> + "%s: context %s is invalid\n", 
> path, spec->lr.ctx_raw);
>   goto err;
>   }
>   }
> @@ -408,105 +389,151 @@ static int load_mmap(struct selabel_handle *rec, 
> const char *path,
>   data->nspec++;
>   }
>  
> - rc = digest_add_specfile(digest, NULL, addr, mmap_stat.st_size,
> - mmap_path);
> - if (rc)
> - goto err;
> -

We should explicitly set rc = 0 here.

>  err:

And maybe this should be out: since it is the only exit path and not
only for errors.

>   free(stem_map);
>  
>   return rc;
>  }
>  
> -static int process_file(const char *path, const char *suffix,
> -   struct selabel_handle *rec,
> -   

Re: [PATCH 1/2] libselinux: add support for pcre2

[Stephen Smalley]

On 09/07/2016 02:25 PM, Stephen Smalley wrote:
> On 09/07/2016 04:08 AM, Janis Danisevskis wrote:
>> From: Janis Danisevskis <jda...@google.com>
>>
>> This patch moves all pcre1/2 dependencies into the new files regex.h
>> and regex.c implementing the common denominator of features needed
>> by libselinux. The compiler flag -DUSE_PCRE2 toggles between the
>> used implementations.
>>
>> As of this patch libselinux supports either pcre or pcre2 but not
>> both at the same time. The persistently stored file contexts
>> information differs. This means libselinux can only load file
>> context files generated by sefcontext_compile build with the
>> same pcre variant.
>>
>> Also, for pcre2 the persistent format is architecture dependant.
>> Stored precompiled regular expressions can only be used on the
>> same architecture they were generated on. If pcre2 is used and
>> sefcontext_compile shall generate portable output, it and libselinux
>> must be compiled with -DNO_PERSISTENTLY_STORED_PATTERNS, at the
>> cost of having to recompile the regular expressions at load time.
>>
>> Signed-off-by: Janis Danisevskis <jda...@google.com>
>> ---
>>  libselinux/Makefile   |  13 ++
>>  libselinux/src/Makefile   |   4 +-
>>  libselinux/src/label_file.c   |  91 ++--
>>  libselinux/src/label_file.h   |  54 ++---
>>  libselinux/src/regex.c| 405 
>> ++
>>  libselinux/src/regex.h| 168 ++
>>  libselinux/utils/Makefile |   4 +-
>>  libselinux/utils/sefcontext_compile.c |  53 +
>>  8 files changed, 637 insertions(+), 155 deletions(-)
>>  create mode 100644 libselinux/src/regex.c
>>  create mode 100644 libselinux/src/regex.h
>>
> 
>> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
>> index c89bb35..6698624 100644
>> --- a/libselinux/src/label_file.c
>> +++ b/libselinux/src/label_file.c
>> @@ -278,7 +280,11 @@ static int load_mmap(struct selabel_handle *rec, const 
>> char *path,
>>  
>>  spec = >spec_arr[data->nspec];
>>  spec->from_mmap = 1;
>> +#if defined USE_PCRE2 && defined NO_PERSISTENTLY_STORED_PATTERNS
>> +spec->regcomp = 0;
>> +#else
>>  spec->regcomp = 1;
>> +#endif
> 
> If we still need this, maybe regex_load_mmap() should take
> >regcomp as an argument and set it internally so that we don't
> need to litter this file with #ifdefs?
> 
>> diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h
>> index 6d1e890..a2e30e5 100644
>> --- a/libselinux/src/label_file.h
>> +++ b/libselinux/src/label_file.h
>> @@ -394,7 +371,8 @@ static inline int process_line(struct selabel_handle 
>> *rec,
>>  struct saved_data *data = (struct saved_data *)rec->data;
>>  struct spec *spec_arr;
>>  unsigned int nspec = data->nspec;
>> -const char *errbuf = NULL;
>> +char const *errbuf;
>> +struct regex_error_data error_data;
>>  
>>  items = read_spec_entries(line_buf, , 3, , , 
>> );
>>  if (items < 0) {
>> @@ -454,7 +432,7 @@ static inline int process_line(struct selabel_handle 
>> *rec,
>>  data->nspec++;
>>  
>>  if (rec->validating &&
>> -compile_regex(data, _arr[nspec], )) {
>> +compile_regex(data, _arr[nspec], _data)) 
>> {
>>  COMPAT_LOG(SELINUX_ERROR,
>> "%s:  line %u has invalid regex %s:  %s\n",
>> path, lineno, regex,
> 
> On the next line (omitted from the diff) we pass errbuf if set as the
> error string.  But your error is hidden in error_data.  Looks like we
> need to use regex_format_error() here?
> 
>> diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c
>> new file mode 100644
>> index 000..6b92b04
>> --- /dev/null
>> +++ b/libselinux/src/regex.c
>> +int regex_load_mmap(struct mmap_area * mmap_area, struct regex_data ** 
>> regex) {
>> +int rc;
>> +size_t entry_len;
>> +#ifndef USE_PCRE2
>> +size_t info_len;
>> +#endif
>> +
>> +rc = next_entry(_len, mmap_area, sizeof(uint32_t));
> 
> This and similar statements are the cause of your uninitialised variable
> use warnings.  entry_len needs to be a uint32_t here.  size_t is 64 bits
> on 64-bit architectures.  Same for info_len.

Oops, sorry, 

Re: [PATCH] libselinux: clean up process file

[Stephen Smalley]

On 09/06/2016 04:06 PM, William Roberts wrote:
> On Sep 6, 2016 13:01, "Stephen Smalley" <s...@tycho.nsa.gov
> <mailto:s...@tycho.nsa.gov>> wrote:
>>
>> On 09/06/2016 11:51 AM, william.c.robe...@intel.com
> <mailto:william.c.robe...@intel.com> wrote:
>> > From: William Roberts <william.c.robe...@intel.com
> <mailto:william.c.robe...@intel.com>>
>> >
>> > The current process_file() code will open the file
>> > twice on the case of a binary file, correct this.
>> >
>> > The general flow through process_file() was a bit
>> > difficult to read, streamline the routine to be
>> > more readable.
>> >
>> > Detailed statistics of before and after:
>> >
>> > Source lines of code reported by cloc on modified files:
>> > before: 1090
>> > after: 1102
>> >
>> > Object size difference:
>> > before: 195530 bytes
>> > after:  195563 bytes
>>
>> Old logic would use the .bin file as long as it is not older than the
>> base file; new logic will only use the .bin file if it is newer.  The
>> end result on my system was that it was using the text file instead.
> 
> I'm not following here. I spent a lot of time with strace comparing old
> vs new behavior here. If x and x.bin exist, it should use x.bin if it's
> newer or the same age as x? Is that correct?

Yes.  strace before shows that it opens the text file and the bin file,
and then proceeds to map the bin file and close the text file without
processing it.  strace after shows that it stats them both and only
opens the text file.  The st_mtime values are the same (possibly we
ought to be comparing the full st_mtim with nanosecond precision
instead, but regardless, if equal, we ought to use the bin file) because
we now generate the bin file in the sandbox and then copy them both to
/etc/selinux at the same time (commit
a7334eb0de98af11ec38b6263536fa01bc2a606c).  You wouldn't see that if
your policy was last built/updated with the older selinux userspace
prior to that commit; if you run semodule -B after installing the
current userspace, you will get that behavior.

> 
>>
>> Also, there are some memory leaks in there; run it under valgrind, e.g.
>> valgrind --leak-check=full matchpathcon /etc
> 
> OK I'll run that test.
> 
>>
>> >
>> > Signed-off-by: William Roberts <william.c.robe...@intel.com
> <mailto:william.c.robe...@intel.com>>
>> > ---
>> >  libselinux/src/label_file.c | 264
> 
>> >  libselinux/src/label_file.h |   1 +
>> >  2 files changed, 147 insertions(+), 118 deletions(-)
>> >
>> > diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
>> > index c89bb35..26b1b87 100644
>> > --- a/libselinux/src/label_file.c
>> > +++ b/libselinux/src/label_file.c
>> > @@ -97,62 +97,44 @@ static int nodups_specs(struct saved_data *data,
> const char *path)
>> >   return rc;
>> >  }
>> >
>> > -static int load_mmap(struct selabel_handle *rec, const char *path,
>> > - struct stat *sb, bool isbinary,
>> > - struct selabel_digest *digest)
>> > +static int process_text_file(FILE *fp, const char *prefix, struct
> selabel_handle *rec)
>> > +{
>> > + /*
>> > +  * Then do detailed validation of the input and fill the spec
> array
>> > +  */
>> > + int rc;
>> > + size_t line_len;
>> > + unsigned lineno = 0;
>> > + char *line_buf = NULL;
>> > + struct saved_data *data = (struct saved_data *)rec->data;
>> > +
>> > + while (getline(_buf, _len, fp) > 0) {
>> > + rc = process_line(rec, data->path, prefix, line_buf,
> ++lineno);
>> > + if (rc)
>> > + goto out;
>> > + }
>> > + rc = 0;
>> > +out:
>> > + free(line_buf);
>> > + return rc;
>> > +}
>> > +
>> > +static int load_mmap(FILE *fp, size_t len, struct selabel_handle *rec)
>> >  {
>> >   struct saved_data *data = (struct saved_data *)rec->data;
>> > - char mmap_path[PATH_MAX + 1];
>> > - int mmapfd;
>> >   int rc;
>> > - struct stat mmap_stat;
>> >   char *addr, *str_buf;
>> > - size_t len;
>> >   int *stem_map;
>> >   struct mmap_area *mmap_area;
>> >   uin

Re: [PATCH 1/2] libsepol: calloc all the *_to_val_structs

[Stephen Smalley]

On 08/19/2016 11:13 AM, Roberts, William C wrote:
> 
> 
>> -Original Message-----
>> From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
>> Sent: Friday, August 19, 2016 6:06 AM
>> To: Roberts, William C <william.c.robe...@intel.com>; seli...@tycho.nsa.gov;
>> jwca...@tycho.nsa.gov; seandroid-list@tycho.nsa.gov
>> Subject: Re: [PATCH 1/2] libsepol: calloc all the *_to_val_structs
>>
>> On 08/18/2016 04:54 PM, william.c.robe...@intel.com wrote:
>>> From: William Roberts <william.c.robe...@intel.com>
>>>
>>> The usage patterns between these structures seem similair to
>>> role_val_to_struct usages. Calloc these up to prevent any unitialized
>>> usages.
>>>
>>> Signed-off-by: William Roberts <william.c.robe...@intel.com>
>>> ---
>>>  libsepol/src/mls.c  | 2 +-
>>>  libsepol/src/policydb.c | 6 +++---
>>>  libsepol/src/users.c| 9 -
>>>  3 files changed, 12 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/libsepol/src/mls.c b/libsepol/src/mls.c index
>>> 2dc5f2b..8047d91 100644
>>> --- a/libsepol/src/mls.c
>>> +++ b/libsepol/src/mls.c
>>> @@ -312,7 +312,7 @@ int mls_context_isvalid(const policydb_t * p, const
>> context_struct_t * c)
>>> if (!c->user || c->user > p->p_users.nprim)
>>> return 0;
>>> usrdatum = p->user_val_to_struct[c->user - 1];
>>> -   if (!mls_range_contains(usrdatum->exp_range, c->range))
>>> +   if (!usrdatum || !mls_range_contains(usrdatum->exp_range, c->range))
>>> return 0;   /* user may not be associated with range */
>>>
>>> return 1;
>>> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c index
>>> c225ac6..5f888d3 100644
>>> --- a/libsepol/src/policydb.c
>>> +++ b/libsepol/src/policydb.c
>>> @@ -1074,7 +1074,7 @@ int policydb_index_others(sepol_handle_t *
>>> handle,
>>>
>>> free(p->user_val_to_struct);
>>> p->user_val_to_struct = (user_datum_t **)
>>> -   malloc(p->p_users.nprim * sizeof(user_datum_t *));
>>> +   calloc(p->p_users.nprim, sizeof(user_datum_t *));
>>> if (!p->user_val_to_struct)
>>> return -1;
>>>
>>> @@ -4006,12 +4006,12 @@ int policydb_reindex_users(policydb_t * p)
>>> free(p->sym_val_to_name[i]);
>>>
>>> p->user_val_to_struct = (user_datum_t **)
>>> -   malloc(p->p_users.nprim * sizeof(user_datum_t *));
>>> +   calloc(p->p_users.nprim, sizeof(user_datum_t *));
>>> if (!p->user_val_to_struct)
>>> return -1;
>>>
>>> p->sym_val_to_name[i] = (char **)
>>> -   malloc(p->symtab[i].nprim * sizeof(char *));
>>> +   calloc(p->symtab[i].nprim, sizeof(char *));
>>> if (!p->sym_val_to_name[i])
>>> return -1;
>>>
>>> diff --git a/libsepol/src/users.c b/libsepol/src/users.c index
>>> ce54c2b..3ffb166 100644
>>> --- a/libsepol/src/users.c
>>> +++ b/libsepol/src/users.c
>>> @@ -19,12 +19,17 @@ static int user_to_record(sepol_handle_t * handle,
>>>
>>> const char *name = policydb->p_user_val_to_name[user_idx];
>>> user_datum_t *usrdatum = policydb->user_val_to_struct[user_idx];
>>> -   ebitmap_t *roles = &(usrdatum->roles.roles);
>>> +   ebitmap_t *roles;
>>> ebitmap_node_t *rnode;
>>> unsigned bit;
>>>
>>> sepol_user_t *tmp_record = NULL;
>>>
>>> +   if (!usrdatum)
>>> +   goto err;
>>> +
>>> +   roles = &(usrdatum->roles.roles);
>>> +
>>> if (sepol_user_create(handle, _record) < 0)
>>> goto err;
>>>
>>> @@ -234,6 +239,7 @@ int sepol_user_modify(sepol_handle_t * handle,
>>> if (!tmp_ptr)
>>> goto omem;
>>> policydb->user_val_to_struct = tmp_ptr;
>>> +   policydb->user_val_to_struct[policydb->p_users.nprim] = NULL;
>>>
>>> tmp_ptr = realloc(policydb->sym_val_to_name[SYM_USERS],
>>>   (policydb->p_users.nprim +
>>> @@ -241,6 +247,7 @@ int sepol_user_modify(sepol_handle_t * handle,
>>> if (!tmp_ptr)
>>> goto omem;
>>> policydb->sym_val_to_name[SYM_USERS] = tmp_ptr;
>>> +   policydb->p_user_val_to_name[policydb->p_users.nprim] =
>> NULL;
>>
>> This one is wrong.
>>
>>>
>>> /* Need to copy the user name */
>>> name = strdup(cname);
>>>
> 
> After looking at the email and the patch again, that’s just a context hunk, I 
> see no + or - next it,
> And I verified it still exists in my source tree. I never touched that hunk 
> or  am I missing
> Some subtle interaction?

I was referring to the lines above that you added.
But on second thought, never mind - I think this one is ok.


___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH 2/2] libsepol: port str_read from kernel

[Stephen Smalley]

On 08/19/2016 10:54 AM, William Roberts wrote:
> On Aug 19, 2016 06:12, "Stephen Smalley" <s...@tycho.nsa.gov
> <mailto:s...@tycho.nsa.gov>> wrote:
>>
>> On 08/18/2016 04:54 PM, william.c.robe...@intel.com
> <mailto:william.c.robe...@intel.com> wrote:
>> > From: William Roberts <william.c.robe...@intel.com
> <mailto:william.c.robe...@intel.com>>
>> >
>> > Rather than duplicating the following sequence:
>> > 1. Read len from file
>> > 2. alloc up space based on 1
>> > 3. read the contents into the buffer from 2
>> > 4. null terminate the buffer from 2
>> >
>> > Use the str_read() function that is in the kernel, which
>> > collapses steps 2 and 4. This not only reduces redundant
>> > code, but also has the side-affect of providing a central
>> > check on zero_or_saturated lengths from step 1 when
>> > generating string values.
>> >
>> > Signed-off-by: William Roberts <william.c.robe...@intel.com
> <mailto:william.c.robe...@intel.com>>
>> > ---
>> >  libsepol/src/conditional.c |  9 +--
>> >  libsepol/src/module.c  | 66
> ++
>> >  libsepol/src/policydb.c| 10 +--
>> >  libsepol/src/private.h |  1 +
>> >  libsepol/src/services.c| 33 +++
>> >  5 files changed, 68 insertions(+), 51 deletions(-)
>> >
>> > diff --git a/libsepol/src/conditional.c b/libsepol/src/conditional.c
>> > index 8680eb2..e1bc961 100644
>> > --- a/libsepol/src/conditional.c
>> > +++ b/libsepol/src/conditional.c
>> > @@ -589,15 +589,8 @@ int cond_read_bool(policydb_t * p,
>> >   goto err;
>> >
>> >   len = le32_to_cpu(buf[2]);
>> > - if (zero_or_saturated(len))
>> > + if (str_read(, fp, len))
>> >   goto err;
>> > - key = malloc(len + 1);
>> > - if (!key)
>> > - goto err;
>> > - rc = next_entry(key, fp, len);
>> > - if (rc < 0)
>> > - goto err;
>> > - key[len] = 0;
>> >
>> >   if (p->policy_type != POLICY_KERN &&
>> >   p->policyvers >= MOD_POLICYDB_VERSION_TUNABLE_SEP) {
>> > diff --git a/libsepol/src/module.c b/libsepol/src/module.c
>> > index f25df95..a9d7c54 100644
>> > --- a/libsepol/src/module.c
>> > +++ b/libsepol/src/module.c
>> > @@ -793,26 +793,26 @@ int sepol_module_package_info(struct
> sepol_policy_file *spf, int *type,
>> >   i);
>> >   goto cleanup;
>> >   }
>> > +
>> >   len = le32_to_cpu(buf[0]);
>> > - if (zero_or_saturated(len)) {
>> > - ERR(file->handle,
>> > - "invalid module name length:
> 0x%"PRIx32,
>> > - len);
>> > - goto cleanup;
>> > - }
>> > - *name = malloc(len + 1);
>> > - if (!*name) {
>> > - ERR(file->handle, "out of memory");
>> > - goto cleanup;
>> > - }
>> > - rc = next_entry(*name, file, len);
>> > - if (rc < 0) {
>> > - ERR(file->handle,
>> > - "cannot get module name string (at
> section %u)",
>> > - i);
>> > + if (str_read(name, file, len)) {
>> > + switch(rc) {
>> > + case EINVAL:
>> > + ERR(file->handle,
>> > + "invalid module name
> length: 0x%"PRIx32,
>> > + len);
>> > + break;
>> > + case ENOMEM:
>> > + ERR(file->handle, "out of
> memory");
>> > + break;
>> > + default:
>> > + ERR(file->handle,
>> > +  

Re: [PATCH 2/2] libsepol: port str_read from kernel

[Stephen Smalley]

On 08/18/2016 04:54 PM, william.c.robe...@intel.com wrote:
> From: William Roberts 
> 
> Rather than duplicating the following sequence:
> 1. Read len from file
> 2. alloc up space based on 1
> 3. read the contents into the buffer from 2
> 4. null terminate the buffer from 2
> 
> Use the str_read() function that is in the kernel, which
> collapses steps 2 and 4. This not only reduces redundant
> code, but also has the side-affect of providing a central
> check on zero_or_saturated lengths from step 1 when
> generating string values.
> 
> Signed-off-by: William Roberts 
> ---
>  libsepol/src/conditional.c |  9 +--
>  libsepol/src/module.c  | 66 
> ++
>  libsepol/src/policydb.c| 10 +--
>  libsepol/src/private.h |  1 +
>  libsepol/src/services.c| 33 +++
>  5 files changed, 68 insertions(+), 51 deletions(-)
> 
> diff --git a/libsepol/src/conditional.c b/libsepol/src/conditional.c
> index 8680eb2..e1bc961 100644
> --- a/libsepol/src/conditional.c
> +++ b/libsepol/src/conditional.c
> @@ -589,15 +589,8 @@ int cond_read_bool(policydb_t * p,
>   goto err;
>  
>   len = le32_to_cpu(buf[2]);
> - if (zero_or_saturated(len))
> + if (str_read(, fp, len))
>   goto err;
> - key = malloc(len + 1);
> - if (!key)
> - goto err;
> - rc = next_entry(key, fp, len);
> - if (rc < 0)
> - goto err;
> - key[len] = 0;
>  
>   if (p->policy_type != POLICY_KERN &&
>   p->policyvers >= MOD_POLICYDB_VERSION_TUNABLE_SEP) {
> diff --git a/libsepol/src/module.c b/libsepol/src/module.c
> index f25df95..a9d7c54 100644
> --- a/libsepol/src/module.c
> +++ b/libsepol/src/module.c
> @@ -793,26 +793,26 @@ int sepol_module_package_info(struct sepol_policy_file 
> *spf, int *type,
>   i);
>   goto cleanup;
>   }
> +
>   len = le32_to_cpu(buf[0]);
> - if (zero_or_saturated(len)) {
> - ERR(file->handle,
> - "invalid module name length: 0x%"PRIx32,
> - len);
> - goto cleanup;
> - }
> - *name = malloc(len + 1);
> - if (!*name) {
> - ERR(file->handle, "out of memory");
> - goto cleanup;
> - }
> - rc = next_entry(*name, file, len);
> - if (rc < 0) {
> - ERR(file->handle,
> - "cannot get module name string (at section 
> %u)",
> - i);
> + if (str_read(name, file, len)) {
> + switch(rc) {
> + case EINVAL:
> + ERR(file->handle,
> + "invalid module name length: 
> 0x%"PRIx32,
> + len);
> + break;
> + case ENOMEM:
> + ERR(file->handle, "out of memory");
> + break;
> + default:
> + ERR(file->handle,
> + "cannot get module name string 
> (at section %u)",
> + i);
> + }

1) You didn't set rc = str_read(), so you can't switch on it above.
2) Using positive values for errors is likely to confuse matters when
interacting with the existing code, which always uses negative values
(either -errno as a legacy of common code with the kernel or -1).
3) I think this overcomplicates the error handling / reporting.  If
str_read() were to set errno and return -1 instead, then you could just
include strerror(errno) in a single error message.  Or you can just
always report the most general error message.  But it isn't worth a
switch statement.

Same applies throughout.

>   goto cleanup;
>   }
> - (*name)[len] = '\0';
> +
>   rc = next_entry(buf, file, sizeof(uint32_t));
>   if (rc < 0) {
>   ERR(file->handle,
> @@ -821,25 +821,23 @@ int sepol_module_package_info(struct sepol_policy_file 
> *spf, int *type,
>   goto cleanup;
>   }
>   len = le32_to_cpu(buf[0]);
> - if (zero_or_saturated(len)) {
> - ERR(file->handle,
> - "invalid module version length: 0x%"PRIx32,

Re: [PATCH 1/2] libsepol: calloc all the *_to_val_structs

[Stephen Smalley]

On 08/18/2016 04:54 PM, william.c.robe...@intel.com wrote:
> From: William Roberts 
> 
> The usage patterns between these structures seem similair
> to role_val_to_struct usages. Calloc these up to prevent
> any unitialized usages.
> 
> Signed-off-by: William Roberts 
> ---
>  libsepol/src/mls.c  | 2 +-
>  libsepol/src/policydb.c | 6 +++---
>  libsepol/src/users.c| 9 -
>  3 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/libsepol/src/mls.c b/libsepol/src/mls.c
> index 2dc5f2b..8047d91 100644
> --- a/libsepol/src/mls.c
> +++ b/libsepol/src/mls.c
> @@ -312,7 +312,7 @@ int mls_context_isvalid(const policydb_t * p, const 
> context_struct_t * c)
>   if (!c->user || c->user > p->p_users.nprim)
>   return 0;
>   usrdatum = p->user_val_to_struct[c->user - 1];
> - if (!mls_range_contains(usrdatum->exp_range, c->range))
> + if (!usrdatum || !mls_range_contains(usrdatum->exp_range, c->range))
>   return 0;   /* user may not be associated with range */
>  
>   return 1;
> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> index c225ac6..5f888d3 100644
> --- a/libsepol/src/policydb.c
> +++ b/libsepol/src/policydb.c
> @@ -1074,7 +1074,7 @@ int policydb_index_others(sepol_handle_t * handle,
>  
>   free(p->user_val_to_struct);
>   p->user_val_to_struct = (user_datum_t **)
> - malloc(p->p_users.nprim * sizeof(user_datum_t *));
> + calloc(p->p_users.nprim, sizeof(user_datum_t *));
>   if (!p->user_val_to_struct)
>   return -1;
>  
> @@ -4006,12 +4006,12 @@ int policydb_reindex_users(policydb_t * p)
>   free(p->sym_val_to_name[i]);
>  
>   p->user_val_to_struct = (user_datum_t **)
> - malloc(p->p_users.nprim * sizeof(user_datum_t *));
> + calloc(p->p_users.nprim, sizeof(user_datum_t *));
>   if (!p->user_val_to_struct)
>   return -1;
>  
>   p->sym_val_to_name[i] = (char **)
> - malloc(p->symtab[i].nprim * sizeof(char *));
> + calloc(p->symtab[i].nprim, sizeof(char *));
>   if (!p->sym_val_to_name[i])
>   return -1;
>  
> diff --git a/libsepol/src/users.c b/libsepol/src/users.c
> index ce54c2b..3ffb166 100644
> --- a/libsepol/src/users.c
> +++ b/libsepol/src/users.c
> @@ -19,12 +19,17 @@ static int user_to_record(sepol_handle_t * handle,
>  
>   const char *name = policydb->p_user_val_to_name[user_idx];
>   user_datum_t *usrdatum = policydb->user_val_to_struct[user_idx];
> - ebitmap_t *roles = &(usrdatum->roles.roles);
> + ebitmap_t *roles;
>   ebitmap_node_t *rnode;
>   unsigned bit;
>  
>   sepol_user_t *tmp_record = NULL;
>  
> + if (!usrdatum)
> + goto err;
> +
> + roles = &(usrdatum->roles.roles);
> +
>   if (sepol_user_create(handle, _record) < 0)
>   goto err;
>  
> @@ -234,6 +239,7 @@ int sepol_user_modify(sepol_handle_t * handle,
>   if (!tmp_ptr)
>   goto omem;
>   policydb->user_val_to_struct = tmp_ptr;
> + policydb->user_val_to_struct[policydb->p_users.nprim] = NULL;
>  
>   tmp_ptr = realloc(policydb->sym_val_to_name[SYM_USERS],
> (policydb->p_users.nprim +
> @@ -241,6 +247,7 @@ int sepol_user_modify(sepol_handle_t * handle,
>   if (!tmp_ptr)
>   goto omem;
>   policydb->sym_val_to_name[SYM_USERS] = tmp_ptr;
> + policydb->p_user_val_to_name[policydb->p_users.nprim] = NULL;

This one is wrong.

>  
>   /* Need to copy the user name */
>   name = strdup(cname);
> 

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [PATCH v4 7/7] libsepol: fix overflow and 0 length allocations

[Stephen Smalley]

On 08/16/2016 01:45 PM, Roberts, William C wrote:
> 
> 
>> -Original Message-
>> From: Roberts, William C
>> Sent: Tuesday, August 16, 2016 10:29 AM
>> To: seli...@tycho.nsa.gov; jwca...@tycho.nsa.gov; seandroid-
>> l...@tycho.nsa.gov; s...@tycho.nsa.gov
>> Cc: Roberts, William C 
>> Subject: [PATCH v4 7/7] libsepol: fix overflow and 0 length allocations
>>
>> From: William Roberts 
>>
>> Throughout libsepol, values taken from sepolicy are used in places where 
>> length
>> == 0 or length ==  matter, find and fix these.
>>
>> Also, correct any type mismatches noticed along the way.
>>
>> Signed-off-by: William Roberts 
>> ---
>>  libsepol/src/conditional.c|  3 +-
>>  libsepol/src/context.c| 16 +++---
>>  libsepol/src/context_record.c | 72 +--
>> 
>>  libsepol/src/module.c | 13 
>>  libsepol/src/module_to_cil.c  |  4 ++-
>>  libsepol/src/policydb.c   | 52 +--
>>  libsepol/src/private.h|  3 ++
>>  7 files changed, 130 insertions(+), 33 deletions(-)
>>
>> diff --git a/libsepol/src/conditional.c b/libsepol/src/conditional.c index
>> ea47cdd..8680eb2 100644
>> --- a/libsepol/src/conditional.c
>> +++ b/libsepol/src/conditional.c
>> @@ -589,7 +589,8 @@ int cond_read_bool(policydb_t * p,
>>  goto err;
>>
>>  len = le32_to_cpu(buf[2]);
>> -
>> +if (zero_or_saturated(len))
>> +goto err;
>>  key = malloc(len + 1);
>>  if (!key)
>>  goto err;
>> diff --git a/libsepol/src/context.c b/libsepol/src/context.c index 
>> 420ee16..a88937f
>> 100644
>> --- a/libsepol/src/context.c
>> +++ b/libsepol/src/context.c
>> @@ -10,6 +10,7 @@
>>  #include "context.h"
>>  #include "handle.h"
>>  #include "mls.h"
>> +#include "private.h"
>>
>>  /* - Compatibility  */
>>  int policydb_context_isvalid(const policydb_t * p, const context_struct_t * 
>> c)
>> @@ -297,10 +298,18 @@ int context_from_string(sepol_handle_t * handle,
>>  char *con_cpy = NULL;
>>  sepol_context_t *ctx_record = NULL;
>>
>> +if (zero_or_saturated(con_str_len)) {
>> +ERR(handle, "Invalid context length");
>> +goto err;
>> +}
>> +
>>  /* sepol_context_from_string expects a NULL-terminated string */
>>  con_cpy = malloc(con_str_len + 1);
>> -if (!con_cpy)
>> -goto omem;
>> +if (!con_cpy) {
>> +ERR(handle, "out of memory");
>> +goto err;
>> +}
>> +
>>  memcpy(con_cpy, con_str, con_str_len);
>>  con_cpy[con_str_len] = '\0';
>>
>> @@ -315,9 +324,6 @@ int context_from_string(sepol_handle_t * handle,
>>  sepol_context_free(ctx_record);
>>  return STATUS_SUCCESS;
>>
>> -  omem:
>> -ERR(handle, "out of memory");
>> -
>>err:
>>  ERR(handle, "could not create context structure");
>>  free(con_cpy);
>> diff --git a/libsepol/src/context_record.c b/libsepol/src/context_record.c 
>> index
>> ac2884a..0a8bbf6 100644
>> --- a/libsepol/src/context_record.c
>> +++ b/libsepol/src/context_record.c
>> @@ -5,6 +5,7 @@
>>
>>  #include "context_internal.h"
>>  #include "debug.h"
>> +#include "private.h"
>>
>>  struct sepol_context {
>>
>> @@ -279,44 +280,69 @@ int sepol_context_from_string(sepol_handle_t *
>> handle,
>>
>>  hidden_def(sepol_context_from_string)
>>
>> +static inline int safe_sum(size_t *sum, const size_t augends[], const
>> +size_t cnt) {
>> +
>> +size_t a, i;
>> +
>> +*sum = 0;
>> +for(i=0; i < cnt; i++) {
>> +/* sum should not be smaller than the addend */
>> +a = augends[i];
>> +*sum += a;
>> +if (*sum < a) {
>> +return i;
> 
> Damn it...this does not work when index 0 is the bad one... I'll wait till
> Tomorrow and aggregate responses again.

Use __builtin_add_overflow()?

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: userdebug_or_eng rules stopped by neverallow?

[Stephen Smalley]

On 07/28/2016 12:11 PM, peter enderborg wrote:
> What is the point with that?

Can you explain a bit further what specific problem you are
encountering, and with which branch/release of AOSP?



___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: genfs contexts labelling issue for special character

[Stephen Smalley]

On 07/01/2016 08:06 AM, Inamdar Sharif wrote:
> Hi Guys,
> 
>  
> 
> I have a node which is  “abc,xyz”
> 
> I want to label this node in genfs_contexts but getting syntax error
> 
>  
> 
> But if I make the below change in
> external/selinux/checksepolicy/policy_scan.l it works fine.
> 
> -"/"({alnum}|[_\.\-/])* { return(PATH); }
> 
> +"/"({alnum}|[_\.\,\-/])*   { return(PATH); }
> 
>  
> 
> Is there any other way I can label the node ??

Quoting the path should also work, btw.
The QPATH pattern already included all non-quote printable characters.

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

[PATCH] Extend checkpolicy pathname matching.

[Stephen Smalley]

checkpolicy currently imposes arbitrary limits on pathnames used
in genfscon and other statements.  This prevents specifying certain
paths in /proc such as those containing comma (,) characters.

Generalize the PATH, QPATH, and FILENAME patterns to support most
legal pathnames.

For simplicity, we do not support pathnames containing newlines or
quotes.

Reported-by: Inamdar Sharif <isha...@nvidia.com>
Signed-off-by: Stephen Smalley <s...@tycho.nsa.gov>
---
 checkpolicy/policy_scan.l | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/checkpolicy/policy_scan.l b/checkpolicy/policy_scan.l
index 22da338..2f7f221 100644
--- a/checkpolicy/policy_scan.l
+++ b/checkpolicy/policy_scan.l
@@ -249,9 +249,9 @@ high |
 HIGH   { return(HIGH); }
 low |
 LOW{ return(LOW); }
-"/"({alnum}|[_\.\-/])* { return(PATH); }
-\""/"[ !#-~]*\"{ return(QPATH); }
-\"({alnum}|[_\.\-\+\~\: ])+\"  { return(FILENAME); }
+"/"[^ \n\r\t\f]*   { return(PATH); }
+\""/"[^\"\n]*\"{ return(QPATH); }
+\"[^"/"\"\n]+\"{ return(FILENAME); }
 {letter}({alnum}|[_\-])*([\.]?({alnum}|[_\-]))*{ return(IDENTIFIER); }
 {digit}+|0x{hexval}+{ return(NUMBER); }
 {alnum}*{letter}{alnum}*{ return(FILESYSTEM); }
-- 
2.5.5

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Re: [android-porting] Strange AVC Denial

[Stephen Smalley]

On 07/12/2016 11:57 AM, Roman Mazur wrote:
> I'm working on a custom build based on Android 6.0.1 for Nexus 7. This
> custom build adds a special daemon that is started from init.rc and
> exposes some API to applications. Particularly, one of available methods
> creates a new file at /data/daemon_dir and returns a file descriptor
> making it possible to write to this file from an app.
> 
> The daemon has its own SELinux context (here it's named custom_daemon).
> And /data/daemon_dir has custom_daemon_file context. There are sepolicy
> rules that grant file creation to the daemon and file writes to
> untrusted_app.
> 
> The configuration described above worked on Android 5. But after merging
> with Android 6, I'm getting the following denial:
> 
> 07-11 21:57:46.735 13389-13389/? W/Binder_2: type=1400 audit(0.0:945):
> avc: denied { write } for path="/data/daemon_dir/some_file"
> dev="mmcblk0p30" ino=496817 scontext=u:r:untrusted_app:s0:c512,c768
> tcontext=u:object_r:custom_daemon_file:s0 tclass=file permissive=0
> 
> 
> Here are the rules that should allow the operation:
> 
> allow untrusted_app custom_daemon_file:file rw_file_perms;
> allow untrusted_app custom_daemon_file:dir r_dir_perms;

(cc seandroid-list)

Assuming that you only want to allow apps to read and write files passed
to them explicitly by your daemon and not to directly open files in this
directory, you should only have:
allow untrusted_app custom_daemon_file:file { read write getattr };

In particular, you would not need to allow open permission to file nor
any permissions to the directory in this usage model, and not allowing
those permissions would be more secure as it would prevent apps from
directly accessing any of those files without going through your daemon.

> allow custom_daemon custom_daemon_file:dir create_dir_perms;
> allow custom_daemon custom_daemon_file:file create_file_perms;
> 
> 
> An interesting thing in this denial report is that scontext is
> untrusted_app. But the denial is logged for the daemon process (13389 is
> one of its thread IDs and Binder_2 is a name of the binder thread that
> handles the API call).
> 
> I believe this mismatch is what is causing the denial but cannot
> understand why this happens and how this can be fixed.

No, the denial is caused by the fact that the app is running with
categories (c512,c768) and the file is not labeled with the same
categories. You can allow this by adding the mlstrustedobject attribute
to your type:
type custom_daemon_file, file_type, data_file_type, mlstrustedobject;

In 6.0, apps and their data files are assigned an automatically
generated category set (c512,c768 above) derived from their user ID in
order to isolate the processes and files of different users from each
other. This prevents cross-user or cross-profile access to /proc/pid
files and app data files, even if world-readable or -writable (sharing
is still possible by having the owning app open the file itself and pass
the file descriptor over binder or local socket IPC to another app, but
direct open is prohibited).  This separation is currently applied to
untrusted apps and platform apps as a result of specifying
levelFrom=user in the seapp_contexts configuration.

With regard to the potentially misleading audit message, it is due to
the fact that this check occurs when your daemon tries to pass the open
file descriptor to the app, so it occurs while the daemon is the current
process.  Before transferring the descriptor into the app's descriptor
table, SELinux checks whether the app is allowed to use it.  So the
check is between the app's context and the file, but happens while the
daemon is still the current process.

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.