Re: [RFC PATCH 1/7] security: Add LSM hooks for Infiniband security

[Or Gerlitz]

On Tue, Apr 5, 2016 at 12:48 AM, Dan Jurgens  wrote:

> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -8,6 +8,7 @@
>   * Copyright (C) 2001 Silicon Graphics, Inc. (Trust Technology Group)
>   * Copyright (C) 2015 Intel Corporation.
>   * Copyright (C) 2015 Casey Schaufler 
> + * Copyright (C) 2016 Mellanox Techonologies. 

Hi Dan,

I don't see the point to use personal copyright credits in our
upstream development. AFAIK, we don't do that for the NIC side
patches, if you don't mind, lets avoid that.

Or.
___
Selinux mailing list
Selinux@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.

Re: [RFC PATCH 1/7] security: Add LSM hooks for Infiniband security

[Daniel Jurgens]

On 4/4/2016 6:48 PM, Casey Schaufler wrote:
> On 4/4/2016 2:48 PM, Dan Jurgens wrote:
>> From: Daniel Jurgens 
>>
>> Add five new hooks
>>  1. Allocate security contexts for Infiniband objects
>>  2. Free security contexts for Infiniband objects
>>  3. Enforce access to Pkeys
>>  4. Enforce access to Infiniband devices subnet management interfaces.
>>  5. A hook to be implemented by IB core to receive notifications of
>> security policy or enforcement changes.  Restricting a QPs access to
>> a pkey will be done during setup and not on a per packet basis
>> access must be enforced again.
>>
>> Because IB core is usually compiled as a module it must be able to
>> delete it's hooks.  Remove the SELinux specific ifdef around
>> security_delete_hooks and update the comment.  Also EXPORT_SYMBOL for
>> security_hook_heads so IB core can access it to add and delete the hook.
> 
> The LSM infrastructure does not actually support dynamic
> loading and unloading of modules. It happens that the SELinux
> code is structured so that it can be safely unloaded if
> the policy has not been loaded.
> 
If a module calls synchronize_rcu after deleting it's hooks but before
unloading isn't safety assured?  I can send an out of context patch in a
reply showing how ib_core manages that hook if that would be helpful for
context.

>> Signed-off-by: Daniel Jurgens 
>> Reviewed-by: Eli Cohen 
>> ---
>>  include/linux/lsm_hooks.h |   43 -
>>  include/linux/security.h  |   37 
>>  security/Kconfig  |9 +++
>>  security/security.c   |   52 
>> +
>>  4 files changed, 135 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 71969de..c0c7a40 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -8,6 +8,7 @@
>>   * Copyright (C) 2001 Silicon Graphics, Inc. (Trust Technology Group)
>>   * Copyright (C) 2015 Intel Corporation.
>>   * Copyright (C) 2015 Casey Schaufler 
>> + * Copyright (C) 2016 Mellanox Techonologies. 
>>   *
>>   *  This program is free software; you can redistribute it and/or modify
>>   *  it under the terms of the GNU General Public License as published by
>> @@ -877,6 +878,21 @@
>>   *  associated with the TUN device's security structure.
>>   *  @security pointer to the TUN devices's security structure.
>>   *
>> + * Security hooks for Infiniband
>> + *
>> + * @pkey_access:
>> + *  Check permission when modifing a QP or transmitting and receiving MADs.
>> + * @ibdev_smi:
>> + *  Check permissions to access the devices subnet management interface 
>> (SMI).
>> + * @infiniband_alloc_security:
>> + *  Allocate a security structure to be used by Infiniband QPs and MAD
>> + *  agents.
>> + * @infiniband_free_security:
>> + *  Free an Infiniband security structure.
>> + * @infiniband_flush:
>> + *  Security modules can use this hook to notify IB core of policy changes
>> + *  or when enforcement changes.
>> + *
>>   * Security hooks for XFRM operations.
>>   *
>>   * @xfrm_policy_alloc_security:
>> @@ -1577,6 +1593,14 @@ union security_list_options {
>>  int (*tun_dev_open)(void *security);
>>  #endif  /* CONFIG_SECURITY_NETWORK */
>>  
>> +#ifdef CONFIG_SECURITY_INFINIBAND
>> +int (*pkey_access)(u64 subnet_prefix, u16 pkey, void *security);
>> +int (*ibdev_smi)(const char *dev_name, u8 port, void *security);
>> +int (*infiniband_alloc_security)(void **security);
>> +void (*infiniband_free_security)(void *security);
> 
> Please attach the security blobs to objects (like an inode) rather
> than just passing a blob pointer. It's going to make module stacking
> lots easier. 
>

That makes sense.  I wondered how modules stacking would work with the
opaque security field, most alloc/free pairs have the lone blob so I
followed that convention.

>> +void (*infiniband_flush)(void);
>> +#endif  /* CONFIG_SECURITY_INFINIBAND */
>> +
>>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
>>  int (*xfrm_policy_alloc_security)(struct xfrm_sec_ctx **ctxp,
>>struct xfrm_user_sec_ctx *sec_ctx,
>> @@ -1805,6 +1829,13 @@ struct security_hook_heads {
>>  struct list_head tun_dev_open;
>>  struct list_head skb_owned_by;
>>  #endif  /* CONFIG_SECURITY_NETWORK */
>> +#ifdef CONFIG_SECURITY_INFINIBAND
>> +struct list_head pkey_access;
>> +struct list_head ibdev_smi;
>> +struct list_head infiniband_alloc_security;
>> +struct list_head infiniband_free_security;
>> +struct list_head infiniband_flush;
>> +#endif  /* CONFIG_SECURITY_INFINIBAND */
>>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
>>  struct list_head xfrm_policy_alloc_security;
>>  struct list_head xfrm_policy_clone_security;
>> @@ -1862,7 +1893,6 @@ static