Re: Questions about execution binary from /data.

[William Roberts]

On Mon, Apr 2, 2018 at 7:37 AM, HAN  wrote:

> Hi Jeffrey, thanks for your quick response.
>
>
>
> My system_app is used to test some components with python script.
>
> This app is not pre-loaded and be installed to test and will be
> uninstalled after all the test-cases are done.
>
> So I have to put my python binary in runtime but dm-verity doesn't allow
> to copy my binary into system partition.
>
> That's why I'm looking for the other partitions to do it.
>
> The python binary shouldn't be pre-loaded on the device.
>
>
I would recommend one of these approaches, assuming your testing on
userdebug or eng variants:
1. Add the testing components to the build
2. adb remount and then push the binaries to system
3. Disable selinux with adb shell setenforce 0

If your testing on user, and have a custom signed testing app, just be
prudent on what keys are
being used to sign that user image. A source of issues have been leaked
system apps signed
with production keys, which is bad!


>
>
> Thanks
>
> HAN
>
>
>
> -Original Message-
> *From:* "Jeffrey Vander Stoep"
> *To:* "HAN";
> *Cc:* ;
> *Sent:* 2018-04-02 (월) 23:14:09
> *Subject:* Re: Questions about execution binary from /data.
>
> Hi Han,
>
> Privileged system components such as system_app are disallowed from
> executing content off the read-write /data partition because it is less
> trusted than dm-verity protected read-only partitions such as /system. I
> recommend you put your python binary on the system partition.
>
> On Mon, Apr 2, 2018 at 6:10 AM HAN  wrote:
>
> Hi everone,
>
>
>
> My system_app needs to execute python binary to run python script.
>
> So I copied the binary into a path "/data/misc/user/0/python"
>
> but execution is blocked by below neverallow.
>
>
>
> Where should I put my python binary into?
>
> Are there any areas that are most commonly used in this case?
>
>
>
>
>
> http://androidxref.com/8.0.0_r4/xref/system/sepolicy/private/app.te#497
>
> 
> --
>
> # Blacklist app domains not allowed to execute from /data
>
> neverallow {
>
>   bluetooth
>
>   isolated_app
>
>   nfc
>
>   radio
>
>   shared_relro
>
>   system_app
>
> } {
>
>   data_file_type
>
>   -dalvikcache_data_file
>
>   -system_data_file # shared libs in apks
>
>   -apk_data_file
>
> }:file no_x_file_perms;
>
> 
> --
>
>
>
> Thanks.
>
> HAN
>
>

Re: /data/misc contents are unlabeled

[William Roberts]

On Tue, Mar 13, 2018 at 11:45 PM, kiran mardi  wrote:
> Hi Stephen,
>
> Please correct me if I am wrong.
> 1. restorecon_recurssive /data in system/core/rootdir/init.rc will not
> run/apply on every bootup?

No, as Stephen stated before, and I quote, "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"

This is for performance reasons, no need to run a costly recursive restorecon
if the file_contexts.bin file hasn't changed. But, if their was a bug
in this feature,
it possibly could cause it not to run.

> expectation is it should be running on every bootup(unlabeled issue
> should be solved).
>
> 2. Looks like the issue is reproduce when we upgrade from M to N. I see the
> change of restorecon_recurssive is already part of M as well.
>
> can this issue occur if the upgrade is partial or something?

I've only ever seen this when you're attempting to access something
before the trigger. This restorecon happens on the post-fs-data
trigger IIRC. Which (obviously) happens after the data partition is decrypted
and mounted.

Considering you saying it's not 100% reproducible,
are you racing the restorecon_recursive?

>
> On Fri, Mar 9, 2018 at 9:38 PM, kiran mardi  wrote:
>>
>> Hi Stephen,
>>
>> The issue I am mentioning is not 100% reproducible. We are seeing this
>> very rarely. So don't know how to get this reproduce. Anyway will try to get
>> more details on the issue and get back to u.
>>
>> Was also thinking what else can be added to address this.
>>
>> Thanks for your help.
>>
>> On 09-Mar-2018 6:41 PM, "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?
>>>
>
>
>
> --
> regards,
> kiran mardi

Re: map on _tmpfs

[William Roberts]

On Wed, Nov 1, 2017 at 11:34 AM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
> 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.
>
Sounds good, ill re-spin with Test: set to something
reasonable.

map on _tmpfs

[William Roberts]

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

-- 
Respectfully,

William C Roberts

local induced CTS issues with property_contexts

[William Roberts]

We recently ran into an issue where CTS was failing on property_contexts.

The best I can tell, is that the CTS build had locale of en_US.utf8 while
the local
build had a locale of C which affected the sort ordering as sort respects
locale.

I proposed a patch to use fc_sort, here:
https://android-review.googlesource.com/#/c/platform/system/sepolicy/+/491917/

Which avoids the locale issue and strips comments so sed can dissapear.

I see it also existing for property contexts, so likely want to submit a
patch for that
too.

-- 
Respectfully,

William C Roberts

m4 processing for macros on all files

[William Roberts]

IIRC, back in the day all files were m4 processed, concatenated and
checked. I made use of these definitions
passed vie BOARD_SEPOLICY_M4DEFS.

Recent changes changed this behavior, so now one has to define the
type always for things like type and
domain for seapp_contexts.

Does anyone object to adding this functionality back in and making
_macros be a part of the
handling (perhaps te_macros)?

The exact use case is that anything debug I try and wrap up in
userdebug_or_eng() macros, this way, if the
release team accidentally forgets to turn something off, the policy
prevents it. In my case, this includes debug
apps, and being able to disable the seapp_contexts entry, as well as
committing all the types would be nice.


Bill

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

[William Roberts]

On Fri, May 19, 2017 at 6:09 AM, Stephen Smalley  wrote:
> 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
>
>

The only problem with Stephen's approach is it won't build
dependencies, so depending on how you have it set up, mmma
might work better.

mmma -B system/sepolicy

You could automate your CI system to do this for patches that
affect the SEAndroid configuration. You want to ensure
that chnages to seapp_contexts and file_contexts are run,
as checkfc and check_seapp have been instrumented to
check certain things.

Re: hook

[William Roberts]

On Apr 11, 2017 04:54, "peng fei"  wrote:

Some research set hook on C API. SEAndroid set hook on syscall.
What's the difference of access control performance  between the C hook and
the syscall hook?


The userspace library hook will be faster, as it avoids the context switch,
however it's insecure as the code doing the check lives inside the process
your checking. There's also different ways of doing this, and they all have
there corner cases.


___
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: add CONFIG_SECURITY_SELINUX_LOAD_ONCE

[William Roberts]

On Apr 7, 2017 12:03, <thomasclinganjo...@gmail.com> wrote:

I think you misunderstand the threat. As we found from boot loader and
firmware update attacks, the ways to subvert policy are manifold. How can I
tell who the “manufacturer” of my android phone that sets the policy really
is and do I trust Android to maintain that trust.

It's part of a signed  image, but it's tied to the trust of that image. An
attacked bootloader or compromise to the kernel subverts the policy. But
we've seen attacks on boot loaders, etc. none of this has anything to do
with the patch nnk submitted.



thx ..tom



*From: *William Roberts <bill.c.robe...@gmail.com>
*Sent: *Friday, April 7, 2017 11:59 AM
*To: *Tom Jones <thomasclinganjo...@gmail.com>
*Cc: *seandroid-list@tycho.nsa.gov; seli...@tycho.nsa.gov; Nick Kralevich
<n...@google.com>
*Subject: *Re: add CONFIG_SECURITY_SELINUX_LOAD_ONCE







On Apr 7, 2017 11:44, "Tom Jones" <thomasclinganjo...@gmail.com> wrote:

I am highly skeptical of policy setting and the supply chain. In the threat
models I create, the supply chain is always the weak, open threat. I could
not begin to guess who the manufacturer of a device is. Android, Samsung,
Verizon or the US Gov't. Is there a threat model for SE Android, or
whatever it is called now?



Well SE Android is fully integrated into Android. Vendors create the policy
that ends up the boot image, which is typically signature verified at boot.
If your supply chain is compromised, the selinux policy is your least
concern. Under treble it ends up in different DM verity protected images.







I looked at the other site and decided it was looking at the technical
problem and not the policy problem at all.



On Fri, Apr 7, 2017 at 11:23 AM, William Roberts <bill.c.robe...@gmail.com>
wrote:





On Fri, Apr 7, 2017 at 11:02 AM, Tom Jones <thomasclinganjo...@gmail.com>
wrote:

I like that, but I wonder at its scope. Would an update to the OS be
allowed to update the policy? For example, Microsoft ships updates to the
Windows O/S 2 times (at least) per month. Would that type of update to
Android allow policy updates?



Part of Android's updates include the policy that is loaded, so the update
mechanism is in place.





Another question involves the list of authoritative CSPs. That can now be
updated in most O/S available on the market. Is that still allowed to be
updated, or is that already allowed by policy?

..tom



The policy is updated, currently, as part of the root file system. In a
feature in progress, TREBLE (FULL_PRODUCT_TREBLE == true), two files, one
from vendor and one from google are used to
generate the policy.



essentially, the policy only comes from those making the device, theirs no
random folks adding/removing policy.





On Fri, Apr 7, 2017 at 10:34 AM, Nick Kralevich <n...@google.com> wrote:

I wanted to draw people's attention to the following proposed change:



  https://android-review.googlesource.com/367695



In the case of Android, it's common for security policy to be loaded once,
and never reloaded again. In that case, the locking / unlocking surrounding
the in-kernel policy is unnecessary and can be avoided. The patch above
turns the locks into no-ops and ensures that the kernel cannot load a
policy more than once. End result is that locking and preemption overhead
is avoided and there's less attack surface / code compiled into the kernel.



I would appreciate comments on the change. This feels like a worthwhile
change for the entire SELinux community.



-- Nick



-- 

Nick Kralevich | Android Security | n...@google.com | 650.214.4037
<(650)%20214-4037>



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




-- 

..tom


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





-- 

Respectfully,

William C Roberts



-- 

..tom
___
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: add CONFIG_SECURITY_SELINUX_LOAD_ONCE

[William Roberts]

On Apr 7, 2017 11:44, "Tom Jones" <thomasclinganjo...@gmail.com> wrote:

I am highly skeptical of policy setting and the supply chain. In the threat
models I create, the supply chain is always the weak, open threat. I could
not begin to guess who the manufacturer of a device is. Android, Samsung,
Verizon or the US Gov't. Is there a threat model for SE Android, or
whatever it is called now?


Well SE Android is fully integrated into Android. Vendors create the policy
that ends up the boot image, which is typically signature verified at boot.
If your supply chain is compromised, the selinux policy is your least
concern. Under treble it ends up in different DM verity protected images.



I looked at the other site and decided it was looking at the technical
problem and not the policy problem at all.

On Fri, Apr 7, 2017 at 11:23 AM, William Roberts <bill.c.robe...@gmail.com>
wrote:

>
>
> On Fri, Apr 7, 2017 at 11:02 AM, Tom Jones <thomasclinganjo...@gmail.com>
> wrote:
>
>> I like that, but I wonder at its scope. Would an update to the OS be
>> allowed to update the policy? For example, Microsoft ships updates to the
>> Windows O/S 2 times (at least) per month. Would that type of update to
>> Android allow policy updates?
>>
>
> Part of Android's updates include the policy that is loaded, so the update
> mechanism is in place.
>
>
>>
>> Another question involves the list of authoritative CSPs. That can now be
>> updated in most O/S available on the market. Is that still allowed to be
>> updated, or is that already allowed by policy?
>> ..tom
>>
>
> The policy is updated, currently, as part of the root file system. In a
> feature in progress, TREBLE (FULL_PRODUCT_TREBLE == true), two files, one
> from vendor and one from google are used to
> generate the policy.
>
> essentially, the policy only comes from those making the device, theirs no
> random folks adding/removing policy.
>
>
>>
>> On Fri, Apr 7, 2017 at 10:34 AM, Nick Kralevich <n...@google.com> wrote:
>>
>>> I wanted to draw people's attention to the following proposed change:
>>>
>>>   https://android-review.googlesource.com/367695
>>>
>>> In the case of Android, it's common for security policy to be loaded
>>> once, and never reloaded again. In that case, the locking / unlocking
>>> surrounding the in-kernel policy is unnecessary and can be avoided. The
>>> patch above turns the locks into no-ops and ensures that the kernel cannot
>>> load a policy more than once. End result is that locking and preemption
>>> overhead is avoided and there's less attack surface / code compiled into
>>> the kernel.
>>>
>>> I would appreciate comments on the change. This feels like a worthwhile
>>> change for the entire SELinux community.
>>>
>>> -- Nick
>>>
>>> --
>>> Nick Kralevich | Android Security | n...@google.com | 650.214.4037
>>> <(650)%20214-4037>
>>>
>>> ___
>>> 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.
>>>
>>
>>
>>
>> --
>> ..tom
>>
>> ___
>> 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.
>>
>
>
>
> --
> Respectfully,
>
> William C Roberts
>
>


-- 
..tom
___
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: service_manager add permission

[William Roberts]

For those following along, the topic was killed, so the patches are:
https://android-review.googlesource.com/#/c/325725/
https://android-review.googlesource.com/#/c/325726/

On Thu, Jan 19, 2017 at 3:43 PM, Nick Kralevich  wrote:
> these are good patches. Thank you for uploading them.
>
> -- Nick
>
> On Thu, Jan 19, 2017 at 2:06 PM, Roberts, William C
>  wrote:
>>
>> I've been seeing some off policy where folks are adding add permissions
>> for service's they shouldn't be adding. This is usually indicative of a
>> mislabel or just plain wrongness.
>> I have some RFC patches on Gerrit that attempt to address this by using a
>> macro to "add a service" to service manager. This macro automatically
>> creates a neverallow preventing
>> others from adding that service type.
>>
>> This may be two strict in some cases, perhaps gpu_service could be an
>> example.
>>
>> Also, doing this found a miss-typeattribute to wificond_service:
>> https://android-review.googlesource.com/#/c/325724/
>>
>> Also, a better mechanism then the macro may exist for this, like just
>> doing it with raw policy statements. I like the macro as it develops a
>> pattern, and you have to break the pattern to get around it,
>> which probably means something is wrong. This seems to have occurred with
>> mediaserver and mediadrmserver both adding a media_service service.
>>
>> For those who wish to view and comment:
>>
>> https://android-review.googlesource.com/#/q/topic:rfc-add-service+(status:open+OR+status:merged)
>>
>> Bill
>>
>
>
>
> --
> Nick Kralevich | Android Security | n...@google.com | 650.214.4037
>
> ___
> 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.



-- 
Respectfully,

William C Roberts
___
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: reload sepolicy

[William Roberts]

On Dec 23, 2016 19:34, "peng fei"  wrote:

Can I modify external/libselinux/src/android.c to force the policy just
load from /data/security/current/sepolicy?
---
This is the original file external/libselinux/src/android.c

> static char const * const seapp_contexts_file[] = {
> "/seapp_contexts",
> "/data/security/current/seapp_contexts",
> NULL };
>
> static const struct selinux_opt seopts[] = {
> { SELABEL_OPT_PATH, "/file_contexts" },
> { SELABEL_OPT_PATH, "/data/security/current/file_contexts" },
> { 0, NULL } };
>
> static const char *const sepolicy_file[] = {
> "/data/security/current/sepolicy",
> NULL };
>
> static const struct selinux_opt seopts_service[] = {
> { SELABEL_OPT_PATH, "/data/security/current/service_contexts" },
> { 0, NULL }
> };
>
---
I want to modify android.c as follows:

> static char const * const seapp_contexts_file[] = {
> "/data/security/current/seapp_contexts",
> NULL };
>
> static const struct selinux_opt seopts[] = {
> { SELABEL_OPT_PATH, "/data/security/current/file_contexts" },
> { 0, NULL } };
>
> static const char *const sepolicy_file[] = {
> "/data/security/current/sepolicy",
> NULL };
>
> static const struct selinux_opt seopts_service[] = {
> { SELABEL_OPT_PATH, "/data/security/current/seapp_contexts" },
> { SELABEL_OPT_PATH, "/data/security/current/service_contexts" },
> { 0, NULL }
> };
>
--
I think building the whole system and rebooting the device to  modify the
sepolicy is complex.


You only need to repackage the boot image for sepolicy changes. If you need
a system relabel, then flash system. Reloading off of data only occurs late
in boot so it's usefulness is limited. Almost everything can be
accomplished via pushing the policy, use load_policy command, restorecon,
start/stop services etc.

Pushing the sepolicy and related file to the /data/security/current/ always
doesn't work.
So , I want a simple way for loading and testing the policy I modified.

If I modify the android.c, could  It  work as I expected or not?


I don't think anyone on the list here is going to help you write code for
this. Also, if you try and ship rules for this you could face CTS/CDD
issues.


Please help me.
Thanks advance.


___
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: Regarding SELinux denial for writing to /tmp from unstrusted_app

[William Roberts]

On Nov 30, 2016 18:14, "Sameer Joshi"  wrote:
>
> Hi All,
>
> I want to give access to untrusted app to write to /tmp directory.
>
> This is on top of 6.0 M code.
>
> Denial was following:
>
> [  151.092299] type=1400 audit(1479910142.370:18): avc: denied { write }
for pid=2947 comm="a.android.flare" name="/" dev="tmpfs" ino=5591
scontext=u:r:untrusted_app:s0:c512,c768 tcontext=u:object_r:tmpfs:s0
tclass=dir permissive=0
>
> To solve this,  I did following in untrusted_app.te ( as per the output
from audit2allow) :
>
> allow untrusted_app tmpfs:dir write;
>
> Even after adding this rule, this denial keeps on appearing again.
>
> Any way to fix this?

typeattribute tmpfs, mlstrustedobject;

This is likely very bad. Allowing a world accessable writable place for
apps allows one app to malform the data another app will access.

>
> 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: add new domain but it does not work well

[William Roberts]

On Nov 23, 2016 02:34, "peng fei"  wrote:
>
> requirement:
> system/bin/setest is a execuble program to read and
write /data/hello.txt . I excepted just setest can read or wirte the file
/data/hello.txt.
> root@generic:/system/bin # ./setest
> Hello, Software Weekly
> --
> the details are as follows:
> 1.add setest.te in /device/asus/flo/sepolicy :
>
> type setest, domain, mlstrustedsubject;
> #setest is also permissive to permit setenforce.
> type setest_exec, exec_type, file_type;
> init_daemon_domain(setest)
> permissive setest;
> allow setest mysec_file:file rw_file_perms;
>
> 2.add setest.te in the file device/asus/flo/BoardConfigCommon.mk as
follows:
>
> BOARD_SEPOLICY_UNION += \
> bluetooth_loader.te \
> bridge.te \
> setest.te \

Newer releases all use BOARD_SEPOLICY_DIRS. Verify which one you need. I
can't recall offhand in what versions the switch was made.

>
> 3.add context in /device/asus/flo/sepolicyfile_contexts as follows:
> /system/bin/setest u:object_r:setest_exec:s0
>
> 4.I modify file.te in /external/sepoicy
>
> add
> # /data/hello.txt
> type mysec_file, file_type, data_file_type;
>
> I modify file_contexts in /external/sepoicy
> add
> /data/hello.txt u:object_r:mysec_file:s0

You really could add all of this in the device policy, no need to ever
modify external sepolicy. On newer versions it's system/sepolicy FYI.

Additionally, that file context entry won't work unless the binary is made
selinux aware. Labels are inherited from the parent on the filesystem
unless other actions are taken. Typically it's best to define a directory
under /data that the init script can create. Init's builtins have the
support to look at file contexts and label things correctly. So a simple
mkdir /data/xxx will suffice in the init script. Then update the file
context to label that and have your service create things under that file.

Fyi: Also, you can't launch setest from the shell. It will run in the
context of the shell and not the domain you defined. It needs to be started
by init. You can use start/stop to start and stop the service.

>
>
-
> the results are as follows:
>
> I use the android5.1.1_r9 ,
> ## I recompile the whole system to make the change effective.###
> [pengfei@pengfei asop]$ source build/envsetup.sh
> [pengfei@pengfei asop]$ lunch aosp_arm-eng
> [pengfei@pengfei asop]$ make
> [pengfei@pengfei asop]$ emulator
> adb shell
> root@generic:/system/bin # ls -Z
> -rwxr-xr-x root shell u:object_r:setest_exec:s0 setest
> root@generic:/system/bin # ./setest
> Hello, Software Weekly
> #setest is a execuble program to read and write /data/hello.txt .#
> But when I use dmesg to have a look at the policy which I have made, it
does not work.
>  details are as follows:
> root@generic:/system/bin # dmesg | grep 'avc'
> 1|root@generic:/system/bin # dmesg | grep 'setest'
> 1|root@generic:/system/bin #
> just get 1.
> What's wrong with my policy change method?
> I think the setest.te does not work as I expeced.
> I cd /data/ and cat hello.txt . It works. I excepted just setest can read
or wirte the file hello.txt
> root@generic:/data # cat hello.txt
> Hello, Software Weeklyroot@generic:/data #
>
> Please help me. Thanks advance.
>
> ___
> 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: fail to extend the policy

[William Roberts]

On Fri, Nov 4, 2016 at 6:47 AM, peng fei  wrote:
> 1.  create an executable C program named setest to create , read and write
> hello.txt.
> 2.  push the setest to /data.  root@grouper:/data # ./setest this will
> create hello.txt in /data
> 3.  add setest.te in external/sepolicy :
>
> setest.te
>
> type setest, domain, mlstrustedsubject;
> type setest_exec, exec_type, file_type;
> permissive setest;
> domain_auto_trans(shell, setest_exec, setest)
> #I think I execute the setest by ./setest , so it can be transform from
> shell to setest.
> auditallow   setest   sec_file :   file rwx_file_perms;

setest is permissive, so it should audit all accesses not
explicitly marked with a dontaudit.

>
>
> 4.  add new context to the file_contexts.
>
> /data/hello.txtu:object_r:sec_file:s0

Labels are inherited by the parent directory unless explicitly changed, thus
/data/hello.txt will have the label of /data unless:
1. setest uses libselinux setfscreatcon() to change it at file creation
2. something (like setest) calls restorecon on the file path.

Android best practicies dictate that services and such have their own working
directory under /data, thus if you have an init.rc to make
/data/setest, init builtins
and relabling will ensure that /data/setest has the label in
file_contexts, and thus
any files underneath will inherit that label.

> /data/setestu:object_r:setest_exec:s0
>
>
> 5.  add new type in the file.te
>
> #/data/hello.txt
> type sec_file, file_type, data_file_type;
>
>
> 6.  compile the policy and adb push sepolicy,
> file_contexts,"seapp_contexts",'service_contexts','property_contexts  to
> /data/security/current.
>
> copy /selinux_version to /data/security/current.
>
> root@grouper:/data/security/current # setprop selinux.reload_policy 1

Dynamically loaded policy is not supported by Android anymore, so make
sure your device
supports this or you build and flash complete images (boot.img and
system.img must be flashed).

> root@grouper:/data/security/current # restorecon file_contexts

This doesn't do what I think you think it does :-P. This relabels the
file_contexts file
to what it is in file_contexts. libselinux opens file_contexts under
the hood, no arguments.

If you want to relabel /data you need to restorecon -R /data or reboot
the device.

> reboot

Reboot may not work with dynamically loaded policy because /data is
mounted and automatically
relabeled by init scripts and installd (for app data dirs). However,
this would occur likely before
the dynamically loaded policy is used for relabeling.

Since I don't know the state of your tree with respect to dynamically
loaded policy, its likely best
to just flash full images.

>
> ---
> but the result is :
>
> root@grouper:/data # ls -Z
> -rw--- root root u:object_r:sec_file:s0 hello.txt
> -rwxrwxrwx shell shell u:object_r:system_data_file:s0 setest
>
>
> the type of hello.txt is I expected as sec_file
> but the type of setest is not setest_exec
>
> 
> please help me,  thanks advance.
>
> ___
> 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.



-- 
Respectfully,

William C Roberts
___
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

[William Roberts]

On Oct 18, 2016 11:08, "Stephen Smalley"  wrote:
>
> 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).

That's fantastic, I didn't notice that change. System apps have been
spewing stuff around system long enough IMO.

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

[William Roberts]

On Oct 18, 2016 10:51, "Stephen Smalley" <s...@tycho.nsa.gov> wrote:
>
> 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>> 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.

Correct, typing from phone is too hard for underscores.
>
> >
> > 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 <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 "help" to
> > seandroid-list-requ...@tycho.nsa.gov
> > <mailto: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

[William Roberts]

On Oct 18, 2016 10:33 AM, "Stephen Smalley" <s...@tycho.nsa.gov> wrote:
>
> 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>> 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.

One could say don't allow platform apps to create things outside of their
sandbox type. I looked at my rule, it was only for untrusted app though.

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.

Yeah, bit we a know that chown 777 is the way to fix that;-p. Don't do
that, platform apps are no designed to be creating resources out the
application sandbox. If you need to create and share data, you can use a
host of mechanisms available on Android. Unix sockets and binder are
readily available. This way your platform app can selectively share it's
isolated data sandbox directory contents via passed fd as an example.
___
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: How to verify my policy?

[William Roberts]

On Oct 18, 2016 9:02 AM, "Stephen Smalley"  wrote:
>
> On 10/17/2016 11:19 PM, peng fei wrote:
> > I want to achieve the result that just allow jd process to open and
> > read /data/audit/log/audit.log.
> > For this target, I add some rules in policy file.
> > And after that, I want to verify my policy. So, I create a test.c to
> > read  /data/audit/log/audit.log. Using gcc to build the test.c to
> > executable test.The file test.c and test is in /home/pengfei.
> > -
> > My modify policy are as follows:
> > --
> > First,
> > add new type in file.te
> > #/data/audit/log/audit.log
> > type sec_file, file_type, data_file_type;
> > #/home/test
> > type jd_exec, file_type;
> > --
> > add the contexts in the file_contexts
> > /data/audit/log/audit.log   u:object_r:sec_file:s0
> > /home/pengfei/test u:object_r:jd_exec:s0
> > --
> > add rule in jd.te
> > allow jd sec_file:file {read, open };
> > allow jd  jd_exec:file rx_file_perms;
> > -
> > How can I verify my policy? Can I create a executable file to imitate
> > jd. How to assign the  conte
> >
> > Please help me. Thanks advance.
>
> Stock Android handles auditing via logd, and those logs can be read via
> logcat, so I'm not sure what your /data/audit/log/audit.log is.
>
> /home doesn't normally exist on Android AFAIK. Executables should
> generally live in /system/bin.  Plenty of /system/bin examples in the
> base AOSP file_contexts file.  If you include the jd binary in your
> system image and specify a context for /system/bin/jd in your
> file_contexts, it will be labeled at build time; you won't need to do
> anything extra to label it.

If you're on OEM it's encouraged to place your executables in the vendor
partition, labelling occurs the same way, at build.

>
> If jd is a daemon started by init, then you want to include:
> init_daemon_domain(jd)
> in your jd.te policy to set up the domain transition.
>
> The easiest way is to use one of the existing domains from AOSP as an
> example.
>
> Use policy macros where possible to make your policy less brittle, e.g.
> allow jd sec_file:file r_file_perms;
>
> On a userdebug or eng build, you can su from an adb shell and then use
> runcon to run the program in a particular context for testing purposes,
> but this may not match the behavior you will see when running it as a
> daemon.
>
> Resources:
> https://source.android.com/security/selinux/index.html
>
https://events.linuxfoundation.org/sites/events/files/slides/abs2014_seforandroid_smalley.pdf
>
>
>
>
> ___
> 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: [PATCH 6/8] libselinux: support ANDROID_HOST=1 on Mac

[William Roberts]

On Oct 18, 2016 08:41, "Stephen Smalley" <s...@tycho.nsa.gov> wrote:
>
> On 10/17/2016 04:24 PM, william.c.robe...@intel.com wrote:
> > From: William Roberts <william.c.robe...@intel.com>
> >
> > To build on mac, first build libsepol with
> > DISABLE_CIL=y and no DESTDIR set.
>
> DISABLE_CIL=y isn't required after the earlier patches, right?

Correct libsepol builds, I forgot to edit the commit message. I'm flying,
so if that's your only issue could you please rewrite the message?

>
> >
> > Secondly, build libselinux with ANDROID_HOST=y
> >
> > This configuration can be used to test the Android
> > host build on Mac.
> >
> > Signed-off-by: William Roberts <william.c.robe...@intel.com>
> > ---
> >  libselinux/Makefile   | 10 ++
> >  libselinux/src/Makefile   | 36 ++--
> >  libselinux/utils/Makefile | 29 +
> >  3 files changed, 57 insertions(+), 18 deletions(-)
> >
> > diff --git a/libselinux/Makefile b/libselinux/Makefile
> > index baa0db3..ef971f4 100644
> > --- a/libselinux/Makefile
> > +++ b/libselinux/Makefile
> > @@ -27,6 +27,16 @@ else
> >  endif
> >  export PCRE_CFLAGS PCRE_LDFLAGS
> >
> > +OS := $(shell uname)
> > +export OS
> > +
> > +ifeq ($(shell $(CC) -v 2>&1 | grep "clang"),)
> > +COMPILER := gcc
> > +else
> > +COMPILER := clang
> > +endif
> > +export COMPILER
> > +
> >  all install relabel clean distclean indent:
> >   @for subdir in $(SUBDIRS); do \
> >   (cd $$subdir && $(MAKE) $@) || exit 1; \
> > diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
> > index 13501cd..7a1ae05 100644
> > --- a/libselinux/src/Makefile
> > +++ b/libselinux/src/Makefile
> > @@ -48,23 +48,39 @@ OBJS= $(patsubst %.c,%.o,$(SRCS))
> >  LOBJS= $(patsubst %.c,%.lo,$(SRCS))
> >  CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security
-Winit-self -Wmissing-include-dirs \
> >-Wunused -Wunknown-pragmas -Wstrict-aliasing -Wshadow
-Wpointer-arith \
> > -  -Wbad-function-cast -Wcast-align -Wwrite-strings
-Wlogical-op -Waggregate-return \
> > +  -Wbad-function-cast -Wcast-align -Wwrite-strings
-Waggregate-return \
> >-Wstrict-prototypes -Wold-style-definition
-Wmissing-prototypes \
> >-Wmissing-declarations -Wmissing-noreturn
-Wmissing-format-attribute \
> >-Wredundant-decls -Wnested-externs -Winline -Winvalid-pch
-Wvolatile-register-var \
> > -  -Wdisabled-optimization -Wbuiltin-macro-redefined
-Wpacked-bitfield-compat \
> > -  -Wsync-nand -Wattributes -Wcoverage-mismatch -Wmultichar
-Wcpp \
> > +  -Wdisabled-optimization -Wbuiltin-macro-redefined \
> > +  -Wattributes -Wmultichar \
> >-Wdeprecated-declarations -Wdiv-by-zero -Wdouble-promotion
-Wendif-labels -Wextra \
> > -  -Wformat-contains-nul -Wformat-extra-args
-Wformat-zero-length -Wformat=2 -Wmultichar \
> > -  -Wnormalized=nfc -Woverflow -Wpointer-to-int-cast -Wpragmas
-Wsuggest-attribute=const \
> > -  -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure
-Wtrampolines \
> > -  -Wno-missing-field-initializers -Wno-sign-compare
-Wjump-misses-init \
> > -  -Wno-format-nonliteral -Wframe-larger-than=$(MAX_STACK_SIZE)
-Wp,-D_FORTIFY_SOURCE=2 \
> > +  -Wformat-extra-args -Wformat-zero-length -Wformat=2
-Wmultichar \
> > +  -Woverflow -Wpointer-to-int-cast -Wpragmas \
> > +  -Wno-missing-field-initializers -Wno-sign-compare \
> > +  -Wno-format-nonliteral -Wframe-larger-than=$(MAX_STACK_SIZE)
\
> >-fstack-protector-all --param=ssp-buffer-size=4 -fexceptions
\
> >-fasynchronous-unwind-tables -fdiagnostics-show-option
-funit-at-a-time \
> > -  -fipa-pure-const -Wno-suggest-attribute=pure
-Wno-suggest-attribute=const \
> >-Werror -Wno-aggregate-return -Wno-redundant-decls
> >
> > +LD_SONAME_FLAGS=-soname,$(LIBSO),-z,defs,-z,relro
> > +
> > +ifeq ($(COMPILER), gcc)
> > +override CFLAGS += -fipa-pure-const -Wlogical-op
-Wpacked-bitfield-compat -Wsync-nand \
> > + -Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc
-Wsuggest-attribute=const \
> > + -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure
-Wtrampolines -Wjump-misses-init \
> > + -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
-Wp,-D_FORTIFY_SOURCE=2
> > +else
> > +override CFLAGS += -Wunused-command-line-argument
&g

Re: Confidentiality and privacy

[William Roberts]

On Thu, Oct 13, 2016 at 5:19 PM, Eduardo Aguirre  wrote:
> Aren't Tomoyo, Apparmor and Smack other LSMs (Linux Security Modules) in the
> Linux Kernel used in Android?

Officially no, just SE Linux. However, I have seen some devices with
TOMOYO enabled,
but those were OEM enabled.

>
>
> El jue., oct. 13, 2016 16:04, Stephen Smalley  escribió:
>>
>> 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 (> > >) 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
>> > (
>> > > >>) 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.



-- 
Respectfully,

William C Roberts

___
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

[William Roberts]

The only "LSM" in Android is SELinux. The term LSM means Linux
Security Module and
is a Linux kernel technology.

If you want to actually look deeper in how SE Linux was integrated, parts of
Exploring SE for Android (my book), may be of help.

As far as Android Security, that internals book you mention is the
best general coverage
I have found.

On Thu, Oct 13, 2016 at 4: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.
>
> El jue., 13 oct. 2016 a las 11:25, Stephen Smalley ()
> 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 (> > >) 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.



-- 
Respectfully,

William C Roberts

___
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

[William Roberts]

On Oct 6, 2016 04:53, "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??

Why would you enable this on Android? It has ueventd support. The best fix
is not to enable it.

>
>
>
> Thanks.
>
> 
> This email message is for the sole use of the intended recipient(s) and
may contain confidential information.  Any unauthorized review, use,
disclosure or distribution is prohibited.  If you are not the intended
recipient, please contact the sender by reply email and destroy all copies
of the original message.
> 
>
> ___
> 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: [PATCH] libselinux: re-introduce DISABLE_BOOL=y

[William Roberts]

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.

-- 
Respectfully,

William C Roberts
___
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

[William Roberts]

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

[William Roberts]

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

[William Roberts]

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.

Seems to be yet-another red-hat contribution from a long time ago:

commit cec06ec8282c538a40bde968ae36fe8356daffaa
Author: Petr Machata <pmach...@redhat.com>
Date:   Tue Apr 10 13:31:55 2012 +0200

Warn when we fail to trace and SELinux boolean deny_ptrace is in effect

diff --git a/ChangeLog b/ChangeLog
index c095263..6107a12 100644

>
>>
>> Signed-off-by: William Roberts <william.c.robe...@intel.com>
>> ---
>>  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)))
>

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

[William Roberts]

do you have the corresponding changes to checkfc on AOSP?

On Thu, Sep 29, 2016 at 7:39 AM, 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 new back end for android
> services with a stricter lookup function. Now the
> service name must match the key of the service label
> exactly.
>
> Signed-off-by: Janis Danisevskis 
> ---
>  libselinux/include/selinux/label.h  |  2 ++
>  libselinux/src/label.c  |  1 +
>  libselinux/src/label_backends_android.c | 54 
> +++--
>  libselinux/src/label_internal.h |  3 ++
>  libselinux/utils/selabel_digest.c   |  2 ++
>  libselinux/utils/selabel_lookup.c   |  2 ++
>  6 files changed, 62 insertions(+), 2 deletions(-)
>
> 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,
>  };
>
>  static void selabel_subs_fini(struct selabel_sub *ptr)
> diff --git a/libselinux/src/label_backends_android.c 
> b/libselinux/src/label_backends_android.c
> index 290b438..4d6ec86 100644
> --- a/libselinux/src/label_backends_android.c
> +++ b/libselinux/src/label_backends_android.c
> @@ -244,7 +244,7 @@ static void closef(struct selabel_handle *rec)
> free(data);
>  }
>
> -static struct selabel_lookup_rec *lookup(struct selabel_handle *rec,
> +static struct selabel_lookup_rec *property_lookup(struct selabel_handle *rec,
>  const char *key,
>  int __attribute__((unused)) type)
>  {
> @@ -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;
> +   }
> +
> +   ret = _arr[i].lr;
> +
> +finish:
> +   return ret;
> +}
> +
>  static void stats(struct selabel_handle __attribute__((unused)) *rec)
>  {
> selinux_log(SELINUX_WARNING, "'stats' functionality not 
> implemented.\n");
> @@ -298,7 +330,25 @@ int selabel_property_init(struct selabel_handle *rec,
> rec->data = data;
> rec->func_close = 
> rec->func_stats = 
> -   rec->func_lookup = 
> +   rec->func_lookup = _lookup;
> +
> +   return init(rec, opts, nopts);
> +}
> +
> +int selabel_service_init(struct selabel_handle *rec,
> +   const struct selinux_opt *opts, unsigned nopts)
> +{
> +   struct saved_data *data;
> +
> +   data = (struct saved_data *)malloc(sizeof(*data));
> +   if (!data)
> +   return -1;
> +   memset(data, 0, sizeof(*data));
> +
> +   rec->data = data;
> +   rec->func_close = 
> +   rec->func_stats = 
> +   rec->func_lookup = _lookup;
>
> return init(rec, opts, nopts);
>  }
> diff --git a/libselinux/src/label_internal.h b/libselinux/src/label_internal.h
> index 7c55531..6a9481a 100644
> --- a/libselinux/src/label_internal.h
> +++ b/libselinux/src/label_internal.h
> @@ -39,6 +39,9 @@ int selabel_db_init(struct selabel_handle *rec,
>  int selabel_property_init(struct selabel_handle *rec,
> const struct selinux_opt *opts,
> unsigned nopts) hidden;
> +int selabel_service_init(struct selabel_handle *rec,
> + 

Re: [RFC] Build ANDROID_HOST=y on mac

[William Roberts]

On Sep 28, 2016 17:07, "Joshua Brindle" <brin...@quarksecurity.com> wrote:
>
> William Roberts wrote:
>>
>> On Sep 28, 2016 16:54, "Joshua Brindle"<brin...@quarksecurity.com>
wrote:
>>>
>>> Joshua Brindle wrote:
>>>>
>>>> William Roberts wrote:
>>>>>
>>>>>  From commit 35d702 on
>>>>> https://github.com/williamcroberts/selinux/tree/fix-mac
>>>>>
>>>>> I have a branch that is building on my elcapitan mac, requesting any
>>>>> comments anyone
>>>>> wishes to make, before I send them out.
>>>>>
>>>>> If you wish to test, this is the procedure
>>>>>
>>>>> 1. Build libsepol (assumes at root of tree)
>>>>> a, cd libsepol
>>>>> b. make
>>>>> 2. Build libselinux
>>>>> a. cd libselinux (assumes at root of tree)
>>>>> b. make ANDROID_HOST=y
>>>>>
>>>> This works for me.
>>>
>>>
>>> make install DESTDIR=/tmp/someidr mostly works, Mac ln does not support
>>
>> --relative so that fails. ANDROID_HOST also needs to be set in the top
>> level makefile so that it propagates down:
>>>
>>> ANDROID_HOST ?= n
>>
>>
>> Yeah install doesn't work on Mac, that's why for Darwin we just set the
>> path to the libsepol location for sefcontext_compile.
>>
>> As for ANDROID_HOST, why does it need to go higher? It's only used in
>> libselinux and is declared and used just like DISABLE_SETRANS...I'm not
>> following you?
>>
>
> Because I was building from the top, basically seeing if I could get a
usable toolchain out of it, but that looks like it would require a bit more
work.

Ahh yeah, that won't work yet, but that would be nice.

>
>
>>>
>>>>
>>>>> This essentially gets us to where a build server/maintainer
>>>>> can test patches quickly on mac, with some assurance
>>>>> it's not busted without downloading all of Android.
>>>>>
>>>>> It's still wise to check in an Android tree if possible IMHO.
>>>>>
>>
>
___
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.

[RFC] Build ANDROID_HOST=y on mac

[William Roberts]

>From commit 35d702 on https://github.com/williamcroberts/selinux/tree/fix-mac

I have a branch that is building on my elcapitan mac, requesting any
comments anyone
wishes to make, before I send them out.

If you wish to test, this is the procedure

1. Build libsepol (assumes at root of tree)
a,  cd libsepol
b. make
2. Build libselinux
a. cd libselinux (assumes at root of tree)
b. make ANDROID_HOST=y

This essentially gets us to where a build server/maintainer
can test patches quickly on mac, with some assurance
it's not busted without downloading all of Android.

It's still wise to check in an Android tree if possible IMHO.

-- 
Respectfully,

William C Roberts
___
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

[William Roberts]

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?

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



-- 
Respectfully,

William C Roberts
___
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

[William Roberts]

On Wed, Sep 28, 2016 at 11:53 AM,  <william.c.robe...@intel.com> wrote:
> From: William Roberts <william.c.robe...@intel.com>
>
> 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];
>

FYI this just happened when building with ANDROID_HOST=y on my
development branch, so I am
assuming some Makefile change introduced a hard error on it... inside
the android tree, it was still
building, without even printing a warning as well.

> Fix it by moving the variable into the ifdef'd usage block.
>
> Signed-off-by: William Roberts <william.c.robe...@intel.com>
> ---
>  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 =
> --
> 1.9.1
>
> ___
> 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: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[William Roberts]

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

Well actually, that means every-time their is a new file added, the
Makefile needs to be modified,
I was trying to avoid that. Also, the remove list is super long, so it
looks pretty unwieldy.


___
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

[William Roberts]

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

[William Roberts]

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

[William Roberts]

On Sep 27, 2016 10:00, "Stephen Smalley" <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>
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.
> >
> >>
> >>>
> >>> 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
>
> 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.
___
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

[William Roberts]

On Sep 27, 2016 09:50, "Stephen Smalley" <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>
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 $@ $^
> >

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

[William Roberts]

On Tue, Sep 27, 2016 at 7:03 AM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
> On 09/26/2016 04:55 PM, William Roberts wrote:
>> On Mon, Sep 26, 2016 at 1: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
>>>
>>> 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 \
>>
>> 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?
>
It's superfluous because it;s not used in the source files built for
ANDROID_HOST, and
it doesn't match the build recipe for Android. So if the goal would be
to provide a quick
flag to test the build against, it wouldn't meet that requirement.
___
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

[William Roberts]

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.

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

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

this prevents us from having multiple definitions of the recipes or
multiple if's on ANDROID_HOST=y.
Both flavors need these wildcard targets.

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

Those things die on linking... so I didn't want to s

Re: Android Fork

[William Roberts]

On Sep 27, 2016 07:52, "Jason Zaman"  wrote:
>
> I just remembered that travis-ci has OSX stuff now.
> https://docs.travis-ci.com/user/osx-ci-environment/
>
> Maybe we should setup a .travis.yml for selinux to build all these
> possible configurations going forward?

At least the Android config would be nice, because certain things
will never build on mac.

>

> On Mon, Sep 26, 2016 at 10:33:37AM -0700, 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.
> >
> > [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.
> ___
> 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: 答复: A question about booting process with SELinux.

[William Roberts]

On Sep 27, 2016 00:00, "Weiyuan (David, Euler)" <weiyuan@huawei.com>
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) ?

There is nothing happening with selinux until userspace mounts selinuxfs
and loads the policy file. The initial sid, which is the initial label for
an object, is encoded in the loaded policy along with the genfscon
statements. Genfscon is used primarily for labeling filesystems that do not
have xattr support.

For filesystems that have xattr, if set, the sid encoded in the xattr
security.selinux is used.

The file_contexts file, file_contexts.bin as of late, is used by userspace
services to.properly set the xattr label on filesystems. Namely init and
ueventd use it.

The basic boot sequence is:
1. Boot the kernel and exec init
2. Initialize selinux mount
3. Load the policy file
3. Restorecon /init
4. setenforce(1) unless non-user build and Android boot variable is set to
permissive.
5. Exec init in the second stage
6. Init script calls restorecon /data and /sys to reset labels if
fike_contexts changed (ie update).

I'm recalling this off the top of my head, some of the ordering might be
slightly off, but the concepts should be correct. You can verify by reading
init.cpp in system/core/init for the userspace load sequence.

To answer your question concisely, selinux isn't doing anything or labeling
anything until the policy is loaded.

>
>
>
> Thanks.
>
>
> -邮件原件-
> 发件人: Stephen Smalley [mailto:s...@tycho.nsa.gov]
> 发送时间: 2016年9月27日 0:43
> 收件人: Weiyuan (David, Euler); William Roberts
> 抄送: seandroid-list@tycho.nsa.gov
> 主题: Re: A question about booting process with SELinux.
>
> 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: [PATCH v2] libselinux: add ANDROID_HOST=y build option

[William Roberts]

On Mon, Sep 26, 2016 at 1: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
>
> 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 \

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.


___
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 Fork

[William Roberts]

On Mon, Sep 26, 2016 at 12:10 PM, Stephen Smalley  wrote:
> 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.

+ AOSP folks.

I've been pondering the same thing, so I am glad one of us started this
discussion. Based on what your stating, I think it makes sense to kill
the Android.mk and Android.bp files upstream, but ill let the AOSP
folks chime in as well.

I think it also makes sense to add ANDROID_HOST=y option for the build
so we can at least test the Android host recipe within the upstream Makefiles,
thoughts?


___
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 Fork

[William Roberts]

On Mon, Sep 26, 2016 at 10:33 AM,   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.
>
> [PATCH 1/2] libselinux: introduce configurable backends
> [PATCH 2/2] libselinux: add ifdef'ing for ANDROID and BUILD_HOST

FYI: AOSP changes:
https://android-review.googlesource.com/#/q/topic:fork-kill+(status:open+OR+status:merged)
___
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

[William Roberts]

On Mon, Sep 26, 2016 at 10:43 AM, Stephen Smalley  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?

>
> 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(),
>>
>



-- 
Respectfully,

William C Roberts
___
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

[William Roberts]

On Mon, Sep 26, 2016 at 10:43 AM, Stephen Smalley  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)
>
> 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...

I think a lot of it might be with how complex your regex's are. Android is
pretty simple, which shows, as textual and binary load times are
pretty close. I think judicious use of -r and knowing what works best for
your arch/build/fc entries,  at the moment, is likely required.


___
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 1/3] libselinux: Add architecture string to file_context.bin

[William Roberts]

On Mon, Sep 26, 2016 at 7:22 AM, Janis Danisevskis  wrote:
> Serialized precompiled regular expressins are architecture
> dependent when using PCRE2. This patch
> - bumps the SELINUX_COMPILED_FCONTEXT version to 5 and
> - adds a field to the output indicating the architecture
>   compatibility.
>
> libselinux can cope with an architecture mismatch by
> ignoring the precompiled data in the input file and recompiling
> the regular expressions at runtime. It can also load older
> versions of file_contexts.bin if they where built with
> sefcontext_compile using the exact same version of the
> pcre1/2 as selinux.
>
> Signed-off-by: Janis Danisevskis 
> ---
>  libselinux/src/label_file.c   | 44 +-
>  libselinux/src/label_file.h   |  4 ++-
>  libselinux/src/regex.c| 50 
> ---
>  libselinux/src/regex.h| 15 ++-
>  libselinux/utils/sefcontext_compile.c | 13 +
>  5 files changed, 120 insertions(+), 6 deletions(-)
>
> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
> index 7156825..9abc1d6 100644
> --- a/libselinux/src/label_file.c
> +++ b/libselinux/src/label_file.c
> @@ -126,6 +126,8 @@ static int load_mmap(FILE *fp, size_t len, struct 
> selabel_handle *rec,
> uint32_t i, magic, version;
> uint32_t entry_len, stem_map_len, regex_array_len;
> const char *reg_version;
> +   const char *reg_arch;
> +   char reg_arch_matches = 1;

It appears all the patchs would reset this to 0 properly, but is their
any reason you
chose the default to be that it matches? I would assume it would be
safer to mark
this as its not a match.

>
> mmap_area = malloc(sizeof(*mmap_area));
> if (!mmap_area) {
> @@ -159,6 +161,10 @@ static int load_mmap(FILE *fp, size_t len, struct 
> selabel_handle *rec,
> if (!reg_version)
> return -1;
>
> +   reg_arch = regex_arch_string();
> +   if (!reg_arch)
> +   return -1;
> +
> if (version >= SELINUX_COMPILED_FCONTEXT_PCRE_VERS) {
>
> len = strlen(reg_version);
> @@ -188,7 +194,43 @@ static int load_mmap(FILE *fp, size_t len, struct 
> selabel_handle *rec,
> return -1;
> }
> free(str_buf);
> +
> +   if (version >= SELINUX_COMPILED_FCONTEXT_REGEX_ARCH) {
> +   len = strlen(reg_arch);
> +
> +   rc = next_entry(_len, mmap_area,
> +   sizeof(uint32_t));
> +   if (rc < 0)
> +   return -1;
> +
> +   /* Check arch string lengths */
> +   if (len != entry_len) {
> +   /*
> +* Skip the entry and conclude that we have
> +* a mismatch, which is not fatal.
> +*/
> +   next_entry(NULL, mmap_area, entry_len);
> +   reg_arch_matches = 0;
> +   goto end_arch_check;
> +   }
> +
> +   /* Check if arch string mismatch */
> +   str_buf = malloc(entry_len + 1);
> +   if (!str_buf)
> +   return -1;
> +
> +   rc = next_entry(str_buf, mmap_area, entry_len);
> +   if (rc < 0) {
> +   free(str_buf);
> +   return -1;
> +   }
> +
> +   str_buf[entry_len] = '\0';
> +   reg_arch_matches = strcmp(str_buf, reg_arch) == 0;
> +   free(str_buf);
> +   }
> }
> +end_arch_check:
>
> /* allocate the stems_data array */
> rc = next_entry(_map_len, mmap_area, sizeof(uint32_t));
> @@ -349,7 +391,7 @@ static int load_mmap(FILE *fp, size_t len, struct 
> selabel_handle *rec,
> spec->prefix_len = prefix_len;
> }
>
> -   rc = regex_load_mmap(mmap_area, >regex);
> +   rc = regex_load_mmap(mmap_area, >regex, 
> reg_arch_matches);
> if (rc < 0)
> goto out;
>
> diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h
> index 88f4294..00c0a5c 100644
> --- a/libselinux/src/label_file.h
> +++ b/libselinux/src/label_file.h
> @@ -24,8 +24,10 @@
>  #define SELINUX_COMPILED_FCONTEXT_PCRE_VERS2
>  #define SELINUX_COMPILED_FCONTEXT_MODE 3
>  #define SELINUX_COMPILED_FCONTEXT_PREFIX_LEN   4
> +#define SELINUX_COMPILED_FCONTEXT_REGEX_ARCH   5
>
> -#define SELINUX_COMPILED_FCONTEXT_MAX_VERS 
> SELINUX_COMPILED_FCONTEXT_PREFIX_LEN
> +#define SELINUX_COMPILED_FCONTEXT_MAX_VERS \
> +   

Re: Killing The Android libselinux Fork (available)

[William Roberts]

I am quite happy to report that what's on the current fork-kill branch on github
libselinux builds on mac with no warnings. Also, we want the libsepol
patches upstream to
cleanse those warnings as well.

All the smaller patches have been sent out and merged, with the exception of
"libselinux: drop unused stdio_ext.h header file", which was just sent
out. I don't
expect an issue on that merge.

This leaves:
https://github.com/williamcroberts/selinux/commit/df022f0f4425498f8537cc2c73064b6bb37c6a05
will be whats needed  upstream to close the gap. I think we might want to
take a revertme patch that comments out all the libselinux build
files, so when it's merged into
aosp theirs no duplicate definitions of libselinux, and then once they
can revert that commit,
we can do the same. They will likely have a window where they can
fetch upstream into external/selinux
before killing external/libselinux and enabling the build files. This
seems to be the approach last
time with a blank toplevel Android.mk

sds let me know how you want the afformentioned commit on the link
above broken up if at all, or any other
issues you  have, and Ill prep the final patch series for the mailing list.

Thanks all for the input provided, and Josh for your late night mac help!

On Fri, Sep 23, 2016 at 1:44 PM, William Roberts
<bill.c.robe...@gmail.com> wrote:
> On Fri, Sep 23, 2016 at 1:24 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
>> On 09/23/2016 04:01 PM, Joshua Brindle wrote:
>>> William Roberts wrote:
>>>> On Fri, Sep 23, 2016 at 6:57 AM, Joshua Brindle
>>>> <brin...@quarksecurity.com>  wrote:
>>>>> William Roberts wrote:
>>>>>> On Sep 22, 2016 9:18 PM, "Jeffrey Vander Stoep"<je...@google.com>
>>>>>> 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.
>>
>
> All those files belong to libsepol, which I am pretty sure is building on Mac.
> However, size is unused on __APPLE__ path, so I will prep patches for those
> unused and uninitialized variables.
>
> I am mostly concerned about libselinux, which on Android we only use a subset
> of the files. It would be possible to have a Makefile setup with the
> files and defines
> but it would only build the build host version and not the target. The
> target would
> be trickier as it links and builds against bionic.
>
> --
> Respectfully,
>
> William C Roberts



-- 
Respectfully,

William C Roberts
___
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)

[William Roberts]

On Fri, Sep 23, 2016 at 1:24 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
> On 09/23/2016 04:01 PM, Joshua Brindle wrote:
>> William Roberts wrote:
>>> On Fri, Sep 23, 2016 at 6:57 AM, Joshua Brindle
>>> <brin...@quarksecurity.com>  wrote:
>>>> William Roberts wrote:
>>>>> On Sep 22, 2016 9:18 PM, "Jeffrey Vander Stoep"<je...@google.com>
>>>>> 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.
>

All those files belong to libsepol, which I am pretty sure is building on Mac.
However, size is unused on __APPLE__ path, so I will prep patches for those
unused and uninitialized variables.

I am mostly concerned about libselinux, which on Android we only use a subset
of the files. It would be possible to have a Makefile setup with the
files and defines
but it would only build the build host version and not the target. The
target would
be trickier as it links and builds against bionic.

-- 
Respectfully,

William C Roberts
___
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)

[William Roberts]

In case anyone is following, Stephen provided some early feedback here:
https://github.com/williamcroberts/selinux/commit/f40d7facbcaf1337f37b5630b98806fd25b1dbf9#diff-ee9fed90a923eef67184cd34ffbb2a9cR551

As promised, the fork was rebased and force pushed.

On Thu, Sep 22, 2016 at 6:39 PM, William Roberts
<bill.c.robe...@gmail.com> wrote:
> On Thu, Sep 22, 2016 at 6:34 PM, William Roberts
> <bill.c.robe...@gmail.com> wrote:
>> So I have been working the last couple of days to understand what it
>> would take to kill external/libselinux (the Android Fork) and fixup
>> upstream so most of the delta is in. The only thing we would keep on
>> the Android side, is android.c and .h. Since those files are self
>> contained, we should just be able to merge upstream without concerns
>> of conflict. If we really wanted to, we could spin off a separate
>> libselinux-android that builds those two files and links to
>> libselinux, but that seems overkill IMHO.
>>
>> The work is available here:
>> https://github.com/williamcroberts/selinux/tree/fork-kill
>>
>> Currently to Build:
>> 1. remove external/libselinux
>> 2. apply this patch to bionic if not present:
>> https://android-review.googlesource.com/#/c/276918
>> 3. either set external/selinux to my fork-kill branch or merge selinux
>> upstream master into external/selinux and apply the two patches listed
>> below:
>>
>> Patches that matter ( I don't know how to make pretty little git summaries):
>>
>> commit e017f48acd2791a6aa62b4ed0c0b44256b26651f
>> Author: William Roberts <william.c.robe...@intel.com>
>> Date:   Wed Sep 21 16:06:37 2016 -0700
>> libselinux: add The Android fork files
>>
>> commit f40d7facbcaf1337f37b5630b98806fd25b1dbf9
>> Author: William Roberts <william.c.robe...@intel.com>
>> Date:   Wed Sep 21 16:00:34 2016 -0700
>> libselinux: rectify the Android fork
>>
>> The goal would be to upstream commit f40d7facb and leave
>> commit e017f48ac on the Android tree.
>>
>> I am going to do some further testing tomorrow, and plan on submitting
>> the upstream patch f40d7facbc on Monday. If anyone wants to leave
>> preliminary feedback, or has a specific thing they want tested, let me know.
>>
>> Currently tested on the emulator and checked that the digest mechanism for
>> last restorecon value is working.
>>
>> --
>> Respectfully,
>>
>> William C Roberts
>
> FYI I may rebase that branch at anytime... you have been warned :-P



-- 
Respectfully,

William C Roberts
___
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)

[William Roberts]

On Fri, Sep 23, 2016 at 6:57 AM, Joshua Brindle
<brin...@quarksecurity.com> wrote:
> William Roberts wrote:
>>
>> On Sep 22, 2016 9:18 PM, "Jeffrey Vander Stoep"<je...@google.com>  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.



>>>>> The work is available here:
>>>>> https://github.com/williamcroberts/selinux/tree/fork-kill
>>>>>
>>>>> Currently to Build:
>>>>> 1. remove external/libselinux
>>>>> 2. apply this patch to bionic if not present:
>>>>> https://android-review.googlesource.com/#/c/276918
>>>>> 3. either set external/selinux to my fork-kill branch or merge selinux
>>>>> upstream master into external/selinux and apply the two patches listed
>>>>> below:
>>>>>
>>>>> Patches that matter ( I don't know how to make pretty little git
>>
>> summaries):
>>>>>
>>>>> commit e017f48acd2791a6aa62b4ed0c0b44256b26651f
>>>>> Author: William Roberts<william.c.robe...@intel.com>
>>>>> Date:   Wed Sep 21 16:06:37 2016 -0700
>>>>> libselinux: add The Android fork files
>>>>>
>>>>> commit f40d7facbcaf1337f37b5630b98806fd25b1dbf9
>>>>> Author: William Roberts<william.c.robe...@intel.com>
>>>>> Date:   Wed Sep 21 16:00:34 2016 -0700
>>>>> libselinux: rectify the Android fork
>>>>>
>>>>> The goal would be to upstream commit f40d7facb and leave
>>>>> commit e017f48ac on the Android tree.
>>>>>
>>>>> I am going to do some further testing tomorrow, and plan on submitting
>>>>> the upstream patch f40d7facbc on Monday. If anyone wants to leave
>>>>> preliminary feedback, or has a specific thing they want tested, let 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: Killing The Android libselinux Fork (available)

[William Roberts]

On Sep 22, 2016 9:18 PM, "Jeffrey Vander Stoep" <je...@google.com> 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?

>
> On Thu, Sep 22, 2016 at 6:39 PM William Roberts <bill.c.robe...@gmail.com>
wrote:
>>
>> On Thu, Sep 22, 2016 at 6:34 PM, William Roberts
>> <bill.c.robe...@gmail.com> wrote:
>> > So I have been working the last couple of days to understand what it
>> > would take to kill external/libselinux (the Android Fork) and fixup
>> > upstream so most of the delta is in. The only thing we would keep on
>> > the Android side, is android.c and .h. Since those files are self
>> > contained, we should just be able to merge upstream without concerns
>> > of conflict. If we really wanted to, we could spin off a separate
>> > libselinux-android that builds those two files and links to
>> > libselinux, but that seems overkill IMHO.
>> >
>> > The work is available here:
>> > https://github.com/williamcroberts/selinux/tree/fork-kill
>> >
>> > Currently to Build:
>> > 1. remove external/libselinux
>> > 2. apply this patch to bionic if not present:
>> > https://android-review.googlesource.com/#/c/276918
>> > 3. either set external/selinux to my fork-kill branch or merge selinux
>> > upstream master into external/selinux and apply the two patches listed
>> > below:
>> >
>> > Patches that matter ( I don't know how to make pretty little git
summaries):
>> >
>> > commit e017f48acd2791a6aa62b4ed0c0b44256b26651f
>> > Author: William Roberts <william.c.robe...@intel.com>
>> > Date:   Wed Sep 21 16:06:37 2016 -0700
>> > libselinux: add The Android fork files
>> >
>> > commit f40d7facbcaf1337f37b5630b98806fd25b1dbf9
>> > Author: William Roberts <william.c.robe...@intel.com>
>> > Date:   Wed Sep 21 16:00:34 2016 -0700
>> > libselinux: rectify the Android fork
>> >
>> > The goal would be to upstream commit f40d7facb and leave
>> > commit e017f48ac on the Android tree.
>> >
>> > I am going to do some further testing tomorrow, and plan on submitting
>> > the upstream patch f40d7facbc on Monday. If anyone wants to leave
>> > preliminary feedback, or has a specific thing they want tested, let me
know.
>> >
>> > Currently tested on the emulator and checked that the digest mechanism
for
>> > last restorecon value is working.
>> >
>> > --
>> > Respectfully,
>> >
>> > William C Roberts
>>
>> FYI I may rebase that branch at anytime... you have been warned :-P
___
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)

[William Roberts]

On Thu, Sep 22, 2016 at 6:34 PM, William Roberts
<bill.c.robe...@gmail.com> wrote:
> So I have been working the last couple of days to understand what it
> would take to kill external/libselinux (the Android Fork) and fixup
> upstream so most of the delta is in. The only thing we would keep on
> the Android side, is android.c and .h. Since those files are self
> contained, we should just be able to merge upstream without concerns
> of conflict. If we really wanted to, we could spin off a separate
> libselinux-android that builds those two files and links to
> libselinux, but that seems overkill IMHO.
>
> The work is available here:
> https://github.com/williamcroberts/selinux/tree/fork-kill
>
> Currently to Build:
> 1. remove external/libselinux
> 2. apply this patch to bionic if not present:
> https://android-review.googlesource.com/#/c/276918
> 3. either set external/selinux to my fork-kill branch or merge selinux
> upstream master into external/selinux and apply the two patches listed
> below:
>
> Patches that matter ( I don't know how to make pretty little git summaries):
>
> commit e017f48acd2791a6aa62b4ed0c0b44256b26651f
> Author: William Roberts <william.c.robe...@intel.com>
> Date:   Wed Sep 21 16:06:37 2016 -0700
> libselinux: add The Android fork files
>
> commit f40d7facbcaf1337f37b5630b98806fd25b1dbf9
> Author: William Roberts <william.c.robe...@intel.com>
> Date:   Wed Sep 21 16:00:34 2016 -0700
> libselinux: rectify the Android fork
>
> The goal would be to upstream commit f40d7facb and leave
> commit e017f48ac on the Android tree.
>
> I am going to do some further testing tomorrow, and plan on submitting
> the upstream patch f40d7facbc on Monday. If anyone wants to leave
> preliminary feedback, or has a specific thing they want tested, let me know.
>
> Currently tested on the emulator and checked that the digest mechanism for
> last restorecon value is working.
>
> --
> Respectfully,
>
> William C Roberts

FYI I may rebase that branch at anytime... you have been warned :-P
___
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.

Killing The Android libselinux Fork (available)

[William Roberts]

So I have been working the last couple of days to understand what it
would take to kill external/libselinux (the Android Fork) and fixup
upstream so most of the delta is in. The only thing we would keep on
the Android side, is android.c and .h. Since those files are self
contained, we should just be able to merge upstream without concerns
of conflict. If we really wanted to, we could spin off a separate
libselinux-android that builds those two files and links to
libselinux, but that seems overkill IMHO.

The work is available here:
https://github.com/williamcroberts/selinux/tree/fork-kill

Currently to Build:
1. remove external/libselinux
2. apply this patch to bionic if not present:
https://android-review.googlesource.com/#/c/276918
3. either set external/selinux to my fork-kill branch or merge selinux
upstream master into external/selinux and apply the two patches listed
below:

Patches that matter ( I don't know how to make pretty little git summaries):

commit e017f48acd2791a6aa62b4ed0c0b44256b26651f
Author: William Roberts <william.c.robe...@intel.com>
Date:   Wed Sep 21 16:06:37 2016 -0700
libselinux: add The Android fork files

commit f40d7facbcaf1337f37b5630b98806fd25b1dbf9
Author: William Roberts <william.c.robe...@intel.com>
Date:   Wed Sep 21 16:00:34 2016 -0700
libselinux: rectify the Android fork

The goal would be to upstream commit f40d7facb and leave
commit e017f48ac on the Android tree.

I am going to do some further testing tomorrow, and plan on submitting
the upstream patch f40d7facbc on Monday. If anyone wants to leave
preliminary feedback, or has a specific thing they want tested, let me know.

Currently tested on the emulator and checked that the digest mechanism for
last restorecon value is working.

-- 
Respectfully,

William C Roberts
___
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

[William Roberts]

On Wed, Sep 21, 2016 at 2:48 PM, William Roberts
<bill.c.robe...@gmail.com> wrote:
> On Sep 21, 2016 13:16, "Stephen Smalley" <s...@tycho.nsa.gov> wrote:
>>
>> 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.
>
> I'm not arguing the fix, just wondering if it was blazingly fast. I didn't
> find the definition or a define in bionic for fgets_unlocked, but when I set
> _GNUC define, it seemed to build OK.

That's an incorrect statement, that was only for the host built
against glibc where
_GNU_SOURCE needs to be defined. For Android, yes the simplest way is as
you suggest.


-- 
Respectfully,

William C Roberts

___
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

[William Roberts]

On Sep 21, 2016 13:16, "Stephen Smalley" <s...@tycho.nsa.gov> wrote:
>
> 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.

I'm not arguing the fix, just wondering if it was blazingly fast. I didn't
find the definition or a define in bionic for fgets_unlocked, but when I
set _GNUC define, it seemed to build 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: unlocked stdio

[William Roberts]

On Sep 21, 2016 13:06, "Stephen Smalley"  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?

>
> >
> >
> >
> > *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
> >
> >
> >
>
> ___
> 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: [RFC] mmap file_contexts and property_contexts:

[William Roberts]

On Sep 19, 2016 22:25, "Jason Zaman" <ja...@perfinion.com> wrote:
>
> On 20 Sep 2016 12:50 pm, "William Roberts" <bill.c.robe...@gmail.com>
wrote:
> >
> > On Sep 19, 2016 21:16, "Jason Zaman" <ja...@perfinion.com> wrote:
> > >
> > > On 20 Sep 2016 5:47 am, <william.c.robe...@intel.com> wrote:
> > > >
> > > > From: William Roberts <william.c.robe...@intel.com>
> > > >
> > > > 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.

>
> -- Jason
> > >
> > > > THINGS TO DO:
> > > > 1. What's arm performance like?
> > > > 2. What interfaces to backends are busted by this (if any)?
> > > > 3. Test Android Properties
> > > > 4. Im pretty sure this breaks sefcontext_compile, fix.
> > > > 5. Test with PCRE2 enabled.
> > > > 6. Spell check this message!
> > > > 7. Run checkpatch
> > > >
> > > > Signed-off-by: William Roberts <william.c.robe...@intel.com>
> > > > ---
> > > >  libselinux/src/label.c  |   2 -
> > > >  libselinux/src/label_android_property.c |  22 ++---
> > > >  libselinux/src/label_file.c | 140
+++-
> > > >  libselinux/src/label_file.h |  66 +--
> > > >  libselinux/src/label_internal.h |   3 +-
> > > >  libselinux/src/label_support.c  |  79 --
> > > >  6 files changed, 172 insertions(+), 140 deletions(-)
> > > >
> > > > diff --git a/libselinux/src/label.c b/libselinux/src/label.c
> > > > index 963bfcb..d767b49 100644
> > > > --- a/libselinux/src/label.c
> > > > +++ b/libselinux/src/label.c
> > > > @@ -15,8 +15,6 @@
> > > >  #include "callbacks.h"
> > > >  #include "label_internal.h"
> > > >
> > > > -#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
> > > > -
> > > >  typedef int (*selabel_initfunc)(struct selabel_handle *rec,
> > > > const struct selinux_opt *opts,
> > > > unsigned nopts);
> > > > diff --git a/libselinux/src/label_android_property.c
b/libselinux/src/label_android_property.c
> > > > index 290b438..2aac394 100644
> > > > --- a/libselinux/src/label_android_property.c
> > > > +++ b/libselinux/src/label_android_property.c
> > > > @@ -85,13 +85,19 @@ static int process_line(struct selabel_handle
*r

Re: [RFC] mmap file_contexts and property_contexts:

[William Roberts]

On Sep 19, 2016 21:16, "Jason Zaman" <ja...@perfinion.com> wrote:
>
> On 20 Sep 2016 5:47 am, <william.c.robe...@intel.com> wrote:
> >
> > From: William Roberts <william.c.robe...@intel.com>
> >
> > 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().

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

>
> -- Jason
>
> > THINGS TO DO:
> > 1. What's arm performance like?
> > 2. What interfaces to backends are busted by this (if any)?
> > 3. Test Android Properties
> > 4. Im pretty sure this breaks sefcontext_compile, fix.
> > 5. Test with PCRE2 enabled.
> > 6. Spell check this message!
> > 7. Run checkpatch
> >
> > Signed-off-by: William Roberts <william.c.robe...@intel.com>
> > ---
> >  libselinux/src/label.c  |   2 -
> >  libselinux/src/label_android_property.c |  22 ++---
> >  libselinux/src/label_file.c | 140
+++-
> >  libselinux/src/label_file.h |  66 +--
> >  libselinux/src/label_internal.h |   3 +-
> >  libselinux/src/label_support.c  |  79 --
> >  6 files changed, 172 insertions(+), 140 deletions(-)
> >
> > diff --git a/libselinux/src/label.c b/libselinux/src/label.c
> > index 963bfcb..d767b49 100644
> > --- a/libselinux/src/label.c
> > +++ b/libselinux/src/label.c
> > @@ -15,8 +15,6 @@
> >  #include "callbacks.h"
> >  #include "label_internal.h"
> >
> > -#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
> > -
> >  typedef int (*selabel_initfunc)(struct selabel_handle *rec,
> > const struct selinux_opt *opts,
> > unsigned nopts);
> > diff --git a/libselinux/src/label_android_property.c
b/libselinux/src/label_android_property.c
> > index 290b438..2aac394 100644
> > --- a/libselinux/src/label_android_property.c
> > +++ b/libselinux/src/label_android_property.c
> > @@ -85,13 +85,19 @@ static int process_line(struct selabel_handle *rec,
> > int pass, unsigned lineno)
> >  {
> > int items;
> > -   char *prop = NULL, *context = NULL;
> > +   union {
> > +   struct {
> > +   char *prop;
> > +   char *context;
> > +   };
> > +   char *array[2];
> > +   } found = { .array = { 0 } };
> > struct saved_data *data = (struct saved_data *)rec->data;
> > spec_t *spec_arr = data->spec_arr;
> > unsigned int nspec = data->nspec;
> > const char *errbuf = NULL;
> >
> > -   items = read_spec_entries(line_buf, , 2, ,
);
> > +   items = read_spec_entries(line_buf, ,
ARRAY_SIZE(found.array), found.array);
> > if (items < 0) {
> > items = errno;
> > selinux_log(SELINUX_ERROR,
> > @@ -108,18 +114,14 @@ static int process_line(struct selabel_handle
*rec,
> > selinux_log(SELINUX_ERROR,
> > "%s:  line %u is missing fields\n", path,
> > lineno);
> > -   free(prop);
> > errno = EINVAL;
> > return -1;
> > }

Re: Avc denied for isolated app

[William Roberts]

On Sep 19, 2016 22:28, "Inamdar Sharif"  wrote:
>
> Hi ,
>
>
>
> I am getting the following avc denied

No, that woukd defeat the purpose if an isolated application. Isolated
applications are sandboxed even away from their own on disk resources.

https://developer.android.com/guide/topics/manifest/service-element.html#isolated

>
>
>
> avc: denied { search } for
pid=3174name="com.google.android.apps.mediashell" dev="mmcblk0p29"
ino=503938 scontext=u:r:isolated_app:s0:c512,c768
tcontext=u:object_r:app_data_file:s0:c512,c768 tclass=dir permissive=0
>
>
>
>
>
> Do we want to add the following rule:
>
> allow isolated_app app_data_file:dir search;
>
>
>
> Thanks.
>
> 
> This email message is for the sole use of the intended recipient(s) and
may contain confidential information.  Any unauthorized review, use,
disclosure or distribution is prohibited.  If you are not the intended
recipient, please contact the sender by reply email and destroy all copies
of the original message.
> 
>
> ___
> 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: [PATCH] Change semantic of -r in sefcontext_compile

[William Roberts]

On Fri, Sep 16, 2016 at 11:44 AM, Janis Danisevskis  wrote:
> I don't really care much about the behavior of sefcontext_compile. I just
> thought making the default behavior the safest would be the best option.
> Before android is using it, I will have to sync the (now modified and
> improved - thank you) patches back into AOSP, which I intend to do. I have
> some benchmarks measuring load and lookup time for file contexts. I am eager
> to review and benchmark William's patches and explore a bit myself. And once
> all options are on the table I can make a case for the fastest solution to
> make it into Android.

+ More Google dudes so they see this.

This is where Android's fork is a PITA. My current TODO list looks like this:
1. Get process_file cleanups up streamed (in progress)
2. Get performance improvements up streamed (I have things local, they
need more work)
3. Rectify the Android/Upstream fork.

For two, I have a UDOO ARM board I was going to add into my performance testing,
since it will run a debian distro I can just use upstream libselinux
and compile checkfc or something
for it. I was going to use that to see if theirs an architectura delta
between arm and x86 with the
performance improvements. All my numbers are only off of an x86 platform.

For three, ideall to me, for phase I this looks like having usptream +
android.c and android.h files.
This way for updating, we can merge, with no conflicts as all android
changes will be confined to
those two files. Ill update upstream to have a method to kill unused
interfaces in a graceful way
as well as fixup android.c/android.h for anything that has been moved
into upstream already,
like the work Richard did with digest files.

After 3, then we can trivially test item 2 on an Android platform,
giving us solid, real world android numbers
and getting us closer to unification.

Does anyone have any objections, concerns on this list, let me know so
I can take them into
consideration.


___
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

[William Roberts]

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


>
> --
> Respectfully,
>
> William C Roberts



-- 
Respectfully,

William C Roberts
___
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

[William Roberts]

On Sep 16, 2016 08:12, "Stephen Smalley" <s...@tycho.nsa.gov> wrote:
>
> 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.

I'm not saying that we break anything, but I think we should really
scrutinize the decision to keep binary fc's on Android. The only way it
could be faster at the moment is mmap and pcre2. We need to get some raw
numbers of pcre2 textual vs binary load times. If it's within 30% I'll have
that gap closed soon. It also takes up about 3 times the disk space for
binary.
___
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

[William Roberts]

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.

-- 
Respectfully,

William C Roberts
___
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

[William Roberts]

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

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

[William Roberts]

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.

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

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

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

[William Roberts]

On Fri, Sep 16, 2016 at 7:30 AM, Stephen Smalley <s...@tycho.nsa.gov> 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.

Functionally that's what it does, you just don't like the implementation.

>
> We shouldn't try loading the text file twice.

It get's a lot more clunky if we go that route, ill take another stab and send
something out this afternoon.

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

I tried using checkpatch yesterday, but Ill have to DL the whole linux tree
to get it to work, do you know if theirs a stand-alone version anywhere? I
am almost out of disk space at the moment, waiting on a new drive.

>
>>
>> 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);
>> +
>> + i

Re: [PATCH] libselinux: add support for pcre2

[William Roberts]

On Sep 16, 2016 07:06, "Jason Zaman" <ja...@perfinion.com> wrote:
>
> On Fri, Sep 16, 2016 at 06:51:25AM -0700, William Roberts wrote:
> > On Fri, Sep 16, 2016 at 6:43 AM, William Roberts
> > <bill.c.robe...@gmail.com> wrote:
> > > On Fri, Sep 16, 2016 at 6:31 AM, Jason Zaman <ja...@perfinion.com>
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 <
jda...@google.com> 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'd prefer it changed myself. Another argument pro-change is that one
doesn't
> > > know if its a PCRE2 or PCRE1 compatable version of the libraries, we
should
> > > probably have an ability to intergoate that via API so we can print it
> > > out in help
> > > dialogue, that this tool so we at least know what it can support.
> > >
> > > The biggest thing that needs to get fixed with this, is that no matter
> > > if it contains the
> > > pre-compiled regexs or not, it should always load and work on Android.
> > > In distros,
> > > it will fall back to file_contexts, but we don't have this in Android.
> > > This ties into the arch
> > > version information below. But if the arch differs, always recompile.
> > > The alternative to
> > > this, is just go back to textual fc file son Android, since we won't
> > > be using any of the
> > > features of binary fc's. Better yet, in the Android build, I would
> > > check to see if host arch
> > > is the same as target arch, or let an OEM set a flag, to do the
> > > compilation. But I would
> > > consider those stretch goals, and just revert android back to textual
files.
> >
> > My ramblings might not be clear here. If we just version it with arch
> > and require
> > an exact match, we don't need -r, so that option goes away.
>
> This is what I was aiming for too, currently the pcre_version() is just
> strcmp'd so changing it to cat(pcre2_version(), arch_string()) should
> avoid any problems. It may turn out to be overzealous but it will always
> be safe. I dont think splitting pcre_version and arch into separate
> fields makes a huge difference if you prefer that. Currently the version

It can make a difference if we ever need to parse any information for some
reason in the future. We'd then have to split them back apart. This is just
a bit more future proof IMO.

> numbers are not parsed tho, so checking arch only for pcre2 seems like a
> lot more work.
>
> -- Jason
___
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

[William Roberts]

On Fri, Sep 16, 2016 at 6:43 AM, William Roberts
<bill.c.robe...@gmail.com> wrote:
> On Fri, Sep 16, 2016 at 6:31 AM, Jason Zaman <ja...@perfinion.com> 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 <jda...@google.com> 
>>> 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'd prefer it changed myself. Another argument pro-change is that one doesn't
> know if its a PCRE2 or PCRE1 compatable version of the libraries, we should
> probably have an ability to intergoate that via API so we can print it
> out in help
> dialogue, that this tool so we at least know what it can support.
>
> The biggest thing that needs to get fixed with this, is that no matter
> if it contains the
> pre-compiled regexs or not, it should always load and work on Android.
> In distros,
> it will fall back to file_contexts, but we don't have this in Android.
> This ties into the arch
> version information below. But if the arch differs, always recompile.
> The alternative to
> this, is just go back to textual fc file son Android, since we won't
> be using any of the
> features of binary fc's. Better yet, in the Android build, I would
> check to see if host arch
> is the same as target arch, or let an OEM set a flag, to do the
> compilation. But I would
> consider those stretch goals, and just revert android back to textual files.

My ramblings might not be clear here. If we just version it with arch
and require
an exact match, we don't need -r, so that option goes away.




___
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

[William Roberts]

On Fri, Sep 16, 2016 at 6:31 AM, Jason Zaman <ja...@perfinion.com> 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 <jda...@google.com> 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'd prefer it changed myself. Another argument pro-change is that one doesn't
know if its a PCRE2 or PCRE1 compatable version of the libraries, we should
probably have an ability to intergoate that via API so we can print it
out in help
dialogue, that this tool so we at least know what it can support.

The biggest thing that needs to get fixed with this, is that no matter
if it contains the
pre-compiled regexs or not, it should always load and work on Android.
In distros,
it will fall back to file_contexts, but we don't have this in Android.
This ties into the arch
version information below. But if the arch differs, always recompile.
The alternative to
this, is just go back to textual fc file son Android, since we won't
be using any of the
features of binary fc's. Better yet, in the Android build, I would
check to see if host arch
is the same as target arch, or let an OEM set a flag, to do the
compilation. But I would
consider those stretch goals, and just revert android back to textual files.

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

For the distros it will make sense most of the time from what I can tell.

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

No don't append it onto the PCRE version string, just check the PCRE
version string
and if its greater than X read out the arch field next. This should
work like policy versioning
in libsepol.


>
> Isnt the selinux policy in android stored in the rootfs or /system?
> isnt that all read-only? how would re-compiling on the device work?
> Does android put the fcontext.bin files on /data? that seems insecure.

This is the cross compile aspect, we build on the host and shove it
into the device
rootfs at build time. We typically build on x86_64 and deploy on ARM.
But this is NOT
always the case, just the most common. For instance, with Intel
tablets it probably can
use the pre-compiled regexes.

>
> -- Jason
>
>> > On Fri, Sep 16, 2016 at 1:35 PM William Roberts <bill.c.robe...@gmail.com>
>> > wrote:
>> >>
>> >> 
>> >> >
>> >> >
>> >> > That's just the thing. Without -r the phone _will_ boot because the
>> >> > regexes
>> >> > are compiled on first use. With -r and an arch mismatch we have an
>> >> > undefined
>> >> > behavior, which is bad.
>> >>
>> >> That's just a limitation of the current design.
>> >>
>> >> >
>> >> > See, I don't currently know what part of the architecture is important.
>> >> > Will
>> >> > word size and endianess be enough, e.g., will regexes generated on
>> >> > x86_64el
>> >> > work on armv8el (apparently this is what I observed but it could change
>> >> > at
>> >> > the whim of the PCRE2 maintainers)? So what will be the encoding? If we
>> >> > encode the complete architecture then the typical case for android
>> >> > (build on
>> >> > x86_64 for armv7/armv8) will yield the same result as without the -r
>> >> > option,
>> >> > namely, we rebuild the regexes on the device. If we just encode word
>> >> > size
>> >> > and endianess it may work for a while... I just don't feel comfortable
>> >> > enough to make a decision a

Re: [PATCH] libselinux: add support for pcre2

[William Roberts]

On Fri, Sep 16, 2016 at 6:09 AM, Janis Danisevskis <jda...@google.com> 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.

>
> On Fri, Sep 16, 2016 at 1:35 PM William Roberts <bill.c.robe...@gmail.com>
> wrote:
>>
>> 
>> >
>> >
>> > That's just the thing. Without -r the phone _will_ boot because the
>> > regexes
>> > are compiled on first use. With -r and an arch mismatch we have an
>> > undefined
>> > behavior, which is bad.
>>
>> That's just a limitation of the current design.
>>
>> >
>> > See, I don't currently know what part of the architecture is important.
>> > Will
>> > word size and endianess be enough, e.g., will regexes generated on
>> > x86_64el
>> > work on armv8el (apparently this is what I observed but it could change
>> > at
>> > the whim of the PCRE2 maintainers)? So what will be the encoding? If we
>> > encode the complete architecture then the typical case for android
>> > (build on
>> > x86_64 for armv7/armv8) will yield the same result as without the -r
>> > option,
>> > namely, we rebuild the regexes on the device. If we just encode word
>> > size
>> > and endianess it may work for a while... I just don't feel comfortable
>> > enough to make a decision at this point.
>>
>> Then shelve it and use textual file contexts. The only purpose of the
>> binary format is to
>> include the pre-compiled regex;s so if you cant use that feature,
>> there's no point in
>> using binary format.
>>
>> My thought would be that it has to be an exact match for architecture
>> upfront, then
>> possibly investigate relaxing the requirement later. But, IIRC, if we get
>> a 30%
>> increase in text file load time, theirs really no point, for the
>> binary format on Android.
>> Android fic files are smaller, and have much simpler regex's compared
>> to our desktop
>> brethren.
>>
>> >
>> > My limited understanding is that the automata built by PCRE2 use
>> > pointers to
>> > link states or other structures. These pointers are symbolized when
>> > serialized, but they keep their width, which is why the regexes are at
>> > least
>> > sensitive to the word size.
>> >
>> > Just a wild Idea:
>> > PCRE2 also supports JIT compiling. I did not use this here because I did
>> > not
>> > feel comfortable for libselinux to depend on the execmem permission. But
>> > this could be developed into pre cross compiling the regexes into native
>> > code, which can than be linked into a device specific library. But I
>> > guess
>> > this would require quite some cooperation with the PCRE2 developers.
>>
>> I was going to ask if the arch dependency was on JIT'd code or just
>> something else
>> which you elaborated above with word size/endiness, etc.
>>
>> >
>> >>
>> >> The other thing is, this is only an issue on binary formated
>> >> file_context
>> >> files,
>> >> this is the ideal reason to move back to text files, since their is no
>> >> speedup
>> >> gained if we have to compile them.
>> >
>> >
>> > Right, or you could see it as an intermediate solution that does not
>> > require
>> > changes to the build system until we can properly cross compile the
>> > regexes.
>>
>> If you add an option to sefcontext_compile it should be to remove
>> them. not add it in.
>> This keeps it consistent with teh behavior for PCRE, it would be add
>> to say, "make me
>> a binary fc, but don't actually embed the regex information", since
>> that is currently not
>> the default behavior. Changing the Android make recipe is trivial, so
>> adding -r shouldn't
>> for Android shouldn't be a show stopper.
>>
>> 



-- 
Respectfully,

William C Roberts
___
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

[William Roberts]

>
>
> That's just the thing. Without -r the phone _will_ boot because the regexes
> are compiled on first use. With -r and an arch mismatch we have an undefined
> behavior, which is bad.

That's just a limitation of the current design.

>
> See, I don't currently know what part of the architecture is important. Will
> word size and endianess be enough, e.g., will regexes generated on x86_64el
> work on armv8el (apparently this is what I observed but it could change at
> the whim of the PCRE2 maintainers)? So what will be the encoding? If we
> encode the complete architecture then the typical case for android (build on
> x86_64 for armv7/armv8) will yield the same result as without the -r option,
> namely, we rebuild the regexes on the device. If we just encode word size
> and endianess it may work for a while... I just don't feel comfortable
> enough to make a decision at this point.

Then shelve it and use textual file contexts. The only purpose of the
binary format is to
include the pre-compiled regex;s so if you cant use that feature,
there's no point in
using binary format.

My thought would be that it has to be an exact match for architecture
upfront, then
possibly investigate relaxing the requirement later. But, IIRC, if we get a 30%
increase in text file load time, theirs really no point, for the
binary format on Android.
Android fic files are smaller, and have much simpler regex's compared
to our desktop
brethren.

>
> My limited understanding is that the automata built by PCRE2 use pointers to
> link states or other structures. These pointers are symbolized when
> serialized, but they keep their width, which is why the regexes are at least
> sensitive to the word size.
>
> Just a wild Idea:
> PCRE2 also supports JIT compiling. I did not use this here because I did not
> feel comfortable for libselinux to depend on the execmem permission. But
> this could be developed into pre cross compiling the regexes into native
> code, which can than be linked into a device specific library. But I guess
> this would require quite some cooperation with the PCRE2 developers.

I was going to ask if the arch dependency was on JIT'd code or just
something else
which you elaborated above with word size/endiness, etc.

>
>>
>> The other thing is, this is only an issue on binary formated file_context
>> files,
>> this is the ideal reason to move back to text files, since their is no
>> speedup
>> gained if we have to compile them.
>
>
> Right, or you could see it as an intermediate solution that does not require
> changes to the build system until we can properly cross compile the regexes.

If you add an option to sefcontext_compile it should be to remove
them. not add it in.
This keeps it consistent with teh behavior for PCRE, it would be add
to say, "make me
a binary fc, but don't actually embed the regex information", since
that is currently not
the default behavior. Changing the Android make recipe is trivial, so
adding -r shouldn't
for Android shouldn't be a show stopper.


___
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

[William Roberts]

On Fri, Sep 16, 2016 at 3:13 AM, Janis Danisevskis <jda...@google.com> wrote:
> First of all, I would like to thank you, Stephen and William, for your
> patience and support.
>
> On Thu, Sep 15, 2016 at 8:34 PM William Roberts <bill.c.robe...@gmail.com>
> wrote:
>>
>> On Thu, Sep 15, 2016 at 7:57 AM, Stephen Smalley <s...@tycho.nsa.gov>
>> wrote:
>> > On 09/15/2016 10:04 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 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?
>>
>> I would make the default include the pre-compiled regex's, if there
>> is an arch mismatch then ditch the pre-compiled ones and reload
>> them. I would make it an option to NOT generate pre-compiled
>> regexes. For the upstream consumers, who compile and deploy
>> on the same system, this means they don't need to update to add
>> -r. For android, we just need to update a makefile to include -r to
>> omit the precompiled regex's.
>
>
> The arch is currently not encoded in the binary, so a mismatch cannot be
> detected. I would sleep better if we left the -r option as is for now,
> because it is kind of "use only if you know what you are doing". Also, this
> way it does not break the android build process.

We probably would want to encode the architecture since this matters.This way
we  can detect a mismatch and compile them. This,prevents the
why isn't my device booting issue for cross compilations that don't have -r.
The other thing is, this is only an issue on binary formated file_context files,
this is the ideal reason to move back to text files, since their is no speedup
gained if we have to compile them.

>
>>
>>
>> In the case of Android, where we cross compile the file_contexts.bin,
>> does using binary fc's even make sense now? The major reason is that
>> the binary format loads faster by skipping the regex compilation, we
>> can no longer skip that with PCRE2 and cross compilation. We should
>> likely move Android back to text based if this is the case. I have
>> speedups in the neighborhood of 30% for text file processing in
>> the pipeline.
>
>
> I realize that the binary PCRE2 variant is currently much slower than the
> PCRE stored regexes. But it still a little faster than the text file
> processing. If you can make text file processing 30% faster, than this may
> well be an option because serializing pcre2 regexes will probably never beat
> pcre, even if we did stunts like compiling for the correct architecture
> using qemu.
> I'd be happy to help perform benchmarks and choose the best option.

Once you dump the pre-compiled regex's from the binary format,
your back to the text files performance. Also, with PCRE, the binary format was
3x the size on disk.

>
> Also I am exploring another option, which may sound a bit exotic. So I would
> like to have a running POC first before I come forward. Maybe nothing comes
> of it.
>
>>
>>
>> >
>> >> Signed-off-by: Janis Danisevskis <jda...@google.com>
>> >> ---
>> >
>> >> 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 

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

[William Roberts]

> +   if (!rc) {
> +   rc = digest_add_specfile(digest, fp, NULL, 
> sb.st_size, found_path);
> +   }
One more time...


___
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

[William Roberts]

On Thu, Sep 15, 2016 at 7:57 AM, Stephen Smalley  wrote:
> 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?

I would make the default include the pre-compiled regex's, if there
is an arch mismatch then ditch the pre-compiled ones and reload
them. I would make it an option to NOT generate pre-compiled
regexes. For the upstream consumers, who compile and deploy
on the same system, this means they don't need to update to add
-r. For android, we just need to update a makefile to include -r to
omit the precompiled regex's.

In the case of Android, where we cross compile the file_contexts.bin,
does using binary fc's even make sense now? The major reason is that
the binary format loads faster by skipping the regex compilation, we
can no longer skip that with PCRE2 and cross compilation. We should
likely move Android back to text based if this is the case. I have
speedups in the neighborhood of 30% for text file processing in
the pipeline.

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



-- 
Respectfully,

William C Roberts
___
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

[William Roberts]

Ill send that right up!

On Thu, Sep 15, 2016 at 7:42 AM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
> 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... */
>&g

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

[William Roberts]

On Sep 7, 2016 11:29, "Jason Zaman" <ja...@perfinion.com> wrote:
>
> On Wed, Sep 07, 2016 at 09:40:43AM -0700, William Roberts wrote:
> > On Wed, Sep 7, 2016 at 8:02 AM, Stephen Smalley <s...@tycho.nsa.gov>
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.
> > >
> > > Shouldn't the pcre variant be encoded in some manner in the
> > > file_contexts.bin file so that libselinux can tell immediately whether
> > > it is supported?
> >
> > Don't we have that in pcre_version()?
> >
> > >
> > >> 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.
>
> I'd rather the arch was just added to the compiled files like is done
> for pcre version. It may not be a bad idea to add arch even for pcre1
> since those also seemed quite fragile in the past.
>
> > > Is it worth supporting the -DNO_PERSISTENTLY_STORED_PATTERNS case?
The
> > > point of using file_contexts.bin was to move the cost of compiling the
> > > regexes to build time rather than load time; if we cannot do that,
then
> > > how much do we gain from using file_contexts.bin instead of just
falling
> > > back to file_contexts?

I have some work that speeds up file_contexts load times by 25% and if we
mmap file_contexts we get a slight bump. I'm confident i can speed it up
even more. The binary format is 3 times the size. Gprof has most if the
slowness shown in nodups on validation, I don't think regex compilation is
the bottleneck. I has hoping I could get the speeds close and then propose
ditching bin file support on Android.

> > >
> > > The #ifdef maze makes it very hard to read and maintain this code;
that
> > > needs to be refactored.
> >
> > Perhaps set up some function pointers and hide the regex structure to
be opaque
> > to the rest of selinux. This makes me think, should we just dlopen the
> > correct version of libpcre based on pcre_version()? Not sure how you
> > feel about dlopen calls
>
> Please no :(. dlopen makes things harder to track in a distro. there are
> a ton of tools in gentoo that figure things out with ldd and dont work
> at all with dlopen (like keeping old libs around if other packages link
> to old .so versions and whatnot).

I didn't think dlopen would be fun.
>
> -- Jason
>
> > > valgrind is reporting numerous errors, including both use of
> > > uninitialised values and memory leaks with both patches applied.  Try:
> > > make DESTDIR=~/obj CFLAGS+=-g clean install
> > > LD_LIBRARY_PATH=~/obj/lib valgrind --leak-check=full
> > > ~/obj/usr/sbin/matchpathcon /etc
> > >
> > > On x86_64.
> > >
> > > Will provide review of the code itself later...
> > >
> > >>
> > >> 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
> > 

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

[William Roberts]

On Wed, Sep 7, 2016 at 8:02 AM, Stephen Smalley  wrote:
> On 09/07/2016 04:08 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.
>
> Shouldn't the pcre variant be encoded in some manner in the
> file_contexts.bin file so that libselinux can tell immediately whether
> it is supported?
>
>> 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.
>
> Is it worth supporting the -DNO_PERSISTENTLY_STORED_PATTERNS case?  The
> point of using file_contexts.bin was to move the cost of compiling the
> regexes to build time rather than load time; if we cannot do that, then
> how much do we gain from using file_contexts.bin instead of just falling
> back to file_contexts?
>
> The #ifdef maze makes it very hard to read and maintain this code; that
> needs to be refactored.
>
> valgrind is reporting numerous errors, including both use of
> uninitialised values and memory leaks with both patches applied.  Try:
> make DESTDIR=~/obj CFLAGS+=-g clean install
> LD_LIBRARY_PATH=~/obj/lib valgrind --leak-check=full
> ~/obj/usr/sbin/matchpathcon /etc

The leak patch was only applied and tested as part of the Android build,
I am OK with that one being squashed down, there is no point in it being
separate. I can send an email from my @intel once I get my VM back
online.

I would through ASAN options into CFLAGS and build with that, its a bit faster
than valgrind.

>
> On x86_64.
>
> Will provide review of the code itself later...
>
>>
>> Signed-off-by: Janis Danisevskis 
>> ---
>>  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/Makefile b/libselinux/Makefile
>> index 6142b60..15d051e 100644
>> --- a/libselinux/Makefile
>> +++ b/libselinux/Makefile
>> @@ -24,6 +24,19 @@ ifeq ($(DISABLE_SETRANS),y)
>>  endif
>>  export DISABLE_AVC DISABLE_SETRANS DISABLE_RPM DISABLE_BOOL EMFLAGS
>>
>> +USE_PCRE2 ?= n
>> +DISABLE_PERSISTENTLY_STORED_REGEX_PATTERNS ?= n
>> +ifeq ($(USE_PCRE2),y)
>> + PCRE_CFLAGS := -DUSE_PCRE2 -DPCRE2_CODE_UNIT_WIDTH=8
>> + ifeq ($(DISABLE_PERSISTENTLY_STORED_REGEX_PATTERNS), y)
>> + PCRE_CFLAGS += -DNO_PERSISTENTLY_STORED_PATTERNS
>> + endif
>> + PCRE_LDFLAGS := -lpcre2-8
>> +else
>> + PCRE_LDFLAGS := -lpcre
>> +endif
>> +export PCRE_CFLAGS PCRE_LDFLAGS
>> +
>>  all install relabel clean distclean indent:
>>   @for subdir in $(SUBDIRS); do \
>>   (cd $$subdir && $(MAKE) $@) || exit 1; \
>> 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 

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

[William Roberts]

On Wed, Sep 7, 2016 at 1:08 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 dependant.

dependant -> 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 
> ---
>  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/Makefile b/libselinux/Makefile
> index 6142b60..15d051e 100644
> --- a/libselinux/Makefile
> +++ b/libselinux/Makefile
> @@ -24,6 +24,19 @@ ifeq ($(DISABLE_SETRANS),y)
>  endif
>  export DISABLE_AVC DISABLE_SETRANS DISABLE_RPM DISABLE_BOOL EMFLAGS
>
> +USE_PCRE2 ?= n
> +DISABLE_PERSISTENTLY_STORED_REGEX_PATTERNS ?= n
> +ifeq ($(USE_PCRE2),y)
> +   PCRE_CFLAGS := -DUSE_PCRE2 -DPCRE2_CODE_UNIT_WIDTH=8
> +   ifeq ($(DISABLE_PERSISTENTLY_STORED_REGEX_PATTERNS), y)
> +   PCRE_CFLAGS += -DNO_PERSISTENTLY_STORED_PATTERNS
> +   endif
> +   PCRE_LDFLAGS := -lpcre2-8
> +else
> +   PCRE_LDFLAGS := -lpcre
> +endif
> +export PCRE_CFLAGS PCRE_LDFLAGS
> +
>  all install relabel clean distclean indent:
> @for subdir in $(SUBDIRS); do \
> (cd $$subdir && $(MAKE) $@) || exit 1; \
> 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
> 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
> @@ -15,7 +15,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -176,7 +175,10 @@ static int load_mmap(struct selabel_handle *rec, const 
> char *path,
> return -1;
>
> if (version >= SELINUX_COMPILED_FCONTEXT_PCRE_VERS) {
> -   len = strlen(pcre_version());
> +   if (!regex_version()) {
> +   return -1;
> +   }
> +   len = strlen(regex_version());
>
> rc = next_entry(_len, mmap_area, sizeof(uint32_t));
> if (rc < 0)
> @@ -198,7 +200,7 @@ static int load_mmap(struct selabel_handle *rec, const 
> char *path,
> }
>
> str_buf[entry_len] = '\0';
> -   if ((strcmp(str_buf, pcre_version()) != 0)) {
> +   if ((strcmp(str_buf, regex_version()) != 0)) {
> free(str_buf);
> return -1;
> }
> @@ -278,7 +280,11 @@ static int 

Re: [PATCH] libselinux: clean up process file

[William Roberts]

 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.
>
> I cant reproduce:
bad send... Can you send your valgrind output? Are you sure its not there
prior to my patch? The only heap alloc I add is the strdup, closef should
always be called despite any flow changes in process_file() as the caller,
init() always goes to label finish.


-- 
Respectfully,

William C Roberts
___
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: clean up process file

[William Roberts]

On Tue, Sep 6, 2016 at 1:22 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
> 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.

Ok this should be simple enough the > should go to >= for the mtime comparison.
We can change the granularity in a separate patch, if you'd like.

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

I cant reproduce:



___
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] selinux: fix overflow and 0 length allocations

[William Roberts]

On Aug 29, 2016 16:56, "Paul Moore" <p...@paul-moore.com> wrote:
>
> On Tue, Aug 23, 2016 at 4:49 PM,  <william.c.robe...@intel.com> wrote:
> > From: William Roberts <william.c.robe...@intel.com>
> >
> > Throughout the SE Linux LSM, values taken from sepolicy are
> > used in places where length == 0 or length == 
> > matter, find and fix these.
> >
> > Signed-off-by: William Roberts <william.c.robe...@intel.com>
> > ---
> >  security/selinux/ss/conditional.c | 3 +++
> >  security/selinux/ss/policydb.c| 4 
> >  security/selinux/ss/private.h | 7 +++
> >  3 files changed, 14 insertions(+)
> >  create mode 100644 security/selinux/ss/private.h
> >
> > diff --git a/security/selinux/ss/conditional.c
b/security/selinux/ss/conditional.c
> > index 456e1a9..ecc0fb6 100644
> > --- a/security/selinux/ss/conditional.c
> > +++ b/security/selinux/ss/conditional.c
> > @@ -16,6 +16,7 @@
> >  #include "security.h"
> >  #include "conditional.h"
> >  #include "services.h"
> > +#include "private.h"
> >
> >  /*
> >   * cond_evaluate_expr evaluates a conditional expr
> > @@ -242,6 +243,8 @@ int cond_read_bool(struct policydb *p, struct
hashtab *h, void *fp)
> > goto err;
> >
> > len = le32_to_cpu(buf[2]);
> > +   if (zero_or_saturated(len))
> > +   goto err;
> >
> > rc = -ENOMEM;
> > key = kmalloc(len + 1, GFP_KERNEL);
> > diff --git a/security/selinux/ss/policydb.c
b/security/selinux/ss/policydb.c
> > index 4b24385..0e881f3 100644
> > --- a/security/selinux/ss/policydb.c
> > +++ b/security/selinux/ss/policydb.c
> > @@ -38,6 +38,7 @@
> >  #include "conditional.h"
> >  #include "mls.h"
> >  #include "services.h"
> > +#include "private.h"
> >
> >  #define _DEBUG_HASHES
> >
> > @@ -1094,6 +1095,9 @@ static int str_read(char **strp, gfp_t flags,
void *fp, u32 len)
> > int rc;
> > char *str;
> >
> > +   if (zero_or_saturated(len))
> > +   return -EINVAL;
> > +
> > str = kmalloc(len + 1, flags);
> > if (!str)
> > return -ENOMEM;
> > diff --git a/security/selinux/ss/private.h
b/security/selinux/ss/private.h
> > new file mode 100644
> > index 000..0e81a78
> > --- /dev/null
> > +++ b/security/selinux/ss/private.h
> > @@ -0,0 +1,7 @@
> > +#ifndef PRIVATE_H_
> > +#define PRIVATE_H_
> > +
> > +#define is_saturated(x) (x == (typeof(x))-1)
> > +#define zero_or_saturated(x) ((x == 0) || is_saturated(x))
> > +
> > +#endif
>
> While I'm not opposed to the idea of using a macro for this purpose,
> e.g. is_saturated() and zero_or_saturated(), I don't see much value in
> creating this macro buried in the SELinux directory.  Especially if we
> end up creating a new header file just for these macros.  Something as
> generic as this should be something we inherit from the generic kernel
> code so we can leverage existing conventions.

It made more sense when I wanted to keep the change similar to libsepol.
But the str_read() function really reduced the number of checks. I ended up
porting that to libsepol. With that said, the macro serves little purpose
with the exception of readability.

>
> If you can find a macro like these in the core kernel code, go ahead

Ill look, doubt I'll find anything.

> and use it, otherwise please respin this with the bound checks open
> code

Sure

>
> Oh, and use "SELinux" ;)
>
> --
> paul moore
> www.paul-moore.com
> ___
> 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 2/2] libsepol: port str_read from kernel

[William Roberts]

On Aug 19, 2016 06:12, "Stephen Smalley" <s...@tycho.nsa.gov> wrote:
>
> On 08/18/2016 04:54 PM, william.c.robe...@intel.com wrote:
> > From: William Roberts <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>
> > ---
> >  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.

It's a new gnuc extension ;-p

> 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 / re

Re: [PATCH] selinux: drop SECURITY_SELINUX_POLICYDB_VERSION_MAX

[William Roberts]

On Aug 18, 2016 17:07, "Paul Moore" <p...@paul-moore.com> wrote:
>
> On Mon, Aug 15, 2016 at 3:42 PM,  <william.c.robe...@intel.com> wrote:
> > From: William Roberts <william.c.robe...@intel.com>
> >
> > Remove the SECURITY_SELINUX_POLICYDB_VERSION_MAX Kconfig option
> >
> > Per: https://github.com/SELinuxProject/selinux/wiki/Kernel-Todo
> >
> > This was only needed on Fedora 3 and 4 and just causes issues now,
> > so drop it.
> >
> > The MAX and MIN should just be whatever the kernel can support.
> >
> > Signed-off-by: William Roberts <william.c.robe...@intel.com>
> > ---
> >  security/selinux/Kconfig| 38
-
> >  security/selinux/include/security.h |  4 
> >  2 files changed, 42 deletions(-)
>
> Merged, thanks for the help!

I plan on tinkering with some of the things on that list, hopefully I can
help whittle it down.
>
> --
> paul moore
> www.paul-moore.com
> ___
> 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 5/7] libsepol: fix overflow and 0 length allocations

[William Roberts]

>> Currently, in file-systems like reiserFS that support scalable xattrs, only
>> VFS is the one limiting the size to 64k. Since their is no constant, and
>> maybe one day this arbitrary VFS limit
>> would be removed, I think we should check correctlly here that were
>> allocating > 1 bytes, and we keep total_sz a const.
>
>
> BTW do you want to take everything up to this patchset and I can
> rebase on the repo
> instead of sending out so many patches each time. This is me assuming the 
> others
> are ok by lack of commentary.
>

BTW Patchset v4 has this patch at the very end, so the order is
different, if you
can apply the ones you like up to X and i'll rebase my local branch accordingly.
Rather than blasting 7 patches on a daily basis.
___
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 5/7] libsepol: fix overflow and 0 length allocations

[William Roberts]

On Tue, Aug 16, 2016 at 8:11 AM, William Roberts <bill.c.robe...@gmail.com>
wrote:

> On Aug 16, 2016 06:12, "James Carter" <jwca...@tycho.nsa.gov> wrote:
> >
> > On 08/15/2016 11:59 AM, william.c.robe...@intel.com wrote:
> >>
> >> From: William Roberts <william.c.robe...@intel.com>
> >>
> >> 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 <william.c.robe...@intel.com>
> >> ---
> >>  libsepol/src/conditional.c|  3 ++-
> >>  libsepol/src/context.c| 16 -
> >>  libsepol/src/context_record.c | 52 --
> -
> >>  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, 110 insertions(+), 33 deletions(-)
> >>
> >
> >
> >> diff --git a/libsepol/src/context_record.c
> b/libsepol/src/context_record.c
> >> index ac2884a..fdf60a0 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 {
> >>
> >> @@ -284,39 +285,44 @@ int sepol_context_to_string(sepol_handle_t *
> handle,
> >>  {
> >>
> >> int rc;
> >> -   const int user_sz = strlen(con->user);
> >> -   const int role_sz = strlen(con->role);
> >> -   const int type_sz = strlen(con->type);
> >> -   const int mls_sz = (con->mls) ? strlen(con->mls) : 0;
> >> -   const int total_sz = user_sz + role_sz + type_sz +
> >> -   mls_sz + ((con->mls) ? 3 : 2);
> >> -
> >> -   char *str = (char *)malloc(total_sz + 1);
> >> -   if (!str)
> >> -   goto omem;
> >> +   char *str = NULL;
> >> +   const size_t user_sz = strlen(con->user);
> >> +   const size_t role_sz = strlen(con->role);
> >> +   const size_t type_sz = strlen(con->type);
> >> +   const size_t mls_sz = (con->mls) ? strlen(con->mls) : 0;
> >> +   const size_t total_sz = user_sz + role_sz + type_sz +
> >> +   mls_sz + ((con->mls) ? 3 : 2) + 1;
> >> +
> >> +   if (zero_or_saturated(total_sz)) {
> >> +   ERR(handle, "invalid size");
> >> +   goto err;
> >> +   }
> >>
> >> +   str = (char *)malloc(total_sz);
> >
> >
> > Before, total_sz + 1 was malloc'd, so it made sense to check for
> saturation, but you add the 1 to total_sz before checking for saturation.
>
> True I didn't want to get rid of the const so we should be checking for 0
> or 1 length, or drop the const and check prior to the increment.
>
> >
> > This case is also different in that multiple string lengths are being
> added, so overflow would be the real concern here, but I don't think that
> it is possible to overflow because of limits in checkpolicy (8K) and xattrs
> (64K).
>
> Check policy isn't generating any of this and the xattr limits are not
> being enforced here. In theory we could make the check on the xattr limits
> if that woukd be preferred.
>
Currently, in file-systems like reiserFS that support scalable xattrs, only
VFS is the one limiting the size to 64k. Since their is no constant, and
maybe one day this arbitrary VFS limit
would be removed, I think we should check correctlly here that were
allocating > 1 bytes, and we keep total_sz a const.

>
> >
> > Jim
> >
> >
> >> +   if (!str) {
> >> +   ERR(handle, "out of memory");
> >> +   goto err;
> >> +   }
> >> if (con->mls) {
> >> -   rc = snprintf(str, total_sz + 1, "%s:%s:%s:%s",
> >> +   rc = snprintf(str, total_sz, "%s:%s:%s:%s",
> >>   con->user, con->role, con->type,
> con->mls);
> >> -   if (rc < 0 || (rc >= total_sz + 1)) {
> >> -

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

[William Roberts]

On Aug 16, 2016 06:12, "James Carter" <jwca...@tycho.nsa.gov> wrote:
>
> On 08/15/2016 11:59 AM, william.c.robe...@intel.com wrote:
>>
>> From: William Roberts <william.c.robe...@intel.com>
>>
>> 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 <william.c.robe...@intel.com>
>> ---
>>  libsepol/src/conditional.c|  3 ++-
>>  libsepol/src/context.c| 16 -
>>  libsepol/src/context_record.c | 52
---
>>  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, 110 insertions(+), 33 deletions(-)
>>
>
>
>> diff --git a/libsepol/src/context_record.c
b/libsepol/src/context_record.c
>> index ac2884a..fdf60a0 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 {
>>
>> @@ -284,39 +285,44 @@ int sepol_context_to_string(sepol_handle_t *
handle,
>>  {
>>
>> int rc;
>> -   const int user_sz = strlen(con->user);
>> -   const int role_sz = strlen(con->role);
>> -   const int type_sz = strlen(con->type);
>> -   const int mls_sz = (con->mls) ? strlen(con->mls) : 0;
>> -   const int total_sz = user_sz + role_sz + type_sz +
>> -   mls_sz + ((con->mls) ? 3 : 2);
>> -
>> -   char *str = (char *)malloc(total_sz + 1);
>> -   if (!str)
>> -   goto omem;
>> +   char *str = NULL;
>> +   const size_t user_sz = strlen(con->user);
>> +   const size_t role_sz = strlen(con->role);
>> +   const size_t type_sz = strlen(con->type);
>> +   const size_t mls_sz = (con->mls) ? strlen(con->mls) : 0;
>> +   const size_t total_sz = user_sz + role_sz + type_sz +
>> +   mls_sz + ((con->mls) ? 3 : 2) + 1;
>> +
>> +   if (zero_or_saturated(total_sz)) {
>> +   ERR(handle, "invalid size");
>> +   goto err;
>> +   }
>>
>> +   str = (char *)malloc(total_sz);
>
>
> Before, total_sz + 1 was malloc'd, so it made sense to check for
saturation, but you add the 1 to total_sz before checking for saturation.

True I didn't want to get rid of the const so we should be checking for 0
or 1 length, or drop the const and check prior to the increment.

>
> This case is also different in that multiple string lengths are being
added, so overflow would be the real concern here, but I don't think that
it is possible to overflow because of limits in checkpolicy (8K) and xattrs
(64K).

Check policy isn't generating any of this and the xattr limits are not
being enforced here. In theory we could make the check on the xattr limits
if that woukd be preferred.
>
> Jim
>
>
>> +   if (!str) {
>> +   ERR(handle, "out of memory");
>> +   goto err;
>> +   }
>> if (con->mls) {
>> -   rc = snprintf(str, total_sz + 1, "%s:%s:%s:%s",
>> +   rc = snprintf(str, total_sz, "%s:%s:%s:%s",
>>   con->user, con->role, con->type, con->mls);
>> -   if (rc < 0 || (rc >= total_sz + 1)) {
>> -   ERR(handle, "print error");
>> -   goto err;
>> -   }
>> } else {
>> -   rc = snprintf(str, total_sz + 1, "%s:%s:%s",
>> +   rc = snprintf(str, total_sz, "%s:%s:%s",
>>   con->user, con->role, con->type);
>> -   if (rc < 0 || (rc >= total_sz + 1)) {
>> -   ERR(handle, "print error");
>> -   goto err;
>> -   }
>> +   }
>> +
>> +   /*
>> +* rc is >= 0 on the size_t cast and is safe to promote
>> +* to an unsigned value.
>> +*/
>> +   if (rc < 0 || (size_t)rc >= total_sz) {
>> +   

Re: Regarding enabling selinux on Android

[William Roberts]

On Aug 1, 2016 04:17, "Sameer Joshi"  wrote:
>
> Hi All,
>
> We are trying to enable SELinux in kernel and have defined following
options in the config file.
>
> CONFIG_SECURITY_SELINUX=y
> CONFIG_SECURITY_SELINUX_BOOTPARAM=y
>
> Command line options for kernel have "selinux=1 security=selinux" set.
>
> However during boot time, we get following error:
>
> [5.549941] SELinux:  policydb version 30 does not match my version
range 15-28
>
> [5.557486] init: SELinux:  Could not load policy:  Invalid argument
>
> [5.563990] init: failed to load policy: Invalid argument
>
> [5.569413] init: Security failure; rebooting into recovery mode...
>
>
> Can someone help us what this error means? Any help in fixing this would
be appreciated.
>

You're kernel is not up to date. You need the patches from Androids kernel
common tree. Bear in mind that their are two version 30s, and you'll need
to have the right one. Marshmallow uses the old version 30. Newer releases
use the new and upstream merged version 30.

I don't have the patch links handy but I'm pretty sure jeffv or nnk at
Google posted them, check the mail archives.

>
> 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: userdebug_or_eng rules stopped by neverallow?

[William Roberts]

On Jul 28, 2016 09:15, "peter enderborg" 
wrote:
>
> What is the point with that?

You could always wrap they aosp never allows in userdebug or eng macros,
and be OK with respect to CTS. However, doing so increases the delta
between user builds and other variants, which may include breakages that
should work when the build flavor is user. Not all engineers at some
Android shops test on user builds frequently.

For userdebug conflicts on neverallows the guidance I've gotten and advised
others is to just use a permissive build, which isn't necessarily the best
answer either, as it requires extra steps to either produce the permissive
image, or to change the shell into a permissive domain, or modify the
kernel command line via a boot shell.

>
> ___
> 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: ioctlcmd=7704 denied on unix_stream_socket for surfaceflinger and system_server domain

[William Roberts]

On Jul 12, 2016 21:20, "Jeffrey Vander Stoep"  wrote:
>
> Hi Yongqin,
>
> Looks like a process is indiscriminately calling ashmem_get_size_region()
(ioctl number 7704=ASHMEM_GET_SIZE) on a unix socket. This is a bug and
should not be allowed. The selinux denial is working as intended.
>
> A similar bug was fixed here:
>
https://android-review.googlesource.com/#/c/198885/4/libs/binder/Parcel.cpp
>
> Does your tree include this patch?
>
> On Tue, Jul 12, 2016 at 8:43 PM YongQin Liu 
wrote:
>>
>> Hi, All
>>
>> When I update to use tag android-6.0.1_r55, I got following avc denials
during the boot up:
>>
>> avc: denied { ioctl } for pid=177 comm="Binder_2" path="socket:[10083]"
dev="sockfs" ino=10083 ioctlcmd=7704 scontext=u:r:surfaceflinger:s0
tcontext=u:r:surfaceflinger:s0 tclass=unix_stream_socket permissive=0
>>
>> and
>> avc: denied { ioctl } for pid=465 comm="Binder_1" path="socket:[14454]"
dev="sockfs" ino=14454 ioctlcmd=7704 scontext=u:r:system_server:s0
tcontext=u:r:system_server:s0 tclass=unix_stream_socket permissive=0
>>
>>
>> With following rules I can make the denials disappeared:
>> 11:29:17 liuyq: hikey$ git diff --staged
>> diff --git a/sepolicy/ioctl_macros b/sepolicy/ioctl_macros
>> new file mode 100644
>> index 000..398976c
>> --- /dev/null
>> +++ b/sepolicy/ioctl_macros
>> @@ -0,0 +1 @@
>> +define(`IOCTLTEST', `0x7704')
>> diff --git a/sepolicy/surfaceflinger.te b/sepolicy/surfaceflinger.te
>> new file mode 100644
>> index 000..7c337a9
>> --- /dev/null
>> +++ b/sepolicy/surfaceflinger.te
>> @@ -0,0 +1 @@
>> +allow surfaceflinger surfaceflinger:unix_stream_socket { IOCTLTEST };
>> diff --git a/sepolicy/system_server.te b/sepolicy/system_server.te
>> new file mode 100644
>> index 000..218a8a2
>> --- /dev/null
>> +++ b/sepolicy/system_server.te
>> @@ -0,0 +1 @@
>> +allow system_server system_server:unix_stream_socket { IOCTLTEST };
>> 11:29:19 liuyq: hikey$
>>
>>
>> but how should I find the real name for the ioctlcmd=7704?
>> searched in kernel, and found 7704 is defined ad following:
>> kernel/linaro/hisilicon/drivers/gpu/drm/radeon/r600d.h:1219:#define
HDMI1_STATUS 0x7704

iirc the ioctlcmd prints as base 10 not hex. Is that correct Jeff?

>>
>> but it does not seem to be used for ioctl command.
>>
>> --
>> Best Regards,
>> Yongqin Liu
>> ---
>> #mailing list
>> linaro-andr...@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/linaro-android
>> ___
>> 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: capable(CAP_SYS_MODULE) causes sys_module denial?

[William Roberts]

On Jul 7, 2016 1:13 PM, "YongQin Liu"  wrote:
>
> Hi, ALL
>
> When I try AOSP master with the hikey board, I see following sys_module
denial on netd domain.
>
>> avc: denied { sys_module } for pid=1775 comm="netd" capability=16
scontext=u:r:netd:s0 tcontext=u:r:netd:s0 tclass=capability permissive=0
>
>
> After some check, I found it was caused by "capable(CAP_SYS_MODULE)" call
in dev_load method of the kernel net/core/dev_ioctl.c file here:
>
>
https://android.googlesource.com/kernel/hikey-linaro/+/refs/heads/android-hikey-linaro-4.4/net/core/dev_ioctl.c#371
>
>
> When I comment the capable(CAP_SYS_MODULE) check, there is no  sys_module
denial output.
>
> I did not dig into the implementation of capable, but should not it just
return false without the sys_module denial?
>
> Could anyone please help point to the source where I should check, why
the  sys_module denial is output?

>From what I know is the cap check is separate. This denial is on
finit_module. See this patch:

http://comments.gmane.org/gmane.comp.security.selinux/24164

>
> Thanks in advance!
>
> --
> Best Regards,
> Yongqin Liu
> ---
> #mailing list
> linaro-andr...@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-android
>
> ___
> 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: Any way to restrict root user to an application directory on runtime

[William Roberts]

On Jul 7, 2016 07:57, "Sameer Joshi"  wrote:
>
> Hi All,
>
> I have a use-case where the root user access

Selinux has no notion of Linux uids like root. So this question doesn't
quite make sense. Selinux is a white list, so if you don't add permissions
it wont be allowed. Also, most apps have MLS enabled, so theirs more
sandboxing than the norm. Is this root user process an init service? If so,
unless it's a mlstrustedsubject and type enforcement allow rules allow
access, it wont be able to access the data directory with the exception of
these where only type enforcement allow rules apply:

System_app
NFC
Bluetooth
Radio
Shell

in Android needs to be restricted to not access one particular file in
/data/data/ directory.

You can label applications using mac_permissions.xml to map a key to an
seinfo string and then using seapp_contexts to then label the application
and it's data directory. More information is in the readme and
seapp_contexts files.

>
> The  is something that I am not aware of at the
build time and will know it only during runtime.

Key (seinfo) will work, you always want to use key with a package name.
Just package name is not safe and iirc check_seapp will complain during the
build and some CTS check will fail.

>
> How do I do it if I need to add this permission runtime?  Is it possible
using SELinux?

Sepolicy is static with the Android image, so all policy must be present in
the build. Almost anything is possible with SELinux. More details would
help me provide a more detailed response.
>
> 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: Regarding giving an app permission

[William Roberts]

On Jul 5, 2016 01:33, "Sameer Joshi"  wrote:
>
> Hi,
>
> I want to provide an application downloaded from app store , permission
to access a particular directory which is shared between the  platform
service started from init.rc and that app.
>
> I wanted to know how can I define an "seinfo" string and assign a domain
to this third party app using mac_permissions.xml, if I don't know the
certificate its signed with.

You could extract the certificate from the apk. And use this in
mac_permissions.xml. better yet, take the pem file from extraction and
configure keys.conf. extracting the pem file, iirc, Robert Craig wrote a
tool that was in the sepolicy project. I'd look through the old but bucket
branches for it. If not you can unzip the apk and use openssl. The exact
syntax and commands to do this I'd have to look up. But it is covered in
exploring we for Android published by pakt (note that I profit from this).

Also if this is a third party app, it's frowned upon to set up custom
policy for it. Typically you would protect resources with the Android
permission model at this level. This was any 3rd party app can request
permissions to access the feature. If you need tight controls that is
usually reserved for tight integration by the OEM.
>
> Is it possible to use only package name ?

Yes but it's not secure. Anyone can select that package name.
>
> Also, is it possible to modify the rules in mac_permissions.xml on
runtime ? This means if the running device gets to know the signing
certificate of the app somehow, is there a way that it can use this
certificate to define the domain for the app on runtime.

We used to support placing the files in data/security, but that is no
longer supported.
>
> 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: genfs contexts labelling issue for special character

[William Roberts]

On Jul 1, 2016 08:15, "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 ??

I noticed the other day the lexer definition for PATH seemed incomplete.
Iirc someone had to add : to it the other day for a similar reason. Maybe I
was just reading this:
http://stackoverflow.com/questions/36926823/escaping-colon-character-in-android-selinux

I'm not sure why the class set doesn't include these characters. A patch to
add them seems reasonable.
>
>
>
> Thanks.
>
> 
> This email message is for the sole use of the intended recipient(s) and
may contain confidential information.  Any unauthorized review, use,
disclosure or distribution is prohibited.  If you are not the intended
recipient, please contact the sender by reply email and destroy all copies
of the original message.
> 
>
> ___
> 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: About dac_override denial on logd

[William Roberts]

On Jun 20, 2016 07:51, "William Roberts" <bill.c.robe...@gmail.com> wrote:
>
>
> On Jun 20, 2016 01:24, "YongQin Liu" <yongqin@linaro.org> wrote:
> >
> > Hi, William
> >
> > Sorry for late response, my laptop OS was crashed last Friday:(
> >
> > Thanks for your suggestion first, and some comments in line.
> > On 17 June 2016 at 07:50, William Roberts <bill.c.robe...@gmail.com>
wrote:
> >>
> >> Typing this from my phone, might not be great. Dac override is
triggered when the process permissions don't match with the owner group and
mode of a file. Dan Walsh has info on it here:
http://danwalsh.livejournal.com/69478.html
> >
> >
> > I checked that, but still not find a good method to know which
file/operation caused that denials.
> >>
> >> I would try and add an auditallow statement(s) to find what file it's
accessing that might be causing this, and then adjust the dac permissions
accordingly.
> >
> > After I added following rule:
> > auditallow logd self:capability dac_override;
>
> No I meant on file types, something like this:
> auditallow logd file_type:{file dir} *;
>
> I'm typing that from memory from a smart phone so it might not be 100%
correct. But you want to get it to show you what files it's accessing, you
already know about DAC override.

Also probably want to do fs_type on the target type as well since it might
be syscall or something like that.

>
> Additionally uou could enable the per auacall audti subsystem but its a
little bit of work in the kernel to do that.

Mangled, I meant per syscall audit system.
>
> >
> > I got following message in console log:
> > [7.076059] audit: type=1400 audit(10.759:3): avc:  denied  {
dac_override } for  pid=1734 comm="logd" capability=1  scontext=u:r:logd:s0
tcontext=u:r:logd:s0 tclass=capability permissive=1
> > [7.093377] audit: type=1400 audit(10.775:4): avc:  granted  {
dac_override } for  pid=1734 comm="logd" capability=1  scontext=u:r:logd:s0
tcontext=u:r:logd:s0 tclass=capability
> > [7.107007] (stk) :ldisc installation timeout
> > [7.114174] (stk) :ldisc_install = 0
> > [7.114176] audit: type=1400 audit(10.795:5): avc:  granted  {
dac_override } for  pid=1734 comm="logd" capability=1  scontext=u:r:logd:s0
tcontext=u:r:logd:s0 tclass=capability
> > [7.149786] logd.auditd: start
> > [7.152983] logd.klogd: 7110154165
> > [7.193079] logd.auditd: policy loaded
> > [7.199590] logd.auditd: integrity enforcement suppressed; not
rebooting
> >
> > Seems no clue on which file caused that denials.
> >
> > Do you have any comments on the output above?
> >
> > Thanks,
> > Yongqin Liu
> >
> >>
> >> On Jun 16, 2016 09:50, "YongQin Liu" <yongqin@linaro.org> wrote:
> >>>
> >>> Hi, ALL
> >>>
> >>> I am playing the AOSP master with hikey board, and I get the
following dac_override  avc denial on logd command:
> >>>
> >>> avc:  denied  { dac_override } for  pid=1763 comm="logd" capability=1
 scontext=u:r:logd:s0 tcontext=u:r:logd:s0 tclass=capability permissive=1
> >>>
> >>> I built the same source for Nexus9 board, and I did not see such
dac_override denial on logd with that Nexus9 board.
> >>>
> >>> Searched "logd" in he device projects for hikey and Nexus9, but did
not find any clue on that
> >>> why I got the dac_override  avc denial on logd with the hikey build,
> >>>
> >>>
> >>> Referenced the document here:
> >>>
http://source.android.com/security/selinux/device-policy.html#granting_the_dac_override_capability
> >>>
> >>> But still have no idea how to change to eliminate the dac_override
denial for logd command.
> >>>
> >>> Do you have any idea on what happens there, or where to check on it?
> >>>
> >>> Thanks in advance!
> >>>
> >>> --
> >>> Best Regards,
> >>> Yongqin Liu
> >>> ---
> >>> #mailing list
> >>> linaro-andr...@lists.linaro.org
> >>> http://lists.linaro.org/mailman/listinfo/linaro-android
> >>>
> >>> ___
> >>> 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.
> >
> >
> >
> >
> > --
> > Best Regards,
> > Yongqin Liu
> > ---
> > #mailing list
> > linaro-andr...@lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/linaro-android
___
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: About dac_override denial on logd

[William Roberts]

On Jun 20, 2016 01:24, "YongQin Liu" <yongqin@linaro.org> wrote:
>
> Hi, William
>
> Sorry for late response, my laptop OS was crashed last Friday:(
>
> Thanks for your suggestion first, and some comments in line.
> On 17 June 2016 at 07:50, William Roberts <bill.c.robe...@gmail.com>
wrote:
>>
>> Typing this from my phone, might not be great. Dac override is triggered
when the process permissions don't match with the owner group and mode of a
file. Dan Walsh has info on it here:
http://danwalsh.livejournal.com/69478.html
>
>
> I checked that, but still not find a good method to know which
file/operation caused that denials.
>>
>> I would try and add an auditallow statement(s) to find what file it's
accessing that might be causing this, and then adjust the dac permissions
accordingly.
>
> After I added following rule:
> auditallow logd self:capability dac_override;

No I meant on file types, something like this:
auditallow logd file_type:{file dir} *;

I'm typing that from memory from a smart phone so it might not be 100%
correct. But you want to get it to show you what files it's accessing, you
already know about DAC override.

Additionally uou could enable the per auacall audti subsystem but its a
little bit of work in the kernel to do that.
>
> I got following message in console log:
> [7.076059] audit: type=1400 audit(10.759:3): avc:  denied  {
dac_override } for  pid=1734 comm="logd" capability=1  scontext=u:r:logd:s0
tcontext=u:r:logd:s0 tclass=capability permissive=1
> [7.093377] audit: type=1400 audit(10.775:4): avc:  granted  {
dac_override } for  pid=1734 comm="logd" capability=1  scontext=u:r:logd:s0
tcontext=u:r:logd:s0 tclass=capability
> [7.107007] (stk) :ldisc installation timeout
> [7.114174] (stk) :ldisc_install = 0
> [7.114176] audit: type=1400 audit(10.795:5): avc:  granted  {
dac_override } for  pid=1734 comm="logd" capability=1  scontext=u:r:logd:s0
tcontext=u:r:logd:s0 tclass=capability
> [7.149786] logd.auditd: start
> [7.152983] logd.klogd: 7110154165
> [7.193079] logd.auditd: policy loaded
> [7.199590] logd.auditd: integrity enforcement suppressed; not
rebooting
>
> Seems no clue on which file caused that denials.
>
> Do you have any comments on the output above?
>
> Thanks,
> Yongqin Liu
>
>>
>> On Jun 16, 2016 09:50, "YongQin Liu" <yongqin@linaro.org> wrote:
>>>
>>> Hi, ALL
>>>
>>> I am playing the AOSP master with hikey board, and I get the
following dac_override  avc denial on logd command:
>>>
>>> avc:  denied  { dac_override } for  pid=1763 comm="logd" capability=1
 scontext=u:r:logd:s0 tcontext=u:r:logd:s0 tclass=capability permissive=1
>>>
>>> I built the same source for Nexus9 board, and I did not see such
dac_override denial on logd with that Nexus9 board.
>>>
>>> Searched "logd" in he device projects for hikey and Nexus9, but did not
find any clue on that
>>> why I got the dac_override  avc denial on logd with the hikey build,
>>>
>>>
>>> Referenced the document here:
>>>
http://source.android.com/security/selinux/device-policy.html#granting_the_dac_override_capability
>>>
>>> But still have no idea how to change to eliminate the dac_override
denial for logd command.
>>>
>>> Do you have any idea on what happens there, or where to check on it?
>>>
>>> Thanks in advance!
>>>
>>> --
>>> Best Regards,
>>> Yongqin Liu
>>> ---
>>> #mailing list
>>> linaro-andr...@lists.linaro.org
>>> http://lists.linaro.org/mailman/listinfo/linaro-android
>>>
>>> ___
>>> 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.
>
>
>
>
> --
> Best Regards,
> Yongqin Liu
> ---
> #mailing list
> linaro-andr...@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-android
___
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 issue in defining file in file_contexts

[William Roberts]

On Thu, Jun 2, 2016 at 6:35 AM, Sameer Joshi  wrote:

> Thanks Stephen. It was my mistake that I did not do mkdir for
> eeprom_data_file correctly.
> After fixing this, it worked fine.
> I was using wrong user. cpnoui executes with user root and group system.
> After modifying user and group for eeprom_data_file during mkdir, issue was
> solved.
>

Running steady services (ie not-oneshot) with root will fail CTS. You
shouldn't really run services as user root, with very few exceptions. In
one case is things writing to kmsg like init.
You should pick a non root uid and use android_filesystem_config.h to
assign any needed capabilities. Examples exist for some nexus devices,
shamu as one.
If your on master, that has switched over to using config.fs ini files.

Additionally, perhaps using a non-core user is best, and you can assign a
uid in the oem reserved range (see android_filesystem_config.h) under
system/core. (just do find).
init services allow for number strings as user and group.



>
> Thanks again.
>
> On Thu, Jun 2, 2016 at 6:09 PM, Stephen Smalley  wrote:
>
>> On 06/02/2016 06:59 AM, Sameer Joshi wrote:
>> > Hi Stephen,
>> >
>> > I tried what you said:
>> >
>> > I have done following on trigger of post-fs-data in init..rc
>> file:
>> >
>> >mkdir /data/eeprom_data 777 system
>>
>> Why mode 777?  What else besides cpnoui needs access, and especially
>> what else needs write access?  Why are you assigning it the system UID?
>> Is that what cpnoui runs in? What group ownership should it have?
>>
>> > And I have defined following in file_contexts
>> >
>> > /data/eeprom_data(/.*)?  u:object_r:eeprom_data_file:s03
>>
>> Is the trailing "3" above a typo?
>> That would make the context invalid, but you should then be getting an
>> error at build time.
>>
>> >
>> > Did following in cpnoui.te:
>> >
>> >allpw cpnoui eeprom_data_file:dir rw_dir_perms;
>> > allow cpnoui eeprom_data_file:file create_file_perms;
>> >
>> > Also my program cpnoui is writing to a file under /data/eeprom_data/
>> >
>> > Still its giving the same error:
>> >
>> > type=1400 audit(1463139871.920:5): avc: denied { create } for pid=167
>> > comm="sh" name="eeprom_data_from_sysprops.tmp" scontext=u:r:cpnoui:s0
>> > tcontext=u:object_r:system_data_file:s0 tclass=file permissive=0
>> >
>> > Do I need to create the directory after specific trigger to make it
>> > readable by init as eeprom_data_file instead of system_data_file.
>>
>> No, if you have a matching entry with a valid context in /file_contexts
>> on the device, and if init is creating the directory (i.e. if the
>> directory does not already exist), then the directory should be created
>> with the context specified by the matching entry.  Further, even if the
>> directory already existed, if /file_contexts has changed since the last
>> boot (e.g. OTA), then the restorecon_recursive /data should relabel it
>> accordingly.
>>
>> Things to check:
>> 1. What is the current label of /data/eeprom_data?
>> ls -Z /data/eeprom_data
>>
>> 2. Does the file_contexts on the device include the entry you added?
>> adb shell grep /data/eeprom_data /file_contexts
>>
>> 3. What does restorecon think the label of /data/eeprom_data should be?
>> adb shell restorecon -nv /data/eeprom_data
>>
>
>
> ___
> 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.
>



-- 
Respectfully,

William C Roberts
___
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: load new policy via application, but this not work

[William Roberts]

On Fri, Jan 22, 2016 at 2:03 AM, 李孟樵  wrote:

> HI,
> ROM: I build aosp-6.0.1-r7 aosp_flo-userdebug
> Devices: Nexus 7 II
> Goal: I want to use my application executes the command "load_policy" in
> this ROM.
>
> I have tried these steps as follows:
> step 1.creat an application
> 1-1 the main code is as follows
> >>Process process = Runtime.getRuntime().exec("su");
>

This does not type transition your suposed system_app to su domain. Thie is
no domain transition set
up for it in base policy. See externel/sepolicy/su.te.


> >>DataOutputStream os = new DataOutputStream(process.getOutputStream());
> >>os.writeBytes("load_policy /data/local/tmp/sepolicy.dontaudit\n");
> >>os.writeBytes("exit\n");
> >>os.flush();
> 1-2 Androidmanifest.xml
> add this
> android:sharedUserId="android.uid.system"
>
> step 2.App into AOSP
> /package/apps
> UserId: system
> context: sysem_app
>
> step 3.modify source code
> 3-1policy
> system_app.te: add "allow system_app su_exec:file rx_file_perms;"
> domain.te: line 399 modify "neverallow { domain
> userdebug_or_eng(`-dumpstate -shell -su -system_app') } su_exec:file
> no_x_file_perms;"
> 3-2/system/core/libcutils/fs_config modify
> { 06755, AID_ROOT,  AID_SHELL, 0, "system/xbin/su" },
>
> app's execution result shows there is no error(nothing) in logcat, but
> didn't load new policy.
>
> Why this would NOT work?
> How can I achieve that?
>

I don't support doing this for many reasons, but if you wanted to do it,
and have control from a system_app you could:

1. in the init.rc do: chown /sys/fs/selinux/load to system system
2. Just write the policy from the java application using standard java file
io apis.

This old init.rc would help you get started:
https://bitbucket.org/seandroid/system-core/src/a4a432bebbb2092a23799056860e236f3de3e61d/rootdir/init.rc?at=seandroid-4.2=file-view-default

Bear in mind, that this will cause CTS failures, so if thats not a concern,
continue on your own accord.




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



-- 
Respectfully,

William C Roberts
___
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: avc denial while enabling zram

[William Roberts]

On Jan 19, 2016 12:20 PM, "Jeffrey Vander Stoep" <je...@google.com> wrote:
>
> Try adding notrim in your fstab. Trimming swap makes no sense.

Does defaults include discard? I haven't looked.
>
> On Tue, Jan 19, 2016 at 9:31 AM William Roberts <bill.c.robe...@gmail.com>
wrote:
>>
>> 1. The no logging on the stat failure is consistent to what I am seeing.
So you might have the correct spot.
>> 2. The fstab entry: /dev/block/zram0none
swapdefaults   zramsize=419430400
>>
>>
>> I wanted to run a test with fsck in permissive to see if it only
requests getattr, or if it requests more. The pattern of requests might help
>> deduce the area of code executing.
>>
>> On Tue, Jan 19, 2016 at 9:28 AM, Jeffrey Vander Stoep <je...@google.com>
wrote:
>>>
>>> Does your zram have the notrim option set in the fstab? e.g.
https://android.googlesource.com/device/htc/flounder/+/master/fstab.flounder#16
>>>
>>>
>>> On Tue, Jan 19, 2016 at 9:18 AM Jeffrey Vander Stoep <je...@google.com>
wrote:
>>>>
>>>> I wonder if
https://android.googlesource.com/platform/external/e2fsprogs/+/master/e2fsck/profile.c#270
is
the cause. It's fstat'ing every file in the directory to see if it exists.
>>>>
>>>>
>>>>
>>>> On Tue, Jan 19, 2016 at 8:44 AM William Roberts <
bill.c.robe...@gmail.com> wrote:
>>>>>
>>>>> I was able to reproduce this on a device with sdcard support.
>>>>>
>>>>> [  171.325196] type=1400 audit(1421405887.930:23): avc: denied {
getattr } for pid=2825 comm="e2fsck" path="/dev/block/zram0" dev="tmpfs"
ino=11308 scontext=u:r:fsck:s0 tcontext=u:object_r:swap_block_device:s0
tclass=blk_file permissive=0
>>>>>
>>>>> It works for us without this rule. This is likely related to vold as
Stephen pointed out calling the Format and Check routines (conjecture).
>>>>>
>>>>> dmesg:
>>>>> [9.405740] fs_mgr: Running /system/bin/e2fsck on
/dev/block/by-name/data
>>>>> [9.719493] e2fsck: e2fsck 1.42.9 (28-Dec-2013)
>>>>> [9.719578] e2fsck: data: clean, 1177/262944 files, 48464/1050112
blocks
>>>>> [9.786987] fs_mgr: Running /system/bin/e2fsck on
/dev/block/by-name/cache
>>>>> [9.817792] e2fsck: e2fsck 1.42.9 (28-Dec-2013)
>>>>> [9.817867] e2fsck: cache: clean, 16/6400 files, 1444/25600 blocks
>>>>>
>>>>> logcat:
>>>>> 01-16 05:56:01.577   168   191 V vold: /system/bin/fsck_msdos
>>>>> 01-16 05:56:25.902   674  1620 I BootReceiver: Checking for fsck
errors
>>>>> 01-16 05:58:07.832   168   191 D Vold: Running /system/bin/e2fsck
on /dev/block/dm-1
>>>>> 01-16 05:58:07.832   168   191 V vold: /system/bin/e2fsck
>>>>> 01-16 05:58:07.940   168   191 I e2fsck  : e2fsck 1.42.9 (28-Dec-2013)
>>>>> 01-16 05:58:07.930  2825  2825 W e2fsck  : type=1400 audit(0.0:23):
avc: denied { getattr } for path="/dev/block/zram0" dev="tmpfs" ino=11308
scontext=u:r:fsck:s0 tcontext=u:object_r:swap_block_device:s0
tclass=blk_file permissive=0
>>>>> 01-16 05:58:07.995   168   191 I e2fsck  : /dev/block/dm-1: clean,
11/485760 files, 34812/1941243 blocks
>>>>>
>>>>>
>>>>> I see nothing failing, and we don't have this rule. As I said before,
and Jeff also re-iterated, if nothing is failing I would dontaudit this
>>>>> and look into whats causing this. I don't really have time to debug
this is an entirety at this point in time.
>>>>>
>>>>> Bill
>>>>>
>>>>> On Tue, Jan 19, 2016 at 8:24 AM, Jeffrey Vander Stoep <
je...@google.com> wrote:
>>>>>>
>>>>>> Some options:
>>>>>> Ignore it. It's working as intended.
>>>>>> dontaudit it. Same as above but removes the denial
>>>>>> track down the source of the denial and fix.
>>>>>> File a bug against AOSP.
>>>>>> On Tue, Jan 19, 2016 at 8:12 AM Inamdar Sharif <isha...@nvidia.com>
wrote:
>>>>>>>
>>>>>>> Checked init.rc as well, that’s perfectly alright.
>>>>>>>
>>>>>>> This avc I am facing while formatting the sdcard as internal
storage. Any more ideas??
>>>>>>>
>>>>>>> Thanks.
>>>>>>>
>>>>>>> -Original Messa

Re: avc denial while enabling zram

[William Roberts]

I had a few minutes over lunch to test Jeff's suggestion:

I did a build with this patch:
-/dev/block/zram0none swapdefaults
  zramsize=419430400
+/dev/block/zram0none swapdefaults
  notrim,zramsize=419430400


Still see the message.
[  255.716737] type=1400 audit(1451649711.930:15): avc: denied { getattr }
for pid=5013 comm="e2fsck" path="/dev/block/zram0" dev="tmpfs" ino=11433
scontext=u:r:fsck:s0 tcontext=u:object_r:swap_block_device:s0
tclass=blk_file permissive=0

I put the device into permissive mode and the only permission request I see
it making is getattr. I have no rules (verified with sesearch) that allow
fsck access to swap_block_device. So it does appear to be a single stat()
call somewhere, perhaps right where Jeff suggested earlier.



On Tue, Jan 19, 2016 at 12:41 PM, William Roberts <bill.c.robe...@gmail.com>
wrote:

>
>
> On Tue, Jan 19, 2016 at 12:26 PM, William Roberts <
> bill.c.robe...@gmail.com> wrote:
>
>>
>> On Jan 19, 2016 12:20 PM, "Jeffrey Vander Stoep" <je...@google.com>
>> wrote:
>> >
>> > Try adding notrim in your fstab. Trimming swap makes no sense.
>>
>> Does defaults include discard? I haven't looked.
>>
> Ok I see that notrim is a fs manager flag, not a mount option.
>
>
>>
>> >
>> > On Tue, Jan 19, 2016 at 9:31 AM William Roberts <
>> bill.c.robe...@gmail.com> wrote:
>> >>
>> >> 1. The no logging on the stat failure is consistent to what I am
>> seeing. So you might have the correct spot.
>> >> 2. The fstab entry: /dev/block/zram0none
>>   swapdefaults   zramsize=419430400
>> >>
>> >>
>> >> I wanted to run a test with fsck in permissive to see if it only
>> requests getattr, or if it requests more. The pattern of requests might help
>> >> deduce the area of code executing.
>> >>
>> >> On Tue, Jan 19, 2016 at 9:28 AM, Jeffrey Vander Stoep <
>> je...@google.com> wrote:
>> >>>
>> >>> Does your zram have the notrim option set in the fstab? e.g.
>> https://android.googlesource.com/device/htc/flounder/+/master/fstab.flounder#16
>> >>>
>> >>>
>> >>> On Tue, Jan 19, 2016 at 9:18 AM Jeffrey Vander Stoep <
>> je...@google.com> wrote:
>> >>>>
>> >>>> I wonder if
>> https://android.googlesource.com/platform/external/e2fsprogs/+/master/e2fsck/profile.c#270
>>  is
>> the cause. It's fstat'ing every file in the directory to see if it exists.
>> >>>>
>> >>>>
>> >>>>
>> >>>> On Tue, Jan 19, 2016 at 8:44 AM William Roberts <
>> bill.c.robe...@gmail.com> wrote:
>> >>>>>
>> >>>>> I was able to reproduce this on a device with sdcard support.
>> >>>>>
>> >>>>> [  171.325196] type=1400 audit(1421405887.930:23): avc: denied {
>> getattr } for pid=2825 comm="e2fsck" path="/dev/block/zram0" dev="tmpfs"
>> ino=11308 scontext=u:r:fsck:s0 tcontext=u:object_r:swap_block_device:s0
>> tclass=blk_file permissive=0
>> >>>>>
>> >>>>> It works for us without this rule. This is likely related to vold
>> as Stephen pointed out calling the Format and Check routines (conjecture).
>> >>>>>
>> >>>>> dmesg:
>> >>>>> [9.405740] fs_mgr: Running /system/bin/e2fsck on
>> /dev/block/by-name/data
>> >>>>> [9.719493] e2fsck: e2fsck 1.42.9 (28-Dec-2013)
>> >>>>> [9.719578] e2fsck: data: clean, 1177/262944 files,
>> 48464/1050112 blocks
>> >>>>> [9.786987] fs_mgr: Running /system/bin/e2fsck on
>> /dev/block/by-name/cache
>> >>>>> [9.817792] e2fsck: e2fsck 1.42.9 (28-Dec-2013)
>> >>>>> [9.817867] e2fsck: cache: clean, 16/6400 files, 1444/25600
>> blocks
>> >>>>>
>> >>>>> logcat:
>> >>>>> 01-16 05:56:01.577   168   191 V vold: /system/bin/fsck_msdos
>> >>>>> 01-16 05:56:25.902   674  1620 I BootReceiver: Checking for fsck
>> errors
>> >>>>> 01-16 05:58:07.832   168   191 D Vold: Running
>> /system/bin/e2fsck on /dev/block/dm-1
>> >>>>> 01-16 05:58:07.