Re: [PATCH 1/1] networkmanager: Grant access to unlabeled PKeys

[Daniel Jurgens]

On 11/27/2017 10:19 AM, Paul Moore wrote:
> On Mon, Nov 27, 2017 at 9:03 AM, Dan Jurgens <dani...@mellanox.com> wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> For controlling IPoIB VLANs
>>
>> Reported-by: Honggang LI <ho...@redhat.com>
>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>> Tested-by: Honggang LI <ho...@redhat.com>
>> ---
>>  networkmanager.te |2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
> [NOTE: resending due to a typo in the refpol mailing list address]
>
> We obviously need something like this now so we don't break IPoIB, but
> I wonder if we should make the IB access controls dynamic like the
> per-packet network access controls.  We could key off the presence of
> the IB pkey and endport definitions: if there are any objects defined
> in the loaded policy we enable the controls, otherwise we disable
> them.

I think I understand what you're saying Paul, but I'm not clear on the 
mechanism.  Are you referring to the netlabel/IPSEC enable checks? They are 
wrapped up in selinux_peerlbl_enabled.

>
>> diff --git a/networkmanager.te b/networkmanager.te
>> index 76d0106..5e881f4 100644
>> --- a/networkmanager.te
>> +++ b/networkmanager.te
>> @@ -184,6 +184,8 @@ userdom_write_user_tmp_sockets(NetworkManager_t)
>>  userdom_dontaudit_use_unpriv_user_fds(NetworkManager_t)
>>  userdom_dontaudit_use_user_ttys(NetworkManager_t)
>>
>> +corenet_ib_access_unlabeled_pkeys(NetworkManager_t)
>> +
>>  optional_policy(`
>> avahi_domtrans(NetworkManager_t)
>> avahi_kill(NetworkManager_t)
>> --
>> 1.7.1

Re: [PATCH] IB/core: Fix static analysis warning in ib_policy_change_task

[Daniel Jurgens]

On 7/3/2017 6:03 PM, Paul Moore wrote:
> On Fri, Jun 30, 2017 at 11:15 AM, Dan Jurgens <dani...@mellanox.com> wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> ib_get_cached_subnet_prefix can technically fail, but the only way it
>> could is not possible based on the loop conditions. Check the return
>> value before using the variable sp to resolve a static analysis warning.
>>
>> Fixes: 8f408ab64be6 ("selinux lsm IB/core: Implement LSM notification
>> system")
>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>> Reported-by: Dan Carpenter <dan.carpen...@oracle.com>
>> ---
>>  drivers/infiniband/core/device.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/core/device.c 
>> b/drivers/infiniband/core/device.c
>> index 631eaa9..dabd9f9 100644
>> --- a/drivers/infiniband/core/device.c
>> +++ b/drivers/infiniband/core/device.c
>> @@ -376,7 +376,8 @@ static void ib_policy_change_task(struct work_struct 
>> *work)
>> WARN_ONCE(ret,
>>   "ib_get_cached_subnet_prefix err: %d, this 
>> should never happen here\n",
>>   ret);
>> -   ib_security_cache_change(dev, i, sp);
>> +   if (ret)
> Should this be "if (!ret)"?

Yes, my apologies. I missed the git add after fixing that locally.  I sent v1 a 
minute ago.
>
>> +   ib_security_cache_change(dev, i, sp);
>> }
>> }
>> up_read(_rwsem);

Re: [PATCH v2 2/2] selinux-testsuite: Infiniband endport tests

[Daniel Jurgens]

On 6/9/2017 3:01 PM, Paul Moore wrote:
> On Fri, Jun 9, 2017 at 10:59 AM, Daniel Jurgens <dani...@mellanox.com> wrote:
>
> Should be all set now, let me know if you notice any problems.  I did
> add a separate third commit to munge the style/formatting (see
> previous emails); I didn't bother posting it to the list as it is just
> style changes, but in case anyone is curious, this is the commit:
>
>   commit 8e0339cef20d0356d3e115c31a133662e9562e65
>   Author: Paul Moore <p...@paul-moore.com>
>   Date:   Fri Jun 9 15:46:37 2017 -0400
>
>infiniband: apply style corrections to the infiniband tests
>
>Patch generated by './tools/check-syntax -f'.
>
>Signed-off-by: Paul Moore <p...@paul-moore.com>
>
>> I recall you saying you do most of your testing in VMs on a laptop.  But if 
>> you have a system with a free pci-e slot we can ship you an HCA if you'd 
>> like to be able to run these yourself.
> Thank you for the offer, and yes I generally run the tests in a VM,
> however we've been working on getting something a bit more automated
> in place for upstream testing (more info on that once everything is
> sorted out).
>
> Let me think about this a bit (and dust off my somewhat neglected
> testing hardware), I generally try to avoid getting tied to specific
> hardware, but it is necessary in this case, and I fear that this may
> be the easiest way to ensure it gets tested regularly.
>
OK, just let me know if you want one.  Once the feature works it's way back to 
mainstream kernel I'll add the tests to our automated regressions too. Thanks 
for all your help getting this whole thing through review!

How often does the fedora-selinux project switch the base refpolicy? It needs 
additions to the unconfined user role to allow access.

Re: [PATCH v2 2/2] selinux-testsuite: Infiniband endport tests

[Daniel Jurgens]

On 6/9/2017 9:50 AM, Paul Moore wrote:
> On Fri, Jun 9, 2017 at 10:44 AM, Daniel Jurgens <dani...@mellanox.com> wrote:
>> On 6/5/2017 5:34 PM, Daniel Jurgens wrote:
>>> On 6/5/2017 5:13 PM, Paul Moore wrote:
>>>> On Tue, May 30, 2017 at 1:52 PM, Stephen Smalley <s...@tycho.nsa.gov> 
>>>> wrote:
>>>>> On Tue, 2017-05-30 at 17:40 +, Daniel Jurgens wrote:
>>>>>> On 5/30/2017 12:05 PM, Stephen Smalley wrote:
>>>>>>> On Tue, 2017-05-30 at 19:34 +0300, Dan Jurgens wrote:
>>>>>>>> From: Daniel Jurgens <dani...@mellanox.com>
>>>>>>>>
>>>>>>>> New tests for Infiniband endports. Most users do not have
>>>>>>>> infiniband
>>>>>>>> hardware, and if they do the device names can vary.  There is a
>>>>>>>> configuration file for enabling the tests and setting environment
>>>>>>>> specific configurations.  If the tests are disabled they always
>>>>>>>> show
>>>>>>>> as
>>>>>>>> passed.
>>>>>>>>
>>>>>>>> A special test application was unnecessary, a standard diagnostic
>>>>>>>> application is used instead.  This required a change to the make
>>>>>>>> file
>>>>>>>> to avoid trying to build an application in the new subdir.
>>>>>>>>
>>>>>>>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>>>> ...
>>>>
>>>>> I wouldn't bother re-spinning unless Paul has other comments.
>>>> Nothing worthy of a respin.
>>>>
>>>> Daniel, have you run these tests against the kernel, userspace, and
>>>> policy code that has been merged?  It would be nice to have a sanity
>>>> check that something didn't break while we were merging everything.
>>>>
>>>> [SIDE NOTE: This afternoon I noticed what I think may be a problem
>>>> with my COPR kernel builds that affects the test suite, so YMMY at the
>>>> moment.]
>>>>
>>> I ran them against the merged kernel and selinux code.  But I used the same 
>>> policy RPMs that I had been using, I didn't try to rebuild the RPMs against 
>>> the new refpolicy.
>>>
>> Are these tests good to go? I haven't gotten any additional comments since 
>> v2.
> Yes, my apologies for not getting back to you sooner; I had hoped to
> talk to some of the IB folks at Red Hat to see if they could verify
> everything (or at least get access to a IB system so I could verify
> it) but I got wrapped in a few audit issues this week and didn't get
> to it.
>
> I'll merge these patches later this afternoon.
>
No problem, just wanted to make sure I wasn't holding it up in anyway.

I recall you saying you do most of your testing in VMs on a laptop.  But if you 
have a system with a free pci-e slot we can ship you an HCA if you'd like to be 
able to run these yourself.

Re: [PATCH v2 2/2] selinux-testsuite: Infiniband endport tests

[Daniel Jurgens]

On 6/5/2017 5:34 PM, Daniel Jurgens wrote:
> On 6/5/2017 5:13 PM, Paul Moore wrote:
>> On Tue, May 30, 2017 at 1:52 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
>>> On Tue, 2017-05-30 at 17:40 +, Daniel Jurgens wrote:
>>>> On 5/30/2017 12:05 PM, Stephen Smalley wrote:
>>>>> On Tue, 2017-05-30 at 19:34 +0300, Dan Jurgens wrote:
>>>>>> From: Daniel Jurgens <dani...@mellanox.com>
>>>>>>
>>>>>> New tests for Infiniband endports. Most users do not have
>>>>>> infiniband
>>>>>> hardware, and if they do the device names can vary.  There is a
>>>>>> configuration file for enabling the tests and setting environment
>>>>>> specific configurations.  If the tests are disabled they always
>>>>>> show
>>>>>> as
>>>>>> passed.
>>>>>>
>>>>>> A special test application was unnecessary, a standard diagnostic
>>>>>> application is used instead.  This required a change to the make
>>>>>> file
>>>>>> to avoid trying to build an application in the new subdir.
>>>>>>
>>>>>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>> ...
>>
>>> I wouldn't bother re-spinning unless Paul has other comments.
>> Nothing worthy of a respin.
>>
>> Daniel, have you run these tests against the kernel, userspace, and
>> policy code that has been merged?  It would be nice to have a sanity
>> check that something didn't break while we were merging everything.
>>
>> [SIDE NOTE: This afternoon I noticed what I think may be a problem
>> with my COPR kernel builds that affects the test suite, so YMMY at the
>> moment.]
>>
> I ran them against the merged kernel and selinux code.  But I used the same 
> policy RPMs that I had been using, I didn't try to rebuild the RPMs against 
> the new refpolicy.
>
Are these tests good to go? I haven't gotten any additional comments since v2.

Re: [PATCH v2 2/2] selinux-testsuite: Infiniband endport tests

[Daniel Jurgens]

On 6/5/2017 5:13 PM, Paul Moore wrote:
> On Tue, May 30, 2017 at 1:52 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
>> On Tue, 2017-05-30 at 17:40 +, Daniel Jurgens wrote:
>>> On 5/30/2017 12:05 PM, Stephen Smalley wrote:
>>>> On Tue, 2017-05-30 at 19:34 +0300, Dan Jurgens wrote:
>>>>> From: Daniel Jurgens <dani...@mellanox.com>
>>>>>
>>>>> New tests for Infiniband endports. Most users do not have
>>>>> infiniband
>>>>> hardware, and if they do the device names can vary.  There is a
>>>>> configuration file for enabling the tests and setting environment
>>>>> specific configurations.  If the tests are disabled they always
>>>>> show
>>>>> as
>>>>> passed.
>>>>>
>>>>> A special test application was unnecessary, a standard diagnostic
>>>>> application is used instead.  This required a change to the make
>>>>> file
>>>>> to avoid trying to build an application in the new subdir.
>>>>>
>>>>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
> ...
>
>> I wouldn't bother re-spinning unless Paul has other comments.
> Nothing worthy of a respin.
>
> Daniel, have you run these tests against the kernel, userspace, and
> policy code that has been merged?  It would be nice to have a sanity
> check that something didn't break while we were merging everything.
>
> [SIDE NOTE: This afternoon I noticed what I think may be a problem
> with my COPR kernel builds that affects the test suite, so YMMY at the
> moment.]
>
I ran them against the merged kernel and selinux code.  But I used the same 
policy RPMs that I had been using, I didn't try to rebuild the RPMs against the 
new refpolicy.

Re: [PATCH v2 2/2] selinux-testsuite: Infiniband endport tests

[Daniel Jurgens]

On 5/30/2017 12:48 PM, Stephen Smalley wrote:
> On Tue, 2017-05-30 at 17:40 +0000, Daniel Jurgens wrote:
>> On 5/30/2017 12:05 PM, Stephen Smalley wrote:
>>> On Tue, 2017-05-30 at 19:34 +0300, Dan Jurgens wrote:
>>>> From: Daniel Jurgens <dani...@mellanox.com>
>>>>
>>>> diff --git a/tests/infiniband_pkey/test
>>>> b/tests/infiniband_pkey/test
>>>> old mode 100644
>>>> new mode 100755
>>> Not a big deal, but it seems odd that this mode change wasn't just
>>> squashed into the first patch.
>>>
>>> Otherwise, it looks ok to me, but I don't have hardware to test it
>>> on.
>>> Did you confirm that when you run the tests, you get the expected
>>> avc
>>> denials in the audit logs?  Also, did you confirm that if you
>>> manually
>>> run the tests in permissive mode, that the tests you expect to fail
>>> do
>>> so (and the rest do not)?
>>>
>>>
>> I'm not sure what happened with the mode there.  I didn't change it
>> manually.  I can clean it up if you want.
> Looks like tests/Makefile does a chmod +x */test.
> I wouldn't bother re-spinning unless Paul has other comments.
>
>> Regarding testing the test. Yes, I did make sure they fail as
>> expected when in permissive mode.  Also I changed setting in the
>> configuration files to make sure all cases fail when they should
>> where that was possible.
> And avc: denied messages are as expected?
>
Yes, here's a sample:

type=AVC msg=audit(1496161222.307:1584): avc:  denied  { manage_subnet } for  
pid=21976 comm="smpquery" device=mlx5_2 port_num=1 
scontext=unconfined_u:unconfined_r:test_ibendport_manage_subnet_t:s0-s0:c0.c1023
 tcontext=system_u:object_r:unlabeled_t:s0 tclass=infiniband_endport 
permissive=0

Re: [PATCH v2 2/2] selinux-testsuite: Infiniband endport tests

[Daniel Jurgens]

On 5/30/2017 12:05 PM, Stephen Smalley wrote:
> On Tue, 2017-05-30 at 19:34 +0300, Dan Jurgens wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> New tests for Infiniband endports. Most users do not have infiniband
>> hardware, and if they do the device names can vary.  There is a
>> configuration file for enabling the tests and setting environment
>> specific configurations.  If the tests are disabled they always show
>> as
>> passed.
>>
>> A special test application was unnecessary, a standard diagnostic
>> application is used instead.  This required a change to the make file
>> to avoid trying to build an application in the new subdir.
>>
>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>>
>> ---
>> v1:
>> - Synchronize interface names with refpolicy changes.
>> - Allowed access to unlabeled pkeys vs default pkey, default pkey is
>> no
>> longer labeled in the refpolicy.
>>
>> v2:
>> Stephen Smalley:
>> - Use a stub makefile instead of a SUBDIRS_NO_MAKE directive.
>> - Use ifdefs around corenet_ib* interfaces.
>> - Only build the test_ibpendport.te file if the infiniband_endport
>> class
>> is available.
>> - use corecmd_bin_entry_type intefrace instead of allow ... bin_t:
>> ---
>>  README   |  7 +++-
>>  policy/Makefile  |  4 +++
>>  policy/test_ibendport.te | 40
>> +++
>>  tests/Makefile   |  2 +-
>>  tests/infiniband_endport/Makefile|  2 ++
>>  tests/infiniband_endport/ibendport_test.conf | 14 
>>  tests/infiniband_endport/test| 49
>> 
>>  tests/infiniband_pkey/test   |  0
>>  8 files changed, 116 insertions(+), 2 deletions(-)
>>  create mode 100644 policy/test_ibendport.te
>>  create mode 100644 tests/infiniband_endport/Makefile
>>  create mode 100644 tests/infiniband_endport/ibendport_test.conf
>>  create mode 100755 tests/infiniband_endport/test
>>  mode change 100644 => 100755 tests/infiniband_pkey/test
>>
>> diff --git a/README b/README
>> index a4c8ebb..de50eb4 100644
>> --- a/README
>> +++ b/README
>> @@ -201,7 +201,12 @@ INFINIBAND TESTS
>>  
>>  Because running Infiniband tests requires specialized hardware you
>> must
>>  set up a configuration file for these tests. The tests are disabled
>> by
>> -default.  See comments in the configuration file for info.
>> +default.  See comments in the configuration file for info. The
>> endport
>> +tests use smpquery, for Fedora it's provided by the infiniband-diags
>> +package.
>>  
>>  Infiniband PKey test conf file:
>>  tests/infiniband_pkey/ibpkey_test.conf
>> +
>> +Infiniband Endport test conf file:
>> +tests/infiniband_endport/ibendport_test.conf
>> diff --git a/policy/Makefile b/policy/Makefile
>> index 46c9fb5..c062009 100644
>> --- a/policy/Makefile
>> +++ b/policy/Makefile
>> @@ -49,6 +49,10 @@ ifeq ($(shell grep -q getrlimit
>> $(POLDEV)/include/support/all_perms.spt && echo
>>  TARGETS += test_prlimit.te
>>  endif
>>  
>> +ifeq ($(shell grep -q infiniband_endport
>> $(POLDEV)/include/support/all_perms.spt && echo true),true)
>> +TARGETS += test_ibendport.te
>> +endif
>> +
>>  ifeq ($(shell grep -q all_file_perms.*map
>> $(POLDEV)/include/support/all_perms.spt && echo true),true)
>>  export M4PARAM = -Dmap_permission_defined
>>  endif
>> diff --git a/policy/test_ibendport.te b/policy/test_ibendport.te
>> new file mode 100644
>> index 000..2a02c57
>> --- /dev/null
>> +++ b/policy/test_ibendport.te
>> @@ -0,0 +1,40 @@
>> +#
>> +#
>> +# Policy for testing Infiniband Pkey access.
>> +#
>> +
>> +gen_require(`
>> +type bin_t;
>> +type infiniband_mgmt_device_t;
>> +')
>> +
>> +attribute ibendportdomain;
>> +
>> +# Domain for process.
>> +type test_ibendport_manage_subnet_t;
>> +domain_type(test_ibendport_manage_subnet_t)
>> +unconfined_runs_test(test_ibendport_manage_subnet_t)
>> +typeattribute test_ibendport_manage_subnet_t testdomain;
>> +typeattribute test_ibendport_manage_subnet_t ibendportdomain;
>> +
>> +type test_ibendport_t;
>> +ifdef(`corenet_ib_endport',`
>> +corenet_ib_endport(test_ibendport_t)
>> +')
>> +
>> +de

Re: [PATCH v1 2/2] selinux-testsuite: Infiniband endport tests

[Daniel Jurgens]

On 5/25/2017 3:04 PM, Stephen Smalley wrote:
> On Wed, 2017-05-24 at 17:18 +0300, Dan Jurgens wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>>
>> +allow test_ibendport_manage_subnet_t bin_t:file entrypoint;
>> +allow test_ibendport_manage_subnet_t bin_t:file execute;
> Just use:
> corecmd_bin_entry_type(test_ibendport_manage_subnet_t)

Done

>
>> +allow test_ibendport_manage_subnet_t
>> infiniband_mgmt_device_t:chr_file { read write open ioctl};
>> +corenet_ib_access_unlabeled_pkeys(test_ibendport_manage_subnet_t)
> This interface needs to be wrapped with an ifdef if this file is not
> excluded when refpolicy lacks the necessary definitions.

Done

>> +
>> +allow test_ibendport_manage_subnet_t
>> test_ibendport_t:infiniband_endport manage_subnet;
> This needs to be conditional on the definition of this class.  You
> could either omit the .te file altogether in the Makefile if not
> defined, as we do for e.g. cap_userns, icmp_socket, etc, or you need to
>  wrap it conditionally as we do for e.g. map permission.

Excluded building the .te file if the class is not defined.

>
>> +@SUBDIRS="$(SUBDIRS) $(SUBDIRS_NO_MAKE)"
>> PATH=/usr/bin:/bin:/usr/sbin:/sbin ./runtests.pl
> This works, but elsewhere we've always just put a trivial Makefile with
> empty all: and clean: targets in it, e.g. entrypoint/Makefile.  No big
> deal either way.

Switched to a stub makefile.

Re: [PATCH v1 1/2] selinux-testsuite: Infiniband pkey tests

[Daniel Jurgens]

On 5/25/2017 2:52 PM, Stephen Smalley wrote:
> On Wed, 2017-05-24 at 17:18 +0300, Dan Jurgens wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> +corenet_ib_pkey(test_ibpkey_t)
>> +corenet_ib_access_unlabeled_pkeys(test_ibpkey_access_t)
> This will break the build on current Fedora and RHEL.
> So, you can either conditionalize inclusion of test_ibpkey.te in the
> Makefile (see other examples already there) so that it is omitted
> entirely if refpolicy lacks the requisite support, or wrap these
> interface calls with suitable ifdefs, e.g.
> ifdef(`corenet_ib_pkey', `
> corenet_ib_pkey(test_ibpkey_t)
> ')
> ...
>
> Probably the latter is best so that we get at least some build testing.

Done, with ifdefs.

>> +
>>
>> +TARGETS=create_modify_qp
>> +
>> +LDLIBS+= -libverbs
> This is a new build dependency (libibverbs-devel), which should be
> listed in the README.

Done

>
>> +int init_ib_rsrc(char* deviceName)
>> +{
>> +int ndev = 0;
>> +struct ibv_device  **dlist = ibv_get_device_list();
>> +struct ibv_device  *device = NULL;;
>> +struct ibv_srq_init_attr srqiattr;
>> +struct ibv_qp_init_attr qpiattr;
>> +int i;
>> +
>> +if (!ndev)
>> +{
> Can you run these test programs through
> astyle --options=none --lineend=linux --mode=c --style=linux --
> indent=force-tab=8 --indent-preprocessor --indent-col1-comments --min-
> conditional-indent=0 --max-instatement-indent=80 --pad-oper --align-
> pointer=name --align-reference=name --max-code-length=80 --break-after-
> logical

Done.

Re: [pcmoore-selinux:next 17/17] security/selinux/ibpkey.c:116:24: sparse: incompatible types in comparison expression (different address spaces)

[Daniel Jurgens]

On 5/22/2017 4:06 PM, Paul Moore wrote:
> On Sun, May 21, 2017 at 5:47 AM, kbuild test robot
>  wrote:
>> tree:   git://git.infradead.org/users/pcmoore/selinux next
>> head:   b76dd295790d44ecb04932110309bb6c15f263a8
>> commit: b76dd295790d44ecb04932110309bb6c15f263a8 [17/17] selinux: Add a 
>> cache for quicker retreival of PKey SIDs
>> reproduce:
>> # apt-get install sparse
>> git checkout b76dd295790d44ecb04932110309bb6c15f263a8
>> make ARCH=x86_64 allmodconfig
>> make C=1 CF=-D__CHECK_ENDIAN__
>>
>>
>> sparse warnings: (new ones prefixed by >>)
>>
>>include/linux/compiler.h:264:8: sparse: attribute 'no_sanitize_address': 
>> unknown attribute
 security/selinux/ibpkey.c:116:24: sparse: incompatible types in comparison 
 expression (different address spaces)
> I haven't looked at this too closely, but this may not be significant
> ... Daniel, can you look into this please?

I looked at this before.  I don't believe it's significant.  The same warning 
pops up in security/selinux/netport.c:125.

>
>> vim +116 security/selinux/ibpkey.c
>>
>>100   * Description:
>>101   * Add a new pkey record to the hash table.
>>102   *
>>103   */
>>104  static void sel_ib_pkey_insert(struct sel_ib_pkey *pkey)
>>105  {
>>106  unsigned int idx;
>>107
>>108  /* we need to impose a limit on the growth of the hash table 
>> so check
>>109   * this bucket to make sure it is within the specified bounds
>>110   */
>>111  idx = sel_ib_pkey_hashfn(pkey->psec.pkey);
>>112  list_add_rcu(>list, _ib_pkey_hash[idx].list);
>>113  if (sel_ib_pkey_hash[idx].size == SEL_PKEY_HASH_BKT_LIMIT) {
>>114  struct sel_ib_pkey *tail;
>>115
>>  > 116  tail = list_entry(
>>117  rcu_dereference_protected(
>>118  sel_ib_pkey_hash[idx].list.prev,
>>119  lockdep_is_held(_ib_pkey_lock)),
>>120  struct sel_ib_pkey, list);
>>121  list_del_rcu(>list);
>>122  kfree_rcu(tail, rcu);
>>123  } else {
>>124  sel_ib_pkey_hash[idx].size++;
>>
>> ---
>> 0-DAY kernel test infrastructureOpen Source Technology Center
>> https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH v7 2/9] IB/core: Enforce PKey security on QPs

[Daniel Jurgens]

On 5/21/2017 7:13 PM, James Morris wrote:
> On Fri, 19 May 2017, Dan Jurgens wrote:
>
>> security/security.c  | 385 ++
> This looks wrong -- merge problem?

Yes, it was a merge problem.  I added back the per field initialization of the 
security head hooks.  Paul stripped this (and the same problem in patch 0004) 
himself before merging.

>
>
>> +if (pps->alt.state != IB_PORT_PKEY_NOT_VALID) {
>> +get_pkey_and_subnet_prefix(>alt,
>> +   ,
>> +   _prefix);
>> +
>> +ret = enforce_qp_pkey_security(pkey,
>> +   subnet_prefix,
>> +   sec);
>> +}
>> +
>> +if (ret)
>> +goto out;
> The above if/goto is unnecessary.

Right.

Paul, do you want me to fix this with a new patch?

>
>> +
>> +out:
>> +return ret;
>> +}
>
>

Re: [PATCH v7 4/9] IB/core: Enforce security on management datagrams

[Daniel Jurgens]

On 5/19/2017 2:21 PM, Paul Moore wrote:
> On Fri, May 19, 2017 at 8:48 AM, Dan Jurgens <dani...@mellanox.com> wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> Allocate and free a security context when creating and destroying a MAD
>> agent.  This context is used for controlling access to PKeys and sending
>> and receiving SMPs.
>>
>> When sending or receiving a MAD check that the agent has permission to
>> access the PKey for the Subnet Prefix of the port.
>>
>> During MAD and snoop agent registration for SMI QPs check that the
>> calling process has permission to access the manage the subnet  and
>> register a callback with the LSM to be notified of policy changes. When
>> notificaiton of a policy change occurs recheck permission and set a flag
>> indicating sending and receiving SMPs is allowed.
>>
>> When sending and receiving MADs check that the agent has access to the
>> SMI if it's on an SMI QP.  Because security policy can change it's
>> possible permission was allowed when creating the agent, but no longer
>> is.
>>
>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>>
>> ---
>> v2:
>> - Squashed LSM hook additions. Paul Moore
>> - Changed security blobs to void*. Paul Moore
>> - Shorten end_port to port. Paul Moore
>> - Change "smp" to "manage_subnet". Paul Moore
>> - Use the LSM policy change notification and a flag to track permission
>>   instead of calling the LSM hook for every SMP. Dan Jurgens
>> - Squashed PKey and SMP enforcement into the same patch and moved the
>>   logic into security.c. Dan Jurgens
>>
>> v3:
>> - ib_port -> ib_endport. Paul Moore
>> - Use notifier chains for LSM notification. Paul Moore
>> - Reorder LSM hook parameters to put sec first. Paul Moore
>>
>>  drivers/infiniband/core/core_priv.h | 35 ++
>>  drivers/infiniband/core/mad.c   | 52 +
>>  drivers/infiniband/core/security.c  | 92 
>> +
>>  include/linux/lsm_hooks.h   |  8 
>>  include/linux/security.h|  6 +++
>>  include/rdma/ib_mad.h   |  4 ++
>>  security/security.c |  8 
>>  7 files changed, 197 insertions(+), 8 deletions(-)
> ...
>
>> diff --git a/security/security.c b/security/security.c
>> index 6eef315..b69fe6f 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -1540,6 +1540,12 @@ int security_ib_pkey_access(void *sec, u64 
>> subnet_prefix, u16 pkey)
>>  }
>>  EXPORT_SYMBOL(security_ib_pkey_access);
>>
>> +int security_ib_endport_manage_subnet(void *sec, const char *dev_name, u8 
>> port_num)
>> +{
>> +   return call_int_hook(ib_endport_manage_subnet, 0, sec, dev_name, 
>> port_num);
>> +}
>> +EXPORT_SYMBOL(security_ib_endport_manage_subnet);
>> +
>>  int security_ib_alloc_security(void **sec)
>>  {
>> return call_int_hook(ib_alloc_security, 0, sec);
>> @@ -2014,6 +2020,8 @@ struct security_hook_heads security_hook_heads 
>> __lsm_ro_after_init = {
>>
>>  #ifdef CONFIG_SECURITY_INFINIBAND
>> .ib_pkey_access = LIST_HEAD_INIT(security_hook_heads.ib_pkey_access),
>> +   .ib_endport_manage_subnet =
>> +   LIST_HEAD_INIT(security_hook_heads.ib_endport_manage_subnet),
>> .ib_alloc_security =
>> LIST_HEAD_INIT(security_hook_heads.ib_alloc_security),
>> .ib_free_security =
> The same problem as 2/9 regarding the LSM hook code.  I'll also drop
> this block, but update your patch in case you need to respin.
>
Thanks, updated locally as well in both cases.

Re: [PATCH v7 0/9] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 5/19/2017 7:49 AM, Dan Jurgens wrote:
> From: Daniel Jurgens <dani...@mellanox.com>
>
> Note on v7, it applies cleanly on Paul Moores' tree.  'git am' fails to
> apply patch 0002* to Dougs' tree, but 'patch' applies it without rejects.
> There's a new file that needs to be added before resolving the git am,
> drivers/infiniband/core/security.c

There's actually a trivial merge conflict in drivers/infiniband/core/uverbs.c 
that doesn't cause patch to create a reject file, in the function create_qp a 
my patch adds a "goto err_destroy;".  In Dougs' tree it needs to be changed to 
"goto err_cb".


>
> Infiniband applications access HW from user-space -- traffic is generated
> directly by HW, bypassing the kernel. Consequently, Infiniband Partitions,
> which are associated directly with HW transport endpoints, are a natural
> choice for enforcing granular mandatory access control for Infiniband. QPs
> may only send or receives packets tagged with the corresponding partition
> key (PKey). The PKey is not a cryptographic key; it's a 16 bit number
> identifying the partition.
>
> Every Infiniband fabric is controlled by a central Subnet Manager (SM).
> The SM provisions the partitions by assigning each port with the
> partitions it can access. In addition, the SM tags each port with a subnet
> prefix, which identifies the subnet. Determining which users are allowed
> to access which partition keys on a given subnet forms an effective policy
> for isolating users on the fabric. Any application that attempts to send
> traffic on a given subnet is automatically subject to the policy,
> regardless of which device and port it uses. SM software configures the
> subnet through a privileged Subnet Management Interface (SMI), which is
> presented by each Infiniband port. Thus, the SMI must also be controlled
> to prevent unauthorized changes to fabric configuration and partitioning. 
>
> To support access control for IB partitions and subnet management,
> security contexts must be provided for two new types of objects - PKeys
> and IB ports.
>
> A PKey label consists of a subnet prefix and a range of PKey values and is
> similar to the labeling mechanism for netports. Each Infiniband port can
> reside on a different subnet. So labeling the PKey values for specific
> subnet prefixes provides the user maximum flexibility, as PKey values may
> be determined independently for different subnets. There is a single
> access vector for PKeys called "access".
>
> An Infiniband port is labeled by device name and port number. There is a
> single access vector for IB ports called "manage_subnet".
>
> Because RDMA allows kernel bypass, enforcement must be done during
> connection setup. Communication over RDMA requires a send and receive
> queue, collectively known as a Queue Pair (QP). A QP must be initialized
> by privileged system calls before it can be used to send or receive data.
> During initialization the user must provide the PKey and port the QP will
> use; at this time access control can be enforced.
>
> Because there is a possibility that the enforcement settings or security
> policy can change, a means of notifying the ib_core module of such changes
> is required. To facilitate this a generic notification callback mechanism
> is added to the LSM. One callback is registered for checking the QP PKey
> associations when the policy changes. Mad agents also register a callback,
> they cache the permission to send and receive SMPs to avoid another per
> packet call to the LSM.
>
> Because frequent accesses to the same PKey's SID is expected a cache is
> implemented which is very similar to the netport cache.
>
> In order to properly enforce security when changes to the PKey table or
> security policy or enforcement occur ib_core must track which QPs are
> using which port, pkey index, and alternate path for every IB device.
> This makes operations that used to be atomic transactional.
>
> When modifying a QP, ib_core must associate it with the PKey index, port,
> and alternate path specified. If the QP was already associated with
> different settings, the QP is added to the new list prior to the
> modification. If the modify succeeds then the old listing is removed. If
> the modify fails the new listing is removed and the old listing remains
> unchanged.
>
> When destroying a QP the ib_qp structure is freed by the decive specific
> driver (i.e. mlx4_ib) if the 'destroy' is successful. This requires storing
> security related information in a separate structure. When a 'destroy'
> request is in process the ib_qp structure is in an undefined state so if
> there are changes to the security policy or PKey table, the security checks
> cannot reset the QP i

Re: [PATCH v1 7/9] semanage: Update semanage to allow runtime labeling of Infiniband Pkeys

[Daniel Jurgens]

On 5/16/2017 2:10 PM, Stephen Smalley wrote:
> On Mon, 2017-05-15 at 23:42 +0300, Dan Jurgens wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> Update libsepol and libsemanage to work with pkey records. Add local
>> storage for new and modified pkey records in pkeys.local. Update
>> semanage
>> to parse the pkey command options to add, modify, and delete pkeys.
>>
>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>>
>> ---
>> v1:
>> Fixed semanage_pkey_exists -> semanage_ibpkey_exists in delete flow
>> in
>> seobject.py
>>
>> Stephen Smalley:
>> - Subnet prefix can't vary in size always 16 bytes, remove size
>> field.
>> - Removed extraneous change in libsepol/VERSION
>> - Removed ifdef DARWIN s6_addr/32 blocks in favor of s6_addr.
>> - Got rid of magic constant for subnet prefix size.
>>
>> Jason Zaman:
>> - Use SETools directly to query types in seobject.py.
>>
>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>> ---
>>  libsemanage/include/semanage/ibpkey_record.h  |  76 +
>>  libsemanage/include/semanage/ibpkeys_local.h  |  36 +++
>>  libsemanage/include/semanage/ibpkeys_policy.h |  28 ++
>>  libsemanage/include/semanage/semanage.h   |   3 +
>>  libsemanage/src/direct_api.c  |  29 +-
>>  libsemanage/src/handle.h  |  36 ++-
>>  libsemanage/src/ibpkey_internal.h |  52 +++
>>  libsemanage/src/ibpkey_record.c   | 185 +++
>>  libsemanage/src/ibpkeys_file.c| 181 +++
>>  libsemanage/src/ibpkeys_local.c   | 178 ++
>>  libsemanage/src/ibpkeys_policy.c  |  52 +++
>>  libsemanage/src/ibpkeys_policydb.c|  62 
>>  libsemanage/src/libsemanage.map   |   1 +
>>  libsemanage/src/policy_components.c   |   5 +-
>>  libsemanage/src/semanage_store.c  |   1 +
>>  libsemanage/src/semanage_store.h  |   1 +
>>  libsemanage/src/semanageswig.i|   3 +
>>  libsemanage/src/semanageswig_python.i |  43 +++
>>  libsemanage/utils/semanage_migrate_store  |   3 +-
>>  libsepol/include/sepol/ibpkey_record.h|  77 +
>>  libsepol/include/sepol/ibpkeys.h  |  44 +++
>>  libsepol/include/sepol/sepol.h|   2 +
>>  libsepol/src/ibpkey_internal.h|  21 ++
>>  libsepol/src/ibpkey_record.c  | 448
>> ++
>>  libsepol/src/ibpkeys.c| 263 +++
>>  python/semanage/semanage  |  60 +++-
>>  python/semanage/seobject.py   | 255 +++
>>  27 files changed, 2129 insertions(+), 16 deletions(-)
>>  create mode 100644 libsemanage/include/semanage/ibpkey_record.h
>>  create mode 100644 libsemanage/include/semanage/ibpkeys_local.h
>>  create mode 100644 libsemanage/include/semanage/ibpkeys_policy.h
>>  create mode 100644 libsemanage/src/ibpkey_internal.h
>>  create mode 100644 libsemanage/src/ibpkey_record.c
>>  create mode 100644 libsemanage/src/ibpkeys_file.c
>>  create mode 100644 libsemanage/src/ibpkeys_local.c
>>  create mode 100644 libsemanage/src/ibpkeys_policy.c
>>  create mode 100644 libsemanage/src/ibpkeys_policydb.c
>>  create mode 100644 libsepol/include/sepol/ibpkey_record.h
>>  create mode 100644 libsepol/include/sepol/ibpkeys.h
>>  create mode 100644 libsepol/src/ibpkey_internal.h
>>  create mode 100644 libsepol/src/ibpkey_record.c
>>  create mode 100644 libsepol/src/ibpkeys.c
>>
>> diff --git a/libsemanage/include/semanage/ibpkey_record.h
>> b/libsemanage/include/semanage/ibpkey_record.h
>> new file mode 100644
>> index 000..d76aaae
>> --- /dev/null
>> +++ b/libsemanage/include/semanage/ibpkey_record.h
>> @@ -0,0 +1,76 @@
>> +/* Copyright (C) 2017 Mellanox Technologies Inc */
>> +
>> +#ifndef _SEMANAGE_IBPKEY_RECORD_H_
>> +#define _SEMANAGE_IBPKEY_RECORD_H_
>> +
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#ifndef _SEMANAGE_IBPKEY_DEFINED_
>> +struct semanage_ibpkey;
>> +struct semanage_ibpkey_key;
>> +typedef struct semanage_ibpkey semanage_ibpkey_t;
>> +typedef struct semanage_ibpkey_key semanage_ibpkey_key_t;
>> +#define _SEMANAGE_IBPKEY_DEFINED_
>> +#endif
>> +
>> +#define INET6_ADDRLEN 16
> We shouldn't expose this in a public header; it's an implementation
> detail.  Likely could/should define it as sizeof(struct in6_addr) to
> ensure consistency?
>
>> +#define INET6_ADDRLEN 16
> Ditto

Changed to sizeof(struct in6_addr) for these.

>> +#ifdef DARWIN
>> +memcpy(_addr[0], subnet_prefix_bytes, 16);
>> +#else
>> +memcpy(_addr32[0], subnet_prefix_bytes, 16);
>> +#endif
> Another case where you can drop #ifdef DARWIN and just use s6_addr.
>
Done

Re: [PATCH v1 5/9] libsepol: Add ibendport ocontext handling

[Daniel Jurgens]

On 5/17/2017 8:53 AM, James Carter wrote:
> On 05/15/2017 04:42 PM, Dan Jurgens wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>>
>> +exit:
>> +if (rc != 0) {
>> +sepol_log_err("Error writing ibendportcon rules to CIL\n");
>> +}
>> +
>> +return rc;
>> +}
>> +
> You need to have the ibendport rules sorted like I mentioned for ibpkey in 
> patch 2.
>
> Jim
Done for both patches.

Re: [PATCH v1 2/9] libsepol: Add ibpkey ocontext handling

[Daniel Jurgens]

On 5/16/2017 1:41 PM, Stephen Smalley wrote:
> On Tue, 2017-05-16 at 14:43 -0400, Stephen Smalley wrote:
>> On Mon, 2017-05-15 at 23:42 +0300, Dan Jurgens wrote:
>>> From: Daniel Jurgens <dani...@mellanox.com>
>>>
>>>
>>> +   case OCON_IBPKEY:
>>> +/* The subnet prefix is in
>>> network
>>> order */
>>> +   for (j = 0; j < 4; j++)
>>> +   buf[j] = c-
>>>> u.ibpkey.subnet_prefix[j];
>> Kernel write code always writes [2] and [3] as 0.
> This btw again raises the question of whether it worth storing them in
> the binary policy at all.
Done, just store the upper 8 bytes in the policy now.

Re: [PATCH v1 2/9] libsepol: Add ibpkey ocontext handling

[Daniel Jurgens]

On 5/16/2017 1:39 PM, Stephen Smalley wrote:
> On Mon, 2017-05-15 at 23:42 +0300, Dan Jurgens wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> Add support for reading, writing, and copying Infinabinda Pkey 
> Infiniband
>
>> ocontext
>> data. Also add support for querying a Pkey sid to checkpolicy.
>>
>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>>
>> ---
>> v1:
>> Stephen Smalley:
>> - Removed domain and type params from sepol_ibpkey_sid.
>> - Removed splen param from sepol_ibpkey_sid, it never varied.
>> - Removed extra XPERMS_IOCTL version from policydb_compat_info.
>> - Confirm that low order bytes of IPv6 addr for subnet prefix is 0's.
>>
>> James Carter:
>> - Added ibpkey handling to kernel_to_cil.c and kernel_to_conf.c
>>
>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>> ---
>>  checkpolicy/checkpolicy.c  | 25 +
>>  libsepol/include/sepol/policydb/services.h |  8 
>>  libsepol/src/expand.c  |  9 +
>>  libsepol/src/kernel_to_cil.c   | 58
>> +
>>  libsepol/src/kernel_to_conf.c  | 59
>> ++
>>  libsepol/src/libsepol.map.in   |  1 +
>>  libsepol/src/module_to_cil.c   | 38 +++
>>  libsepol/src/policydb.c| 37 +++
>>  libsepol/src/services.c| 51
>> ++
>>  libsepol/src/write.c   | 16 
>>  10 files changed, 302 insertions(+)
>>
>> diff --git a/libsepol/include/sepol/policydb/services.h
>> b/libsepol/include/sepol/policydb/services.h
>> index 9162149..459254e 100644
>> --- a/libsepol/include/sepol/policydb/services.h
>> +++ b/libsepol/include/sepol/policydb/services.h
>> @@ -188,6 +188,14 @@ extern int sepol_port_sid(uint16_t domain,
>>uint16_t port, sepol_security_id_t *
>> out_sid);
>>  
>>  /*
>> + * Return the SID of the ibpkey specified by
>> + * `subnet prefix', and `pkey'.
>> + */
>> +extern int sepol_ibpkey_sid(void *subnet_prefix_p,
> Why void *?  Can't this just be struct in6_addr *subnet_prefix or
> uint32_t subnet_prefix[]?  The only reason we use void *addr in
> sepol_node_sid() is because that argument can actually vary depending
> on the domain.  Likely can be const too.

Changed to uint32_t *, since that's what the type is in the ocontext structure. 
 Added const.

>> +n->u.ibpkey.subnet_prefix[2] = c-
>>> u.ibpkey.subnet_prefix[2];
>> +n->u.ibpkey.subnet_prefix[3] = c-
>>> u.ibpkey.subnet_prefix[3];
> [2] and [3] should always be zero.

Done

>
>> +n->u.ibpkey.low_pkey = c-
>>> u.ibpkey.low_pkey;
>> +n->u.ibpkey.high_pkey = c-
>>> u.ibpkey.high_pkey;
>> +break;
>>  case OCON_PORT:
>>  n->u.port.protocol = c-
>>> u.port.protocol;
>>  n->u.port.low_port = c-
>>> u.port.low_port;
>> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
>> index 7093b29..d6e8e6f 100644
>> --- a/libsepol/src/policydb.c
>> +++ b/libsepol/src/policydb.c
>>
>> @@ -2782,6 +2804,21 @@ static int ocontext_read_selinux(struct
>> policydb_compat_info *info,
>>  (>context[1], p, fp))
>>  return -1;
>>  break;
>> +case OCON_IBPKEY:
>> +rc = next_entry(buf, fp,
>> sizeof(uint32_t) * 6);
>> +if (rc < 0 || buf[2] || buf[3])
>> +return -1;
> Kernel code also rejects buf[4] or buf[5] > 0x.

Done

>> @@ -1410,6 +1411,21 @@ static int ocontext_write_selinux(struct
>> policydb_compat_info *info,
>>  if (context_write(p, >context[1], 
>> fp))
>>  return POLICYDB_ERROR;
>>  break;
>> +case OCON_IBPKEY:
>> + /* The subnet prefix is in network
>> order */
>> +for (j = 0; j < 4; j++)
>> +buf[j] = c-
>>> u.ibpkey.subnet_prefix[j];
> Kernel write code always writes [2] and [3] as 0.

Done

Re: [PATCH v1 1/9] checkpolicy: Add support for ibpkeycon labels

[Daniel Jurgens]

On 5/16/2017 1:18 PM, Stephen Smalley wrote:
> On Mon, 2017-05-15 at 23:42 +0300, Dan Jurgens wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> +if (subnet_prefix.s6_addr[2] || subnet_prefix.s6_addr[3]) {
>> +yyerror("subnet prefix should be 0's in the low
>> order 64 bits.");
>> +rc = -1;
>> +goto out;
>> +}
>> +
>> +memcpy(>u.ibpkey.subnet_prefix[0],
>> _prefix.s6_addr[0],
>> +   sizeof(newc->u.ibpkey.subnet_prefix));
>> +
>> +newc->u.ibpkey.low_pkey = low;
>> +newc->u.ibpkey.high_pkey = high;
> Kernel patch also rejects low or high > 0x, so we likely ought to
> do the same here?

Done

Re: [PATCH v1 7/9] semanage: Update semanage to allow runtime labeling of Infiniband Pkeys

[Daniel Jurgens]

On 5/16/2017 2:36 PM, Stephen Smalley wrote:
> On Tue, 2017-05-16 at 19:34 +0000, Daniel Jurgens wrote:
>> On 5/16/2017 2:30 PM, Stephen Smalley wrote:
>>> On Mon, 2017-05-15 at 23:42 +0300, Dan Jurgens wrote:
>>>> From: Daniel Jurgens <dani...@mellanox.com>
>>>>
>>>> Update libsepol and libsemanage to work with pkey records. Add
>>>> local
>>>> storage for new and modified pkey records in pkeys.local. Update
>>>> semanage
>>>> to parse the pkey command options to add, modify, and delete
>>>> pkeys.
>>>>
>>>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>>>>
>>>> ---
>>>> v1:
>>>> Fixed semanage_pkey_exists -> semanage_ibpkey_exists in delete
>>>> flow
>>>> in
>>>> seobject.py
>>>>
>>>> Stephen Smalley:
>>>> - Subnet prefix can't vary in size always 16 bytes, remove size
>>>> field.
>>>> - Removed extraneous change in libsepol/VERSION
>>>> - Removed ifdef DARWIN s6_addr/32 blocks in favor of s6_addr.
>>>> - Got rid of magic constant for subnet prefix size.
>>>>
>>>> Jason Zaman:
>>>> - Use SETools directly to query types in seobject.py.
>>>>
>>>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>>>> ---
>>>>  libsemanage/include/semanage/ibpkey_record.h  |  76 +
>>>>  libsemanage/include/semanage/ibpkeys_local.h  |  36 +++
>>>>  libsemanage/include/semanage/ibpkeys_policy.h |  28 ++
>>>>  libsemanage/include/semanage/semanage.h   |   3 +
>>>>  libsemanage/src/direct_api.c  |  29 +-
>>>>  libsemanage/src/handle.h  |  36 ++-
>>>>  libsemanage/src/ibpkey_internal.h |  52 +++
>>>>  libsemanage/src/ibpkey_record.c   | 185 +++
>>>>  libsemanage/src/ibpkeys_file.c| 181 +++
>>>>  libsemanage/src/ibpkeys_local.c   | 178 ++
>>>>  libsemanage/src/ibpkeys_policy.c  |  52 +++
>>>>  libsemanage/src/ibpkeys_policydb.c|  62 
>>>>  libsemanage/src/libsemanage.map   |   1 +
>>>>  libsemanage/src/policy_components.c   |   5 +-
>>>>  libsemanage/src/semanage_store.c  |   1 +
>>>>  libsemanage/src/semanage_store.h  |   1 +
>>>>  libsemanage/src/semanageswig.i|   3 +
>>>>  libsemanage/src/semanageswig_python.i |  43 +++
>>>>  libsemanage/utils/semanage_migrate_store  |   3 +-
>>>>  libsepol/include/sepol/ibpkey_record.h|  77 +
>>>>  libsepol/include/sepol/ibpkeys.h  |  44 +++
>>>>  libsepol/include/sepol/sepol.h|   2 +
>>>>  libsepol/src/ibpkey_internal.h|  21 ++
>>>>  libsepol/src/ibpkey_record.c  | 448
>>>> ++
>>>>  libsepol/src/ibpkeys.c| 263
>>>> +++
>>>>  python/semanage/semanage  |  60 +++-
>>>>  python/semanage/seobject.py   | 255
>>>> +++
>>>>  27 files changed, 2129 insertions(+), 16 deletions(-)
>>>>  create mode 100644 libsemanage/include/semanage/ibpkey_record.h
>>>>  create mode 100644 libsemanage/include/semanage/ibpkeys_local.h
>>>>  create mode 100644 libsemanage/include/semanage/ibpkeys_policy.h
>>>>  create mode 100644 libsemanage/src/ibpkey_internal.h
>>>>  create mode 100644 libsemanage/src/ibpkey_record.c
>>>>  create mode 100644 libsemanage/src/ibpkeys_file.c
>>>>  create mode 100644 libsemanage/src/ibpkeys_local.c
>>>>  create mode 100644 libsemanage/src/ibpkeys_policy.c
>>>>  create mode 100644 libsemanage/src/ibpkeys_policydb.c
>>>>  create mode 100644 libsepol/include/sepol/ibpkey_record.h
>>>>  create mode 100644 libsepol/include/sepol/ibpkeys.h
>>>>  create mode 100644 libsepol/src/ibpkey_internal.h
>>>>  create mode 100644 libsepol/src/ibpkey_record.c
>>>>  create mode 100644 libsepol/src/ibpkeys.c
>>>>
>>>> diff --git a/python/semanage/seobject.py
>>>> b/python/semanage/seobject.py
>>>> index 7a54373..41b0aca 100644
>>>> --- a/python/semanage/seobject.py
>>>> +++ b/python/semanage/seobject.py
>&g

Re: [PATCH v1 7/9] semanage: Update semanage to allow runtime labeling of Infiniband Pkeys

[Daniel Jurgens]

On 5/16/2017 2:30 PM, Stephen Smalley wrote:
> On Mon, 2017-05-15 at 23:42 +0300, Dan Jurgens wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> Update libsepol and libsemanage to work with pkey records. Add local
>> storage for new and modified pkey records in pkeys.local. Update
>> semanage
>> to parse the pkey command options to add, modify, and delete pkeys.
>>
>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>>
>> ---
>> v1:
>> Fixed semanage_pkey_exists -> semanage_ibpkey_exists in delete flow
>> in
>> seobject.py
>>
>> Stephen Smalley:
>> - Subnet prefix can't vary in size always 16 bytes, remove size
>> field.
>> - Removed extraneous change in libsepol/VERSION
>> - Removed ifdef DARWIN s6_addr/32 blocks in favor of s6_addr.
>> - Got rid of magic constant for subnet prefix size.
>>
>> Jason Zaman:
>> - Use SETools directly to query types in seobject.py.
>>
>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>> ---
>>  libsemanage/include/semanage/ibpkey_record.h  |  76 +
>>  libsemanage/include/semanage/ibpkeys_local.h  |  36 +++
>>  libsemanage/include/semanage/ibpkeys_policy.h |  28 ++
>>  libsemanage/include/semanage/semanage.h   |   3 +
>>  libsemanage/src/direct_api.c  |  29 +-
>>  libsemanage/src/handle.h  |  36 ++-
>>  libsemanage/src/ibpkey_internal.h |  52 +++
>>  libsemanage/src/ibpkey_record.c   | 185 +++
>>  libsemanage/src/ibpkeys_file.c| 181 +++
>>  libsemanage/src/ibpkeys_local.c   | 178 ++
>>  libsemanage/src/ibpkeys_policy.c  |  52 +++
>>  libsemanage/src/ibpkeys_policydb.c|  62 
>>  libsemanage/src/libsemanage.map   |   1 +
>>  libsemanage/src/policy_components.c   |   5 +-
>>  libsemanage/src/semanage_store.c  |   1 +
>>  libsemanage/src/semanage_store.h  |   1 +
>>  libsemanage/src/semanageswig.i|   3 +
>>  libsemanage/src/semanageswig_python.i |  43 +++
>>  libsemanage/utils/semanage_migrate_store  |   3 +-
>>  libsepol/include/sepol/ibpkey_record.h|  77 +
>>  libsepol/include/sepol/ibpkeys.h  |  44 +++
>>  libsepol/include/sepol/sepol.h|   2 +
>>  libsepol/src/ibpkey_internal.h|  21 ++
>>  libsepol/src/ibpkey_record.c  | 448
>> ++
>>  libsepol/src/ibpkeys.c| 263 +++
>>  python/semanage/semanage  |  60 +++-
>>  python/semanage/seobject.py   | 255 +++
>>  27 files changed, 2129 insertions(+), 16 deletions(-)
>>  create mode 100644 libsemanage/include/semanage/ibpkey_record.h
>>  create mode 100644 libsemanage/include/semanage/ibpkeys_local.h
>>  create mode 100644 libsemanage/include/semanage/ibpkeys_policy.h
>>  create mode 100644 libsemanage/src/ibpkey_internal.h
>>  create mode 100644 libsemanage/src/ibpkey_record.c
>>  create mode 100644 libsemanage/src/ibpkeys_file.c
>>  create mode 100644 libsemanage/src/ibpkeys_local.c
>>  create mode 100644 libsemanage/src/ibpkeys_policy.c
>>  create mode 100644 libsemanage/src/ibpkeys_policydb.c
>>  create mode 100644 libsepol/include/sepol/ibpkey_record.h
>>  create mode 100644 libsepol/include/sepol/ibpkeys.h
>>  create mode 100644 libsepol/src/ibpkey_internal.h
>>  create mode 100644 libsepol/src/ibpkey_record.c
>>  create mode 100644 libsepol/src/ibpkeys.c
>>
>> diff --git a/python/semanage/seobject.py
>> b/python/semanage/seobject.py
>> index 7a54373..41b0aca 100644
>> --- a/python/semanage/seobject.py
>> +++ b/python/semanage/seobject.py
>> @@ -32,6 +32,7 @@ import socket
>>  from semanage import *
>>  PROGNAME = "policycoreutils"
>>  import sepolicy
>> +import setools
>>  from IPy import IP
>>  
>>  try:
>> @@ -1309,6 +1310,260 @@ class portRecords(semanageRecords):
>>  rec += ", %s" % p
>>  print(rec)
>>  
>> +class ibpkeyRecords(semanageRecords):
>> +try:
>> +q =
>> setools.TypeQuery(setools.SELinuxPolicy(sepolicy.get_installed_policy
>> ()), attrs=["ibpkey_type"])
>> +valid_types = sorted(str(t) for t in q.results())
>> +except RuntimeError:
>> +valid_types = []
> This causes all semanage commands to fail (without a patched re

Re: [PATCH v1 8/9] semanage: Update semanage to allow runtime labeling of ibendports

[Daniel Jurgens]

On 5/16/2017 11:48 AM, Jason Zaman wrote:
> On Mon, May 15, 2017 at 11:42:40PM +0300, Dan Jurgens wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> Update libsepol and libsemanage to work with ibendport records. Add local
>> storage for new and modified ibendport records in ibendports.local.
>> Update semanage to parse the ibendport command options to add, modify,
>> and delete them.
>>
>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>>
>> ---
>> v1:
>> Jason Zaman:
>> - Use SETools directly to query types in seobject.py
>>
>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>> diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py
>> index 41b0aca..037c9ae 100644
>> --- a/python/semanage/seobject.py
>> +++ b/python/semanage/seobject.py
>> @@ -1565,6 +1565,245 @@ class ibpkeyRecords(semanageRecords):
>>  rec += ", %s" % p
>>  print rec
>>  
>> +class ibendportRecords(semanageRecords):
>> +try:
>> +q = 
>> setools.TypeQuery(setools.SELinuxPolicy(sepolicy.get_installed_policy()), 
>> attrs=["ibendport_type"])
>> +valid_types = sorted(str(t) for t in q.results())
> Super minor nit pick here: set() (or even frozenset()) is probably
> better than sorted() since you appear to only just check membership.
>
> I probably wouldnt bother updating unless there is a v2 for some other
> reason.
>
> And if want it on this or an updated version:
> Acked-by: Jason Zaman <ja...@perfinion.com>
>
> -- Jason
>
Thanks Jason, I'll have to rebase over a patch Stephen submitted the other day, 
so there will be a v2 and I'll address this in it.

Re: [PATCH 2/9] libsepol: Add ibpkey ocontext handling

[Daniel Jurgens]

On 5/11/2017 10:18 AM, James Carter wrote:
> libsepol now has the functionality to write cil or a policy.conf from a 
> kernel 
> policy, so kernel_to_cil.c and kernel_to_conf.c need to be updated as well. 
> Doing that shouldn't be any more complicated than what was done for 
> module_to_c.
>
> Jim
Added.  Thanks for reviewing, completely missed when those files were added.
>
> On 05/09/2017 04:50 PM, Dan Jurgens wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> Add support for reading, writing, and copying Infinabinda Pkey ocontext
>> data. Also add support for querying a Pkey sid to checkpolicy.
>>

Re: [PATCH 5/9] libsepol: Add ibendport ocontext handling

[Daniel Jurgens]

On 5/10/2017 2:05 PM, Stephen Smalley wrote:
> On Tue, 2017-05-09 at 23:50 +0300, Dan Jurgens wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> --- a/libsepol/include/sepol/policydb/services.h
>> +++ b/libsepol/include/sepol/policydb/services.h
>> @@ -199,6 +199,16 @@ extern int sepol_ibpkey_sid(uint16_t domain,
>>sepol_security_id_t *out_sid);
>>  
>>  /*
>> + * Return the SID of the ibendport specified by
>> + * `domain', `type', `dev_name', and `port'.
>> + */
>> +extern int sepol_ibendport_sid(uint16_t domain,
>> +   uint16_t type,
>> +   char *dev_name,
>> +   uint8_t port,
>> +   sepol_security_id_t *out_sid);
> Why (domain, type) arguments?

Same case as the pkey one.  Removed.

>> --- a/libsepol/src/module_to_cil.c
>> +++ b/libsepol/src/module_to_cil.c
>> @@ -2585,6 +2585,7 @@ static int ocontext_selinux_isid_to_cil(struct
>> policydb *pdb, struct ocontext *i
>>  "scmp_packet",
>>  "devnull",
>>  "ibpkey",
>> +"ibendport",
> No new initial SIDs.

Removed


>>
>> @@ -2829,6 +2829,23 @@ static int ocontext_read_selinux(struct
>> policydb_compat_info *info,
>>  (>context[0], p, fp))
>>  return -1;
>>  break;
>> +case OCON_IBENDPORT:
>> +rc = next_entry(buf, fp,
>> sizeof(uint32_t) * 2);
>> +if (rc < 0)
>> +return -1;
>> +len = le32_to_cpu(buf[0]);
> if (zero_or_saturated(len))
>   return -1;

Added, but slightly differently because I'm checking for a smaller max length 
due to the next comment.

>> +int hidden sepol_ibendport_sid(uint16_t domain __attribute__
>> ((unused)),
>> +   uint16_t type __attribute__
>> ((unused)),
>> +   char *dev_name,
>> +   uint8_t port,
>> +   sepol_security_id_t *out_sid)
>> +{
>> +ocontext_t *c;
>> +int rc = 0;
>> +
>> +c = policydb->ocontexts[OCON_IBENDPORT];
>> +while (c) {
>> +if (c->u.ibendport.port == port &&
>> +!strncmp(dev_name, c->u.ibendport.dev_name, 64))
>> +break;
> Do you ensure that dev_name cannot be > 64 bytes in checkpolicy and in
> ocontext_read_selinux()?  And do we really want strncmp() here rather
> than just strcmp()?  What's the advantage?
The maximum size for an Infiniband device name is 64 bytes. But there really 
isn't an advantage for the comparison.  I'll switch it, and enforce a length 
check.

Re: [PATCH 4/9] checkpolicy: Add support for ibendportcon labels

[Daniel Jurgens]

On 5/10/2017 1:56 PM, Stephen Smalley wrote:
> On Tue, 2017-05-09 at 23:50 +0300, Dan Jurgens wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>  } ibendport;
> These were pkey and ib_endport in the kernel patch, and port was
> port_num.  Either way is fine but they probably ought to be consistent.

Yes, I received an internal comment that pkey wouldn't be an especially strange 
name collision in the future.  When I repost the kernel series I'll synchronize 
the names there for consistency.

>> @@ -396,6 +400,7 @@ typedef struct genfs {
>>  #define OCON_FSUSE 5/* fs_use */
>>  #define OCON_NODE6 6/* IPv6 nodes */
>>  #define OCON_IBPKEY 7   /* Infiniband PKEY */
>> +#define OCON_IBENDPORT 8/* Infiniband End Port */
> These were OCON_PKEY and OCON_IB_ENDPORT in the last kernel patches I
> saw.  Ok either way but they probably ought to be consistent.
Same here.

Re: [PATCH 2/9] libsepol: Add ibpkey ocontext handling

[Daniel Jurgens]

On 5/10/2017 1:51 PM, Stephen Smalley wrote:
> On Tue, 2017-05-09 at 23:50 +0300, Dan Jurgens wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> Add support for reading, writing, and copying Infinabinda Pkey 
> s/Infinabinda/Infiniband/
Done
>
>> --- a/libsepol/include/sepol/policydb/services.h
>> +++ b/libsepol/include/sepol/policydb/services.h
>> @@ -188,6 +188,17 @@ extern int sepol_port_sid(uint16_t domain,
>>uint16_t port, sepol_security_id_t *
>> out_sid);
>>  
>>  /*
>> + * Return the SID of the ibpkey specified by
>> + * `domain', `type', `subnet prefix', and `pkey'.
>> + */
> Can you explain why you are passing a (domain,type) pair to this
> interface and why subnet_prefix is not fixed length as it is in
> corresponding kernel interface (security_pkey_sid)?  Will these
> arguments ever be used?  Could the length change in the future?
>
> For that matter, and I guess I should have asked this on the kernel
> patches, why are you storing and passing the subnet prefix as a
> complete IPv6 address?  Is that just for the convenience of being able
> to use inet_pton() and inet_ntop()?  Is this typical for handling of IB
> subnet prefixes?  Seems a bit wasteful.
I modeled it after sepol_port_sid, which has the unused type and domain.  They 
are not needed and I've removed them. The length was also not needed, it is 
always the same size and will never change. 

Regarding using an IPv6 address for the subnet prefix, it is for convenience. 
There is already code to deal with IPv6 addresses, not just inet_pton and 
inet_ntop, but in the CIL code as well.  The subnet prefix is just the top half 
of the IPv6 address.  Using IPv6 address to store it allowed code reuse.  When 
the policy is loaded into the kernel the lower 8 bytes are not stored, subnet 
prefix is stored as a u64, so the space is not permanently wasted.
>>
>> @@ -2583,6 +2584,7 @@ static int ocontext_selinux_isid_to_cil(struct
>> policydb *pdb, struct ocontext *i
>>  "policy",
>>  "scmp_packet",
>>  "devnull",
>> +"ibpkey",
> I thought we dropped the separate initial SID for it?
You're right.  Overlooked this when I changed that during the kernel series 
review.
>>
>> @@ -185,6 +186,21 @@ static struct policydb_compat_info
>> policydb_compat[] = {
>>   .ocon_num = OCON_NODE6 + 1,
>>   .target_platform = SEPOL_TARGET_SELINUX,
>>  },
>> +
>> +{
>> + .type = POLICY_KERN,
>> + .version = POLICYDB_VERSION_XPERMS_IOCTL,
>> + .sym_num = SYM_NUM,
>> + .ocon_num = OCON_NODE6 + 1,
>> + .target_platform = SEPOL_TARGET_SELINUX,
>> +},
> This seems duplicated?

Removed

>> @@ -2782,6 +2812,23 @@ static int ocontext_read_selinux(struct
>> policydb_compat_info *info,
>>  (>context[1], p, fp))
>>  return -1;
>>  break;
>> +case OCON_IBPKEY:
>> +rc = next_entry(buf, fp,
>> sizeof(uint32_t) * 6);
>> +if (rc < 0)
>> +return -1;
>> +
>> +c->u.ibpkey.subnet_prefix[0] =
>> buf[0];
>> +c->u.ibpkey.subnet_prefix[1] =
>> buf[1];
>> +c->u.ibpkey.subnet_prefix[2] =
>> buf[2];
>> +c->u.ibpkey.subnet_prefix[3] =
>> buf[3];
> Why load all the values rather than just confirming that [2] and [3]
> are zero as in the kernel?

Changed to confirm they are 0.

Re: [PATCH 1/9] checkpolicy: Add support for ibpkeycon labels

[Daniel Jurgens]

On 5/10/2017 1:18 PM, Stephen Smalley wrote:
> On Tue, 2017-05-09 at 23:50 +0300, Dan Jurgens wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>>
>> +#ifdef DARWIN
>> +memcpy(>u.ibpkey.subnet_prefix[0],
>> _prefix.s6_addr[0],
>> +   sizeof(newc->u.ibpkey.subnet_prefix));
>> +#else
>> +memcpy(>u.ibpkey.subnet_prefix[0],
>> _prefix.s6_addr32[0],
>> +   sizeof(newc->u.ibpkey.subnet_prefix));
>> +#endif
> We can just always use s6_addr instead of s6_addr32 and drop the
> #ifdef.  Just pushed a commit to fix that elsewhere. Also we switched
> from #ifdef DARWIN to __APPLE__ a while ago, but that won't matter once
> you drop the #ifdef altogether.
OK
>
>> @@ -722,10 +728,11 @@ extern int
>> policydb_set_target_platform(policydb_t *p, int platform);
>>  #define POLICYDB_VERSION_CONSTRAINT_NAMES   29
>>  #define POLICYDB_VERSION_XEN_DEVICETREE 30 /* Xen-
>> specific */
>>  #define POLICYDB_VERSION_XPERMS_IOCTL   30 /* Linux-specific */
>> +#define POLICYDB_VERSION_INFINIBAND 31
> This is Linux-specific too.
I'll add a similar comment.
>
>>  
>>  /* Range of policy versions we understand*/
>>  #define POLICYDB_VERSION_MINPOLICYDB_VERSION_BASE
>> -#define POLICYDB_VERSION_MAXPOLICYDB_VERSION_XPERMS_IOCTL
>> +#define POLICYDB_VERSION_MAXPOLICYDB_VERSION_INFINIBAND
>>  
>>  /* Module versions and specific changes*/
>>  #define MOD_POLICYDB_VERSION_BASE   4
>> @@ -743,10 +750,11 @@ extern int
>> policydb_set_target_platform(policydb_t *p, int platform);
>>  #define MOD_POLICYDB_VERSION_TUNABLE_SEP14
>>  #define MOD_POLICYDB_VERSION_NEW_OBJECT_DEFAULTS15
>>  #define MOD_POLICYDB_VERSION_DEFAULT_TYPE   16
>> -#define MOD_POLICYDB_VERSION_CONSTRAINT_NAMES  17
>> +#define MOD_POLICYDB_VERSION_CONSTRAINT_NAMES   17
>> +#define MOD_POLICYDB_VERSION_INFINIBAND 18
>>  
>>  #define MOD_POLICYDB_VERSION_MIN MOD_POLICYDB_VERSION_BASE
>> -#define MOD_POLICYDB_VERSION_MAX
>> MOD_POLICYDB_VERSION_CONSTRAINT_NAMES
>> +#define MOD_POLICYDB_VERSION_MAX MOD_POLICYDB_VERSION_INFINIBAND
>>  
>>  #define POLICYDB_CONFIG_MLS1
> Hmmm...we never introduced a binary module version for xperms, since
> the only user is presently Android and they don't use binary modules
> and in general we'd like to get rid of binary modules altogether and
> switch entirely to source modules (either .te modules with a te2cil
> converter or cil modules).  But I guess you probably want to support
> this in the interim for convenient usage within existing Fedora/RHEL
> policies.
>
Yes, we want to pull this back into RHEL once it's available upstream.

Thank you for your quick review.  I'll continue going through your comments on 
the other patches and post a v1 after giving some more time for others to 
comment as well.

Re: [PATCH v6 0/9] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 5/3/2017 9:41 AM, Paul Moore wrote:
> On Wed, Nov 23, 2016 at 9:17 AM, Dan Jurgens <dani...@mellanox.com> wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> Infiniband applications access HW from user-space -- traffic is generated
>> directly by HW, bypassing the kernel. Consequently, Infiniband Partitions,
>> which are associated directly with HW transport endpoints, are a natural
>> choice for enforcing granular mandatory access control for Infiniband. QPs 
>> may
>> only send or receives packets tagged with the corresponding partition key
>> (PKey). The PKey is not a cryptographic key; it's a 16 bit number identifying
>> the partition.
>>
>> Every Infiniband fabric is controlled by a central Subnet Manager (SM). The 
>> SM
>> provisions the partitions by assigning each port with the partitions it can
>> access. In addition, the SM tags each port with a subnet prefix, which
>> identifies the subnet. Determining which users are allowed to access which
>> partition keys on a given subnet forms an effective policy for isolating 
>> users
>> on the fabric. Any application that attempts to send traffic on a given 
>> subnet
>> is automatically subject to the policy, regardless of which device and port 
>> it
>> uses. SM software configures the subnet through a privileged Subnet 
>> Management
>> Interface (SMI), which is presented by each Infiniband port. Thus, the SMI 
>> must
>> also be controlled to prevent unauthorized changes to fabric configuration 
>> and
>> partitioning.
>>
>> To support access control for IB partitions and subnet management, security
>> contexts must be provided for two new types of objects - PKeys and IB ports.
>>
>> A PKey label consists of a subnet prefix and a range of PKey values and is
>> similar to the labeling mechanism for netports. Each Infiniband port can 
>> reside
>> on a different subnet. So labeling the PKey values for specific subnet 
>> prefixes
>> provides the user maximum flexibility, as PKey values may be determined
>> independently for different subnets. There is a single access vector for 
>> PKeys
>> called "access".
>>
>> An Infiniband port is labeled by device name and port number. There is a 
>> single
>> access vector for IB ports called "manage_subnet".
>>
>> Because RDMA allows kernel bypass, enforcement must be done during connection
>> setup. Communication over RDMA requires a send and receive queue, 
>> collectively
>> known as a Queue Pair (QP). A QP must be initialized by privileged system 
>> calls
>> before it can be used to send or receive data. During initialization the user
>> must provide the PKey and port the QP will use; at this time access control 
>> can
>> be enforced.
>>
>> Because there is a possibility that the enforcement settings or security
>> policy can change, a means of notifying the ib_core module of such changes
>> is required. To facilitate this a generic notification callback mechanism
>> is added to the LSM. One callback is registered for checking the QP PKey
>> associations when the policy changes. Mad agents also register a callback,
>> they cache the permission to send and receive SMPs to avoid another per
>> packet call to the LSM.
>>
>> Because frequent accesses to the same PKey's SID is expected a cache is
>> implemented which is very similar to the netport cache.
>>
>> In order to properly enforce security when changes to the PKey table or
>> security policy or enforcement occur ib_core must track which QPs are
>> using which port, pkey index, and alternate path for every IB device.
>> This makes operations that used to be atomic transactional.
>>
>> When modifying a QP, ib_core must associate it with the PKey index, port,
>> and alternate path specified. If the QP was already associated with
>> different settings, the QP is added to the new list prior to the
>> modification. If the modify succeeds then the old listing is removed. If
>> the modify fails the new listing is removed and the old listing remains
>> unchanged.
>>
>> When destroying a QP the ib_qp structure is freed by the decive specific
>> driver (i.e. mlx4_ib) if the 'destroy' is successful. This requires storing
>> security related information in a separate structure. When a 'destroy'
>> request is in process the ib_qp structure is in an undefined state so if
>> there are changes to the security policy or PKey table, the security checks
>> cannot reset the QP if it doesn't have permission for the n

Re: [PATCH 1/3] selinux: Implement LSM notification system

[Daniel Jurgens]

On 4/26/2017 10:38 AM, Casey Schaufler wrote:
> On 4/26/2017 8:02 AM, Sebastien Buisson wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> Add a generic notification mechanism in the LSM. Interested consumers
>> can register a callback with the LSM and security modules can produce
>> events.
> Why is this a generic mechanism? Do you ever see anyone
> other than SELinux using it?
I had created an SELinux specific mechanism, Paul Moore requested I make it 
generic.
>> Add a call to the notification mechanism from SELinux when the AVC
>> cache changes.
> This seems like a whole lot of mechanism for
> something you could accomplish with a log message.
> What am I missing?
This was part of a larger patch set that hasn't been accepted yet.  SELinux 
support for Inifiniband.  Subsequent patches in that patch set will use it as 
well.
>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>> Signed-off-by: Sebastien Buisson <sbuis...@ddn.com>
>> ---
>>  include/linux/security.h | 23 +++
>>  security/security.c  | 20 
>>  security/selinux/hooks.c | 12 
>>  3 files changed, 55 insertions(+)
>>
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index af675b5..73a9c93 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -68,6 +68,10 @@
>>  struct user_namespace;
>>  struct timezone;
>>  
>> +enum lsm_event {
>> +LSM_POLICY_CHANGE,
>> +};
>> +
>>  /* These functions are in security/commoncap.c */
>>  extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
>> int cap, int audit);
>> @@ -163,6 +167,10 @@ struct security_mnt_opts {
>>  int num_mnt_opts;
>>  };
>>  
>> +int call_lsm_notifier(enum lsm_event event, void *data);
>> +int register_lsm_notifier(struct notifier_block *nb);
>> +int unregister_lsm_notifier(struct notifier_block *nb);
>> +
>>  static inline void security_init_mnt_opts(struct security_mnt_opts *opts)
>>  {
>>  opts->mnt_opts = NULL;
>> @@ -381,6 +389,21 @@ int security_sem_semop(struct sem_array *sma, struct 
>> sembuf *sops,
>>  struct security_mnt_opts {
>>  };
>>  
>> +static inline int call_lsm_notifier(enum lsm_event event, void *data)
>> +{
>> +return 0;
>> +}
>> +
>> +static inline int register_lsm_notifier(struct notifier_block *nb)
>> +{
>> +return 0;
>> +}
>> +
>> +static inline  int unregister_lsm_notifier(struct notifier_block *nb)
>> +{
>> +return 0;
>> +}
>> +
>>  static inline void security_init_mnt_opts(struct security_mnt_opts *opts)
>>  {
>>  }
>> diff --git a/security/security.c b/security/security.c
>> index b9fea39..ef9d9e1 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -32,6 +32,8 @@
>>  /* Maximum number of letters for an LSM name string */
>>  #define SECURITY_NAME_MAX   10
>>  
>> +static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
>> +
>>  struct security_hook_heads security_hook_heads __lsm_ro_after_init;
>>  char *lsm_names;
>>  /* Boot-time LSM user choice */
>> @@ -146,6 +148,24 @@ void __init security_add_hooks(struct 
>> security_hook_list *hooks, int count,
>>  panic("%s - Cannot get early memory.\n", __func__);
>>  }
>>  
>> +int call_lsm_notifier(enum lsm_event event, void *data)
>> +{
>> +return atomic_notifier_call_chain(_notifier_chain, event, data);
>> +}
>> +EXPORT_SYMBOL(call_lsm_notifier);
>> +
>> +int register_lsm_notifier(struct notifier_block *nb)
>> +{
>> +return atomic_notifier_chain_register(_notifier_chain, nb);
>> +}
>> +EXPORT_SYMBOL(register_lsm_notifier);
>> +
>> +int unregister_lsm_notifier(struct notifier_block *nb)
>> +{
>> +return atomic_notifier_chain_unregister(_notifier_chain, nb);
>> +}
>> +EXPORT_SYMBOL(unregister_lsm_notifier);
>> +
>>  /*
>>   * Hook list operation macros.
>>   *
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index e67a526..a4d36f8 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -171,6 +171,14 @@ static int selinux_netcache_avc_callback(u32 event)
>>  return 0;
>>  }
>>  
>> +static int selinux_lsm_notifier_avc_callback(u32 event)
>> +{
>> +if (event == AVC_CALLBACK_RESET)
>> +call_lsm_notifier(LSM_POLICY_CHANGE, NULL);
>> +
>> +return 0;
>> +}
>> +
>>  /*
>>   * initialise the security for the init task
>>   */
>> @@ -6379,6 +6387,10 @@ static __init int selinux_init(void)
>>  if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET))
>>  panic("SELinux: Unable to register AVC netcache callback\n");
>>  
>> +if (avc_add_callback(selinux_lsm_notifier_avc_callback,
>> + AVC_CALLBACK_RESET))
>> +panic("SELinux: Unable to register AVC LSM notifier 
>> callback\n");
>> +
>>  if (selinux_enforcing)
>>  printk(KERN_DEBUG "SELinux:  Starting in enforcing mode\n");
>>  else
>

Re: [PATCH v6 0/9] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 1/24/2017 3:45 PM, Doug Ledford wrote:
> On Tue, 2017-01-24 at 16:40 -0500, Doug Ledford wrote:
>> On Tue, 2016-12-13 at 17:17 -0500, Paul Moore wrote:
>>> On Tue, Dec 13, 2016 at 11:25 AM, Daniel Jurgens <danielj@mellanox.
>>> co
>>> m> wrote:
>>>>
>>>> On 12/13/2016 9:01 AM, Stephen Smalley wrote:
>>>>>
>>>>> For the LSM/SELinux bits,
>>>>> Acked-by: Stephen Smalley <s...@tycho.nsa.gov>
>>>>>
>>>>> Note that there will be a merge conflict on classmap.h due to
>>>>> commits in
>>>>> the selinux next branch, but that should be easy to resolve.
>>>>>
>>>>> We'll need the patches for the selinux userspace and refpolicy.
>>>> Thanks Stephen, I need to rebase the user space and do some patch
>>>> breakup.  I'll start on that soon.
>>> Sorry, I haven't had a chance to look at v6, but considering all
>>> our
>>> discussions on the previous versions I don't expect any issues from
>>> me.  I was hoping for some more generic hooks/controls, but that
>>> doesn't look to be possible given the nature of RDMA.  I also want
>>> to
>>> mention again the need for tests; we've talked about this in the
>>> past
>>> and while it isn't possible to run the tests without IB hardware, I
>>> would like to see us merge tests into the selinux-testsuite so that
>>> those who do have the required h/w available could run the tests.
>>>
>>> Assuming we can sort out the SELinux userspace and and tests by the
>>> end of January, I see no reason why this couldn't go in for v4.11.
>> Daniel, can you work with people on the userspace and tests?  I'll
>> pull
>> this into a branch (I assume by Paul's and Stephen's comments that
>> they
>> expect it to go through my tree) ready to go, but hold actually
>> submitting it in the merge window until I've heard more from you all
>> that userspace is ready.
> Although it would also be helpful if you could rebase on a current tree
> (preferably my k.o/for-4.11 branch).
>
Doug, I'm working on the user space patches, hope to be ready to submit in the 
next 1-2 weeks.  Do you want me to rebase now or wait until those are submitted?


___
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: [PATCH v6 0/9] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 12/13/2016 9:01 AM, Stephen Smalley wrote:
> On 11/23/2016 09:17 AM, Dan Jurgens wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> Infiniband applications access HW from user-space -- traffic is generated
>> directly by HW, bypassing the kernel. Consequently, Infiniband Partitions,
>> which are associated directly with HW transport endpoints, are a natural
>> choice for enforcing granular mandatory access control for Infiniband. QPs 
>> may
>> only send or receives packets tagged with the corresponding partition key
>> (PKey). The PKey is not a cryptographic key; it's a 16 bit number identifying
>> the partition.
>>
>> Every Infiniband fabric is controlled by a central Subnet Manager (SM). The 
>> SM
>> provisions the partitions by assigning each port with the partitions it can
>> access. In addition, the SM tags each port with a subnet prefix, which
>> identifies the subnet. Determining which users are allowed to access which
>> partition keys on a given subnet forms an effective policy for isolating 
>> users
>> on the fabric. Any application that attempts to send traffic on a given 
>> subnet
>> is automatically subject to the policy, regardless of which device and port 
>> it
>> uses. SM software configures the subnet through a privileged Subnet 
>> Management
>> Interface (SMI), which is presented by each Infiniband port. Thus, the SMI 
>> must
>> also be controlled to prevent unauthorized changes to fabric configuration 
>> and
>> partitioning. 
>>
>> To support access control for IB partitions and subnet management, security
>> contexts must be provided for two new types of objects - PKeys and IB ports.
>>
>> A PKey label consists of a subnet prefix and a range of PKey values and is
>> similar to the labeling mechanism for netports. Each Infiniband port can 
>> reside
>> on a different subnet. So labeling the PKey values for specific subnet 
>> prefixes
>> provides the user maximum flexibility, as PKey values may be determined
>> independently for different subnets. There is a single access vector for 
>> PKeys
>> called "access".
>>
>> An Infiniband port is labeled by device name and port number. There is a 
>> single
>> access vector for IB ports called "manage_subnet".
>>
>> Because RDMA allows kernel bypass, enforcement must be done during connection
>> setup. Communication over RDMA requires a send and receive queue, 
>> collectively
>> known as a Queue Pair (QP). A QP must be initialized by privileged system 
>> calls
>> before it can be used to send or receive data. During initialization the user
>> must provide the PKey and port the QP will use; at this time access control 
>> can
>> be enforced.
>>
>> Because there is a possibility that the enforcement settings or security
>> policy can change, a means of notifying the ib_core module of such changes
>> is required. To facilitate this a generic notification callback mechanism
>> is added to the LSM. One callback is registered for checking the QP PKey
>> associations when the policy changes. Mad agents also register a callback,
>> they cache the permission to send and receive SMPs to avoid another per
>> packet call to the LSM.
>>
>> Because frequent accesses to the same PKey's SID is expected a cache is
>> implemented which is very similar to the netport cache.
>>
>> In order to properly enforce security when changes to the PKey table or
>> security policy or enforcement occur ib_core must track which QPs are
>> using which port, pkey index, and alternate path for every IB device.
>> This makes operations that used to be atomic transactional.
>>
>> When modifying a QP, ib_core must associate it with the PKey index, port,
>> and alternate path specified. If the QP was already associated with
>> different settings, the QP is added to the new list prior to the
>> modification. If the modify succeeds then the old listing is removed. If
>> the modify fails the new listing is removed and the old listing remains
>> unchanged.
>>
>> When destroying a QP the ib_qp structure is freed by the decive specific
>> driver (i.e. mlx4_ib) if the 'destroy' is successful. This requires storing
>> security related information in a separate structure. When a 'destroy'
>> request is in process the ib_qp structure is in an undefined state so if
>> there are changes to the security policy or PKey table, the security checks
>> cannot reset the QP if it doesn't have permission for the new setting. If
>> the 'de

Re: [PATCH v6 5/9] selinux: Create policydb version for Infiniband support

[Daniel Jurgens]

On 12/13/2016 8:35 AM, Stephen Smalley wrote:
> On 11/23/2016 09:17 AM, Dan Jurgens wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> Support for Infiniband requires the addition of two new object contexts,
>> one for infiniband PKeys and another IB Ports. Added handlers to read
>> and write the new ocontext types when reading or writing a binary policy
>> representation.
>>
>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>> Reviewed-by: Eli Cohen <e...@mellanox.com>
> I assume you have libsepol/checkpolicy patches for this as well?
>
Yes, I plan to submit them once the kernel changes are accepted.

___
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: [PATCH v6 3/9] selinux lsm IB/core: Implement LSM notification system

[Daniel Jurgens]

On 12/13/2016 8:26 AM, Stephen Smalley wrote:
> On 11/23/2016 09:17 AM, Dan Jurgens wrote:
>> @@ -177,6 +177,8 @@ static ssize_t sel_write_enforce(struct file *file, 
>> const char __user *buf,
>>  avc_ss_reset(0);
>>  selnl_notify_setenforce(selinux_enforcing);
>>  selinux_status_update_setenforce(selinux_enforcing);
>> +if (!selinux_enforcing)
>> +call_lsm_notifier(LSM_POLICY_CHANGE, NULL);
> Why do you need this notification?  When switching from permissive to
> enforcing, you need (and already get) a notification since you may need
> to revoke previously granted permissions.  But what action do you need
> to take on switching to permissive?
MAD (management datagram) Agents cache if they are allowed to send and receive 
subnet management protocol (SMP) datagrams.  Without this notification they 
will still drop all SMPs in permissive mode if they weren't allowed in 
enforcing mode.  This is handled in [PATCH v6 4/9] IB/core: Enforce security on 
management datagrams.


___
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: [PATCH v5 2/9] IB/core: Enforce PKey security on QPs

[Daniel Jurgens]

On 11/22/2016 5:24 PM, James Morris wrote:
> On Tue, 22 Nov 2016, Dan Jurgens wrote:
>
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> Add new LSM hooks to allocate and free security contexts and check for
>> permission to access a PKey.
> I guess Doug's is best tree for these patches?
Maybe? In earlier versions I had kept the LSM, RDMA, and SELinux changes in 
separate patches Paul Moore thought it was best to squash them together 
functionally. Once everyone agrees they can merged they could all go to one 
tree.  They apply cleanly to Paul and Doug's trees for sure.
>
>> ---
>> v2:
>> - Squashed LSM hook additions. Paul Moore
>> - Changed security blobs to void*. Paul Moore
>>
>> v3:
>> - Change parameter order of pkey_access hook. Paul Moore
>> ---
>>  drivers/infiniband/core/Makefile |   3 +-
>>  drivers/infiniband/core/cache.c  |  21 +-
>>  drivers/infiniband/core/core_priv.h  |  77 +
>>  drivers/infiniband/core/device.c |  33 ++
>>  drivers/infiniband/core/security.c   | 617 
>> +++
>>  drivers/infiniband/core/uverbs_cmd.c |  20 +-
>>  drivers/infiniband/core/verbs.c  |  27 +-
>>  include/linux/lsm_hooks.h|  27 ++
>>  include/linux/security.h |  21 ++
>>  include/rdma/ib_verbs.h  |  48 +++
>>  security/Kconfig |   9 +
>>  security/security.c  |  31 ++
>>  12 files changed, 925 insertions(+), 9 deletions(-)
>>  create mode 100644 drivers/infiniband/core/security.c
>
> Acked-by: James Morris <james.l.mor...@oracle.com>
>
>


___
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: [PATCH v5 9/9] selinux: Add a cache for quicker retreival of PKey SIDs

[Daniel Jurgens]

On 11/22/2016 4:53 PM, James Morris wrote:
> On Tue, 22 Nov 2016, Dan Jurgens wrote:
>
>> +static int sel_pkey_sid_slow(u64 subnet_prefix, u16 pkey_num, u32 *sid)
>> +{
>> +int ret = -ENOMEM;
>> +struct sel_pkey *pkey;
>> +struct sel_pkey *new = NULL;
>> +unsigned long flags;
>> +
>> +spin_lock_irqsave(_pkey_lock, flags);
>> +pkey = sel_pkey_find(subnet_prefix, pkey_num);
>> +if (pkey) {
>> +*sid = pkey->psec.sid;
>> +spin_unlock_irqrestore(_pkey_lock, flags);
>> +return 0;
>> +}
>> +
>> +ret = security_pkey_sid(subnet_prefix, pkey_num, sid);
>> +if (ret != 0)
>> +goto out;
>> +
>> +new = kzalloc(sizeof(*new), GFP_ATOMIC);
>> +if (!new)
>> +goto out;
>> +
>> +new->psec.subnet_prefix = subnet_prefix;
>> +new->psec.pkey = pkey_num;
>> +new->psec.sid = *sid;
>> +sel_pkey_insert(new);
>> +
>> +out:
>> +spin_unlock_irqrestore(_pkey_lock, flags);
>> +if (unlikely(ret))
>> +kfree(new);
>> +
>> +return ret;
> The error handling is messed up here.
>
> You'll return 0 on kzalloc failure.
>
> Also, if security_pkey_sid fails, you'll attempt to kfree 'new', and you 
> don't need an unlikely() in a slow path.
>
>
Right.  I'll remove the if/kfree in the out path completely.  There's no way 
ret becomes non-zero after the kzalloc.  If the kzalloc fails I'll still have 
it return 0, the SID retrieved is valid, it just won't be added the cache so 
returning with an error is overkill.

Thank you for your review.


___
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: [PATCH v5 8/9] selinux: Add IB Port SMP access vector

[Daniel Jurgens]

On 11/22/2016 4:47 PM, James Morris wrote:
> On Tue, 22 Nov 2016, Dan Jurgens wrote:
>
>> +*out_sid = c->sid[0];
>> +} else {
>> +*out_sid = SECINITSID_UNLABELED;
>> +}
> Per previous comment about the braces.
The coding style says if one branch requires brackets they should be used on 
all branches:

"This does not apply if only one branch of a conditional statement is a single 
statement; in the latter case use braces in both branches: "


___
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: [PATCH v4 9/9] selinux: Add a cache for quicker retreival of PKey SIDs

[Daniel Jurgens]

On 11/9/2016 1:05 AM, Leon Romanovsky wrote:
> On Tue, Nov 08, 2016 at 11:06:25PM +0200, Dan Jurgens wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> It is likely that the SID for the same PKey will be requested many
>> times. To reduce the time to modify QPs and process MADs use a cache to
>> store PKey SIDs.
>>
>> This code is heavily based on the "netif" and "netport" concept
>> originally developed by James Morris <jmor...@redhat.com> and Paul Moore
>> <p...@paul-moore.com> (see security/selinux/netif.c and
>> security/selinux/netport.c for more information)
>>
>> issue: 736423
>> Change-Id: I176c3079d5d84d06839b4f750100ac47a6081e94
> It doesn't belong to commit message.
>
>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>

Yes, sorry silly oversight on my part.  I will address for all patches in v5.


___
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: [PATCH v4 3/9] selinux lsm IB/core: Implement LSM notification system

[Daniel Jurgens]

On 11/8/2016 4:36 PM, kbuild test robot wrote:
> Hi Daniel,
>
> [auto build test ERROR on rdma/master]
> [also build test ERROR on v4.9-rc4]
> [cannot apply to next-20161108]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Dan-Jurgens/SELinux-support-for-Infiniband-RDMA/20161109-053432
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/dledford/rdma.git 
> master
> config: i386-randconfig-s1-201645 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386 
>
> All errors (new ones prefixed by >>):
>
>>> ERROR: "unregister_lsm_notifier" [drivers/infiniband/core/ib_core.ko] 
>>> undefined!
>>> ERROR: "register_lsm_notifier" [drivers/infiniband/core/ib_core.ko] 
>>> undefined!
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation

This link error happens when CONFIG_SECURITY is not set.  I will address it in 
v5 after giving some time for additional comments.


___
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: [PATCH v3 0/9] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 9/20/2016 6:43 PM, Paul Moore wrote:
> On Tue, Sep 6, 2016 at 4:02 PM, Jason Gunthorpe
>  wrote:
>> On Thu, Sep 01, 2016 at 02:06:46PM -0400, Paul Moore wrote:
>>
>>> Jason and/or Daniel, I think it would be helpful if you could explain
>>> both the InifiniBand and IP based approaches for those of us who know
>>> SELinux, but not necessarily the RDMA and InfiniBand portions of this
>>> discussion.  Be verbose and explain it as if we were idiots (I get
>>> called that enough, it must be true).
>> Well, I'm not really familiar with SELinux, I know a little bit about
>> how labels are applied in the netstack, but not that much...
>>
>> The RDMA subsystem supports 4 different networking standards, and they
>> each have their own objects..
> All right, I'm done traveling for a bit and it seems like this
> discussion has settled into a stalemate so let's try to pick things
> back up and sort this out.
>
> Starting we a better RDMA education for me.
>
> So far the discussion has been around providing access controls at the
> transport layer, are there any RDMA entities that are transport
> agnostic that might be better suited for what we are trying to do?  Or
> is it simply that the RDMA layer is tied so tightly to the underlying
> transport that we can't separate the two and have to consider them as
> one?
Welcome back Paul.

I don't think there is a transport agnostic way to provide the kind of control 
I use in this patch set, which is very Infiniband specific.  RoCE uses VLANs 
and they are conceptually similar to subnet partitions, but the means of using 
them is completely different.  To use a different VLAN the user must select a 
GID for that VLAN.  One could provide a means to control RoCE access to VLANs 
by labeling GIDs and controlling them in a similar way to how I do PKeys.  That 
approach doesn't help with Infiniband partitions though, because the same GID 
can be used on multiple partitions.  It's also not very desirable from a policy 
writers perspective because it makes it so a bespoke policy is required per 
node.

Regardless of any other approaches one might like to use to provide access 
control for RDMA non-Infiniband transport I think controlling access to 
Infiniband PKeys is still a desirable feature and I don't see any other way to 
have that.


___
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: [PATCH v3 0/9] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 9/8/2016 1:38 PM, Jason Gunthorpe wrote:
> On Thu, Sep 08, 2016 at 05:47:46PM +, Liran Liss wrote:
>
>> This patch-set enables partition-based isolation for Infiniband networks in 
>> a very intuitive manner, that's it.
>> IB partitions don't have anything to do with VLANs.
> You guys need to do a better job at supporting the whole subsystem
> when you propose new uapi features.
>
> Jason
> --
> 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
>
The uapi of this subsystem isn't changed.


___
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: [PATCH v3 0/9] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 9/8/2016 1:36 PM, Jason Gunthorpe wrote:
> On Thu, Sep 08, 2016 at 04:44:36PM +0000, Daniel Jurgens wrote:
>
>> Net has variety of means of enforcement, one of which is controlling
>> access to ports , which is the most like what
>> I'm doing here.
> No, the analog the tcp/udp,port number is <ib, service_id> 
I should have been clearer here.  From the SELinux perspective this scheme is 
very similar to net ports.
>> It will work like any other SELinux policy.  You label the things
>> you want to control with a type and setup rules about which
>> roles/types can interact with them and how.  I'm sure the default
>> policy from distros will be to not restrict access.  Policy is
>> loaded into the kernel, the disk and filesystem has nothing to do
> Eh? I thought the main utility of selinux was using the labels written
> to the filesystem to constrain access, eg I might label
> /usr/bin/apache in a way that gets the <tcp,80> policy applied to it.
Filesystems can be labeled, but so can other things without a filesystem 
representation.
>> with this aside from it being where the policy is stored before
>> being loaded.  What is this dynamic injector you are talking about?
> The container projects (eg docker) somehow setup selinux on the
> fly for each container. I'm not sure how.
SELinux policy is modular and can be changed or updated while running, I'm not 
very familiar with docker so I'm not sure what they do regarding SELinux.  I'm 
also not sure it's relevant to the issues at hand.
>
>> Assume you have machines on one subnet (0xfe80::) one has a device
>> called mlx5_0, the another mlx4_0 and you want to grant access to
>> system administrators.
> So do this in userspace? Why should the kernel do the translation?
I'm still not clear on what translation you are talking about.  To look up a 
label for something the kernel uses the same attributes the policy writer used 
to create the label.  In this patch set when modify_qp is called there is a 
search of all the labels for pkeys for one that matches subnet prefix for the 
relevant port and the pkey number.


___
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: [PATCH v3 0/9] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 9/7/2016 7:01 PM, ira.weiny wrote:
> On Tue, Sep 06, 2016 at 03:55:48PM -0600, Jason Gunthorpe wrote:
>> On Tue, Sep 06, 2016 at 08:35:56PM +0000, Daniel Jurgens wrote:
>>
>>> I think to control access to a VLAN for RoCE there would have to
>>> labels for GIDs, since that's how you select which VLAN to use.
>> Since people are talking about using GIDs for containers adding a GID
>> constraint for all technologies makes sense to me..
>>
>> But rocev1 (at least mlx4) does not use vlan ids from the GID, the
>> vlan id is set directly in the id, so it still seems to need direct
>> containment. I also see vlan related stuff in the iwarp providers, so
>> they probably have a similar requirement.
>>
>>> required.  RDMA device handle labeling isn't granular enough for
>>> what I'm trying to accomplish.  We want users with different levels
>>> of permission to be able to use the same device, but restrict who
>>> they can communicate with by isolating them to separate partitions.
>> Sure, but maybe you should use the (device handle:pkey/vlan_id) as your
>> labeling tuple not (Subnet Prefix, pkey)
> Would "device handle" here specify the port?
>
> Ira

It would have to include the port, but idea of using a device name for this is 
pretty ugly.  <subnet_prefix,pkey> makes it very easy to write a policy that 
can be deployed widely.  <device,port,pkey/vlan> could require many different 
policies depending on the configuration of each machine.

I've added Liran Liss, he devised the approach that's implemented.  This would 
be a pretty big change, with worse usability so I'd like to get his feedback. 


___
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: [PATCH v3 0/9] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 9/6/2016 3:02 PM, Jason Gunthorpe wrote:
> On Thu, Sep 01, 2016 at 02:06:46PM -0400, Paul Moore wrote:
>
>> Jason and/or Daniel, I think it would be helpful if you could explain
>> both the InifiniBand and IP based approaches for those of us who know
>> SELinux, but not necessarily the RDMA and InfiniBand portions of this
>> discussion.  Be verbose and explain it as if we were idiots (I get
>> called that enough, it must be true).
> Well, I'm not really familiar with SELinux, I know a little bit about
> how labels are applied in the netstack, but not that much...
>
> The RDMA subsystem supports 4 different networking standards, and they
> each have their own objects..
>
> Just focusing on the pkey/vlan ideas. Every packet placed on the
> network has either a pkey or vlan label, the networking switches and
> receivers use these labels to create strong access control.
>
> The labels are not-global, they are isolated to a site, or even a
> single network within a site.
>
> ipoib also uses pkey in the same way netdev does (with these
> patches it looks like a userspace can still access a pkey via ipoib
> even if selinux is restricting access to it).
>
> Daniel's patch also touched on the QP1 and QP0 concepts. Packets can
> be labeled as being for QP0/1 and the recievers process them under the
> assumption they were sent by something with privilege (eg like the low
> port numbers in IP)
>
> So, from my perspective, we shouldn't be talking about doing pkey
> without also addressing vlan. It sounds like Daniel's concern is how to
> identify the number space (eg he is using a GID prefix for IB, which
> won't work on anything else, maybe rdma device handle is a better choice)
>
> Jason
>
I think to control access to a VLAN for RoCE there would have to labels for 
GIDs, since that's how you select which VLAN to use.  It'd be very similar to 
how the pkey labels works, but it doesn't help with Infiniband, so I think the 
pkey labeling scheme is still required.  RDMA device handle labeling isn't 
granular enough for what I'm trying to accomplish.  We want users with 
different levels of permission to be able to use the same device, but restrict 
who they can communicate with by isolating them to separate partitions.


___
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: [PATCH v3 0/9] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 8/30/2016 1:56 PM, Jason Gunthorpe wrote:
>
> Are subsystems usually SELinux enabled in such a piecemeal way?
>
> Are you sure the 'partition' SELinux label should not be more general
> to cover more of the similar RDMA cases?
>
> Jason
>
In order to label something you have to be able to describe something unique 
about an instance of it, like a Subnet Prefix/PKey value pair.  What other 
thing could we label more generally to control access to a partition/VLAN?


___
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: [PATCH v3 0/9] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 8/30/2016 1:46 PM, Jason Gunthorpe wrote:
> On Tue, Aug 30, 2016 at 02:06:53PM +0000, Daniel Jurgens wrote:
>
>> I don't this will be useful, RoCE doesn't have partitions/PKeys
>> because it uses Ethernet as the transport instead of Infiniband.
> The vlan stuff in roce should be just as restricted as the pkey is in
> IB
>
> Jason
>
This patch set introduces a mechanism for controlling access to Infiniband 
partitions.  If someone is interested in writing SELinux tests regarding RoCE 
and VLANs then RXE may very well be useful for them.  It just doesn't apply 
here.


___
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: [PATCH v2 8/9] selinux: Add IB Port SMP access vector

[Daniel Jurgens]

On 7/22/2016 2:26 PM, Paul Moore wrote:
> On Thu, Jul 14, 2016 at 6:56 PM, Dan Jurgens  wrote:
>
>> +   audit_log_format(ab, " port=%u", a->u.ib_port->port);
> Based on our other conversations, I'm guessing that should be " endport=%u"?

I think port is fine there, device name and port number.  Together they are an 
"endport".

___
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: [PATCH v2 5/9] selinux: Create policydb version for Infiniband support

[Daniel Jurgens]

On 7/22/2016 11:47 AM, Jason Gunthorpe wrote:
> On Fri, Jul 22, 2016 at 12:29:25PM -0400, Paul Moore wrote:
>> We had a discussion about this in the last patchset and I think things
>> may have gotten confused.  From what I remember, according to the IB
>> developers the proper term is "end port"; if that is true, we probably
>> should use "end port", but I would prefer if we drop the underscore in
>> most of the places, e.g. "ib_endport" instead of "ib_end_port".
> Yes, the proper term is end port, (contrasted with switch port).
>
> Jason
>
I can add the 'end'.


___
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: [PATCH v2 0/9] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 7/22/2016 10:46 AM, Paul Moore wrote:
> On Thu, Jul 14, 2016 at 6:56 PM, Dan Jurgens <dani...@mellanox.com> wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> The selinux next tree is missing some patches for IB/core.  This series
>> applies cleanly to ib-next, and should apply cleanly to selinux-next once
>> the IB patches are merged.
> Hi Dan,
>
> Thanks for the updated patchset, I'm taking a look at it today.  I do
> have one question, related but independent to your patches: is there a
> way to test the LSM controlled portions of the Infiniband stack
> without any IB hardware (e.g. is there an IB "loopback" device)?  I'm
> asking because we are really trying to make sure we have tests for any
> new code/functionality we add to SELinux, and requiring IB hardware to
> test the IB access controls would make this difficult.
>
> * https://github.com/SELinuxProject/selinux-testsuite
>
> -Paul
>
I found this: https://github.com/nminoru/pib but haven't used it.  I'll try 
playing with it and see if it can serve this purpose.



___
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: [PATCH 04/12] selinux: Allocate and free infiniband security hooks

[Daniel Jurgens]

On 7/1/2016 3:13 PM, Casey Schaufler wrote:
> On 7/1/2016 12:17 PM, Paul Moore wrote:
>> On Fri, Jul 1, 2016 at 2:59 PM, Daniel Jurgens <dani...@mellanox.com> wrote:
>>> On 7/1/2016 1:54 PM, Paul Moore wrote:
>>>> On Thu, Jun 30, 2016 at 5:48 PM, Daniel Jurgens <dani...@mellanox.com> 
>>>> wrote:
>>>>> On 6/30/2016 4:06 PM, Casey Schaufler wrote:
>>>>>> On 6/30/2016 1:42 PM, Paul Moore wrote:
>>>>>>>>  };
>>>>>>>>
>>>>>>>>  /**
>>>>>>>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
>>>>>>>> index 3f6780b..e522acb 100644
>>>>>>>> --- a/include/rdma/ib_verbs.h
>>>>>>>> +++ b/include/rdma/ib_verbs.h
>>>>>>>> @@ -1454,6 +1454,7 @@ struct ib_qp {
>>>>>>>> void   *qp_context;
>>>>>>>> u32 qp_num;
>>>>>>>> enum ib_qp_type qp_type;
>>>>>>>> +   struct ib_qp_security  *qp_sec;
>>>>>>> See my earlier question/comment about just using a void pointer here.
>>>>>> I think that this is in response to my comments to the
>>>>>> effect that I would like to see the LSM infrastructure
>>>>>> using the inode like (inode->i_security) to the xfrm
>>>>>> (void *) approach. I haven't been looking at the IB patches
>>>>>> too carefully to date. It's possible I have not been clear.
>>>>> My understanding at the time was that by using something other than a 
>>>>> void * different security modules could maintain their own opaque blobs 
>>>>> with in and keep the same prototype for the hook.  It's possible I 
>>>>> misunderstood you, but it made sense to me.  I don't know of any plans 
>>>>> for other security modules to support Infiniband, but this leaves the 
>>>>> door open.
>>>> All of what you describe above can still happen with a void pointer;
>>>> in some ways it is even easier with a void pointer.
>>> If multiple security modules register an alloc_security hook for example, 
>>> how would you coordinate between them to allocate the memory?
>> You worry about that in the LSM framework and hide the details behind
>> the void pointer.  For example, you create an array/list of LSM
>> specific blobs and just stash a pointer to the head of the data in the
>> void pointer.
> Don't worry about it at this point. Patches pending.
> If I have to change modules to accommodate the
> infrastructure I'm not afraid to do so.

So I should revert to void* blobs?  I just want to be clear before making the 
change.

___
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: [PATCH 05/12] selinux: Implement Infiniband PKey "Access" access vector

[Daniel Jurgens]

On 7/1/2016 2:26 PM, Paul Moore wrote:
> On Fri, Jul 1, 2016 at 3:16 PM, Daniel Jurgens <dani...@mellanox.com> wrote:
>> On 7/1/2016 1:59 PM, Paul Moore wrote:
>>> On Fri, Jul 1, 2016 at 2:21 PM, Daniel Jurgens <dani...@mellanox.com> wrote:
>>>> On 7/1/2016 11:29 AM, Paul Moore wrote:
>>>>> I wondered about this earlier in the patchset when we were discussing
>>>>> the policy format, and I'm still wondering; perhaps you can help me
>>>>> understand IB a bit better ...
>>>>>
>>>>> From what I gather, the partition key is the IB security boundary, not
>>>>> the subnet, is that true?  If so, why are we including the subnet with
>>>>> the partition key value/label?  I understand the low/high pkey range
>>>>> as a way of simplifying the policy, but I don't quite understand the
>>>>> point of tying the subnet to the partition key label.  Would you ever
>>>>> want to have multiple labels for a single partition key, or should it
>>>>> be a single label for the partition key regardless of the subnet?
>>>>>
>>>> Each subnet can have a different partition configuration and a node can be 
>>>> on multiple subnets.  By specifying the subnet prefix along with the pkey 
>>>> value the user has flexibility to have different policy for different 
>>>> subnets, instead of a global PKey space that would require coordinating 
>>>> the partition configuration across all subnets.
>>> Perhaps a better explanation of partitions and subnets are in order,
>>> especially for those of like me who are new to IB.
>>>
>> A subnet is a set of ports managed by a common subnet manager, which sets up 
>> the partition configuration.
> So there can be multiple partitions inside a subnet and not multiple
> subnets inside a partition?

Yes, a each subnet can have many partitions.  The partitions are contained 
within that subnet, a different subnet can have a partition that uses same PKey 
value, but that's a different partition.  So if we have 2 subnets, fe80:: and 
fe81:: they can each have a partition that uses PKey X but it doesn't mean 
nodes with access to that partition on 0xfe80 can reach nodes on 0xfe81 on that 
partition.

>
>> A partition is a virtual fabric, similar to an VLAN.
> Yeah, I've read that in multiple places and I think that is what I
> find confusing as it doesn't seem to mesh with my understanding of
> what you are intending.
>
>> If there are multiple IB ports each could be connected to a different subnet.
> Ports are just end points, I get that.  That's important, but it isn't
> helping me understand the relationship between subnets and partitions,
> that is where I'm struggling at the moment.

Subnets have one or more partitions.  Partitions belong to one subnet.

>> By including the subnet prefix in the label the subnets can use the same 
>> PKey values and policy can restrict access appropriately.
> This doesn't make any sense to me right now.
>
>> Without that mechanism if one of the subnets had a partition with PKey 1 the 
>> other partition couldn't reuse that PKey if a different security policy is 
>> desired for that subnet.
> 
Going back to the example above assume fe80:: has a partition using PKey 0x8001 
and we grant a user access.  Without the subnet prefix in the label we are 
granting that user the ability to use pkey 0x8001 on any partition available.  
So subnet fe81:: can't use PKey 0x8001 if it doesn't want to grant that same 
user access.  By labeling with the subnet prefix we can grant access to PKey 
0x8001 on subnet fe80:: only, leaving the subnet manager on fe81:: the ability 
to use that same PKey but not grant access to said user.

___
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: [PATCH 05/12] selinux: Implement Infiniband PKey "Access" access vector

[Daniel Jurgens]

On 7/1/2016 1:59 PM, Paul Moore wrote:
> On Fri, Jul 1, 2016 at 2:21 PM, Daniel Jurgens <dani...@mellanox.com> wrote:
>> On 7/1/2016 11:29 AM, Paul Moore wrote:
>>> I wondered about this earlier in the patchset when we were discussing
>>> the policy format, and I'm still wondering; perhaps you can help me
>>> understand IB a bit better ...
>>>
>>> From what I gather, the partition key is the IB security boundary, not
>>> the subnet, is that true?  If so, why are we including the subnet with
>>> the partition key value/label?  I understand the low/high pkey range
>>> as a way of simplifying the policy, but I don't quite understand the
>>> point of tying the subnet to the partition key label.  Would you ever
>>> want to have multiple labels for a single partition key, or should it
>>> be a single label for the partition key regardless of the subnet?
>>>
>> Each subnet can have a different partition configuration and a node can be 
>> on multiple subnets.  By specifying the subnet prefix along with the pkey 
>> value the user has flexibility to have different policy for different 
>> subnets, instead of a global PKey space that would require coordinating the 
>> partition configuration across all subnets.
> Perhaps a better explanation of partitions and subnets are in order,
> especially for those of like me who are new to IB.
>

A subnet is a set of ports managed by a common subnet manager, which sets up 
the partition configuration.  A partition is a virtual fabric, similar to an 
VLAN.  If there are multiple IB ports each could be connected to a different 
subnet.  By including the subnet prefix in the label the subnets can use the 
same PKey values and policy can restrict access appropriately.  Without that 
mechanism if one of the subnets had a partition with PKey 1 the other partition 
couldn't reuse that PKey if a different security policy is desired for that 
subnet.


___
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: [PATCH 04/12] selinux: Allocate and free infiniband security hooks

[Daniel Jurgens]

On 7/1/2016 1:54 PM, Paul Moore wrote:
> On Thu, Jun 30, 2016 at 5:48 PM, Daniel Jurgens <dani...@mellanox.com> wrote:
>> On 6/30/2016 4:06 PM, Casey Schaufler wrote:
>>> On 6/30/2016 1:42 PM, Paul Moore wrote:
>>>>>  };
>>>>>
>>>>>  /**
>>>>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
>>>>> index 3f6780b..e522acb 100644
>>>>> --- a/include/rdma/ib_verbs.h
>>>>> +++ b/include/rdma/ib_verbs.h
>>>>> @@ -1454,6 +1454,7 @@ struct ib_qp {
>>>>> void   *qp_context;
>>>>> u32 qp_num;
>>>>> enum ib_qp_type qp_type;
>>>>> +   struct ib_qp_security  *qp_sec;
>>>> See my earlier question/comment about just using a void pointer here.
>>> I think that this is in response to my comments to the
>>> effect that I would like to see the LSM infrastructure
>>> using the inode like (inode->i_security) to the xfrm
>>> (void *) approach. I haven't been looking at the IB patches
>>> too carefully to date. It's possible I have not been clear.
>> My understanding at the time was that by using something other than a void * 
>> different security modules could maintain their own opaque blobs with in and 
>> keep the same prototype for the hook.  It's possible I misunderstood you, 
>> but it made sense to me.  I don't know of any plans for other security 
>> modules to support Infiniband, but this leaves the door open.
> All of what you describe above can still happen with a void pointer;
> in some ways it is even easier with a void pointer.

If multiple security modules register an alloc_security hook for example, how 
would you coordinate between them to allocate the memory?

___
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: [PATCH 05/12] selinux: Implement Infiniband PKey "Access" access vector

[Daniel Jurgens]

On 7/1/2016 11:29 AM, Paul Moore wrote:
> On Thu, Jun 23, 2016 at 3:52 PM, Dan Jurgens <dani...@mellanox.com> wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> Add a type and access vector for PKeys. Implement the qp_pkey_access
>> and mad_agent_pkey_access hooks to check that the caller has
>> permission to access the PKey on the given subnet prefix.  Add an
>> interface to get the PKey SID. Walk the PKey ocontexts to find an entry
>> for the given subnet prefix and pkey.
>>
>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>> Reviewed-by: Eli Cohen <e...@mellanox.com>
>> ---
>>  include/linux/lsm_audit.h|  7 
>>  security/selinux/hooks.c | 41 
>> 
>>  security/selinux/include/classmap.h  |  2 ++
>>  security/selinux/include/initial_sid_to_string.h |  1 +
>>  security/selinux/include/security.h  |  2 ++
>>  security/selinux/ss/services.c   | 41 
>> 
>>  6 files changed, 94 insertions(+)
>>
>> diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h
>> index ffb9c9d..8ff7eae 100644
>> --- a/include/linux/lsm_audit.h
>> +++ b/include/linux/lsm_audit.h
>> @@ -45,6 +45,11 @@ struct lsm_ioctlop_audit {
>> u16 cmd;
>>  };
>>
>> +struct lsm_pkey_audit {
>> +   u64 subnet_prefix;
>> +   u16 pkey;
>> +};
>> +
>>  /* Auxiliary data to use in generating the audit record. */
>>  struct common_audit_data {
>> char type;
>> @@ -59,6 +64,7 @@ struct common_audit_data {
>>  #define LSM_AUDIT_DATA_INODE   9
>>  #define LSM_AUDIT_DATA_DENTRY  10
>>  #define LSM_AUDIT_DATA_IOCTL_OP11
>> +#define LSM_AUDIT_DATA_PKEY12
>> union   {
>> struct path path;
>> struct dentry *dentry;
>> @@ -75,6 +81,7 @@ struct common_audit_data {
>>  #endif
>> char *kmod_name;
>> struct lsm_ioctlop_audit *op;
>> +   struct lsm_pkey_audit *pkey;
>> } u;
>> /* this union contains LSM specific data */
>> union {
> Please correct me if I'm wrong, but I don't recall seeing any code in
> the patchset which actually logs the extra IB information in the audit
> record, did I miss it?
I didn't make any changes to the audit logging.  The messages look like this.
[245259.895597] audit: type=1400 audit(1467392186.710:631): avc:  denied  { 
access } for  pid=27519 comm="ib_write_bw" scontext=root:sysadm_r:sysadm_t:s0 
tcontext=system_u:object_r:staff_allowed_pkey_t:s0 tclass=infiniband_pkey 
permissive=1

> I wondered about this earlier in the patchset when we were discussing
> the policy format, and I'm still wondering; perhaps you can help me
> understand IB a bit better ...
>
> From what I gather, the partition key is the IB security boundary, not
> the subnet, is that true?  If so, why are we including the subnet with
> the partition key value/label?  I understand the low/high pkey range
> as a way of simplifying the policy, but I don't quite understand the
> point of tying the subnet to the partition key label.  Would you ever
> want to have multiple labels for a single partition key, or should it
> be a single label for the partition key regardless of the subnet?
>
Each subnet can have a different partition configuration and a node can be on 
multiple subnets.  By specifying the subnet prefix along with the pkey value 
the user has flexibility to have different policy for different subnets, 
instead of a global PKey space that would require coordinating the partition 
configuration across all subnets.


___
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: [PATCH 02/12] selinux: Create policydb version for Infiniband support

[Daniel Jurgens]

On 7/1/2016 7:50 AM, Leon Romanovsky wrote:
> On Thu, Jun 30, 2016 at 06:01:42PM +0300, Yuval Shaia wrote:
>> On Thu, Jun 23, 2016 at 10:52:48PM +0300, Dan Jurgens wrote:
>>
>>> if (rc)
>>> return rc;
>>> break;
>>> +   case OCON_PKEY: {
>> Is "{" needed?
> No, I agree, need to remove.
The { is needed here, unless the variable sbn_pfx is defined at function scope.
>
>>> +   __be64 *sbn_pfx = (__be64 *)nodebuf;
>>> +   *sbn_pfx = cpu_to_be64(c->u.pkey.subnet_prefix);


___
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: [PATCH 04/12] selinux: Allocate and free infiniband security hooks

[Daniel Jurgens]

On 6/30/2016 4:06 PM, Casey Schaufler wrote:
> On 6/30/2016 1:42 PM, Paul Moore wrote:
>> On Thu, Jun 23, 2016 at 3:52 PM, Dan Jurgens <dani...@mellanox.com> wrote:
>>> From: Daniel Jurgens <dani...@mellanox.com>
>>>
>>> Implement and attach hooks to allocate and free Infiniband QP and MAD
>>> agent security structures.
>>>
>>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>>> Reviewed-by: Eli Cohen <e...@mellanox.com>
>>> ---
>>>  include/rdma/ib_mad.h |  1 +
>>>  include/rdma/ib_verbs.h   |  1 +
>>>  security/selinux/hooks.c  | 53 
>>> +++
>>>  security/selinux/include/objsec.h |  5 
>>>  4 files changed, 60 insertions(+)
>>>
>>> diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h
>>> index c8a773f..a1ed025 100644
>>> --- a/include/rdma/ib_mad.h
>>> +++ b/include/rdma/ib_mad.h
>>> @@ -537,6 +537,7 @@ struct ib_mad_agent {
>>> u32 flags;
>>> u8  port_num;
>>> u8  rmpp_version;
>>> +   void*m_security;
>> General convention is to just call the LSM blobs "security" unless
>> there is already a field with that name.
> Not that it really matters all that much, but an unadorned "security"
> makes it unnecessarily difficult to match "p->security" to the data
> involved when you're looking at keys, creds and ipc. I like having
> the prefix. I think the other fields in the structure should have it,
> too, but as I'm not an acknowledged authority on good style I hesitate
> to suggest it in general.

Now that you mention it I think this was part of your comment about not using 
void*.

>>>  };
>>>
>>>  /**
>>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
>>> index 3f6780b..e522acb 100644
>>> --- a/include/rdma/ib_verbs.h
>>> +++ b/include/rdma/ib_verbs.h
>>> @@ -1454,6 +1454,7 @@ struct ib_qp {
>>> void   *qp_context;
>>> u32 qp_num;
>>> enum ib_qp_type qp_type;
>>> +   struct ib_qp_security  *qp_sec;
>> See my earlier question/comment about just using a void pointer here.
> I think that this is in response to my comments to the
> effect that I would like to see the LSM infrastructure
> using the inode like (inode->i_security) to the xfrm
> (void *) approach. I haven't been looking at the IB patches
> too carefully to date. It's possible I have not been clear.
My understanding at the time was that by using something other than a void * 
different security modules could maintain their own opaque blobs with in and 
keep the same prototype for the hook.  It's possible I misunderstood you, but 
it made sense to me.  I don't know of any plans for other security modules to 
support Infiniband, but this leaves the door open.
>>>  };
>>>
>>>  struct ib_mr {
>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>>> index 6a8841d..4f13ea4 100644
>>> --- a/security/selinux/hooks.c
>>> +++ b/security/selinux/hooks.c
>>> @@ -17,6 +17,7 @@
>>>   * Paul Moore <p...@paul-moore.com>
>>>   *  Copyright (C) 2007 Hitachi Software Engineering Co., Ltd.
>>>   *Yuichi Nakamura <yna...@hitachisoft.jp>
>>> + *  Copyright (C) 2016 Mellanox Technologies
>>>   *
>>>   * This program is free software; you can redistribute it and/or modify
>>>   * it under the terms of the GNU General Public License version 2,
>>> @@ -83,6 +84,8 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>> +#include 
>>>
>>>  #include "avc.h"
>>>  #include "objsec.h"
>>> @@ -6015,6 +6018,47 @@ static void 
>>> selinux_unregister_ib_flush_callback(void)
>>> mutex_unlock(_flush_mutex);
>>>  }
>>>
>>> +static int selinux_ib_qp_alloc_security(struct ib_qp_security *qp_sec)
>>> +{
>>> +   struct ib_security_struct *sec;
>>> +
>>> +   sec = kzalloc(sizeof(*sec), GFP_ATOMIC);
>>> +   if (!sec)
>>> +   return -ENOMEM;
>>> +   sec->sid = current_sid();
>>> +
>>> +   qp_sec->q_security = sec;
>>> +   return 0;
>>> +}
>> If you get rid of the ip_qp_security

Re: [PATCH 01/12] security: Add LSM hooks for Infiniband security

[Daniel Jurgens]

On 6/30/2016 3:33 PM, Paul Moore wrote:
> On Thu, Jun 23, 2016 at 3:52 PM, Dan Jurgens  wrote:
>
>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
>> index 432bed5..3f6780b 100644
>> --- a/include/rdma/ib_verbs.h
>> +++ b/include/rdma/ib_verbs.h
>> @@ -1428,6 +1428,10 @@ struct ib_srq {
>> } ext;
>>  };
>>
>> +struct ib_qp_security {
>> +   void *q_security;
>> +};
> Sorry, I missed this earlier and didn't realize it until I was going
> through 4/12 ... why both with ib_qp_security?  Why not just use a
> straight void pointer?
>
In the RFC series Casey Schaufler asked me to not use void blobs to make module 
stacking easier.  Also, in the IB/Core part of the series much is added to the 
ib_qp_security structure to track security info needed for proper enforcement.


___
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: [PATCH 01/12] security: Add LSM hooks for Infiniband security

[Daniel Jurgens]

On 6/30/2016 3:28 PM, Paul Moore wrote:
> On Thu, Jun 23, 2016 at 3:52 PM, Dan Jurgens <dani...@mellanox.com> wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> Add nine new hooks
>>  1. Allocate security contexts for Infiniband QPs.
>>  2. Free security contexts for Infiniband QPs.
>>  3. Allocate security contexts for Infiniband MAD agents.
>>  4. Free security contexts for Infiniband MAD agents.
>>  5. Enforce QP access to Pkeys
>>  6. Enforce MAD agent access to Pkeys
>>  7. Enforce MAD agent access to Infiniband End Ports for sending Subnet
>> Management Packets (SMP)
>>  8. A hook to register a callback 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.
>>  9. A hook to unregister the callback.
>>
>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>> Reviewed-by: Eli Cohen <e...@mellanox.com>
>> ---
>>  include/linux/lsm_hooks.h | 71 
>>  include/linux/security.h  | 63 +++
>>  include/rdma/ib_verbs.h   |  4 +++
>>  security/Kconfig  |  9 +
>>  security/security.c   | 83 
>> +++
>>  5 files changed, 230 insertions(+)
> I'd recommend putting the IB hook calls into this patch as well, it
> helps make the hooks a bit more concrete as you can see where, and how
> they are called.
Do you mean add them with SELinux hook implementations?  Or with the the 
IB/Core code where they are called?  I tried as best as I could to avoid 
mingling LSM, IB/Core, and SELinux changes.  Hoping to minimize the burden of a 
single patch needing acceptance from multiple maintainers and synchronization 
problems that could create.  I could split this up and add the hooks where they 
are actually used if you don't think that's problem though.


___
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: [PATCH 02/12] selinux: Create policydb version for Infiniband support

[Daniel Jurgens]

On 6/30/2016 3:17 PM, Paul Moore wrote:
> On Thu, Jun 23, 2016 at 3:52 PM, Dan Jurgens <dani...@mellanox.com> wrote:
>> From: Daniel Jurgens <dani...@mellanox.com>
>>
>> Support for Infiniband requires the addition of two new object contexts,
>> one for infiniband PKeys and another IB End Ports.  Added handlers to read
>> and write the new ocontext types when reading or writing a binary policy
>> representation.
>>
>> Signed-off-by: Daniel Jurgens <dani...@mellanox.com>
>> Reviewed-by: Eli Cohen <e...@mellanox.com>
>> ---
>>  security/selinux/include/security.h |   3 +-
>>  security/selinux/ss/policydb.c  | 129 
>> +++-
>>  security/selinux/ss/policydb.h  |  27 +---
>>  3 files changed, 135 insertions(+), 24 deletions(-)
> ...
>
>> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
>> index 992a315..78b819c 100644
>> --- a/security/selinux/ss/policydb.c
>> +++ b/security/selinux/ss/policydb.c
>> @@ -2219,6 +2229,58 @@ static int ocontext_read(struct policydb *p, struct 
>> policydb_compat_info *info,
>> goto out;
>> break;
>> }
>> +   case OCON_PKEY: {
>> +   rc = next_entry(nodebuf, fp, sizeof(u32) * 
>> 6);
>> +   if (rc)
>> +   goto out;
>> +
>> +   c->u.pkey.subnet_prefix = 
>> be64_to_cpu(*((__be64 *)nodebuf));
>> +   /* The subnet prefix is stored as an IPv6
>> +* address in the policy.
>> +*
>> +* Check that the lower 2 DWORDS are 0.
>> +*/
> Any particular reason why you reusing an IPv6 address format here?
> Why not use a u64 for the prefix and u16/u32 fields for the partition
> keys?
The subnet prefix is the high order bytes of an IPv6 address and there is 
infrastructure in place in the userland utilities that deal with IPv6 addresses 
(parsing them with a :: to eliminate the need to fill out the 0's for example).

Regarding u16, the policy is packed with everything in u32, as you can see in 
OCON_NODE6 and OCON_PORT handling.
>> +   if (nodebuf[2] || nodebuf[3]) {
>> +   rc = -EINVAL;
>> +   goto out;
>> +   }
>> +
>> +   if (nodebuf[4] > 0x ||
>> +   nodebuf[5] > 0x) {
>> +   rc = -EINVAL;
>> +   goto out;
>> +   }
>> +
>> +   c->u.pkey.low_pkey = le32_to_cpu(nodebuf[4]);
>> +   c->u.pkey.high_pkey = 
>> le32_to_cpu(nodebuf[5]);
>> +
>> +   rc = 
>> context_read_and_validate(>context[0],
>> +  p,
>> +  fp);
>> +   if (rc)
>> +   goto out;
>> +   break;
>> +   }
>> +   case OCON_IB_END_PORT:
> This is a little bit of bikeshedding, but is there such thing as an IB
> "port" that isn't an *end* "port"?  Could we simply use OCON_IB_PORT?
Jason Gunthorpe requested that the name be end_port  in the RFC series.
>> +   rc = next_entry(buf, fp, sizeof(u32) * 2);
>> +   if (rc)
>> +   goto out;
>> +   len = le32_to_cpu(buf[0]);
>> +
>> +   rc = str_read(>u.ib_end_port.dev_name, 
>> GFP_KERNEL,
>> + fp,
>> + len);
>> +   if (rc)
>> +   goto out;
>> +
>> +   c->u.ib_end_port.port = le32_to_cpu(buf[1]);
> No range checking on the port value like you do on the partition keys above?

I can add a similar check.


___
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: [PATCH 03/12] selinux: Implement Infiniband flush callback

[Daniel Jurgens]

On 6/30/2016 2:52 PM, Paul Moore wrote:
> I'm still working on understanding IB, but my current thinking is very
> similar to Yuval's suggestions.  There is a risk of creating a general
> purpose mechanism to solve a specific, isolated problem, but adding a
> LSM notification mechanism does seem like a reasonable thing to do.
>
> My current thinking is to have the LSM framework itself, e.g.
> security/security.c, maintain a list of callbacks (BTW, please make it
> a RCU protected list) with other non-LSM subsystems registering
> callbacks, and specific LSMs making notification calls into the LSM
> framework itself which would handle iterating through the registered
> callbacks.  Since we're going down the general purpose solution route,
> I might add an event field and a void pointer to the callback, for
> example:
>
>   void lsm_notifier_callback(unsigned int event, void *ptr);
>
> ... I would expect at first we would only have a POLICY_CHANGE event
> (ptr set to NULL), but we may want/need to add other events in the
> future.
>
> Make sense?

Yes, I think so.  I'll make this change.


___
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: [PATCH 03/12] selinux: Implement Infiniband flush callback

[Daniel Jurgens]

On 6/30/2016 10:10 AM, Yuval Shaia wrote:
> On Thu, Jun 23, 2016 at 10:52:49PM +0300, Dan Jurgens wrote:
>
>> +static void (*ib_flush_callback)(void);
> Do we really want to have such ib_ prefix in security/ directory?
>
>> +if (ib_flush_callback)
>> +ib_flush_callback();
> How about some generic mechanism (such as a list) in case more
> modules/drivers would like to register callbacks?
> ( assuming this is no longer RFC :) )
>
Paul Moore and I were having a higher level discussion about this in the 00/12 
thread.  I think your suggestion makes sense, perhaps Paul will weigh in when 
he reaches this patch.

___
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: [PATCH 00/12] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 6/30/2016 9:43 AM, Yuval Shaia wrote:
> Few extremely minor cosmetic suggestions to commit message.
>
Thanks Yuval, I'll address these in the eventual v2 series.

___
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: [PATCH 00/12] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 6/29/2016 12:33 PM, Paul Moore wrote:
> On Thu, Jun 23, 2016 at 3:52 PM, Dan Jurgens  wrote:
>> When destroying a QP the ib_qp structure is freed by the hardware driver
>> if the destroy is successful.  This requires storing security related
>> information in a separate structure. When a destroy request is in process
>> the ib_qp structure is in an undefined state so if there are changes to the
>> security policy or PKey table the security checks cannot reset the QP if it
>> doesn't have permission for the new setting.  If the destroy fails security
>> for that QP must be enforced again, and its status in the list restored.
>> If the destroy succeeds the security info can be cleaned up and freed.
> Perhaps I'll end up answering this for myself as I work my way through
> the patches, but hopefully you are handling the case where a destroy
> operation fails and the QP needs to be revalidated?

Yes, if the destroy fails the security is checked again.  You can see
this in security.c
ib_destroy_qp_security_abort which is added in "[PATCH 09/12] IB/core:
Enforce PKey
security on QPs"

> I'm also wondering if QP revalidation on a security policy change is
> worth the trouble; we've historically not been able to provide any
> revoke guarantees so I'm not sure if it is worth a lot of added
> complexity to gain this functionality just for Infiniband.  That said,
> it would be *nice* to have revalidation/revocation working, even if
> only for IB.  It may be that we need similar code to handle the
> various corner cases, so we may be stuck with the complexity anyway, I
> just thought it was worth bringing up as a topic of discussion.

QP re-validation on policy change comes cheap because it's possible for the
PKey table to change.  So a mechanism to recheck all the QPs is needed
regardless.  I'd be fine with getting rid of it if you think that's
best.  In a
production environment SELinux will always be enforcing so it's probably not
really needed.  During my testing it left a funny taste in my mouth when
I had
QPs that shouldn't be allowed continue to exist after setenforce 1.  On
the other
hand I'm not in love with the callback registration for policy change
notification
one off for Infiniband.  In on of the RFCs I used an LSM hook that
ib/core would
implement.  I think Casey commented on that, so I changed it to what you see
now.

Thank you for reviewing this.

___
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 v2 00/13] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 4/14/2016 11:26 AM, Ira Weiny wrote:
> On Thu, Apr 14, 2016 at 01:11:15PM +0000, Daniel Jurgens wrote:
>> On 4/13/2016 11:23 PM, Hefty, Sean wrote:
>>>>>> Former (multicast modifications of fabric) also requires restricting
>>>>>> arbitrary UD QPs as well as QP1 as SA access is QPn (n > 0) <-> QP1.
>>>>>
>>>>> The SA could have an option to ignore all requests that do not originate
>>>> QP1,
>>>>> then protect access to QP1 on the client nodes.
>>>>
>>>> I'm not really sure what we are protecting against here.  Is it simply DoS
>>>> against the SA?
>>>
>>> This would protect against a non-privileged QP trying to change multicast 
>>> or event subscription, for example.  Though it could help with DoS, by 
>>> avoiding the processing associated with requests.  Jason's original 
>>> question was why would you want to leave qp1 open, and I think the answer 
>>> to that depends on what restrictions could be enforced for qpX: X > 1.  
>>> Restricting both seem desirable, IMO.
>>>
>>
>> There are no qpX to qpX restrictions.  As long as the users that created
>> both QPs have permission to use the PKey in the PKey index they
>> specified then they are free to communicate in any way.
> 
> For GSI (from 'random' QPs) this seams reasonable.
> 
> In InfiniBand, the pkey is ignored in SMPs (those packets on QP0/VL15).  So it
> does not really make sense to me to control SMPs via pkey.  (OPA does check
> pkeys in SMPs.)
> 
> Therefore, I agree with Jason that need to control access to QP0.  
> Furthermore,
> I think you can/should do that when the agent is registered.  This will be 
> more
> performant for SMs as well.

QP0 is already controlled, just some naming cleanup there.  Agent
registration is the ideal time to do this, unfortunately security policy
can change at run time, and there's no existing way to destroy a mad
agent from the "bottom up".  I added per MAD checks for that reason.

> For SA/GSI access using umad I think the best control is to limit access to 
> the
> umad char device itself?  ibacm has been changed to be an SA cache (with
> plugins available for both IB and OPA) and therefore it can run with 
> sufficient
> privileges to access the umad char device.  Then regular user applications
> don't need umad access at all.

Denying access for all MADs could be managed via existing filesystem
mechanism.

> For SA/GSI interactions from 'random' UD QPs, those can be controlled via the
> Pkey mechanism you mention above.
> 
> If you do add a check on every packet in the mad stack I would like time to
> evaluate how that will affect the mad interface for performance.

I don't anticipate a large performance impact, due to caching of
security ID's.  I can try to quantify it though.  I don't know of any
MAD performance tests off hand, do have a recommendation?

Thanks,
Dan



___
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 v2 00/13] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 4/13/2016 11:23 PM, Hefty, Sean wrote:
 Former (multicast modifications of fabric) also requires restricting
 arbitrary UD QPs as well as QP1 as SA access is QPn (n > 0) <-> QP1.
>>>
>>> The SA could have an option to ignore all requests that do not originate
>> QP1,
>>> then protect access to QP1 on the client nodes.
>>
>> I'm not really sure what we are protecting against here.  Is it simply DoS
>> against the SA?
> 
> This would protect against a non-privileged QP trying to change multicast or 
> event subscription, for example.  Though it could help with DoS, by avoiding 
> the processing associated with requests.  Jason's original question was why 
> would you want to leave qp1 open, and I think the answer to that depends on 
> what restrictions could be enforced for qpX: X > 1.  Restricting both seem 
> desirable, IMO.
> 

There are no qpX to qpX restrictions.  As long as the users that created
both QPs have permission to use the PKey in the PKey index they
specified then they are free to communicate in any way.

___
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 v2 00/13] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 4/13/2016 7:27 PM, Ira Weiny wrote:
> On Wed, Apr 13, 2016 at 04:47:48PM +, Sean Hefty wrote:
>>> Former (multicast modifications of fabric) also requires restricting
>>> arbitrary UD QPs as well as QP1 as SA access is QPn (n > 0) <-> QP1.
>>
>> The SA could have an option to ignore all requests that do not originate QP1,
>> then protect access to QP1 on the client nodes.
> 
> I'm not really sure what we are protecting against here.  Is it simply DoS
> against the SA?
> 
> Ira
> 
>> --
>>
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

A DoS attack against the SA is out of scope for this proposed change.
SELinux provides access control.  Preventing a user from maliciously
doing something you've given them permission to do is a different problem.

___
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] selinux: always return a value from the netport/netnode/netif caches

[Daniel Jurgens]

On 4/13/2016 4:43 PM, Paul Moore wrote:
> From: Paul Moore 
> 
> Even if we are under memory pressure and can't allocate a new cache
> node we can still return the port/node/iface value we looked up from
> the policy.
> 
> Reported-by: Greg 
> Signed-off-by: Paul Moore 
> ---
>  security/selinux/netif.c   |   35 +--
>  security/selinux/netnode.c |   31 +--
>  security/selinux/netport.c |   19 ---
>  3 files changed, 38 insertions(+), 47 deletions(-)
> 
> diff --git a/security/selinux/netif.c b/security/selinux/netif.c
> index e607b44..5c3bfa4 100644
> --- a/security/selinux/netif.c
> +++ b/security/selinux/netif.c
> @@ -91,18 +91,16 @@ static inline struct sel_netif *sel_netif_find(const 
> struct net *ns,
>   * zero on success, negative values on failure.
>   *
>   */
> -static int sel_netif_insert(struct sel_netif *netif)
> +static void sel_netif_insert(struct sel_netif *netif)
>  {
>   int idx;
>  
>   if (sel_netif_total >= SEL_NETIF_HASH_MAX)
> - return -ENOSPC;
> + return;
>  
>   idx = sel_netif_hashfn(netif->nsec.ns, netif->nsec.ifindex);
>   list_add_rcu(>list, _netif_hash[idx]);
>   sel_netif_total++;
> -
> - return 0;
>  }
>  
>  /**
> @@ -135,7 +133,7 @@ static void sel_netif_destroy(struct sel_netif *netif)
>   */
>  static int sel_netif_sid_slow(struct net *ns, int ifindex, u32 *sid)
>  {
> - int ret;
> + int ret = 0;
>   struct sel_netif *netif;
>   struct sel_netif *new = NULL;
>   struct net_device *dev;
> @@ -155,34 +153,27 @@ static int sel_netif_sid_slow(struct net *ns, int 
> ifindex, u32 *sid)
>   netif = sel_netif_find(ns, ifindex);

I know this is out of context for this patch, but isn't this find
redundant?  It was already checked in sel_netif_sid.

>   if (netif != NULL) {
>   *sid = netif->nsec.sid;
> - ret = 0;
>   goto out;
>   }
> - new = kzalloc(sizeof(*new), GFP_ATOMIC);
> - if (new == NULL) {
> - ret = -ENOMEM;
> + ret = security_netif_sid(dev->name, sid);
> + if (ret != 0) {
> + printk(KERN_WARNING
> +"SELinux: failure in sel_netif_sid_slow(),"
> +" unable to determine network interface label (%d)\n",
> +ifindex);
>   goto out;
>   }
> - ret = security_netif_sid(dev->name, >nsec.sid);
> - if (ret != 0)
> + new = kzalloc(sizeof(*new), GFP_ATOMIC);
> + if (new == NULL)
>   goto out;
>   new->nsec.ns = ns;
>   new->nsec.ifindex = ifindex;
> - ret = sel_netif_insert(new);
> - if (ret != 0)
> - goto out;
> - *sid = new->nsec.sid;
> + new->nsec.sid = *sid;
> + sel_netif_insert(new);
>  
>  out:
>   spin_unlock_bh(_netif_lock);
>   dev_put(dev);
> - if (unlikely(ret)) {
> - printk(KERN_WARNING
> -"SELinux: failure in sel_netif_sid_slow(),"
> -" unable to determine network interface label (%d)\n",
> -ifindex);
> - kfree(new);
> - }
>   return ret;
>  }
>  
> diff --git a/security/selinux/netnode.c b/security/selinux/netnode.c
> index da923f8..b752bd2 100644
> --- a/security/selinux/netnode.c
> +++ b/security/selinux/netnode.c
> @@ -199,7 +199,7 @@ static void sel_netnode_insert(struct sel_netnode *node)
>   */
>  static int sel_netnode_sid_slow(void *addr, u16 family, u32 *sid)
>  {
> - int ret = -ENOMEM;
> + int ret;
>   struct sel_netnode *node;
>   struct sel_netnode *new = NULL;
>  
> @@ -210,39 +210,42 @@ static int sel_netnode_sid_slow(void *addr, u16 family, 
> u32 *sid)
>   spin_unlock_bh(_netnode_lock);
>   return 0;
>   }
> - new = kzalloc(sizeof(*new), GFP_ATOMIC);
> - if (new == NULL)
> - goto out;
>   switch (family) {
>   case PF_INET:
>   ret = security_node_sid(PF_INET,
>   addr, sizeof(struct in_addr), sid);
> - new->nsec.addr.ipv4 = *(__be32 *)addr;
>   break;
>   case PF_INET6:
>   ret = security_node_sid(PF_INET6,
>   addr, sizeof(struct in6_addr), sid);
> - new->nsec.addr.ipv6 = *(struct in6_addr *)addr;
>   break;
>   default:
>   BUG();
>   ret = -EINVAL;
>   }
> - if (ret != 0)
> + if (ret != 0) {
> + printk(KERN_WARNING
> +"SELinux: failure in sel_netnode_sid_slow(),"
> +" unable to determine network node label\n");
>   goto out;
> -
> + }
> + new = kzalloc(sizeof(*new), GFP_ATOMIC);
> + if (new == NULL)
> + goto out;
> + switch (family) {
> + case PF_INET:
> + 

Re: [RFC PATCH v2 00/13] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 4/13/2016 7:10 AM, Hal Rosenstock wrote:
> On 4/12/2016 1:58 PM, Jason Gunthorpe wrote:
>> On Tue, Apr 12, 2016 at 05:06:45PM +, Hefty, Sean wrote:
 Wouldn't QP1 require different access control than QP0 due to SA clients
 on every end node ?
>>>
>>> QP1 still allows modification of the fabric (e.g. multicast join) or
>>> an DoS attack against the SA.  Though the latter probably requires
>>> restricting how a UD QP may be used.
>>
>> Right, I don't disagree we should have smp and gmp 'just in case'
>> (fine names as well) labels, I just don't really understand why you'd
>> trust something enough to grant gmp but not enough for smp...
>>
>> Particularly encouraging people to grant gmp as though that was 'safe'
>> is really bad advice.
> 
> I'm not sure what the motivation is either. The nature of the QP1 threat
> is somewhat different from the QP0 threat. Only thing I can think of is
> that it's hard to protect GMPs/QP1 since any UD QP can send to QP1.
> 
> -- Hal
> 
>> Which in turn makes me wonder why the umad dev node label is not
>> sufficient.
>>
>> Jason
>>
> 

I've asked Liran to look over this thread, I'd like him to weigh in.  He
said he will have time tomorrow.

___
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 v2 00/13] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 4/12/2016 12:12 AM, Hal Rosenstock wrote:
> On 4/11/2016 7:35 PM, Daniel Jurgens wrote:
>> On 4/11/2016 6:12 PM, Jason Gunthorpe wrote:
>>> On Mon, Apr 11, 2016 at 10:30:54PM +0000, Daniel Jurgens wrote:
>>>
>>> Like I said, the user facing name should be QP0 in that case.
>>>
>>> Jason
>>>
>>
>> OK, I'll change idbev to ibendport and smi to qp0, or qpzero if the
>> SELinux user space code doesn't allow numbers in access vector identifiers.
> 
> Another possible name for this is smp.
> 
> -- Hal
> 
> 

Thanks Hal, I'll go with that as long as Jason doesn't object.

___
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 v2 00/13] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 4/11/2016 6:12 PM, Jason Gunthorpe wrote:
> On Mon, Apr 11, 2016 at 10:30:54PM +0000, Daniel Jurgens wrote:
> 
> Like I said, the user facing name should be QP0 in that case.
> 
> Jason
> 

OK, I'll change idbev to ibendport and smi to qp0, or qpzero if the
SELinux user space code doesn't allow numbers in access vector identifiers.


___
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 v2 00/13] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 4/11/2016 5:12 PM, Jason Gunthorpe wrote:
> On Mon, Apr 11, 2016 at 08:38:50PM +0000, Daniel Jurgens wrote:
>>>> An Infiniband device (ibdev) is labeled by name and port number.  There is 
>>>> a
>>>> single access vector for ibdevs as well, called "smi".
>>>
>>> This is called an End Port (SMI is something else in the IB
>>> spec). Please use the standard terminology.
>> I see your point on the end port, I'll address this is the next series
>> by updating the commit messages and replacing ibdev with ibendport.
>>
>> I don't understand where you think I've gone wrong on SMI.
> 
> Well, this makes no sense:
>  There is a single access vector for ibdevs as well, called "smi".

Access vector is an SELinux term.  Object have access vectors.  For
example a file object has has many access vectors like "read", "write",
"unlink", etc.  Policy rules allow access to a type of object on a
subset of its access vectors.

Controlling the "smi" is to prevent someone from starting a subnet manager.

> SMI is not umad. SMI should only refer to the SMA access channel on a
> specific node, and I have no idea why someone would want to restrict
> local SMA access independently of generic umad qp0 access. Just call
> it QP0 or QP1 or umad.
> 
> SMI is an obscure internal term that should not be user facing.
> 

The point of control here is MAD agent registration and MAD transmit and
receive.  When a MAD agent is created it inherits the security ID of
it's parent task.  For MAD agents that have a QP of type IB_QPT_SMI,
when an attempt is made to send a MAD the security ID of the MAD agent
is checked for access to the SMI vector of the IB device (to become End
Port).  This is only for MAD agents that have a qp with of type
IB_QPT_SMI.  So having umad as the access vector is too broad.


___
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 v2 00/13] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 4/11/2016 3:12 PM, Jason Gunthorpe wrote:
> On Thu, Apr 07, 2016 at 02:33:45AM +0300, Dan Jurgens wrote:
> 
>> Currently there is no way to provide granular access control to an Infiniband
>> fabric.  By providing an ability to restrict user access to specific virtual
>> subfabrics administrators can limit access to bandwidth and isolate users on
>> the fabric.
> 
> Do you actually have a concrete use case for this?

We know the national labs are interested in this.

> This seems superficially similar to netlabel, which I guess targets a
> certain niche, but I'm really wondering with all the other container
> patches if this was supposed to be done with namespaces...

I can't speak to the goals of the other container patches.

Netlabel can't label kernel bypassed packets.  It can be used for IPoIB
though.

>> An Infiniband device (ibdev) is labeled by name and port number.  There is a
>> single access vector for ibdevs as well, called "smi".
> 
> This is called an End Port (SMI is something else in the IB
> spec). Please use the standard terminology.
I see your point on the end port, I'll address this is the next series
by updating the commit messages and replacing ibdev with ibendport.

I don't understand where you think I've gone wrong on SMI.

>From section 3.4.5.2: "Each node provides a Subnet Management Agent
(SMA) that the SM access through a well known interface called the
Subnet Management Interface (SMI)."

Thanks,
Dan


___
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 v2 09/13] ib/core: Enforce PKey security when modifying QPs

[Daniel Jurgens]

On 4/7/2016 12:40 PM, l...@leon.nu wrote:
> On Thu, Apr 07, 2016 at 05:03:50PM +0000, Daniel Jurgens wrote:
>> On 4/7/2016 11:31 AM, Leon Romanovsky wrote:
>>> On Thu, Apr 07, 2016 at 02:33:54AM +0300, Dan Jurgens wrote:
>>>> From: Daniel Jurgens <dani...@mellanox.com>
>>>> +  kfree(qp->qp_sec);
>>>> +
>>>> +  return err;
>>>> +}
>>>> +EXPORT_SYMBOL(ib_security_create_qp_security);
>>>> +
>>>> +void ib_security_destroy_qp(struct ib_qp_security *sec)
>>>> +{
>>>> +  security_ib_qp_free_security(sec);
>>>> +  kfree(sec);
>>>> +}
>>>
>>> Did you want to EXPORT_SYMBOL here too?
>>>
>>
>> It's not called from outside ib_core, I only exported the functions that
>> are. ib_security_modify_qp and ib_security_create_qp security are called
>> from ib_core and ib_uverbs.  ib_security_enforce_mad_agent_pkey_access
>> is called from ib_mad.
> 
> So, how are you releasing the memory which is taken by
> ib_security_create_qp_security ?

It is freed in ib_security_destroy_qp, which is called by ib_destroy_qp
after the QP is destroyed successfully.



___
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 0/7] SELinux support for Infiniband RDMA

[Daniel Jurgens]

On 4/4/2016 8:55 PM, James Morris wrote:
> On Tue, 5 Apr 2016, Daniel Jurgens wrote:
> 
>> On 4/4/2016 8:13 PM, James Morris wrote:
>>> On Tue, 5 Apr 2016, Dan Jurgens wrote:
>>>
>>>> From: Daniel Jurgens <dani...@mellanox.com>
>>>>
>>>> Currently there is no way to provide granular access control to an 
>>>> Infiniband
>>>> fabric.  By providing an ability to restrict user access to specific 
>>>> virtual
>>>> subfabrics administrators can limit access to bandwidth and isolate users 
>>>> on
>>>> the fabric.
>>>
>>> Where are the LSM hooks placed?
>>>
>>>
>>>
>> The LSM hooks are defined in patch 1/7 of this series.  There are 4 that
>> will be called from ib_core, and one that's implemented by ib_core to be
>> called by a security module if the policy or enforcement setting change
>> (infiniband_flush).  That call from SELinux is added in patch 3/7 of
>> this series.
> 
> Can you post the ib_core patches, too?
> 
> 
Yes, I'll post them soon.

___
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 <dani...@mellanox.com>
>>
>> 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 <dani...@mellanox.com>
>> Reviewed-by: Eli Cohen <e...@mellanox.com>
>> ---
>>  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 <ca...@schaufler-ca.com>
>> + * Copyright (C) 2016 Mellanox Techonologies. <dani...@mellanox.com>
>>   *
>>   *  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_