RE: add CONFIG_SECURITY_SELINUX_LOAD_ONCE

[Roberts, William C]

Nack… use Booleans

Allow Android to have 1 boolean that init trips, once innit trips it, the allow 
to load policy is removed and also the rule to allow toggling that Boolean is 
removed


From: Seandroid-list [mailto:seandroid-list-boun...@tycho.nsa.gov] On Behalf Of 
Nick Kralevich
Sent: Friday, April 7, 2017 10:34 AM
To: SELinux ; seandroid-list@tycho.nsa.gov
Subject: add CONFIG_SECURITY_SELINUX_LOAD_ONCE

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
___
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: Using non-native executables from native services

[Roberts, William C]

> -Original Message-
> From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
> Sent: Friday, February 10, 2017 9:26 AM
> To: Roberts, William C <william.c.robe...@intel.com>; 'seandroid-
> l...@tycho.nsa.gov' <seandroid-list@tycho.nsa.gov>
> Subject: Re: Using non-native executables from native services
> 
> On Fri, 2017-02-10 at 16:44 +, Roberts, William C wrote:
> > Bump anyone have any feedback?
> >
> > From: Roberts, William C
> > Sent: Wednesday, February 8, 2017 10:45 AM
> > To: seandroid-list@tycho.nsa.gov
> > Subject: Using non-native executables from native services
> >
> > If a native service wishes to execute a non-native tool, like AM, it
> > would require being able to execute the dalvikcache_data_file for
> > that. However, doing so hits my neverallow:
> >
> > #
> > # Assert that, to the extent possible, we're not loading executable
> > content from # outside the rootfs or /system partition except for a
> > few whitelisted domains.
> > #
> > neverallow {
> >     domain
> >     -appdomain
> >     -dumpstate
> >     -shell
> >     userdebug_or_eng(`-su')
> >     -system_server
> >    -webview_zygote
> >     -zygote
> > } { file_type -system_file -exec_type -postinstall_file }:file
> > execute; neverallow {
> >     domain
> >     -appdomain # for oemfs
> >     -recovery # for /tmp/update_binary in tmpfs } { fs_type -rootfs
> > }:file execute;
> >
> > Before, I would just typeattribute the service into appdomain, which
> > obviously has some non-desirable consequences since it was not a full
> > app. This new neverallow precludes that:
> >
> > # Only domains spawned from zygote and runas may have the appdomain
> > attribute.
> > neverallow { domain -runas -webview_zygote -zygote } {
> >   appdomain -shell userdebug_or_eng(`-su') -bluetooth }:process {
> > transition dyntransition };
> >
> > What’s the best answer for this? In my particular case they wish to
> > send a broadcast from their native service, should they just use some
> > native broadcast API?
> 
> I agree that would be better.  Maybe that's a question for android- platform 
> or
> one of the other android groups as to what is the recommended way to perform
> such things from native services.

That's not a bad suggestion, ill hit the platform group up. If I get any 
feedback Ill post a link
to this thread for others to find.


___
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: CIL Typepermissive Symbol not inside parenthesis

[Roberts, William C]

> -Original Message-
> From: Roberts, William C
> Sent: Thursday, January 26, 2017 11:17 AM
> To: 'seandroid-list@tycho.nsa.gov' <seandroid-list@tycho.nsa.gov>
> Cc: 'Stephen Smalley' <s...@tycho.nsa.gov>; 'Nick Kralevich' 
> <n...@google.com>;
> 'seli...@tycho.nsa.gov' <seli...@tycho.nsa.gov>
> Subject: RE: CIL Typepermissive Symbol not inside parenthesis
> 
> 
> 
> > -Original Message-
> > From: Roberts, William C
> > Sent: Thursday, January 26, 2017 10:39 AM
> > To: seandroid-list@tycho.nsa.gov
> > Cc: 'Stephen Smalley' <s...@tycho.nsa.gov>; 'Nick Kralevich'
> > <n...@google.com>; seli...@tycho.nsa.gov
> > Subject: CIL Typepermissive Symbol not inside parenthesis
> >
> > Building for Hikey (Android) with a type permissive statement on
> > hci_attach, yields this error:
> >
> > /bin/bash -c "(out/host/linux-x86/bin/secilc -M true -c 30
> > out/target/product/hikey/obj/ETC/plat_sepolicy.cil_intermediates/plat_
> > policy_n
> > vr.cil
> > out/target/product/hikey/obj/ETC/mapping_sepolicy.cil_intermediates/ma
> > pping
> > /current.cil
> > out/target/product/hikey/obj/ETC/nonplat_sepolicy.cil_intermediates/no
> > nplat_
> > policy_nvr.cil  -o
> > out/target/product/hikey/obj/ETC/sepolicy_intermediates/sepolicy.tmp )
> > && (out/host/linux-x86/bin/sepolicy-analyze
> > out/target/product/hikey/obj/ETC/sepolicy_intermediates/sepolicy.tmp
> > permissive >
> > out/target/product/hikey/obj/ETC/sepolicy_intermediates/sepolicy.permi
> > ssived omains ) && (if [ \"userdebug\" = \"user\" -a -s
> >
> out/target/product/hikey/obj/ETC/sepolicy_intermediates/sepolicy.permissived
> > omains ]; then  echo \"==\" 1>&2;   echo
> \"ERROR:
> > permissive domains not allowed in user builds\" 1>&2;   echo
> \"List of
> > invalid domains:\" 1>&2;cat
> >
> out/target/product/hikey/obj/ETC/sepolicy_intermediates/sepolicy.permissived
> > omains 1>&2;exit 1; fi ) && (mv
> > out/target/product/hikey/obj/ETC/sepolicy_intermediates/sepolicy.tmp
> > out/target/product/hikey/obj/ETC/sepolicy_intermediates/sepolicy )"
> > Symbol not inside parenthesis at line 1239 of
> > out/target/product/hikey/obj/ETC/nonplat_sepolicy.cil_intermediates/no
> > nplat_
> > policy_nvr.cil
> >
> > To reproduce apply this patch to device/linaro/hikey:
> > diff --git a/sepolicy/hci_attach.te b/sepolicy/hci_attach.te index
> > d87f444..1990d54 100644
> > --- a/sepolicy/hci_attach.te
> > +++ b/sepolicy/hci_attach.te
> > @@ -1,6 +1,8 @@
> >  type hci_attach, domain;
> >  type hci_attach_exec, exec_type, file_type;
> >
> > +permissive hci_attach;
> > +
> >  init_daemon_domain(hci_attach)
> >
> >  allow hci_attach kernel:system module_request;
> >
> > and build sepolicy
> >
> > make -j4 sepolicy
> >
> > I have no idea what's hgappening, but the statement looks different
> > than all the other CIL statements:
> >
> > Failing CIL snippet:
> >
> > (type hci_attach)
> > (roletype object_r hci_attach)
> > CIL_TYPEPERMISSIVE (type hci_attach_exec) (roletype object_r
> > hci_attach_exec) (type hci_attach_tmpfs)
> >
> >
> 
> Some of things call routines like cil_write_roletype() in write_ast.c, but 
> some just
> frpintf(CIL_). Are these features not implemented?
> 
> If I apply this hack it works:
> diff --git a/libsepol/cil/src/cil_write_ast.c 
> b/libsepol/cil/src/cil_write_ast.c
> index 4ebda6a..8a25680 100644
> --- a/libsepol/cil/src/cil_write_ast.c
> +++ b/libsepol/cil/src/cil_write_ast.c
> @@ -1255,7 +1255,7 @@ int __cil_write_node_helper(struct cil_tree_node
> *node, uint32_t *finished, void
> fprintf(cil_out, "CIL_TYPEBOUNDS ");
> break;
> case CIL_TYPEPERMISSIVE:
> -   fprintf(cil_out, "CIL_TYPEPERMISSIVE ");
> +   fprintf(cil_out, "(typepermissive hci_attach)\n");
> break;
> case CIL_TYPEATTRIBUTE:
> 
> The output looks ok from sepolicy-analyze:
> 
> $ sepolicy-analyze $OUT/root/sepolicy permissive crash_dump su hci_attach

FYI This does not affect upstream SE Linux, it looks like Dan Cashman over at 
Google authored the file,
So ill drop common selinux mailing listr on further responses. I'll take a look 
at fixing this today...

___
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: CIL Typepermissive Symbol not inside parenthesis

[Roberts, William C]

> -Original Message-
> From: Roberts, William C
> Sent: Thursday, January 26, 2017 10:39 AM
> To: seandroid-list@tycho.nsa.gov
> Cc: 'Stephen Smalley' <s...@tycho.nsa.gov>; 'Nick Kralevich' 
> <n...@google.com>;
> seli...@tycho.nsa.gov
> Subject: CIL Typepermissive Symbol not inside parenthesis
> 
> Building for Hikey (Android) with a type permissive statement on hci_attach,
> yields this error:
> 
> /bin/bash -c "(out/host/linux-x86/bin/secilc -M true -c 30
> out/target/product/hikey/obj/ETC/plat_sepolicy.cil_intermediates/plat_policy_n
> vr.cil
> out/target/product/hikey/obj/ETC/mapping_sepolicy.cil_intermediates/mapping
> /current.cil
> out/target/product/hikey/obj/ETC/nonplat_sepolicy.cil_intermediates/nonplat_
> policy_nvr.cil  -o
> out/target/product/hikey/obj/ETC/sepolicy_intermediates/sepolicy.tmp ) &&
> (out/host/linux-x86/bin/sepolicy-analyze
> out/target/product/hikey/obj/ETC/sepolicy_intermediates/sepolicy.tmp
> permissive >
> out/target/product/hikey/obj/ETC/sepolicy_intermediates/sepolicy.permissived
> omains ) && (if [ \"userdebug\" = \"user\" -a -s
> out/target/product/hikey/obj/ETC/sepolicy_intermediates/sepolicy.permissived
> omains ]; thenecho \"==\" 1>&2;   echo 
> \"ERROR:
> permissive domains not allowed in user builds\" 1>&2; echo 
> \"List of
> invalid domains:\" 1>&2;  cat
> out/target/product/hikey/obj/ETC/sepolicy_intermediates/sepolicy.permissived
> omains 1>&2;  exit 1; fi ) && (mv
> out/target/product/hikey/obj/ETC/sepolicy_intermediates/sepolicy.tmp
> out/target/product/hikey/obj/ETC/sepolicy_intermediates/sepolicy )"
> Symbol not inside parenthesis at line 1239 of
> out/target/product/hikey/obj/ETC/nonplat_sepolicy.cil_intermediates/nonplat_
> policy_nvr.cil
> 
> To reproduce apply this patch to device/linaro/hikey:
> diff --git a/sepolicy/hci_attach.te b/sepolicy/hci_attach.te index
> d87f444..1990d54 100644
> --- a/sepolicy/hci_attach.te
> +++ b/sepolicy/hci_attach.te
> @@ -1,6 +1,8 @@
>  type hci_attach, domain;
>  type hci_attach_exec, exec_type, file_type;
> 
> +permissive hci_attach;
> +
>  init_daemon_domain(hci_attach)
> 
>  allow hci_attach kernel:system module_request;
> 
> and build sepolicy
> 
> make -j4 sepolicy
> 
> I have no idea what's hgappening, but the statement looks different than all 
> the
> other CIL statements:
> 
> Failing CIL snippet:
> 
> (type hci_attach)
> (roletype object_r hci_attach)
> CIL_TYPEPERMISSIVE (type hci_attach_exec) (roletype object_r hci_attach_exec)
> (type hci_attach_tmpfs)
> 
> 

Some of things call routines like cil_write_roletype() in write_ast.c, but some 
just frpintf(CIL_). Are these features not implemented?

If I apply this hack it works:
diff --git a/libsepol/cil/src/cil_write_ast.c b/libsepol/cil/src/cil_write_ast.c
index 4ebda6a..8a25680 100644
--- a/libsepol/cil/src/cil_write_ast.c
+++ b/libsepol/cil/src/cil_write_ast.c
@@ -1255,7 +1255,7 @@ int __cil_write_node_helper(struct cil_tree_node *node, 
uint32_t *finished, void
fprintf(cil_out, "CIL_TYPEBOUNDS ");
break;
case CIL_TYPEPERMISSIVE:
-   fprintf(cil_out, "CIL_TYPEPERMISSIVE ");
+   fprintf(cil_out, "(typepermissive hci_attach)\n");
break;
case CIL_TYPEATTRIBUTE:

The output looks ok from sepolicy-analyze:

$ sepolicy-analyze $OUT/root/sepolicy permissive
crash_dump
su
hci_attach

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

service_manager add permission

[Roberts, William C]

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


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

RE: labelling /sys/kernel/debug aka debugfs

[Roberts, William C]

If Bin is using our N tree, then all the stuff for debugfs are exact matches:

/sys/kernel/debug/sync  u:object_r:debugfs_graphics_sync:s0
/sys/kernel/debug/dri/0/i915_frequency_info u:object_r:debugfs_graphics:s0
/sys/kernel/debug/pstate_snb/setpoint u:object_r:debugfs_pstate:s0

Bin, can you confirm the state of your tree? You can run this command to find 
the entries:

find . -name file_contexts | xargs grep 'kernel/debug'

Thanks,
Bill


From: Nick Kralevich [mailto:n...@google.com]
Sent: Wednesday, October 12, 2016 11:42 AM
To: Roberts, William C <william.c.robe...@intel.com>
Cc: Stephen Smalley <s...@tycho.nsa.gov>; seandroid-list@tycho.nsa.gov; Yang, 
Bin Y <bin.y.y...@intel.com>
Subject: Re: labelling /sys/kernel/debug aka debugfs

Can you attach the file_contexts file you're using? Or at least all the entries 
starting with /sys? In the past, when I've seen this kind of slow restorecon on 
boot, it's been due to a regex which defeats the restorecon directory tree 
walking optimizations.

-- Nick

On Wed, Oct 12, 2016 at 6:42 AM, Roberts, William C 
<william.c.robe...@intel.com<mailto:william.c.robe...@intel.com>> wrote:


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

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



--
Nick Kralevich | Android Security | n...@google.com<mailto: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.

RE: labelling /sys/kernel/debug aka debugfs

[Roberts, William C]

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

I don't see any uses where the lack of regex support would be a problem.
I'll see if Bin, the reporter, can collect more stats for us, Yang could you 
collect
those?

___
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
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.

labelling /sys/kernel/debug aka debugfs

[Roberts, William C]

It's been reported that labelling via restorecon_recursive  /sys/kernel/debug 
is taking 0.25s on a device. I wanted to verify a thought:

It looks like genfscon per file labeling is supported by selinux (like procfs), 
on linux master branch, I see:

selinux_set_mnt_opts():

815 if (!strcmp(sb->s_type->name, "debugfs") ||
816 !strcmp(sb->s_type->name, "sysfs") ||
817 !strcmp(sb->s_type->name, "pstore"))
818 sbsec->flags |= SE_SBGENFS;


Would using genfscon statements and removing the restorecon_recursive be faster 
since it avoids the tree walk? Any caveats, issues one can think of?

Bill

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

RE: unlocked stdio

[Roberts, William C]

Correction, it's just fgets_unlocked, it appears to support the others.

From: Roberts, William C
Sent: Wednesday, September 21, 2016 12:53 PM
To: 'seandroid-list@tycho.nsa.gov' <seandroid-list@tycho.nsa.gov>; 
'seli...@tycho.nsa.gov' <seli...@tycho.nsa.gov>; 's...@tycho.nsa.gov' 
<s...@tycho.nsa.gov>
Subject: unlocked stdio

What was the purpose of using unlocked stdio in libselinux? Bionic doesn't 
support it, is it really necessary?

Bill

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

unlocked stdio

[Roberts, William C]

What was the purpose of using unlocked stdio in libselinux? Bionic doesn't 
support it, is it really necessary?

Bill

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

RE: [RFC] mmap file_contexts and property_contexts:

[Roberts, William C]

> > -Original Message-
> > From: Roberts, William C
> > Sent: Monday, September 19, 2016 2:45 PM
> > To: seli...@tycho.nsa.gov; seandroid-list@tycho.nsa.gov;
> > s...@tycho.nsa.gov; jda...@google.com
> > Cc: Roberts, William C <william.c.robe...@intel.com>
> > Subject: [RFC] mmap file_contexts and property_contexts:
> >
> > 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

Another potential issue, is that mmaping with textfiles also has the sideffect
of mapping in all the spaces or tabs, etc used, between fields, so on files with
heavy whitespace, it could be really memory intensive. 

I think Janis's patch that has -r option is a more useful way to go. Without 
precompiled
Regex's its essentially a textfile without the whitespace, and it automatically 
would
get pulled into the mmap interface.

I'm abandoning this, since we already get this functionality, without the 
negatives.
Janis, IIRC, you are re-configuring your patch so -r omits pre-compiled regex's 
(submitted
I believe on the list and awaiting other patches) and adding an arch string so 
we can
Detect when we need to recompile the regular expressions if they are supplied 
but
unusable.

I think we may want to investigate to have the "omit pre-compiled regex's" work 
on PCRE
library variants as well, but I doubt it would be used since Android is moving 
to PCRE2.




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

[Roberts, William C]

> > On 09/19/2016 05:51 PM, Roberts, William C wrote:
> > > FYI I only tested this with checkfc...
> >
> > Evidently.  matchpathcon and sefcontext_compile both report calls to
> > free() on invalid pointers and abort.
> 
> That doesn’t surprise me, I only tested the checkfc usages. Hence #4 in todo's
> 

I looked at the sefcontext_compile bug, and its yet another conglomeration 
where internal
Interfaces into libselinux are presumed and duplicate code exists (for loading 
a file).
It implements its own routine for the freeing of Spec data. Why doesn't it use 
proper
libselinux interfaces, or libselinux expose the proper Interfaces for this type 
of work?
Is this just an example of something that should be fixed or is there some 
deeper
reasoning to its construction?

I could not get matchpatchcon to reproduce (built with ASAN):
./selinux/libselinux/utils/matchpathcon /etc -f file_contexts
/etcu:object_r:rootfs:s0

Bill


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

RE: [RFC] mmap file_contexts and property_contexts:

[Roberts, William C]

> -Original Message-
> From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
> Sent: Tuesday, September 20, 2016 6:18 AM
> To: Roberts, William C <william.c.robe...@intel.com>; seli...@tycho.nsa.gov;
> seandroid-list@tycho.nsa.gov; jda...@google.com
> Subject: Re: [RFC] mmap file_contexts and property_contexts:
> 
> On 09/19/2016 05:51 PM, Roberts, William C wrote:
> > FYI I only tested this with checkfc...
> 
> Evidently.  matchpathcon and sefcontext_compile both report calls to
> free() on invalid pointers and abort.

That doesn’t surprise me, I only tested the checkfc usages. Hence #4 in todo's

> 
> >
> >> -Original Message-
> >> From: Roberts, William C
> >> Sent: Monday, September 19, 2016 2:45 PM
> >> To: seli...@tycho.nsa.gov; seandroid-list@tycho.nsa.gov;
> >> s...@tycho.nsa.gov; jda...@google.com
> >> Cc: Roberts, William C <william.c.robe...@intel.com>
> >> Subject: [RFC] mmap file_contexts and property_contexts:
> >>
> >> 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
> >>
> >> 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);
> &

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

[Roberts, William C]

> > > +static FILE *open_file(const char *path, const char *suffix,
> > > +char *save_path, size_t len, struct stat *sb) {
> > > + unsigned i;
> > > + int rc;
> > > + char stack_path[len];
> >
> > Ew, what is this?  C99 magic.  Probably just make it PATH_MAX and be
> > done with it.
> 
> You already have C99 magic with mixed code and data declarations, so I don't 
> see
> the problem.
> https://gcc.gnu.org/onlinedocs/gcc/Mixed-Declarations.html
> https://gcc.gnu.org/onlinedocs/gcc/Variable-Length.html
> 

You also seem to have more magic with inline, when passing
CFLAGS='-std=c90' to make it complains about:

cc -std=c90 -I../include -I/usr/include -D_GNU_SOURCE   -c -o booleans.o 
booleans.c
In file included from avc.c:14:0:
avc_internal.h:36:15: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ 
before ‘void’
 static inline void set_callbacks(const struct avc_memory_callback *mem_cb,

Looking at:
https://gcc.gnu.org/onlinedocs/gcc/Inline.html
http://stackoverflow.com/questions/12151168/does-ansi-c-not-know-the-inline-keyword

It appears you need something like __inline__

If this code base was met to compile C90, it already failed and is only C99 
compliant + GNUC extensions.
So I don't see the harm in using the C99 capabilities.




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

[Roberts, William C]

> -Original Message-
> From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
> Sent: Thursday, September 8, 2016 12:41 PM
> To: Roberts, William C <william.c.robe...@intel.com>; seli...@tycho.nsa.gov;
> seandroid-list@tycho.nsa.gov; jwca...@tycho.nsa.gov
> Subject: Re: [PATCH v2] libselinux: clean up process file
> 
> On 09/08/2016 03:30 PM, Roberts, William C wrote:
> > 
> >
> >>>> +/* Append any given suffix */
> >>>> +char *to = stpcpy([current_size], ".");
> >>>
> >>> Simpler as:
> >>>   char *to = current + current_size;
> >>>   *to++ = '.';
> >>
> >> I don't think this is simpler, but I'll do it.
> >
> > Doing that as is gets us this:
> > ==26050== Conditional jump or move depends on uninitialised value(s)
> > ==26050==at 0x4C2DD9A: strcat (in /usr/lib/valgrind/vgpreload_memcheck-
> amd64-linux.so)
> > ==26050==by 0x4E4B6D8: rolling_append (label_file.c:429)
> > ==26050==by 0x4E4B8A3: open_file (label_file.c:472)
> > ==26050==by 0x4E4BAC6: process_file (label_file.c:519)
> > ==26050==by 0x4E4BE6D: init (label_file.c:582)
> > ==26050==by 0x4E4D02B: selabel_file_init (label_file.c:965)
> > ==26050==by 0x4E481F5: selabel_open (label.c:340)
> > ==26050==by 0x4E513E0: matchpathcon_init_prefix (matchpathcon.c:322)
> > ==26050==by 0x4E51725: matchpathcon (matchpathcon.c:413)
> > ==26050==by 0x400D86: printmatchpathcon (matchpathcon.c:26)
> > ==26050==by 0x40141F: main (matchpathcon.c:196)
> >
> > Because strcat() needs to fastforward to the null byte, this would
> > need to change to an strcpy.
> 
> Yes, sorry - switch the strcat() to strcpy() too.  Simpler and more 
> efficient, and if
> you had to append further components, you could use
> stpcpy() rather than strcpy() each time until the last one.
> 
FYI: That change actually dropped off aprox 100 bytes on the obj code.


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

[Roberts, William C]

> > > + /* Append any given suffix */
> > > + char *to = stpcpy([current_size], ".");
> >
> > Simpler as:
> > char *to = current + current_size;
> > *to++ = '.';
> 
> I don't think this is simpler, but I'll do it.

Doing that as is gets us this:
==26050== Conditional jump or move depends on uninitialised value(s)
==26050==at 0x4C2DD9A: strcat (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==26050==by 0x4E4B6D8: rolling_append (label_file.c:429)
==26050==by 0x4E4B8A3: open_file (label_file.c:472)
==26050==by 0x4E4BAC6: process_file (label_file.c:519)
==26050==by 0x4E4BE6D: init (label_file.c:582)
==26050==by 0x4E4D02B: selabel_file_init (label_file.c:965)
==26050==by 0x4E481F5: selabel_open (label.c:340)
==26050==by 0x4E513E0: matchpathcon_init_prefix (matchpathcon.c:322)
==26050==by 0x4E51725: matchpathcon (matchpathcon.c:413)
==26050==by 0x400D86: printmatchpathcon (matchpathcon.c:26)
==26050==by 0x40141F: main (matchpathcon.c:196)

Because strcat() needs to fastforward to the null byte, this would need to
change to an strcpy.




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

[Roberts, William C]

> -Original Message-
> From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
> Sent: Thursday, September 8, 2016 8:15 AM
> To: Roberts, William C <william.c.robe...@intel.com>; seli...@tycho.nsa.gov;
> seandroid-list@tycho.nsa.gov; jwca...@tycho.nsa.gov
> Subject: Re: [PATCH v2] libselinux: clean up process file
> 
> On 09/06/2016 08:07 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: 740
> >
> > Object size difference:
> > before: 195530 bytes
> > after:  195529 bytes
> >
> > Signed-off-by: William Roberts <william.c.robe...@intel.com>
> > ---
> >  libselinux/src/label_file.c | 263
> > 
> >  1 file changed, 145 insertions(+), 118 deletions(-)
> >
> > diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
> > index c89bb35..6839a77 100644
> > --- a/libselinux/src/label_file.c
> > +++ b/libselinux/src/label_file.c
> > @@ -97,62 +97,43 @@ static int nodups_specs(struct saved_data *data, const
> char *path)
> > return rc;
> >  }
> >
> > -static int load_mmap(struct selabel_handle *rec, const char *path,
> > -   struct stat *sb, bool isbinary,
> > -   struct selabel_digest *digest)
> > +static int process_text_file(FILE *fp, const char *prefix, struct
> > +selabel_handle *rec, const char *path) {
> > +   /*
> > +* Then do detailed validation of the input and fill the spec array
> > +*/
> 
> You can drop this comment; it is no longer helpful/relevant.
> 
> > +   int rc;
> > +   size_t line_len;
> > +   unsigned lineno = 0;
> > +   char *line_buf = NULL;
> > +
> > +   while (getline(_buf, _len, fp) > 0) {
> > +   rc = process_line(rec, path, prefix, line_buf, ++lineno);
> > +   if (rc)
> > +   goto out;
> > +   }
> > +   rc = 0;
> > +out:
> > +   free(line_buf);
> > +   return rc;
> > +}
> > +
> > +static int load_mmap(FILE *fp, size_t len, struct selabel_handle
> > +*rec, const char *path)
> >  {
> > struct saved_data *data = (struct saved_data *)rec->data;
> > -   char mmap_path[PATH_MAX + 1];
> > -   int mmapfd;
> > int rc;
> > -   struct stat mmap_stat;
> > char *addr, *str_buf;
> > -   size_t len;
> > int *stem_map;
> > struct mmap_area *mmap_area;
> > uint32_t i, magic, version;
> > uint32_t entry_len, stem_map_len, regex_array_len;
> >
> > -   if (isbinary) {
> > -   len = strlen(path);
> > -   if (len >= sizeof(mmap_path))
> > -   return -1;
> > -   strcpy(mmap_path, path);
> > -   } else {
> > -   rc = snprintf(mmap_path, sizeof(mmap_path), "%s.bin", path);
> > -   if (rc >= (int)sizeof(mmap_path))
> > -   return -1;
> > -   }
> > -
> > -   mmapfd = open(mmap_path, O_RDONLY | O_CLOEXEC);
> > -   if (mmapfd < 0)
> > -   return -1;
> > -
> > -   rc = fstat(mmapfd, _stat);
> > -   if (rc < 0) {
> > -   close(mmapfd);
> > -   return -1;
> > -   }
> > -
> > -   /* if mmap is old, ignore it */
> > -   if (mmap_stat.st_mtime < sb->st_mtime) {
> > -   close(mmapfd);
> > -   return -1;
> > -   }
> > -
> > -   /* ok, read it in... */
> > -   len = mmap_stat.st_size;
> > -   len += (sysconf(_SC_PAGE_SIZE) - 1);
> > -   len &= ~(sysconf(_SC_PAGE_SIZE) - 1);
> > -
> > mmap_area = malloc(sizeof(*mmap_area));
> > if (!mmap_area) {
> > -   close(mmapfd);
> > return -1;
> > }
> >
> > -   addr = mmap(NULL, len, PROT_READ, MAP_PRIVATE, mmapfd, 0);
> > -   close(mmapfd);
> > +   addr = mmap(NULL, len, PROT_READ, MAP_PRIVATE, fileno(fp), 0);
> > if (addr == MAP_FAILED) {
> > free(mmap_area);
> > perror("mmap");
&g

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

[Roberts, William C]

> -Original Message-
> From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
> Sent: Thursday, September 8, 2016 8:15 AM
> To: Roberts, William C <william.c.robe...@intel.com>; seli...@tycho.nsa.gov;
> seandroid-list@tycho.nsa.gov; jwca...@tycho.nsa.gov
> Subject: Re: [PATCH v2] libselinux: clean up process file
> 
> On 09/06/2016 08:07 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: 740
> >
> > Object size difference:
> > before: 195530 bytes
> > after:  195529 bytes
> >
> > Signed-off-by: William Roberts <william.c.robe...@intel.com>
> > ---
> >  libselinux/src/label_file.c | 263
> > 
> >  1 file changed, 145 insertions(+), 118 deletions(-)
> >
> > diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
> > index c89bb35..6839a77 100644
> > --- a/libselinux/src/label_file.c
> > +++ b/libselinux/src/label_file.c
> > @@ -97,62 +97,43 @@ static int nodups_specs(struct saved_data *data, const
> char *path)
> > return rc;
> >  }
> >
> > -static int load_mmap(struct selabel_handle *rec, const char *path,
> > -   struct stat *sb, bool isbinary,
> > -   struct selabel_digest *digest)
> > +static int process_text_file(FILE *fp, const char *prefix, struct
> > +selabel_handle *rec, const char *path) {
> > +   /*
> > +* Then do detailed validation of the input and fill the spec array
> > +*/
> 
> You can drop this comment; it is no longer helpful/relevant.
> 
> > +   int rc;
> > +   size_t line_len;
> > +   unsigned lineno = 0;
> > +   char *line_buf = NULL;
> > +
> > +   while (getline(_buf, _len, fp) > 0) {
> > +   rc = process_line(rec, path, prefix, line_buf, ++lineno);
> > +   if (rc)
> > +   goto out;
> > +   }
> > +   rc = 0;
> > +out:
> > +   free(line_buf);
> > +   return rc;
> > +}
> > +
> > +static int load_mmap(FILE *fp, size_t len, struct selabel_handle
> > +*rec, const char *path)
> >  {
> > struct saved_data *data = (struct saved_data *)rec->data;
> > -   char mmap_path[PATH_MAX + 1];
> > -   int mmapfd;
> > int rc;
> > -   struct stat mmap_stat;
> > char *addr, *str_buf;
> > -   size_t len;
> > int *stem_map;
> > struct mmap_area *mmap_area;
> > uint32_t i, magic, version;
> > uint32_t entry_len, stem_map_len, regex_array_len;
> >
> > -   if (isbinary) {
> > -   len = strlen(path);
> > -   if (len >= sizeof(mmap_path))
> > -   return -1;
> > -   strcpy(mmap_path, path);
> > -   } else {
> > -   rc = snprintf(mmap_path, sizeof(mmap_path), "%s.bin", path);
> > -   if (rc >= (int)sizeof(mmap_path))
> > -   return -1;
> > -   }
> > -
> > -   mmapfd = open(mmap_path, O_RDONLY | O_CLOEXEC);
> > -   if (mmapfd < 0)
> > -   return -1;
> > -
> > -   rc = fstat(mmapfd, _stat);
> > -   if (rc < 0) {
> > -   close(mmapfd);
> > -   return -1;
> > -   }
> > -
> > -   /* if mmap is old, ignore it */
> > -   if (mmap_stat.st_mtime < sb->st_mtime) {
> > -   close(mmapfd);
> > -   return -1;
> > -   }
> > -
> > -   /* ok, read it in... */
> > -   len = mmap_stat.st_size;
> > -   len += (sysconf(_SC_PAGE_SIZE) - 1);
> > -   len &= ~(sysconf(_SC_PAGE_SIZE) - 1);
> > -
> > mmap_area = malloc(sizeof(*mmap_area));
> > if (!mmap_area) {
> > -   close(mmapfd);
> > return -1;
> > }

I just noticed this, I made this a single line if and didn't drop the braces, 
this
Will be in v3

> >
> > -   addr = mmap(NULL, len, PROT_READ, MAP_PRIVATE, mmapfd, 0);
> > -   close(mmapfd);
> > +   addr = mmap(NULL, len, PROT_READ, MAP_PRIVATE, fileno(fp), 0);
> > if (addr == MAP_FAILED) {
> > free(mm

RE: [PATCH] libselinux: add support for pcre2

[Roberts, William C]

> -Original Message-
> From: Janis Danisevskis [mailto:jda...@android.com]
> Sent: Thursday, September 8, 2016 8:52 AM
> To: seli...@tycho.nsa.gov; seandroid-list@tycho.nsa.gov; s...@tycho.nsa.gov;
> jwca...@tycho.nsa.gov
> Cc: Janis Danisevskis <jda...@google.com>; Roberts, William C
> <william.c.robe...@intel.com>
> Subject: [PATCH] libselinux: add support for pcre2
> 
> 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 and sefcontext_compile
> shall generate portable output, it and libselinux must be compiled with -
> DNO_PERSISTENTLY_STORED_PATTERNS, at the cost of having to recompile the
> regular expressions at load time.
> 
> Signed-off-by: Janis Danisevskis <jda...@google.com>
> 
> This patch includes includes:

Double includes

> 
> libselinux: fix memory leak on pcre2
> 
> Introduced a malloc on pcre_version(). Libselinux expected this to be static, 
> just
> use a static internal buffer.
> 
> Signed-off-by: William Roberts <william.c.robe...@intel.com>

You can remove any attribution since its squashed down, this really doesn't
Apply, so don't feel obligated to include any of this information.

> ---
>  libselinux/Makefile   |  13 +
>  libselinux/src/Makefile   |   4 +-
>  libselinux/src/label_file.c   |  93 ++-
>  libselinux/src/label_file.h   |  59 ++---
>  libselinux/src/regex.c| 461 
> ++
>  libselinux/src/regex.h| 169 +
>  libselinux/utils/Makefile |   4 +-
>  libselinux/utils/sefcontext_compile.c |  55 +---
>  8 files changed, 697 insertions(+), 161 deletions(-)  create mode 100644
> libselinux/src/regex.c  create mode 100644 libselinux/src/regex.h
> 
> diff --git a/libselinux/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..e41c351 100644
> --- a/libselinux/src/label_file.c
> +

RE: [PATCH] libselinux: clean up process file

[Roberts, William C]

> -Original Message-
> From: Roberts, William C
> Sent: Friday, September 2, 2016 10:34 AM
> To: 'Stephen Smalley' <s...@tycho.nsa.gov>; 'seli...@tycho.nsa.gov'
> <seli...@tycho.nsa.gov>; 'jwca...@tycho.nsa.gov' <jwca...@tycho.nsa.gov>;
> 'seandroid-list@tycho.nsa.gov' <seandroid-list@tycho.nsa.gov>
> Subject: RE: [PATCH] libselinux: clean up process file
> 
> 
> > since M, I think it makes sense to have this in the following order:
> > 1. patch to only open the file once
> > 2. patch to switch to stdio fmemopen
> 
> Actually looking at this, it would be write it once for item 1, then discard 
> that as
> Most of it would change for item two. So I see no value in splitting this up.
> I think we just implement item 2 and as a sideffect it won't open the file up
> multiple times.

Looking at this in the context of fmemopen() or even some of the others like 
open_memstream()
They both seem to copy data into an internal buffer, so I did not see a way, 
for instance to just
set pointers into the mapping. The hand rolled getline like routine that worked 
on the area
struct should really not be allocating and using the pointers into the map, 
that was a poor
decision.

> 
> 

___
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

[Roberts, William C]

> since M, I think it makes sense to have this in the following order:
> 1. patch to only open the file once
> 2. patch to switch to stdio fmemopen

Actually looking at this, it would be write it once for item 1, then discard 
that as
Most of it would change for item two. So I see no value in splitting this up.
I think we just implement item 2 and as a sideffect it won't open the file up
multiple times.



___
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

[Roberts, William C]

> 
> I'm not sure what the motivation is for this patch.

It simplifies and streamlines the code. The existing code will  cause ones
eyes to bleed and I was sick of the bleeding.

> In any event, I think it would be better to do it as a series of smaller 
> changes,
> preferably starting with any actual fixes/improvements and only then moving
> into code rewriting.  Some specific comments below.

Sure I'll split this into two things.
After reading everything and now knowing that bionic supports fmemopen() since 
M,
I think it makes sense to have this in the following order:
1. patch to only open the file once
2. patch to switch to stdio fmemopen

The second thing takes care of a lot of stuff and kills of area struct which is 
nice.

> 
> >
> > Detailed statistics of before and after:
> >
> > Source lines of code reported by cloc on modified files:
> > before: 1090
> > after: 1119
> >
> > Object size difference:
> > before: 195530 bytes
> > after:  195569 bytes
> >
> > Performance:
> >
> > text fc files (avg of 3 runs with selabel_open()):
> > before: 248 ms
> > after: 243 ms
> >
> > bin fc files (avg of 3 runs with selabel_open()):
> > before: 236 ms
> > after:  236 ms
> >
> > Signed-off-by: William Roberts 
> > ---
> >  libselinux/src/label_file.c | 606
> > ++--
> >  libselinux/src/label_file.h |  17 +-
> >  2 files changed, 359 insertions(+), 264 deletions(-)
> >
> > diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
> > index c89bb35..bd65ee7 100644
> > --- a/libselinux/src/label_file.c
> > +++ b/libselinux/src/label_file.c
> > @@ -97,92 +97,270 @@ 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 load_mmap(FILE *fp, off_t size, struct selabel_handle
> > +*rec)
> >  {
> > -   struct saved_data *data = (struct saved_data *)rec->data;
> > -   char mmap_path[PATH_MAX + 1];
> > -   int mmapfd;
> > -   int rc;
> > -   struct stat mmap_stat;
> > -   char *addr, *str_buf;
> > -   size_t len;
> > -   int *stem_map;
> > +   char *addr;
> > 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;
> > -   }
> > +   struct saved_data *data = (struct saved_data *) rec->data;
> >
> > -   mmapfd = open(mmap_path, O_RDONLY | O_CLOEXEC);
> > -   if (mmapfd < 0)
> > +   addr = mmap(NULL, size, PROT_READ, MAP_PRIVATE, fileno(fp), 0);
> > +   if (addr == MAP_FAILED )
> > return -1;
> >
> > -   rc = fstat(mmapfd, _stat);
> > -   if (rc < 0) {
> > -   close(mmapfd);
> > +   mmap_area = malloc(sizeof(*mmap_area));
> > +   if (!mmap_area) {
> > +   munmap(addr, size);
> > return -1;
> > }
> >
> > -   /* if mmap is old, ignore it */
> > -   if (mmap_stat.st_mtime < sb->st_mtime) {
> > -   close(mmapfd);
> > -   return -1;
> > +   /* save where we mmap'd the file to cleanup on close() */
> > +   mmap_area->addr = mmap_area->next_addr = addr;
> > +   mmap_area->len = mmap_area->next_len = size;
> > +   mmap_area->next = data->mmap_areas;
> > +   data->mmap_areas = mmap_area;
> > +
> > +   return 0;
> > +}
> > +
> > +struct file_details {
> > +   const char *suffix;
> > +   struct stat sb;
> > +};
> > +
> > +static char *rolling_append(char *current, const char *suffix, size_t
> > +max) {
> > +   size_t size;
> > +
> > +   if (!suffix)
> > +   return current;
> > +
> > +   /*
> > +* Overflow check:
> > +* current contents of tmp (stack_path) + '.' + suffix  + '\0'
> 
> Comment refers to variables that don't exist in this scope.
> 
> > +* should never be too big.
> > +*/
> > +   size = strlen(current) + 1 + strlen(suffix) + 1;
> 
> If we are worried about overflow, then are we worried about integer overflow
> above?

Initially I thought no, because we already checked that current is PATH_MAX, 
but we
Didn't check suffix, so it would be possible. So I think we should be worried 
here. I know
You said libsepol gets hit with old compilers, but what about libselinux? Can 
we use any
Intrinsics here?

> 
> > +   if (size > max)
> > +   return NULL ;
> 
> Here and elsewhere: extraneous whitespace before ;
Weird, done.
> 
> > +
> > +   /* Append any given suffix */
> > +   strcat(current, ".");
> > +   strcat(current, suffix);
> 
> This could be done more 

RE: [PATCH] libselinux: clean up process file

[Roberts, William C]

> + rc = 0;
> 
> + out: free(stem_map);

Dang eclipse formatter, now I see why libsepol has the indents for the labels 
way out,
Ill aggregate feedback and possible send v2 next week.

>   return rc;
>  }



___
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: unix_stream_socket erros on latest Marshmallow

[Roberts, William C]

> -Original Message-
> From: Seandroid-list [mailto:seandroid-list-boun...@tycho.nsa.gov] On Behalf
> Of Mahaveer, Vishal
> Sent: Friday, August 19, 2016 8:00 AM
> To: seandroid-list@tycho.nsa.gov
> Subject: unix_stream_socket erros on latest Marshmallow
> 
> Hi,
> 
> After updating to latest AOSP Marshmallow release (moved from MOB30M to
> MOB30Z) We started seeing bunch of new selinux denials regarding
> unix_stream_socket.
> 
> [  141.546027] type=1400 audit(141.529:16): avc: denied { ioctl } for pid=233
> comm="Binder_3" path="socket:[11553]" dev="sockfs" ino=11553 ioctlcmd=7704
> scontext=u:r:surfaceflinger:s0 tcontext=u:r:surfaceflinger:s0
> tclass=unix_stream_socket permissive=1 [  141.570289] type=1400
> audit(141.529:17): avc: denied { ioctl } for pid=233 comm="Binder_3"
> path="socket:[11553]" dev="sockfs" ino=11553 ioctlcmd=7704
> scontext=u:r:surfaceflinger:s0 tcontext=u:r:surfaceflinger:s0
> tclass=unix_stream_socket permissive=1 [  141.833709] type=1400
> audit(141.819:18): avc: denied { ioctl } for pid=233 comm="Binder_3"
> path="socket:[11665]" dev="sockfs" ino=11665 ioctlcmd=7704
> scontext=u:r:surfaceflinger:s0 tcontext=u:r:surfaceflinger:s0
> tclass=unix_stream_socket permissive=1 [  141.857664] type=1400
> audit(141.819:19): avc: denied { ioctl } for pid=233 comm="Binder_3"
> path="socket:[11665]" dev="sockfs" ino=11665 ioctlcmd=7704
> scontext=u:r:surfaceflinger:s0 tcontext=u:r:surfaceflinger:s0
> tclass=unix_stream_socket permissive=1
> 
> 
> I guess this change that came in as part of the update is causing the issue
> https://android.googlesource.com/platform/external/sepolicy/+/556bb0f55324e
> 8839d7b735a0de9bc31028e839e
> 
> How do I resolve them, is fix available for the same in AOSP?
> 

This has been answered here:
http://www.mail-archive.com/seandroid-list%40tycho.nsa.gov/msg02806.html

It should be this patch and IIRC it worked for me:
https://android-review.googlesource.com/#/c/198885/4/libs/binder/Parcel.cpp

___
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/2] libsepol: calloc all the *_to_val_structs

[Roberts, William C]

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

After looking at the email and the patch again, that’s just a context hunk, I 
see no + or - next it,
And I verified it still exists in my source tree. I never touched that hunk or  
am I missing
Some subtle interaction?


___
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: Fix AFL Found Bugs in libsepol

[Roberts, William C]

> -Original Message-
> From: Selinux [mailto:selinux-boun...@tycho.nsa.gov] On Behalf Of
> william.c.robe...@intel.com
> Sent: Monday, August 15, 2016 8:59 AM
> To: seli...@tycho.nsa.gov; jwca...@tycho.nsa.gov; seandroid-
> l...@tycho.nsa.gov; s...@tycho.nsa.gov
> Subject: Fix AFL Found Bugs in libsepol
> 
> With the patches mentioned below, I have been able to run the AFL fuzzer for 3
> days without issue. I didn't get much feedback in v2 of the patchset, but 
> these
> should address the issues in that series.

As well as fix a few more issues found along the way.

> 
> [PATCH v3 1/7] libsepol: fix invalid access of NULL on [PATCH v3 2/7] 
> libsepol:
> ensure key is valid before doing search [PATCH v3 3/7] ebitmap: detect invalid
> bitmap [PATCH v3 4/7] genfs_read: fix use heap-use-after-free [PATCH v3 5/7]
> libsepol: fix overflow and 0 length allocations [PATCH v3 6/7] libsepol: bound
> attr_type_map access by nprim [PATCH v3 7/7] libsepol: fix unitialized jmp and
> invalid dereference
> ___
> 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] libsepol: fix memory leak in expand.c

[Roberts, William C]

> >
> > -  out:
> > + out:
> > +   rc = 0;
> >
> > -   ebitmap_destroy();
> > + err_neg:
> > ebitmap_destroy(_types);
> > + err_types:
> > +   ebitmap_destroy();
> >
> > -   return 0;
> > +   return rc;
> >  }

I just noticed the indenting on the label changed, I went through the
File and it has:
1. labels with 7 leading spaces
2. labels left justified

Which one is the way you want them, left justified?

Bill


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

RE: unlabeled file access for logd

[Roberts, William C]

> >> >> I am getting the following denial
> >> >>
> >> >> avc: denied { search } for pid=218 comm="logd" name="/"
> dev="mmcblk0p29"
> >> >> ino=2 scontext=u:r:logd:s0 tcontext=u:object_r:unlabeled:s0
> >> >> tclass=dir
> >> >> permissive=0
> >> >>
> >> >>
> >> >>
> >> >> Does anyone know about this??
> >> >>
> >> >>
> >> >>
> >> >> I recall William facing a similar issue.
> >> >
> >> >Do you have this change:
> >> >https://android-review.googlesource.com/#/c/229794/
> >>
> >> Still facing the same denial after adding the patch.
> >> Any idea??
> >
> >That fixed it for me, so no, I don't have any other ideas. What block device 
> >is
> that referring too? Is it for the data partition?
> 
> Yes its referring to user data partition.
> 

Sorry, I really don't know then. Based on the silence, it appears no one else 
has faced this
Issue post patch.



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

RE: [PATCH v2] module_to_cil: fix use of uninitialized variables

[Roberts, William C]

> -Original Message-
> From: Steve Lawrence [mailto:slawre...@tresys.com]
> Sent: Wednesday, August 3, 2016 2:51 PM
> To: Roberts, William C <william.c.robe...@intel.com>; seli...@tycho.nsa.gov;
> seandroid-list@tycho.nsa.gov; s...@tycho.nsa.gov
> Subject: Re: [PATCH v2] module_to_cil: fix use of uninitialized variables
> 
> On 08/03/2016 05:42 PM, william.c.robe...@intel.com wrote:
> > From: William Roberts <william.c.robe...@intel.com>
> >
> > Correct errors like these reported by gcc:
> >
> > module_to_cil.c: In function ‘block_to_cil’:
> > module_to_cil.c:229:20: error: ‘attr_list’ may be used uninitialized in this
> function [-Werror=maybe-uninitialized]
> >   struct list_node *curr = (*attr_list)->head;
> >
> > Usages of attr_list_destroy() were called when list_init() fails. Just
> > bail early on failure.
> >
> > stack_init() and stack_destroy() also suffered from the aforementioned
> > issue.
> >
> > Signed-off-by: William Roberts <william.c.robe...@intel.com>
> > ---
> >  libsepol/src/module_to_cil.c | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/libsepol/src/module_to_cil.c
> > b/libsepol/src/module_to_cil.c index b9a4af7..9d0d064 100644
> > --- a/libsepol/src/module_to_cil.c
> > +++ b/libsepol/src/module_to_cil.c
> > @@ -1307,7 +1307,7 @@ static int cond_list_to_cil(int indent, struct
> > policydb *pdb, struct cond_node *
> >
> > rc = list_init(_list);
> > if (rc != 0) {
> > -   goto exit;
> > +   return rc;
> > }
> >
> > for (cond = cond_list; cond != NULL; cond = cond->next) { @@ -3488,7
> > +3488,7 @@ static int block_to_cil(struct policydb *pdb, struct
> > avrule_block *block, struct
> >
> > rc = list_init(_list);
> > if (rc != 0) {
> > -   goto exit;
> > +   return rc;
> > }
> >
> > rc = typealiases_to_cil(indent, pdb, block, stack); @@ -3635,7
> > +3635,7 @@ static int blocks_to_cil(struct policydb *pdb)
> >
> > rc = stack_init();
> > if (rc != 0) {
> > -   goto exit;
> > +   return rc;
> > }
> >
> > block = pdb->global;
> > @@ -3703,7 +3703,7 @@ static int linked_blocks_to_cil(struct policydb
> > *pdb)
> >
> > rc = stack_init();
> > if (rc != 0) {
> > -   goto exit;
> > +   return rc;
> > }
> >
> > block = pdb->global;
> >
> 
> I would recommend just initializing the variables to NULL and keeping the 
> "goto
> exit"'s. That would maintain the single return point, allows for extra 
> cleanup code
> to be run in the future if necessary, and is consistent with the rest of the
> module_to_cil code.

That's fine, but requiring only one exit point isn't consistent with the rest 
of libsepol,
some do it this way, some don’t. There will be slightly more overhead In the 
approach
of initializing it first and then jumping on failure, then checking if its NULL 
in the
destruction routine, then finally returning versus just checking the init 
routine
and returning.

> 
> - Steve
> 


___
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] module_to_cil: fix possible use of initialized value

[Roberts, William C]

> -Original Message-
> From: Roberts, William C
> Sent: Wednesday, August 3, 2016 2:37 PM
> To: seli...@tycho.nsa.gov; seandroid-list@tycho.nsa.gov; s...@tycho.nsa.gov
> Cc: Roberts, William C <william.c.robe...@intel.com>
> Subject: [PATCH] module_to_cil: fix possible use of initialized value

Let's try this commit message again... unitialized!

> 
> From: William Roberts <william.c.robe...@intel.com>
> 
> Correct errors like these reported by gcc:
> 
> module_to_cil.c: In function ‘block_to_cil’:
> module_to_cil.c:229:20: error: ‘attr_list’ may be used uninitialized in this 
> function
> [-Werror=maybe-uninitialized]
>   struct list_node *curr = (*attr_list)->head;
> 
> Usages of attr_list_destroy() were called when list_init() fails. Just bail 
> early on
> failure.
> 
> stack_init() and stack_destroy() also suffered from the aforementioned issue.
> 
> Signed-off-by: William Roberts <william.c.robe...@intel.com>
> ---
>  libsepol/src/module_to_cil.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c index
> b9a4af7..9d0d064 100644
> --- a/libsepol/src/module_to_cil.c
> +++ b/libsepol/src/module_to_cil.c
> @@ -1307,7 +1307,7 @@ static int cond_list_to_cil(int indent, struct policydb
> *pdb, struct cond_node *
> 
>   rc = list_init(_list);
>   if (rc != 0) {
> - goto exit;
> + return rc;
>   }
> 
>   for (cond = cond_list; cond != NULL; cond = cond->next) { @@ -3488,7
> +3488,7 @@ static int block_to_cil(struct policydb *pdb, struct avrule_block
> *block, struct
> 
>   rc = list_init(_list);
>   if (rc != 0) {
> - goto exit;
> + return rc;
>   }
> 
>   rc = typealiases_to_cil(indent, pdb, block, stack); @@ -3635,7 +3635,7
> @@ static int blocks_to_cil(struct policydb *pdb)
> 
>   rc = stack_init();
>   if (rc != 0) {
> - goto exit;
> + return rc;
>   }
> 
>   block = pdb->global;
> @@ -3703,7 +3703,7 @@ static int linked_blocks_to_cil(struct policydb *pdb)
> 
>   rc = stack_init();
>   if (rc != 0) {
> - goto exit;
> + return rc;
>   }
> 
>   block = pdb->global;
> --
> 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.

RE: unlabeled file access for logd

[Roberts, William C]

> -Original Message-
> From: Seandroid-list [mailto:seandroid-list-boun...@tycho.nsa.gov] On Behalf
> Of Inamdar Sharif
> Sent: Wednesday, August 3, 2016 2:59 AM
> To: Stephen Smalley ; seandroid-list@tycho.nsa.gov
> Subject: RE: unlabeled file access for logd
> 
> >On 07/18/2016 06:55 AM, Inamdar Sharif wrote:
> >> Hi Guys,
> >>
> >>
> >>
> >> I am getting the following denial
> >>
> >> avc: denied { search } for pid=218 comm="logd" name="/" dev="mmcblk0p29"
> >> ino=2 scontext=u:r:logd:s0 tcontext=u:object_r:unlabeled:s0
> >> tclass=dir
> >> permissive=0
> >>
> >>
> >>
> >> Does anyone know about this??
> >>
> >>
> >>
> >> I recall William facing a similar issue.
> >
> >Do you have this change:
> >https://android-review.googlesource.com/#/c/229794/
> 
> Still facing the same denial after adding the patch.
> Any idea??

That fixed it for me, so no, I don't have any other ideas. What block device
is that referring too? Is it for the data partition?

___
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] selinux: print leading 0x on ioctlcmd audits

[Roberts, William C]

> -Original Message-
> From: Steve Grubb [mailto:sgr...@redhat.com]
> Sent: Friday, July 15, 2016 12:42 PM
> To: Roberts, William C <william.c.robe...@intel.com>
> Cc: Paul Moore <pmo...@redhat.com>; William Roberts
> <bill.c.robe...@gmail.com>; seandroid-list@tycho.nsa.gov;
> seli...@tycho.nsa.gov; linux-au...@redhat.com
> Subject: Re: [PATCH] selinux: print leading 0x on ioctlcmd audits
> 
> On Friday, July 15, 2016 7:33:09 PM EDT Roberts, William C wrote:
> > 
> >
> > > > This is important so that people don't make up new ones that do
> > > > the same thing. The ioctlcmd field name should be recorded. Are
> > > > there more that need documenting?
> > >
> > > Steve/William, one of you want to send a patch/PR for the field
> > > dictionary?
> >
> > I'll send it over.
> 
> I also asked some other questions.  Is this the ioctl number? As in syscall 
> arg a1? I
> need to know if its the same thing so that I can hook up its translation if 
> so.

Yes, per man ioctl, it's the "request number".  Assuming a0 is the file 
descriptor, then a1 is the
Ioctlcmd value.


> 
> Thanks,
> -Steve

___
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] selinux: print leading 0x on ioctlcmd audits

[Roberts, William C]

> > This is important so that people don't make up new ones that do the
> > same thing. The ioctlcmd field name should be recorded. Are there more
> > that need documenting?
> 
> Steve/William, one of you want to send a patch/PR for the field dictionary?

I'll send it over.

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

transient unlabeled /data

[Roberts, William C]

For anyone experiencing a transient unlabeled /data file system, the root cause 
seems to be when fs_mgr_do_format() is invoked and a new
ext4 filesystem is created.  That file system is unlabeled until the 
restorecon_recursive is kicked off. The solution seems to be passing the 
sehandle
for file_contexts to the make_ext4 routine so the created file system is 
properly labelled.

I saw them with logd myself:
avc: denied { search } for pid=2535 comm="logd" name="/" dev="mmcblk0p9" ino=2 
scontext=u:r:logd:s0 tcontext=u:object_r:unlabeled:s0 tclass=dir permissive=1

https://android-review.googlesource.com/#/c/229794/

Hopefully this saves someone a couple of hours.

Bill

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

app_fuse

[Roberts, William C]

https://android-review.googlesource.com/#/c/198907/

Is their any documentation or information surrounding app_fuse feature?

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

RE: platform_app property_service:set

[Roberts, William C]

I was leaning towards bug myself, thanks for clarifying. Patch uploaded:
https://android-review.googlesource.com/223300

From: Nick Kralevich [mailto:n...@google.com]
Sent: Friday, May 6, 2016 3:43 AM
To: Roberts, William C <william.c.robe...@intel.com>
Cc: seandroid-list@tycho.nsa.gov
Subject: Re: platform_app property_service:set

This seems like a bug in the policy. The intention of 
https://android-review.googlesource.com/194717 was to allow property setting 
access for platform_app.

Can you upload a change to fix this?

-- Nick

On Thu, May 5, 2016 at 8:37 AM, Roberts, William C 
<william.c.robe...@intel.com<mailto:william.c.robe...@intel.com>> wrote:
With platform app runing with a category set, it cannot access the 
property_socket since it is not mlstrustedobject. Which one of the following is 
this an example of:

1.   A bug in the policy

2.   Something you shouldn’t do

3.   Up to OEMs and if they want to add that typeattribute who cares… 
(property_set is restricted via neverallows on a few domains including 
untrusted_app).

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



--
Nick Kralevich | Android Security | n...@google.com<mailto: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.

platform_app property_service:set

[Roberts, William C]

With platform app runing with a category set, it cannot access the 
property_socket since it is not mlstrustedobject. Which one of the following is 
this an example of:

1.   A bug in the policy

2.   Something you shouldn't do

3.   Up to OEMs and if they want to add that typeattribute who cares... 
(property_set is restricted via neverallows on a few domains including 
untrusted_app).
___
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: google apps, untrusted_app domains

[Roberts, William C]

> -Original Message-
> From: Seandroid-list [mailto:seandroid-list-boun...@tycho.nsa.gov] On Behalf
> Of James Muir
> Sent: Monday, March 21, 2016 2:08 PM
> To: seandroid-list@tycho.nsa.gov
> Subject: google apps, untrusted_app domains
> 
> I just installed a Marshmallow factory image (hammerhead-mra58) on my N5 and
> I notice that the google apps all run under "untrusted_app" domains:
> 
> $ adb shell
> shell@hammerhead:/ $ ps -Z | grep 'google'
> u:r:untrusted_app:s0:c512,c768 u0_a571597  212
> com.google.android.inputmethod.latin
> u:r:untrusted_app:s0:c512,c768 u0_a222283  212
> com.google.android.setupwizard
> u:r:untrusted_app:s0:c512,c768 u0_a532349  212   com.google.android.tts
> u:r:untrusted_app:s0:c512,c768 u0_a117063  212
> com.google.android.gms.persistent
> u:r:untrusted_app:s0:c512,c768 u0_a117082  212
> com.google.process.gapps
> u:r:untrusted_app:s0:c512,c768 u0_a117111  212   com.google.android.gms
> u:r:untrusted_app:s0:c512,c768 u0_a178880  212
> com.google.android.partnersetup
> u:r:untrusted_app:s0:c512,c768 u0_a3812054 212
> com.google.android.apps.cloudprint
> u:r:untrusted_app:s0:c512,c768 u0_a2712395 212
> com.google.android.googlequicksearchbox:search
> u:r:untrusted_app:s0:c512,c768 u0_a2712414 212
> com.google.android.googlequicksearchbox:interactor
> u:r:untrusted_app:s0:c512,c768 u0_a5513115 212   com.google.android.talk
> u:r:untrusted_app:s0:c512,c768 u0_a6214057 212
> com.google.android.apps.magazines
> u:r:untrusted_app:s0:c512,c768 u0_a6515070 212
> com.google.android.apps.photos
> u:r:untrusted_app:s0:c512,c768 u0_a7115130 212   com.google.android.gm
> u:r:untrusted_app:s0:c512,c768 u0_a7015156 212
> com.google.android.gm.exchange
> u:r:untrusted_app:s0:c512,c768 u0_a6915426 212
> com.google.android.deskclock
> u:r:untrusted_app:s0:c512,c768 u0_a4616580 212
> com.google.android.apps.fitness
> u:r:untrusted_app:s0:c512,c768 u0_a2716604 212
> com.google.android.googlequicksearchbox
> u:r:untrusted_app:s0:c512,c768 u0_a3417204 212
> com.google.android.calendar
> 
> At one time, the recommendation was that google apps included in a ROM
> should be assigned "platform_app"; there is a post from Stephen S (Feb
> 2014) to this effect:
> 
> https://marc.info/?l=seandroid-list=139353406611165=2
> 
> Does that recommendation still stand or is it obsolete now?  Google presumably
> has the gapps working correctly using "untrusted_app" now (however, I do see
> lots of selinux denials with this factory image).

They run under untrusted_app. IIRS back in the day they had to be signed, I 
remember
dealing with that issue on CM (again IIRC). I've seen denials for chrome on 
isolated_process,
nothing really of note for the google apps though.

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

init search/verification tool

[Roberts, William C]

I'd like to announce a tool for the verification and searching of init.rc files 
for Android.

https://github.com/01org/initsearchtool

The search functionality is nice for verifying policy to init.rc files, etc.. 
However, the verification aspect of the tool is likely of most use to those on 
this list.

Under test (in the repo) you'll see a file called assert.xml:












If anyone adds something to this test that violates this, it will fail when run:
$ ./isearch.py verify --assert=test/assert.xml test/init.aosp.rc
Failed test(No world sockets):
service(test/init.aosp.rc : 584): ueventd /sbin/ueventd
socket(588) : danger 0666

This is still early code, so likely some bugs. Any bug reports can be filed on 
guthub and pull requests are always reviewed.

Thanks,
Bill



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

RE: Enable System App to use IPTables

[Roberts, William C]

Iptables even with ‘su’ from a system_app won’t work.


1.   Zygote spawned applications have 0 caps (you need net_admin IIRC)

2.   Zygote spawned applications have NO_NEW_PRIVS set, which means setuid 
(and gaining caps) is pretty worthless (man prctl)

3.   You would never get this far (hopefully) but selinux also bolsters 
these restrictions by not granting capabilities, etc.

A recent thread just explained this, if you’re looking for further details:
http://marc.info/?l=seandroid-list=145623380322624=2

As far as enabling netd to support your use-cases, I cannot say for sure, but 
it sounds plausible, but is a bit off topic for this mailing list.

Bill

From: Seandroid-list [mailto:seandroid-list-boun...@tycho.nsa.gov] On Behalf Of 
Alex Boyd
Sent: Wednesday, February 24, 2016 10:27 AM
To: William Roberts ; Joshua Brindle 

Cc: seandroid-list@tycho.nsa.gov
Subject: RE: Enable System App to use IPTables

Just to clarify because I don’t want to lose anything in translation and I’m 
rather new at this ( I’ve been an application engineer for sometime, but 
nothing this close to the kernel ).

1. Just invoking iptables really isn’t an option without su ( which basically 
opens up everything ).
2. A Linux system service would need to handle communication with iptables, one 
already exists (netd, but I am assuming this probably doesn’t have the 
capability to block a specific uid access to a specific interface )

To do this the right way, I would need to :
1. add the necessary capabilities inside of netd ( poking around in the source 
right now, I only see a few commands that it supports ("list", "getcfg", 
"setcfg", "clearaddrs", "ipv6privacyextensions", "ipv6", "setmtu")
2. add an interface to those capabilities in the android framework most likely 
in the NetworkManagementService.java and the INetworkManagementService.aidl
3. inside of the system_app use 
Context.getSystemService(Context.NETWORKMANAGEMENT_SERVICE) to interact with 
the newly added methods to the NetowrkManagementService ?

I hope I am not totally missing the message.

Thanks again for all the help.

From: William Roberts
Sent: Wednesday, February 24, 2016 12:28 PM
To: Joshua Brindle
Cc: seandroid-list@tycho.nsa.gov; Alex 
Boyd
Subject: Re: Enable System App to use IPTables


On Feb 24, 2016 09:19, "Joshua Brindle" 
> wrote:
>
> Alex Boyd wrote:
>>
>> Unlock iptables with SELinux policy
>> I am trying to customize Android so that it has a built in firewall. I
>> want to allow my Settings app to block different apps from using
>> mobile data and/or wifi.
>>
>
> You probably want to modify netd to handle the firewalling and send a message 
> over its socket to tell it what to do. system_app isn't terribly privileged 
> and netd already manages iptables rules.

Also doesn't iptables need capabilities? apps on Android have their capability 
set cleared in the zygote.

>
>> My approach so far has been to add new selinux policy rules to allow
>> system level apps to interact with iptables. I have tried multiple
>> different policies, but here is what I currently have.
>>
>>
>> file_contexts
>>
>>  /system/bin/iptables   u:object_r:iptables_exec:s0
>>
>>
>> system_app.te
>>
>>  type iptables_exec;
>>
>>  allow system_app iptables_exec:file { rx_file_perms };
>>
>>
>> I didn't define a new "domain" for iptables and I wasn't sure if I
>> needed to declare the system_app domain again, or if this would just
>> be appended to that.
>>
>> Thanks in advance for any help. If anyone has any pointers on where to
>> look to get a better understanding of SELinux inside of android,
>> please let me know.
>>
>>
>>
>>
>>
>>
>> ___
>> 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.

Patch that removed untrusted_app cache file create/unlink

[Roberts, William C]

I am trying to find the patch that removed cache_file access for untrusted_app, 
my git-foo is failing me.

I see it was added here: 9ba844fea12a0b

And this patch neverallows it: 0d8e9adf4

But I cannot find the one that drops it. I would like to cherry-pick this to my 
tree.

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

RE: avc denial while enabling zram

[Roberts, William C]

The only thing we have is the label and for some reason (not sure why offhand) 
a getattr for the swap_block file for vold.

file_contexts:1:# ZRam device configured as swap space
file_contexts:2:/dev/block/zram0u:object_r:swap_block_device:s0

vold.te:allow vold swap_block_device:blk_file getattr;

We never had to allow any access from fsck. I see no dontaudits, so perhaps 
were just ignoring the audit messages.

Bill

From: Seandroid-list [mailto:seandroid-list-boun...@tycho.nsa.gov] On Behalf Of 
William Roberts
Sent: Monday, January 18, 2016 8:53 AM
To: Inamdar Sharif 
Cc: seandroid-list@tycho.nsa.gov
Subject: RE: avc denial while enabling zram

Interesting, we have swap on zram on Intel devices and I don't recall hearing 
of anything related to this. So we may just be doing a dontaudit or ignoring 
it, not sure offhand.
On Jan 18, 2016 8:41 AM, "Inamdar Sharif"  wrote:
>
> >>Is that denial actually manifesting itself as some broken functionality?
>
> As such it is not breaking anything. But this is seen while formatting sdcard 
> as internal storage.
>
>  
>
> Also, why is fsck getting invoked on swap, especially one backed by zram?
>
> Not sure about this but got something from the commit message which says that 
> we don’t have swap device on AOSP.
>
>  
>
> 
>
> e2fsck is invoked on any partitions marked with the check mount
>
> option in the fstab file, typically userdata and cache but never
>
> system.  We allow it to read/write the userdata_block_device and
>
> cache_block_device types but also allow it to read/write the default
>
> block_device type until we can get the more specific types assigned
>
> in all of the device-specific policies.
>
>  
>
> mkswap is invoked on any swap partition defined in the fstab file.
>
> We introduce a new swap_block_device type for this purpose, to be
>
> assigned to any such block devices in the device-specific policies,
>
> and only allow it to read/write such block devices.  As there seem to be
>
> no devices in AOSP with swap partitions in their fstab files, this does
>
> not appear to risk any breakage for existing devices.
>
> 
>
>  
>
> Thanks.
>
>  
>
> From: William Roberts [mailto:bill.c.robe...@gmail.com] 
> Sent: Monday, January 18, 2016 9:54 PM
> To: Inamdar Sharif
> Cc: seandroid-list@tycho.nsa.gov
> Subject: Re: avc denial while enabling zram
>
>  
>
> Is that denial actually manifesting itself as some broken functionality?
>
> Also, why is fsck getting invoked on swap, especially one backed by zram?
>
> On Jan 18, 2016 8:20 AM, "Inamdar Sharif"  wrote:
>
> Hi Guys,
>
>  
>
> I am facing the below avc denial while enabling zram.
>
> avc: denied  { getattr } for  pid=7545 comm="e2fsck" path="/dev/block/zram0" 
> dev="tmpfs" ino=11973 scontext=u:r:fsck:s0 
> tcontext=u:object_r:swap_block_device:s0 tclass=blk_file permissive=0 
>
>  
>
> I have labelled dev/block/zram0 as swap_block_device
>
> Also I have an entry in the fstab :
>
> /dev/block/zram0   none swap    defaults  
> zramsize=536870912
>
>  
>
> But due to neverallow rule in fsck.te  the above permission cannot be granted.
>
> # fsck should never be run on these block devices
>
> neverallow fsck {
>
>   boot_block_device
>
>   frp_block_device
>
>   metadata_block_device
>
>   recovery_block_device
>
>   root_block_device
>
>   swap_block_device
>
>   system_block_device
>
>   vold_device
>
> }:blk_file no_rw_file_perms;
>
>  
>
> So I think we have to remove swap_block_device from the neverallow. Any 
> suggestions??
>
>  
>
> 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: fc ordering

[Roberts, William C]

>> libselinux only "sorts" in the sense of giving precedence to exact (no
>> regex characters) entries.  The sorting described in the page you
>> referenced is done by libsemanage or by the fc_sort helper program
>> used in the refpolicy build and is not part of Android at all.  That
>> sorting was introduced to help with ambiguities that occur when
>> file_contexts was split into per-module .fc files.  
>That's essentially the problem we have in our build. Each module is added 
>during the build via sepolicy dirs variable. Perhaps then we should look at 
>adding fc_sort during build? 
>Android however
>> only has a single monolithic file_contexts file, and even with the
>> device-specific file_contexts, the assumption is that those entries
>> should always take precedence over the generic ones (as long as they
>> are not identical and conflict).  So order does matter.  Last matching
>> entry wins.

Does anyone object to adding something like sort_fc to the build to alleviate 
this ordering issue?

Now that we switch to checking the fc files with checkfc, I'm not aware of any 
other CTS issues.

BTW in the CTS test, why didn't we just modify checkfc to return status codes 
for equal, subset, superset? We have all this "complicated"
stdio checking for exact matching.

___
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: fc ordering

[Roberts, William C]

> -Original Message-
> From: Stephen Smalley [mailto:stephen.smal...@gmail.com]
> Sent: Thursday, December 31, 2015 11:01 AM
> To: Roberts, William C <william.c.robe...@intel.com>
> Cc: William Roberts <bill.c.robe...@gmail.com>; seandroid-list@tycho.nsa.gov
> Subject: Re: fc ordering
> 
> On Thu, Dec 31, 2015 at 12:53 PM, Roberts, William C
> <william.c.robe...@intel.com> wrote:
> >
> > 
> >
> >>> libselinux only "sorts" in the sense of giving precedence to exact
> >>> (no regex characters) entries.  The sorting described in the page
> >>> you referenced is done by libsemanage or by the fc_sort helper
> >>> program used in the refpolicy build and is not part of Android at
> >>> all.  That sorting was introduced to help with ambiguities that
> >>> occur when file_contexts was split into per-module .fc files.
> >>That's essentially the problem we have in our build. Each module is added
> during the build via sepolicy dirs variable. Perhaps then we should look at 
> adding
> fc_sort during build?
> >>Android however
> >>> only has a single monolithic file_contexts file, and even with the
> >>> device-specific file_contexts, the assumption is that those entries
> >>> should always take precedence over the generic ones (as long as they
> >>> are not identical and conflict).  So order does matter.  Last
> >>> matching entry wins.
> >
> > Does anyone object to adding something like sort_fc to the build to 
> > alleviate
> this ordering issue?
> >
> > Now that we switch to checking the fc files with checkfc, I'm not aware of 
> > any
> other CTS issues.
> 
> I guess the question is whether we want to sort the entire file_contexts this 
> way
> (which could potentially cause an entry from the AOSP general file_contexts to
> override a device-specific entry) or individually sort the general and device-
> specific file_contexts and then concatenate the two.  The sorting is 
> heuristic-
> based, as there is no precise ordering that can be defined among regular
> expressions.

IIUC,  If we sort the entire fc entries (general + device), then the heuristics 
per that fedora page
would allow "more specific" device entries to override general entries.

However, just sorting the device policy would also be Ok with me, since it 
would solve my issue
In the modular style policy build.

> 
> > BTW in the CTS test, why didn't we just modify checkfc to return status 
> > codes
> for equal, subset, superset? We have all this "complicated"
> > stdio checking for exact matching.
> 
> I followed the example of other tests which were output-based, and also exit
> status can be misleading, e.g. you have to ensure that you are not
> misinterpreting a shell exit code (e.g. checkfc could not be found or could 
> not be
> executed) as the exit code of checkfc itself, and that you cleanly delineate 
> all
> possible exit statuses of the program.

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

fc ordering

[Roberts, William C]

I was under the impression that fc entry order does not matter. That they are 
sorted based on how specific the match is..

As per:
https://fedoraproject.org/wiki/SELinux/ManagingFileContext

In the current Android 6.0 release, I don't see the function sort_specs() 
anywhere in external/libselinux, or any code that sorts the fc entries.

Looks like this code from 6.0:

512 /* Move exact pathname specifications to the end. */
513 spec_copy = (spec_t *) malloc(sizeof(spec_t) * data->nspec);
514 if (!spec_copy)
515 goto finish;
516 j = 0;
517 for (i = 0; i < data->nspec; i++)
518 if (data->spec_arr[i].hasMetaChars)
519 memcpy(_copy[j++],
520>spec_arr[i], sizeof(spec_t));
521 for (i = 0; i < data->nspec; i++)
522 if (!data->spec_arr[i].hasMetaChars)
523 memcpy(_copy[j++],
524>spec_arr[i], sizeof(spec_t));
525 free(data->spec_arr);

Was replaced with sort_specs() on upstream libselinux.

Does anyone perhaps have any more light to shed? Am I way off here?

Bill

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

mac_override: What does ignore mean?

[Roberts, William C]

According to: http://selinuxproject.org/page/ObjectClassesPerms#capability2, 
mac_override is ignored. What does that actually mean? Is it always denied (my 
guess) or always allowed?

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

RE: mac_override: What does ignore mean?

[Roberts, William C]

> -Original Message-
> From: Roberts, William C
> Sent: Monday, December 14, 2015 10:38 AM
> To: 'Stephen Smalley' <s...@tycho.nsa.gov>; seandroid-list@tycho.nsa.gov
> Subject: RE: mac_override: What does ignore mean?
> 
> > >> On 12/14/2015 11:57 AM, Roberts, William C wrote:
> > >>> According to:
> > >>> http://selinuxproject.org/page/ObjectClassesPerms#capability2,
> > >>> mac_override is ignored. What does that actually mean? Is it
> > >>> always denied (my guess) or always allowed?
> > >>
> > >> It is never checked by SELinux, only by Smack.
> > >>
> > >
> > > What does that entail exactly? The messages printed to dmesg are
> > > "avc denied". Does the "is capable" checks call into SE Linux and
> > > EPERM is always
> > returned?
> > >
> > > I ask this in the context of an out of tree driver that is currently
> > > and incorrectly
> > coded with a capable(MAC_OVERRIDE) check.
> >
> > No, the logic performed by the capable hook is not specific to any
> > capability; it just checks whether that permission bit is set in the 
> > corresponding
> access vector.
> > So you can allow it in policy and it should be fine.  But it is wrong
> > for the driver to be using that capability...
> 
> That's what I thought based on looking at the code. I advised the driver team 
> that
> they Should be doing some other type of is_capable() check, likely SYS_ADMIN
> for their needs.
> 
> Thanks, I just wanted to confirm.

FYI more details:

Here is the code:
https://android.googlesource.com/kernel/x86_64.git/+/android-x86_64-fugu-3.10-marshmallow/drivers/staging/sep54/sepfs.c

Line 240.

They have a UID/GID access list for each command, if the process has cap 
MAC_OVERRIDE, the check is skipped. I don't know why
They even need this capable check, they should just add the privileged 
components into their ACL and drop this. Better yet, if they
need finer access controls, they could do an implementation and out of tree 
patch ala binder (would like to avoid this).



___
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: mac_override: What does ignore mean?

[Roberts, William C]

> -Original Message-
> From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
> Sent: Monday, December 14, 2015 9:18 AM
> To: Roberts, William C <william.c.robe...@intel.com>; seandroid-
> l...@tycho.nsa.gov
> Subject: Re: mac_override: What does ignore mean?
> 
> On 12/14/2015 11:57 AM, Roberts, William C wrote:
> > According to:
> > http://selinuxproject.org/page/ObjectClassesPerms#capability2,
> > mac_override is ignored. What does that actually mean? Is it always
> > denied (my guess) or always allowed?
> 
> It is never checked by SELinux, only by Smack.
> 

What does that entail exactly? The messages printed to dmesg are "avc denied". 
Does the "is capable" checks
call into SE Linux and EPERM is always returned?

I ask this in the context of an out of tree driver that is currently and 
incorrectly coded with a capable(MAC_OVERRIDE) check.




___
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: mac_override: What does ignore mean?

[Roberts, William C]

> >> On 12/14/2015 11:57 AM, Roberts, William C wrote:
> >>> According to:
> >>> http://selinuxproject.org/page/ObjectClassesPerms#capability2,
> >>> mac_override is ignored. What does that actually mean? Is it always
> >>> denied (my guess) or always allowed?
> >>
> >> It is never checked by SELinux, only by Smack.
> >>
> >
> > What does that entail exactly? The messages printed to dmesg are "avc
> > denied". Does the "is capable" checks call into SE Linux and EPERM is always
> returned?
> >
> > I ask this in the context of an out of tree driver that is currently and 
> > incorrectly
> coded with a capable(MAC_OVERRIDE) check.
> 
> No, the logic performed by the capable hook is not specific to any 
> capability; it
> just checks whether that permission bit is set in the corresponding access 
> vector.
> So you can allow it in policy and it should be fine.  But it is wrong for the 
> driver to
> be using that capability...

That's what I thought based on looking at the code. I advised the driver team 
that they
Should be doing some other type of is_capable() check, likely SYS_ADMIN for 
their needs.

Thanks, I just wanted to confirm.

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

anonymous unix socket fd passed

[Roberts, William C]

We have an application that creates an anonymous local socket and passes the fd 
via IPC to 3rd party (untrusted_app) applications. The socket itself has the 
label of the application. What are my options here? Mlstrustedsubject the 
application or get the callers to setsockcreatecon() prior to creating the 
socket? Any others?

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

RE: av_decision on audit callback

[Roberts, William C]

> -Original Message-
> From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
> Sent: Friday, October 2, 2015 1:13 PM
> To: Roberts, William C; seandroid-list@tycho.nsa.gov; seli...@tycho.nsa.gov
> Subject: Re: av_decision on audit callback
> 
> On 10/02/2015 04:07 PM, Roberts, William C wrote:
> >
> >
> >> -Original Message-
> >> From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
> >> Sent: Friday, October 2, 2015 12:12 PM
> >> To: Roberts, William C; seandroid-list@tycho.nsa.gov;
> >> seli...@tycho.nsa.gov
> >> Subject: Re: av_decision on audit callback
> >>
> >> On 10/02/2015 02:54 PM, Stephen Smalley wrote:
> >>> On 10/02/2015 02:48 PM, Roberts, William C wrote:
> >>>> I would like to be able to gather the result of permissive mode per
> >>>> domain from a check_access() call for the userspace object managers
> >>>> on Android.
> >>>>
> >>>>   From what I can tell check_access() calls avc_has_perm with a
> >>>> NULL 5th argument. That argument is for the struct avc_entry_ref.
> >>>>
> >>>> That structure has a pointer to an opaque type, avc_entry. Which
> >>>> contains struct av_decision.
> >>>>
> >>>> Which contains flags that have a permissive flag:
> >>>>
> >>>> struct av_decision {
> >>>>
> >>>>   access_vector_t allowed;
> >>>>
> >>>>   access_vector_t decided;
> >>>>
> >>>>   access_vector_t auditallow;
> >>>>
> >>>>   access_vector_t auditdeny;
> >>>>
> >>>>   unsigned int seqno;
> >>>>
> >>>>   unsigned int flags;
> >>>>
> >>>> };
> >>>>
> >>>> /* Definitions of av_decision.flags */
> >>>>
> >>>> #define SELINUX_AVD_FLAGS_PERMISSIVE0x0001
> >>>>
> >>>> It looks like if check_access just passes this structure and then
> >>>> avc_has_perm() when it calls avc_audit, it could supply the
> >>>> av_decision structure to the avc_suppl_audit() call. We could then
> >>>> have an audit2 callback that takes this parameter.
> >>>>
> >>>> Is this mostly right, seem sane? Better way to do this?
> >>>
> >>> It doesn't need to be exposed at that level; the libselinux
> >>> avc_audit() routine can log it, similar to what is done in the kernel.
> >>> It already has the av_decision structure available to it.
> >>
> >> To clarify, anything directly known to the AVC, like the permissive
> >> flag, can be directly logged by it.  The audit callback is for
> >> logging auxiliary audit information not known to the AVC (the pid of the 
> >> client
> process being a good example).
> >
> > I was wondering if we could just dump permissive=0|1 from the AVC
> > logging routine, but that would affect everyone.  I guess then you
> > would be ok with that? Does order matter with the fields wrt parsing?
> > I don't want to break any desktop tooling I am aware of, would we upstream
> this change as well?
> 
> Just put it at the end (i.e. log_append() after the avc_dump_query() call), 
> like we
> do in the kernel.  Shouldn't be a problem.  Technically order shouldn't 
> matter but
> safer to just append it to the current fields.
> 
> Yes, we'd upstream it.
> 
Done. Ill post back with a patch on android-review. And once merged there I can 
send one to the
Mailing list or you can cherry-pick. Are you ok with that?


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

RE: av_decision on audit callback

[Roberts, William C]

> -Original Message-
> From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
> Sent: Friday, October 2, 2015 12:12 PM
> To: Roberts, William C; seandroid-list@tycho.nsa.gov; seli...@tycho.nsa.gov
> Subject: Re: av_decision on audit callback
> 
> On 10/02/2015 02:54 PM, Stephen Smalley wrote:
> > On 10/02/2015 02:48 PM, Roberts, William C wrote:
> >> I would like to be able to gather the result of permissive mode per
> >> domain from a check_access() call for the userspace object managers
> >> on Android.
> >>
> >>  From what I can tell check_access() calls avc_has_perm with a NULL
> >> 5th argument. That argument is for the struct avc_entry_ref.
> >>
> >> That structure has a pointer to an opaque type, avc_entry. Which
> >> contains struct av_decision.
> >>
> >> Which contains flags that have a permissive flag:
> >>
> >> struct av_decision {
> >>
> >>  access_vector_t allowed;
> >>
> >>  access_vector_t decided;
> >>
> >>  access_vector_t auditallow;
> >>
> >>  access_vector_t auditdeny;
> >>
> >>  unsigned int seqno;
> >>
> >>  unsigned int flags;
> >>
> >> };
> >>
> >> /* Definitions of av_decision.flags */
> >>
> >> #define SELINUX_AVD_FLAGS_PERMISSIVE0x0001
> >>
> >> It looks like if check_access just passes this structure and then
> >> avc_has_perm() when it calls avc_audit, it could supply the
> >> av_decision structure to the avc_suppl_audit() call. We could then
> >> have an audit2 callback that takes this parameter.
> >>
> >> Is this mostly right, seem sane? Better way to do this?
> >
> > It doesn't need to be exposed at that level; the libselinux
> > avc_audit() routine can log it, similar to what is done in the kernel.
> > It already has the av_decision structure available to it.
> 
> To clarify, anything directly known to the AVC, like the permissive flag, can 
> be
> directly logged by it.  The audit callback is for logging auxiliary audit 
> information
> not known to the AVC (the pid of the client process being a good example).

I was wondering if we could just dump permissive=0|1 from the AVC logging 
routine, but that
would affect everyone.  I guess then you would be ok with that? Does order 
matter with
the fields wrt parsing? I don't want to break any desktop tooling I am aware 
of, would we upstream
this change as well?



___
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: av_decision on audit callback

[Roberts, William C]

> -Original Message-
> From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
> Sent: Friday, October 2, 2015 1:26 PM
> To: Roberts, William C; seandroid-list@tycho.nsa.gov; seli...@tycho.nsa.gov
> Subject: Re: av_decision on audit callback
> 
> On 10/02/2015 04:22 PM, Roberts, William C wrote:
> >
> >
> >> -Original Message-
> >> From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
> >> Sent: Friday, October 2, 2015 1:13 PM
> >> To: Roberts, William C; seandroid-list@tycho.nsa.gov;
> >> seli...@tycho.nsa.gov
> >> Subject: Re: av_decision on audit callback
> >>
> >> On 10/02/2015 04:07 PM, Roberts, William C wrote:
> >>>
> >>>
> >>>> -Original Message-
> >>>> From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
> >>>> Sent: Friday, October 2, 2015 12:12 PM
> >>>> To: Roberts, William C; seandroid-list@tycho.nsa.gov;
> >>>> seli...@tycho.nsa.gov
> >>>> Subject: Re: av_decision on audit callback
> >>>>
> >>>> On 10/02/2015 02:54 PM, Stephen Smalley wrote:
> >>>>> On 10/02/2015 02:48 PM, Roberts, William C wrote:
> >>>>>> I would like to be able to gather the result of permissive mode
> >>>>>> per domain from a check_access() call for the userspace object
> >>>>>> managers on Android.
> >>>>>>
> >>>>>>From what I can tell check_access() calls avc_has_perm with a
> >>>>>> NULL 5th argument. That argument is for the struct avc_entry_ref.
> >>>>>>
> >>>>>> That structure has a pointer to an opaque type, avc_entry. Which
> >>>>>> contains struct av_decision.
> >>>>>>
> >>>>>> Which contains flags that have a permissive flag:
> >>>>>>
> >>>>>> struct av_decision {
> >>>>>>
> >>>>>>access_vector_t allowed;
> >>>>>>
> >>>>>>access_vector_t decided;
> >>>>>>
> >>>>>>access_vector_t auditallow;
> >>>>>>
> >>>>>>access_vector_t auditdeny;
> >>>>>>
> >>>>>>unsigned int seqno;
> >>>>>>
> >>>>>>unsigned int flags;
> >>>>>>
> >>>>>> };
> >>>>>>
> >>>>>> /* Definitions of av_decision.flags */
> >>>>>>
> >>>>>> #define SELINUX_AVD_FLAGS_PERMISSIVE0x0001
> >>>>>>
> >>>>>> It looks like if check_access just passes this structure and then
> >>>>>> avc_has_perm() when it calls avc_audit, it could supply the
> >>>>>> av_decision structure to the avc_suppl_audit() call. We could
> >>>>>> then have an audit2 callback that takes this parameter.
> >>>>>>
> >>>>>> Is this mostly right, seem sane? Better way to do this?
> >>>>>
> >>>>> It doesn't need to be exposed at that level; the libselinux
> >>>>> avc_audit() routine can log it, similar to what is done in the kernel.
> >>>>> It already has the av_decision structure available to it.
> >>>>
> >>>> To clarify, anything directly known to the AVC, like the permissive
> >>>> flag, can be directly logged by it.  The audit callback is for
> >>>> logging auxiliary audit information not known to the AVC (the pid
> >>>> of the client
> >> process being a good example).
> >>>
> >>> I was wondering if we could just dump permissive=0|1 from the AVC
> >>> logging routine, but that would affect everyone.  I guess then you
> >>> would be ok with that? Does order matter with the fields wrt parsing?
> >>> I don't want to break any desktop tooling I am aware of, would we
> >>> upstream
> >> this change as well?
> >>
> >> Just put it at the end (i.e. log_append() after the avc_dump_query()
> >> call), like we do in the kernel.  Shouldn't be a problem.
> >> Technically order shouldn't matter but safer to just append it to the 
> >> current
> fields.
> >>
> >> Yes, we'd upstream it.
> >>
> > Done. Ill post back with a patch on android-review. And once merged
> > there I can send one to the Mailing list or you can cherry-pick. Are you ok 
> > with
> that?
> 
> Yes.  As with the kernel, only add permissive= to avc: denied messages, not 
> avc:
> granted ones.  See the kernel logic (but note that the code and data 
> structures
> aren't the same as the libselinux AVC anymore).

Ok. Yeah it wouldn't make sense for granted. Its good thing you said something.


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

av_decision on audit callback

[Roberts, William C]

I would like to be able to gather the result of permissive mode per domain from 
a check_access() call for the userspace object managers on Android.

>From what I can tell check_access() calls avc_has_perm with a NULL 5th 
>argument. That argument is for the struct avc_entry_ref.

That structure has a pointer to an opaque type, avc_entry. Which contains 
struct av_decision.

Which contains flags that have a permissive flag:

struct av_decision {
access_vector_t allowed;
access_vector_t decided;
access_vector_t auditallow;
access_vector_t auditdeny;
unsigned int seqno;
unsigned int flags;
};

/* Definitions of av_decision.flags */
#define SELINUX_AVD_FLAGS_PERMISSIVE0x0001

It looks like if check_access just passes this structure and then 
avc_has_perm() when it calls avc_audit, it could supply the av_decision 
structure to the avc_suppl_audit() call. We could then have an audit2 callback 
that takes this parameter.

Is this mostly right, seem sane? Better way to do this?


___
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: property service set doesn't have pid

[Roberts, William C]

Im an idiot... sorry

From: Roberts, William C
Sent: Thursday, October 1, 2015 1:46 PM
To: 'seandroid-list@tycho.nsa.gov'
Subject: RE: property service set doesn't have pid

Looks like the check_access() has an additional field that can get passed 
avc_has_perm()

http://manned.org/selinux_check_access



From: Roberts, William C
Sent: Thursday, October 1, 2015 1:38 PM
To: seandroid-list@tycho.nsa.gov<mailto:seandroid-list@tycho.nsa.gov>
Subject: property service set doesn't have pid

We have the peer ucred structure in property service, but the messages don't 
have much information. How can I plumb pid into the check_access 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.

kernel module request personality-8

[Roberts, William C]

This email serves a public record for anyone else wondering what to do with 
these audits.

Recently when working on Android with a 3.14.37 kernel, I came across the 
following audits:


[  170.456077] type=1400 audit(1432616225.277:76): avc: denied {

module_request } for pid=3742 comm="debuggerd" kmod="personality-8"

scontext=u:r:debuggerd:s0 tcontext=u:r:kernel:s0 tclass=system

permissive=1



$ adb shell ps -Z | grep debuggerd

u:r:debuggerd:s0   root  3742  1 /system/bin/debuggerd

u:r:debuggerd:s0   root  3743  1 /system/bin/debuggerd64



I saw these for pretty much any 32 variant of an executable. I was also on an 
intel x86 Android platform, I am not sure if this will

Be relevant to other arch's or not. I was able to safely "dontaudit" this,  and 
it seems removed from 4.0 kernels. Stephen (paraphrasing the email) provided

The following informative links as well as suggesting a dontaudit:



http://marc.info/?l=linux-kernel=128934356223514=2



More context:

http://marc.info/?l=linux-kernel=142878569719810=2



Looks like it got removed entirely in 4.0:

http://marc.info/?l=linux-arch=142912798314177=2



Hopefully this helps some lost soul in the future.


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

ioctl 0x5879 on class DIR

[Roberts, William C]

Does anyone know what this ioctl is on class dir?

[  258.646056] type=1400 audit(1432616313.483:91): avc: denied { ioctl } for 
pid=4814 comm="xxx" path="/factory" dev="mmcblk0p12" ino=2 ioctlcmd=5879 
scontext=u:r::s0 tcontext=u:object_r:factory_file:s0 tclass=dir permissive=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.

kernel access to device comm is kdevtmpfs

[Roberts, William C]

I get these records with comm set to kdevtempfs. The targert context is device, 
however when interrogating the node from userspace, I notice 2 things:


1.   The inode doesn't match

2.   The label is correct per file_contexts

root@device:/dev # ls -laiZ media0
   1 crw-rw system   camerau:object_r:camera_device:s0 
media0
root@device:/dev # ls -laiZ ttyS1
1217 crw-rw bluetooth bluetooth  u:object_r:hci_attach_dev:s0 
ttyS1

[4.421817] audit: type=1400 audit(1263534127.178:4): avc:  denied  { write 
} for  pid=24 comm=kdevtmpfs name=/ dev=devtmpfs ino=1025 
scontext=u:r:kernel:s0 tcontext=u:object_r:device:s0 tclass=dir permissive=1
[4.421859] audit: type=1400 audit(1263534127.178:5): avc:  denied  { 
add_name } for  pid=24 comm=kdevtmpfs name=dm-0 scontext=u:r:kernel:s0 
tcontext=u:object_r:device:s0 tclass=dir permissive=1
[5.745165] type=1400 audit(1263534128.499:23): avc: denied { getattr } for 
pid=24 comm=kdevtmpfs path=/ttyS1 dev=devtmpfs ino=1051 
scontext=u:r:kernel:s0 tcontext=u:object_r:device:s0 tclass=chr_file 
permissive=1
[5.746180] type=1400 audit(1263534128.499:24): avc: denied { setattr } for 
pid=24 comm=kdevtmpfs name=ttyS1 dev=devtmpfs ino=1051 
scontext=u:r:kernel:s0 tcontext=u:object_r:device:s0 tclass=chr_file 
permissive=1
[5.746384] type=1400 audit(1263534128.499:25): avc: denied { remove_name } 
for pid=24 comm=kdevtmpfs name=ttyS1 dev=devtmpfs ino=1051 
scontext=u:r:kernel:s0 tcontext=u:object_r:device:s0 tclass=dir permissive=1
[5.746742] type=1400 audit(1263534128.499:26): avc: denied { unlink } for 
pid=24 comm=kdevtmpfs name=ttyS1 dev=devtmpfs ino=1051 
scontext=u:r:kernel:s0 tcontext=u:object_r:device:s0 tclass=chr_file 
permissive=1
[5.746966] type=1400 audit(1263534128.500:27): avc: denied { create } for 
pid=24 comm=kdevtmpfs name=ttyS1 scontext=u:r:kernel:s0 
tcontext=u:object_r:device:s0 tclass=chr_file permissive=1
[7.605775] type=1400 audit(1263534130.358:35): avc: denied { write } for 
pid=24 comm=kdevtmpfs name=/ dev=devtmpfs ino=1025 scontext=u:r:kernel:s0 
tcontext=u:object_r:device:s0 tclass=dir permissive=1
[7.606116] type=1400 audit(1263534130.358:36): avc: denied { add_name } for 
pid=24 comm=kdevtmpfs name=media0 scontext=u:r:kernel:s0 
tcontext=u:object_r:device:s0 tclass=dir permissive=1
[7.606350] type=1400 audit(1263534130.358:37): avc: denied { create } for 
pid=24 comm=kdevtmpfs name=media0 scontext=u:r:kernel:s0 
tcontext=u:object_r:device:s0 tclass=chr_file permissive=1
[7.606582] type=1400 audit(1263534130.358:38): avc: denied { setattr } for 
pid=24 comm=kdevtmpfs name=media0 dev=devtmpfs ino= 
scontext=u:r:kernel:s0 tcontext=u:object_r:device:s0 tclass=chr_file 
permissive=1
[   10.152747] type=1400 audit(1263534132.902:52): avc: denied { write } for 
pid=24 comm=kdevtmpfs name=/ dev=devtmpfs ino=1025 scontext=u:r:kernel:s0 
tcontext=u:object_r:device:s0 tclass=dir permissive=1
[   10.153026] type=1400 audit(1263534132.902:53): avc: denied { add_name } for 
pid=24 comm=kdevtmpfs name=dm-1 scontext=u:r:kernel:s0 
tcontext=u:object_r:device:s0 tclass=dir permissive=1
[4.421817] audit: type=1400 audit(1263534127.178:4): avc:  denied  { write 
} for  pid=24 comm=kdevtmpfs name=/ dev=devtmpfs ino=1025 
scontext=u:r:kernel:s0 tcontext=u:object_r:device:s0 tclass=dir permissive=1
[4.421859] audit: type=1400 audit(1263534127.178:5): avc:  denied  { 
add_name } for  pid=24 comm=kdevtmpfs name=dm-0 scontext=u:r:kernel:s0 
tcontext=u:object_r:device:s0 tclass=dir permissive=1
[5.745165] type=1400 audit(1263534128.499:23): avc: denied { getattr } for 
pid=24 comm=kdevtmpfs path=/ttyS1 dev=devtmpfs ino=1051 
scontext=u:r:kernel:s0 tcontext=u:object_r:device:s0 tclass=chr_file 
permissive=1
[5.746180] type=1400 audit(1263534128.499:24): avc: denied { setattr } for 
pid=24 comm=kdevtmpfs name=ttyS1 dev=devtmpfs ino=1051 
scontext=u:r:kernel:s0 tcontext=u:object_r:device:s0 tclass=chr_file 
permissive=1
[5.746384] type=1400 audit(1263534128.499:25): avc: denied { remove_name } 
for pid=24 comm=kdevtmpfs name=ttyS1 dev=devtmpfs ino=1051 
scontext=u:r:kernel:s0 tcontext=u:object_r:device:s0 tclass=dir permissive=1
[5.746742] type=1400 audit(1263534128.499:26): avc: denied { unlink } for 
pid=24 comm=kdevtmpfs name=ttyS1 dev=devtmpfs ino=1051 
scontext=u:r:kernel:s0 tcontext=u:object_r:device:s0 tclass=chr_file 
permissive=1
[5.746966] type=1400 audit(1263534128.500:27): avc: denied { create } for 
pid=24 comm=kdevtmpfs name=ttyS1 scontext=u:r:kernel:s0 
tcontext=u:object_r:device:s0 tclass=chr_file permissive=1
[7.605775] type=1400 audit(1263534130.358:35): avc: denied { write } for 
pid=24 comm=kdevtmpfs name=/ dev=devtmpfs ino=1025 scontext=u:r:kernel:s0 
tcontext=u:object_r:device:s0 tclass=dir permissive=1
[7.606116] type=1400 audit(1263534130.358:36): avc: denied { add_name } for 
pid=24 comm=kdevtmpfs name=media0 

m4 synclines always 1?

[Roberts, William C]

Are the m4 syncline's guaranteed to start with the number 1, I can't seem to 
come across or generate a case otherwise on Android.

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

Booting Flo on AOSP SELinux denials

[Roberts, William C]

Just noticed these on Flo when I have been booting off of master the last few 
days, on a permissive boot I get:

[   81.106384] type=1400 audit(1438356485.185:3): avc: denied { read } for 
pid=920 comm=vold name=/ dev=mmcblk0p4 ino=2 scontext=u:r:vold:s0 
tcontext=u:object_r:persist_file:s0 tclass=dir
[   81.106994] type=1400 audit(1438356485.185:4): avc: denied { open } for 
pid=920 comm=vold name=/ dev=mmcblk0p4 ino=2 scontext=u:r:vold:s0 
tcontext=u:object_r:persist_file:s0 tclass=dir
[   81.107299] type=1400 audit(1438356485.185:5): avc: denied { ioctl } for 
pid=920 comm=vold path=/persist dev=mmcblk0p4 ino=2 scontext=u:r:vold:s0 
tcontext=u:object_r:persist_file:s0 tclass=dir


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

file_contexts intermediate deleted on error

[Roberts, William C]

When detecting issues with targets processed with checkfc (property_contexts 
and file_contexts), on error the intermediate is removed, so one is only left 
with the output which is void of the m4 synclines. I am proposing handling the 
m4 pre-processing to a separate intermediate like policy.conf so one can review 
the file for errors reported by checkfc. Is anyone opposed to this?
___
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: modprobe in kernel context

[Roberts, William C]

 -Original Message-
 From: Stephen Smalley [mailto:stephen.smal...@gmail.com]
 Sent: Tuesday, July 21, 2015 11:15 AM
 To: Roberts, William C
 Cc: seandroid-list@tycho.nsa.gov
 Subject: Re: modprobe in kernel context
 
 Most Android kernels are built with CONFIG_MODULES=n these days.  Is yours
 built with it enabled?  If so, then yes, you need to define a domain 
 transition
 from kernel to a new domain on modprobe execution.

No modules are enabled. Is the transition on exec of usermodehelper

According to your comment in kernel.te here we should really only define a 
transition if the executable
Is outside of the rootfs

# The kernel domain is never entered via an exec, nor should it
# ever execute a program outside the rootfs without changing to another domain.
# If you encounter an execute_no_trans denial on the kernel domain, then
# possible causes include:
# - The program is a kernel usermodehelper.  In this case, define a domain
#   for the program and domain_auto_trans() to it.
# - You failed to setcon u:r:init:s0 in your init.rc and thus your init
#   program was left in the kernel domain and is now trying to execute
#   some other program.  Fix your init.rc file.
# - You are running an exploit which switched to the init task credentials
#   and is then trying to exec a shell or other program.  You lose!


 
 On Mon, Jul 20, 2015 at 6:43 PM, Roberts, William C
 william.c.robe...@intel.com wrote:
  I am getting this message and having a fun time tracking it down. My
  best guess is that a user mode helper is executing this.
 
 
 
  38[   29.983923] type=1400 audit(946684830.959:3): avc: denied {
  sys_module } for pid=236 comm=modprobe capability=16
  scontext=u:r:kernel:s0 tcontext=u:r:kernel:s0 tclass=capability
  permissive=1
 
 
 
  # cat /proc/sys/kernel/modprobe
 
  /sbin/modprobe
 
 
 
  # ls -laZ /proc//sys/kernel/modprobe
 
  -rw-r--r-- root root  u:object_r:usermodehelper:s0 modprobe
 
 
 
  Should I just label this and set up a domain transition? Our kernel
  does contain the rootfs patch https://android-
 review.googlesource.com/#/c/58360/.
 
 
 
  Is this barking down the right path, or should I just give kernel
  domain this capability?
 
 
  ___
  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.

modprobe in kernel context

[Roberts, William C]

I am getting this message and having a fun time tracking it down. My best guess 
is that a user mode helper is executing this.

38[   29.983923] type=1400 audit(946684830.959:3): avc: denied { sys_module } 
for pid=236 comm=modprobe capability=16 scontext=u:r:kernel:s0 
tcontext=u:r:kernel:s0 tclass=capability permissive=1

# cat /proc/sys/kernel/modprobe
/sbin/modprobe

# ls -laZ /proc//sys/kernel/modprobe
-rw-r--r-- root root  u:object_r:usermodehelper:s0 modprobe

Should I just label this and set up a domain transition? Our kernel does 
contain the rootfs patch https://android-review.googlesource.com/#/c/58360/.

Is this barking down the right path, or should I just give kernel domain this 
capability?
___
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: New Categories on Non System App of Android M Preview

[Roberts, William C]

 -Original Message-
 From: Seandroid-list [mailto:seandroid-list-boun...@tycho.nsa.gov] On Behalf
 Of Stephen Smalley
 Sent: Friday, July 17, 2015 11:55 AM
 To: Enfeng Huang
 Cc: seandroid-list@tycho.nsa.gov
 Subject: Re: New Categories on Non System App of Android M Preview
 
 See https://android-review.googlesource.com/#/c/107443/
 
 On Fri, Jul 17, 2015 at 2:33 PM, Enfeng Huang enfen...@samsung.com wrote:
  Hi all,
 
 
 
  Recently, I found that there are 2 new SEAndroid categories
  (c512,c768) added to the non system app.
 
  Why should there be such a change? On the previous Android version,
  there is no category at all.
 
  Another question is why 2 categories are used instead of 1? I think
  that 1 category may be enough.

If you look into external/libselinux/src/android.c at function 
seapp_context_lookup() it should
Become apparent (hopefully) the underpinnings of this. You don't want 
collisions between various
levelFrom options in seapp_contexts between category sets. IE you don't want 
levelFrom=user and
levelFrom=app to collide and thus defeat MLS separation.

levelFrom=app gets a category set where one might be from 0-255 and the other 
form 256-511
levelFrom=user gets a category set where one might be from 512-767 and the 
other form 768-1023
levelFrom=all is the above two and thus has 4 categories in the set.

levelFrom user will provide MLS isolation between physical Android users (not 
the uid sandboxing mechanism that is internal to a user)
levelFrom app will provide MLS isolation between apps and this reinforce the 
app sandboxing, however IIRC breaks inter app file sharing on open().
levelFrom all does both inter-user and inter-application

This is all based on the current MLS rules as I can best remember offhand. To 
understand category sets, make sure you understand the MLS syntax
and domby, etc statements. I found this reference handy for that (its terse but 
I found that to be the most helpful):
http://selinuxproject.org/page/NB_MLS

Another resource is the SELinux notebook:
http://www.freetechbooks.com/the-selinux-notebook-the-foundations-t785.html

Relevant code:
   if (cur-levelFrom != LEVELFROM_NONE) {
char level[255];
switch (cur-levelFrom) {
case LEVELFROM_APP:
snprintf(level, sizeof level, s0:c%u,c%u,
 appid  0xff,
 256 + (appid8  0xff));
break;
case LEVELFROM_USER:
snprintf(level, sizeof level, s0:c%u,c%u,
 512 + (userid  0xff),
 768 + (userid8  0xff));
break;
case LEVELFROM_ALL:
snprintf(level, sizeof level, 
s0:c%u,c%u,c%u,c%u,
 appid  0xff,
 256 + (appid8  0xff),
 512 + (userid  0xff),
 768 + (userid8  0xff));


 
 
 
  Thanks,
 
  Enfeng Huang, software engineer @ Samsung Research America
 
 
  ___
  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: executing off of a fusefs

[Roberts, William C]

From: William Roberts [mailto:bill.c.robe...@gmail.com]
Sent: Friday, July 10, 2015 8:10 AM
To: Stephen Smalley
Cc: seandroid-list@tycho.nsa.gov; seli...@tycho.nsa.gov; Roberts, William C
Subject: Re: executing off of a fusefs


On Jul 10, 2015 5:48 AM, Stephen Smalley 
s...@tycho.nsa.govmailto:s...@tycho.nsa.gov wrote:

 On 07/09/2015 07:04 PM, Roberts, William C wrote:
  I have this problem on Android, but it’s SELinux general. I’d like to
  get some info from those in the know.
 
  If you wanted to be able to execute files off of fuse and have the
  kernel handle the transition, would it be as simple as having the file
  system return the getxattr call for the security label?

 Aren't fuse mounts mounted with nosuid?  That disables SELinux context
 transitions as well as suid/sgid/file-capabilities. Otherwise, the
 kernel is trusting the userspace filesystem to not cause a privilege
 escalation, potentially in excess of the privileges of the userspace
 filesystem daemon itself.

On AOSP yes others no. BTW I'm not condoning doing this either.


 Aside from that, you would also need to configure SELinux to use xattrs
 from fuse (i.e. fs_use_xattr fuse ...), and change the daemon to support
 getxattr, and ensure that you do not have the deadlock problem that
 exists with upstream FUSE where it

This old problem, I think if you don't use libfuse you can get around it.

Also, I see the manpage for mount has rootcontext, does this provide the
rootnode context so the xattr won’t be queried, or does it provide some 
transient
label that is replaced at  mount with the xattr query?

does not support handling a getxattr
 request on the root directory during the mount(2) call.  And doing this
 would affect all fuse mounts (e.g. /storage/emulated), not just a
 specific one.  There was an attempt to add per-subtype support for
 fs_use so that one could specify a different labeling behavior for a
 specific kind of fuse filesystem, see kernel commit
 102aefdda4d8275ce7d7100bc16c88c74272b260, but this was reverted in
 4d546f81717d253ab67643bf072c6d8821a9249c and
 29b1deb2a48a9dd02b93597aa4c055a24c0e989f as it did not work and caused a
 deadlock for other fuse filesystems.







 ___
 Selinux mailing list
 seli...@tycho.nsa.govmailto:seli...@tycho.nsa.gov
 To unsubscribe, send email to 
 selinux-le...@tycho.nsa.govmailto:selinux-le...@tycho.nsa.gov.
 To get help, send an email containing help to 
 selinux-requ...@tycho.nsa.govmailto: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.

executing off of a fusefs

[Roberts, William C]

I have this problem on Android, but it's SELinux general. I'd like to get some 
info from those in the know.
If you wanted to be able to execute files off of fuse and have the kernel 
handle the transition, would it be as simple as having the file system return 
the getxattr call for the security label?
___
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.

system_app execute on system_app_data_file

[Roberts, William C]

Thoughts on allowing system_app domain execute on its system_app_data_file 
type? I would be leaning towards a no for this with a neverallow and have 
anything that needs to do this moved into its own domain with its own data file 
type. Such a neverallow doesn't exist on AOSP master as of today.
___
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] neverallows on seapp_contexts

[Roberts, William C]

I have a very preliminary change that adds neverallow like assertions to the 
checkseapp tool.

https://android-review.googlesource.com/#/c/153291/

I am not looking for a code review, but rather any assertions that we can come 
up with now, to make sure that we
Will be able to encode them with the proposed design. If anyone has any 
assertions they would like to see, please fire away.

Thanks,
Bill


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

RE: Seeing new denial on AOSP 5.1.1 w.r.t shell request read access on lnk_file .

[Roberts, William C]

 -Original Message-
 From: Selinux [mailto:selinux-boun...@tycho.nsa.gov] On Behalf Of
 Stephen Smalley
 Sent: Friday, June 5, 2015 5:21 AM
 To: Ravi Kumar; seli...@tycho.nsa.gov; seandroid-list@tycho.nsa.gov
 Subject: Re: Seeing new denial on AOSP 5.1.1 w.r.t shell request read access
 on lnk_file .
 
 On 06/05/2015 02:31 AM, Ravi Kumar wrote:
  Hi Team  ,
  I am seeing some new denial when running the CTS test cases  which is
  as follows
 
  avc: denied { read } for pid=6013 comm=sh name=app_process
  dev=mmcblk0p24 ino=410 scontext=u:r:shell:s0
  tcontext=u:object_r:zygote_exec:s0 tclass=lnk_file permissive=0
 
  where app_process is link created to point app_process32/app_process64
  binary when doing a  ls -Z  i see the context of this is as expected
  and  tcontext on the denial  is also as expected (as below ).
 
  root# ls -Z |grep app
  lrwxr-xr-x root shell u:object_r:zygote_exec:s0
  app_process - app_process64
  -rwxr-xr-x root shell u:object_r:zygote_exec:s0
  app_process32
  -rwxr-xr-x root shell u:object_r:zygote_exec:s0
  app_process64
 
 
  I can see that there are NO changes in sepolicy for shell domain /
  Zygote domain.
  Only change is the kernel migration from 3.10.48  to 3.10.73(policy
  version 28)  and i see there are  couple of changes done in security .
  Looking at the changes  I don't see any suspicious changes which could
  impact the shell domain nature. Adding the rule is surely  going to
  addressing the issue but wonder why at first place it is needed  from
  security point of view i don't think adding just read should create a
  problem as the  lnk_files is as good as common_file and to as
  write/execute are not given should not be at risk.  Please let me know
  if we have any changes which could cause this or any comment on this
  will be of great help .
 
 (cc seandroid-list, as that is the list for Android-specific SELinux
 questions)
 
 On lollipop-mr1-release, we have the following in file_contexts:
 $ grep app_process file_contexts
 /system/bin/app_process32 u:object_r:zygote_exec:s0
 /system/bin/app_process64 u:object_r:zygote_exec:s0
 so the /system/bin/app_process symbolic link should just be labeled
 system_file.
 

Yes and once you get labeled properly all domains can read system_file
See domain.te: allow domain system_file:lnk_file r_file_perms;
The read permission is required for symbolic links to read the location to 
follow.

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

killing init seclabel

[Roberts, William C]

Given that rootfs supports restorecon can we kill seclabel and just label 
things in sbin and set up transitions? Can we perhaps support genfscon path 
name labeling like in sysfs/procfs and thus avoid the need for a restorecon?

Any objections to this or preference in approach?

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