Re: [PATCH] Add security hooks to binder and implement the hooks for SELinux.

2015-01-23 Thread Greg KH
On Fri, Jan 23, 2015 at 01:56:28PM -0800, Casey Schaufler wrote:
> On 1/22/2015 6:30 PM, Greg KH wrote:
> > On Thu, Jan 22, 2015 at 01:47:29PM -0500, Stephen Smalley wrote:
> >> On Thu, Jan 22, 2015 at 1:09 PM, Casey Schaufler  
> >> wrote:
> >>> On 1/22/2015 12:51 AM, Greg KH wrote:
>  On Wed, Jan 21, 2015 at 10:54:10AM -0500, Stephen Smalley wrote:
> > Add security hooks to the binder and implement the hooks for SELinux.
> > The security hooks enable security modules such as SELinux to implement
> > controls over binder IPC.  The security hooks include support for
> > controlling what process can become the binder context manager
> > (binder_set_context_mgr), controlling the ability of a process
> > to invoke a binder transaction/IPC to another process 
> > (binder_transaction),
> > controlling the ability of a process to transfer a binder reference to
> > another process (binder_transfer_binder), and controlling the ability
> > of a process to transfer an open file to another process 
> > (binder_transfer_file).
> >
> > These hooks have been included in the Android kernel trees since 
> > Android 4.3.
>  Very interesting, I missed the fact that these were added in that tree,
>  thanks for digging it out and submitting it.
> 
>  I'd like some acks from some Android developers before I take these.
>  Or, if it's easier for them to go through the security tree, that's fine
>  with me as well.
> >>> My only concern is that we're about to see a set of hooks proposed
> >>> for kdbus as well, and it would be a shame if we had two sets of hooks
> >>> that do roughly the same thing (ok, *very roughly*) introduced back to 
> >>> back.
> >> Not sure how much commonality there truly is among them (based on the
> >> last set of proposed kdbus lsm hooks that I saw, admittedly a while
> >> ago) and modules may want to distinguish between the two
> >> forms of IPC regardless.  The binder hooks have been in place in the
> >> Android kernel trees for quite some time, so this patch is just making
> >> the mainline binder driver consistent with what is already in Android.
> >> If it turns out that there is significant duplication when the kdbus
> >> lsm hooks land, I'd be happy to help coalesce them.
> > Yeah, I would wait and see what happens with how the kdbus hooks look
> > before worrying about this all that much.  As people are using the
> > binder hooks today, and binder is in the kernel tree, but kdbus isn't,
> > let's not worry about kdbus until that is merged.
> 
> That seems like a sane plan to me in light of the situation. I am somewhat
> concerned about the growth in special case security behavior. The tun hooks,
> now the binder hooks and later the kdbus hooks. It looks like we're going
> in the direction of special policies for each kind of communication rather
> than a system IPC policy. On the other hand, that appears to be the trend in
> system security overall, so even I can see that it may be prudent.

If you know of some way to make a more "unified" system IPC policy, that
would be great to have, I don't know if anyone is even considering that
work, as it seems people are just more concerned about addressing the
more real issues of the different IPC methods these days.

Maybe it would be a good research project for someone to write a thesis
on?  :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add security hooks to binder and implement the hooks for SELinux.

2015-01-23 Thread Casey Schaufler
On 1/22/2015 6:30 PM, Greg KH wrote:
> On Thu, Jan 22, 2015 at 01:47:29PM -0500, Stephen Smalley wrote:
>> On Thu, Jan 22, 2015 at 1:09 PM, Casey Schaufler  
>> wrote:
>>> On 1/22/2015 12:51 AM, Greg KH wrote:
 On Wed, Jan 21, 2015 at 10:54:10AM -0500, Stephen Smalley wrote:
> Add security hooks to the binder and implement the hooks for SELinux.
> The security hooks enable security modules such as SELinux to implement
> controls over binder IPC.  The security hooks include support for
> controlling what process can become the binder context manager
> (binder_set_context_mgr), controlling the ability of a process
> to invoke a binder transaction/IPC to another process 
> (binder_transaction),
> controlling the ability of a process to transfer a binder reference to
> another process (binder_transfer_binder), and controlling the ability
> of a process to transfer an open file to another process 
> (binder_transfer_file).
>
> These hooks have been included in the Android kernel trees since Android 
> 4.3.
 Very interesting, I missed the fact that these were added in that tree,
 thanks for digging it out and submitting it.

 I'd like some acks from some Android developers before I take these.
 Or, if it's easier for them to go through the security tree, that's fine
 with me as well.
>>> My only concern is that we're about to see a set of hooks proposed
>>> for kdbus as well, and it would be a shame if we had two sets of hooks
>>> that do roughly the same thing (ok, *very roughly*) introduced back to back.
>> Not sure how much commonality there truly is among them (based on the
>> last set of proposed kdbus lsm hooks that I saw, admittedly a while
>> ago) and modules may want to distinguish between the two
>> forms of IPC regardless.  The binder hooks have been in place in the
>> Android kernel trees for quite some time, so this patch is just making
>> the mainline binder driver consistent with what is already in Android.
>> If it turns out that there is significant duplication when the kdbus
>> lsm hooks land, I'd be happy to help coalesce them.
> Yeah, I would wait and see what happens with how the kdbus hooks look
> before worrying about this all that much.  As people are using the
> binder hooks today, and binder is in the kernel tree, but kdbus isn't,
> let's not worry about kdbus until that is merged.

That seems like a sane plan to me in light of the situation. I am somewhat
concerned about the growth in special case security behavior. The tun hooks,
now the binder hooks and later the kdbus hooks. It looks like we're going
in the direction of special policies for each kind of communication rather
than a system IPC policy. On the other hand, that appears to be the trend in
system security overall, so even I can see that it may be prudent.


>
> thanks,
>
> greg k-h
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add security hooks to binder and implement the hooks for SELinux.

2015-01-22 Thread Greg KH
On Thu, Jan 22, 2015 at 01:47:29PM -0500, Stephen Smalley wrote:
> On Thu, Jan 22, 2015 at 1:09 PM, Casey Schaufler  
> wrote:
> > On 1/22/2015 12:51 AM, Greg KH wrote:
> >> On Wed, Jan 21, 2015 at 10:54:10AM -0500, Stephen Smalley wrote:
> >>> Add security hooks to the binder and implement the hooks for SELinux.
> >>> The security hooks enable security modules such as SELinux to implement
> >>> controls over binder IPC.  The security hooks include support for
> >>> controlling what process can become the binder context manager
> >>> (binder_set_context_mgr), controlling the ability of a process
> >>> to invoke a binder transaction/IPC to another process 
> >>> (binder_transaction),
> >>> controlling the ability of a process to transfer a binder reference to
> >>> another process (binder_transfer_binder), and controlling the ability
> >>> of a process to transfer an open file to another process 
> >>> (binder_transfer_file).
> >>>
> >>> These hooks have been included in the Android kernel trees since Android 
> >>> 4.3.
> >> Very interesting, I missed the fact that these were added in that tree,
> >> thanks for digging it out and submitting it.
> >>
> >> I'd like some acks from some Android developers before I take these.
> >> Or, if it's easier for them to go through the security tree, that's fine
> >> with me as well.
> >
> > My only concern is that we're about to see a set of hooks proposed
> > for kdbus as well, and it would be a shame if we had two sets of hooks
> > that do roughly the same thing (ok, *very roughly*) introduced back to back.
> 
> Not sure how much commonality there truly is among them (based on the
> last set of proposed kdbus lsm hooks that I saw, admittedly a while
> ago) and modules may want to distinguish between the two
> forms of IPC regardless.  The binder hooks have been in place in the
> Android kernel trees for quite some time, so this patch is just making
> the mainline binder driver consistent with what is already in Android.
> If it turns out that there is significant duplication when the kdbus
> lsm hooks land, I'd be happy to help coalesce them.

Yeah, I would wait and see what happens with how the kdbus hooks look
before worrying about this all that much.  As people are using the
binder hooks today, and binder is in the kernel tree, but kdbus isn't,
let's not worry about kdbus until that is merged.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add security hooks to binder and implement the hooks for SELinux.

2015-01-22 Thread Stephen Smalley
On Thu, Jan 22, 2015 at 1:09 PM, Casey Schaufler  wrote:
> On 1/22/2015 12:51 AM, Greg KH wrote:
>> On Wed, Jan 21, 2015 at 10:54:10AM -0500, Stephen Smalley wrote:
>>> Add security hooks to the binder and implement the hooks for SELinux.
>>> The security hooks enable security modules such as SELinux to implement
>>> controls over binder IPC.  The security hooks include support for
>>> controlling what process can become the binder context manager
>>> (binder_set_context_mgr), controlling the ability of a process
>>> to invoke a binder transaction/IPC to another process (binder_transaction),
>>> controlling the ability of a process to transfer a binder reference to
>>> another process (binder_transfer_binder), and controlling the ability
>>> of a process to transfer an open file to another process 
>>> (binder_transfer_file).
>>>
>>> These hooks have been included in the Android kernel trees since Android 
>>> 4.3.
>> Very interesting, I missed the fact that these were added in that tree,
>> thanks for digging it out and submitting it.
>>
>> I'd like some acks from some Android developers before I take these.
>> Or, if it's easier for them to go through the security tree, that's fine
>> with me as well.
>
> My only concern is that we're about to see a set of hooks proposed
> for kdbus as well, and it would be a shame if we had two sets of hooks
> that do roughly the same thing (ok, *very roughly*) introduced back to back.

Not sure how much commonality there truly is among them (based on the
last set of proposed kdbus lsm hooks that I saw, admittedly a while
ago) and modules may want to distinguish between the two
forms of IPC regardless.  The binder hooks have been in place in the
Android kernel trees for quite some time, so this patch is just making
the mainline binder driver consistent with what is already in Android.
If it turns out that there is significant duplication when the kdbus
lsm hooks land, I'd be happy to help coalesce them.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add security hooks to binder and implement the hooks for SELinux.

2015-01-22 Thread Casey Schaufler
On 1/22/2015 12:51 AM, Greg KH wrote:
> On Wed, Jan 21, 2015 at 10:54:10AM -0500, Stephen Smalley wrote:
>> Add security hooks to the binder and implement the hooks for SELinux.
>> The security hooks enable security modules such as SELinux to implement
>> controls over binder IPC.  The security hooks include support for
>> controlling what process can become the binder context manager
>> (binder_set_context_mgr), controlling the ability of a process
>> to invoke a binder transaction/IPC to another process (binder_transaction),
>> controlling the ability of a process to transfer a binder reference to
>> another process (binder_transfer_binder), and controlling the ability
>> of a process to transfer an open file to another process 
>> (binder_transfer_file).
>>
>> These hooks have been included in the Android kernel trees since Android 4.3.
> Very interesting, I missed the fact that these were added in that tree,
> thanks for digging it out and submitting it.
>
> I'd like some acks from some Android developers before I take these.
> Or, if it's easier for them to go through the security tree, that's fine
> with me as well.

My only concern is that we're about to see a set of hooks proposed
for kdbus as well, and it would be a shame if we had two sets of hooks
that do roughly the same thing (ok, *very roughly*) introduced back to back.

>
> thanks,
>
> greg k-h
> --
> To unsubscribe from this list: send the line "unsubscribe 
> linux-security-module" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add security hooks to binder and implement the hooks for SELinux.

2015-01-22 Thread Nick Kralevich
Acked-By: Nick Kralevich 

On Thu, Jan 22, 2015 at 12:51 AM, Greg KH  wrote:
> On Wed, Jan 21, 2015 at 10:54:10AM -0500, Stephen Smalley wrote:
>> Add security hooks to the binder and implement the hooks for SELinux.
>> The security hooks enable security modules such as SELinux to implement
>> controls over binder IPC.  The security hooks include support for
>> controlling what process can become the binder context manager
>> (binder_set_context_mgr), controlling the ability of a process
>> to invoke a binder transaction/IPC to another process (binder_transaction),
>> controlling the ability of a process to transfer a binder reference to
>> another process (binder_transfer_binder), and controlling the ability
>> of a process to transfer an open file to another process 
>> (binder_transfer_file).
>>
>> These hooks have been included in the Android kernel trees since Android 4.3.
>
> Very interesting, I missed the fact that these were added in that tree,
> thanks for digging it out and submitting it.
>
> I'd like some acks from some Android developers before I take these.
> Or, if it's easier for them to go through the security tree, that's fine
> with me as well.
>
> thanks,
>
> greg k-h



-- 
Nick Kralevich | Android Security | n...@google.com | 650.214.4037
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add security hooks to binder and implement the hooks for SELinux.

2015-01-22 Thread Jeffrey Vander Stoep
ACK.

This has been in the android tree since Nov 2012.

Forward port of commit: 6639e3d91a05bafa2a85c24c211c43fcaa1b17c5
in https://android.googlesource.com/kernel/common.git

Apologies for the double send. Forgot to disable formatting.

Thanks,
Jeff

On Thu, Jan 22, 2015 at 12:51 AM, Greg KH  wrote:
> On Wed, Jan 21, 2015 at 10:54:10AM -0500, Stephen Smalley wrote:
>> Add security hooks to the binder and implement the hooks for SELinux.
>> The security hooks enable security modules such as SELinux to implement
>> controls over binder IPC.  The security hooks include support for
>> controlling what process can become the binder context manager
>> (binder_set_context_mgr), controlling the ability of a process
>> to invoke a binder transaction/IPC to another process (binder_transaction),
>> controlling the ability of a process to transfer a binder reference to
>> another process (binder_transfer_binder), and controlling the ability
>> of a process to transfer an open file to another process 
>> (binder_transfer_file).
>>
>> These hooks have been included in the Android kernel trees since Android 4.3.
>
> Very interesting, I missed the fact that these were added in that tree,
> thanks for digging it out and submitting it.
>
> I'd like some acks from some Android developers before I take these.
> Or, if it's easier for them to go through the security tree, that's fine
> with me as well.
>
> thanks,
>
> greg k-h
> ___
> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add security hooks to binder and implement the hooks for SELinux.

2015-01-22 Thread Greg KH
On Wed, Jan 21, 2015 at 10:54:10AM -0500, Stephen Smalley wrote:
> Add security hooks to the binder and implement the hooks for SELinux.
> The security hooks enable security modules such as SELinux to implement
> controls over binder IPC.  The security hooks include support for
> controlling what process can become the binder context manager
> (binder_set_context_mgr), controlling the ability of a process
> to invoke a binder transaction/IPC to another process (binder_transaction),
> controlling the ability of a process to transfer a binder reference to
> another process (binder_transfer_binder), and controlling the ability
> of a process to transfer an open file to another process 
> (binder_transfer_file).
> 
> These hooks have been included in the Android kernel trees since Android 4.3.

Very interesting, I missed the fact that these were added in that tree,
thanks for digging it out and submitting it.

I'd like some acks from some Android developers before I take these.
Or, if it's easier for them to go through the security tree, that's fine
with me as well.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] Add security hooks to binder and implement the hooks for SELinux.

2015-01-21 Thread Stephen Smalley
Add security hooks to the binder and implement the hooks for SELinux.
The security hooks enable security modules such as SELinux to implement
controls over binder IPC.  The security hooks include support for
controlling what process can become the binder context manager
(binder_set_context_mgr), controlling the ability of a process
to invoke a binder transaction/IPC to another process (binder_transaction),
controlling the ability of a process to transfer a binder reference to
another process (binder_transfer_binder), and controlling the ability
of a process to transfer an open file to another process (binder_transfer_file).

These hooks have been included in the Android kernel trees since Android 4.3.

(Updated to reflect upstream relocation and changes to the binder driver,
changes to the LSM audit data structures, coding style cleanups, and
to add inline documentation for the hooks).

Signed-off-by: Stephen Smalley 
---
 drivers/android/binder.c| 26 +
 include/linux/security.h| 58 +
 security/capability.c   | 27 ++
 security/security.c | 23 
 security/selinux/hooks.c| 73 +
 security/selinux/include/classmap.h |  2 +
 6 files changed, 209 insertions(+)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 8c43521..33b09b6 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifdef CONFIG_ANDROID_BINDER_IPC_32BIT
 #define BINDER_IPC_32BIT 1
@@ -1400,6 +1401,11 @@ static void binder_transaction(struct binder_proc *proc,
return_error = BR_DEAD_REPLY;
goto err_dead_binder;
}
+   if (security_binder_transaction(proc->tsk,
+   target_proc->tsk) < 0) {
+   return_error = BR_FAILED_REPLY;
+   goto err_invalid_target_handle;
+   }
if (!(tr->flags & TF_ONE_WAY) && thread->transaction_stack) {
struct binder_transaction *tmp;
 
@@ -1551,6 +1557,11 @@ static void binder_transaction(struct binder_proc *proc,
return_error = BR_FAILED_REPLY;
goto err_binder_get_ref_for_node_failed;
}
+   if (security_binder_transfer_binder(proc->tsk,
+   target_proc->tsk)) {
+   return_error = BR_FAILED_REPLY;
+   goto err_binder_get_ref_for_node_failed;
+   }
ref = binder_get_ref_for_node(target_proc, node);
if (ref == NULL) {
return_error = BR_FAILED_REPLY;
@@ -1581,6 +1592,11 @@ static void binder_transaction(struct binder_proc *proc,
return_error = BR_FAILED_REPLY;
goto err_binder_get_ref_failed;
}
+   if (security_binder_transfer_binder(proc->tsk,
+   target_proc->tsk)) {
+   return_error = BR_FAILED_REPLY;
+   goto err_binder_get_ref_failed;
+   }
if (ref->node->proc == target_proc) {
if (fp->type == BINDER_TYPE_HANDLE)
fp->type = BINDER_TYPE_BINDER;
@@ -1638,6 +1654,13 @@ static void binder_transaction(struct binder_proc *proc,
return_error = BR_FAILED_REPLY;
goto err_fget_failed;
}
+   if (security_binder_transfer_file(proc->tsk,
+ target_proc->tsk,
+ file) < 0) {
+   fput(file);
+   return_error = BR_FAILED_REPLY;
+   goto err_get_unused_fd_failed;
+   }
target_fd = task_get_unused_fd_flags(target_proc, 
O_CLOEXEC);
if (target_fd < 0) {
fput(file);
@@ -2675,6 +2698,9 @@ static int binder_ioctl_set_ctx_mgr(struct file *filp)
ret = -EBUSY;
goto out;
}
+   ret = security_binder_set_context_mgr(proc->tsk);
+   if (ret < 0)
+   goto out;
if (uid_valid(binder_context_mgr_uid)) {
if (!uid_eq(binder_context_mgr_uid, curr_euid)) {
pr_err("BINDER_SET_CONTEXT_MGR bad uid %d != %d\n",
diff --git a/include/linux/security.h b/inclu