Re: [RFC PATCH v2 11/13] ib/core: Enforce Infiniband device SMI security
On 4/7/2016 3:54 PM, Leon Romanovsky wrote: > On Thu, Apr 07, 2016 at 02:33:56AM +0300, Dan Jurgens wrote: >> +dev_err(&device->dev, >> +"%s: Access Denied. Err: %d\n", > > Please convert it to lower case. > Can malicious user flood the system with this print? > >> +dev_err(&device->dev, >> +"%s: Access Denied. Err: %d\n", > > The same as above. > I will just remove the prints unless you object. The denial will be captured in the audit log. ___ 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
On 4/7/2016 4:11 PM, l...@leon.nu wrote:
> On Thu, Apr 07, 2016 at 09:02:43PM +, 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:
>>>
+ if (sec->qp == sec->qp->real_qp) {
+ /* The caller of this function holds the QP security
+ * mutex so this list traversal is safe
+ */
>>>
>>> Did the comment below pass checkpatch.pl?
>>
>> It did.
>>
+ list_for_each_entry(shared_qp_sec,
+ &sec->shared_qp_list,
+ shared_qp_list) {
>>>
>>> Is this list always needed to be protected by lock?
>>> In general, I prefer to see lock/unlock operations near protected code.
>>> Otherwise, it is hard to follow all lock/unlock paths.
>>
>> The mutex is required, and I'm not sure how to push it lower safely.
>> Consider the situation where a QP is being modified concurrently with an
>> open_shared_qp. Unless the lock is held the entire time for each
>> operation open_shared_qp could check permission using a ib_qp_security
>> struct in a partially modified state, i.e. there could be a mix of new
>> and old settings for the port/pkey index/alternate path. Shared QPs use
>> the port and pkey setting for the underlying real QP with the shared qp
>> q_security.
>
> Common flow is to do everything in one place:
> 1. lock
> 2. check permissions
> 3. change
> 4. release lock
>
> Your flow is much broader.
> 1. lock
> 2. check permission
> 3. do work
> 4. change
> 5. do work
> 6. release lock.
>
I see what your saying. This is an artifact of me breaking this up into
two patches after the fact. "[RFC PATCH v2 12/13] ib/core: Track which
QPs are using which port and PKey index" introduces the reason that is
needed. In that patch lists of QP security structures are maintained
and walked when there are changes to pkey table or GIDs. Those list
operations don't hold the QP security mutexes for all the QPs so it con
run concurrently with a QP modify. This means the QP needs to be added
to the new setting lists, and then clean up the lists after the modify
finishes.
I can change this patch to the common flow above, and then introduce "my
flow" in the patch that requires it. Another option would be to squash
the two together. They are both relatively large so I am hesitant to do
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: [RFC PATCH v2 09/13] ib/core: Enforce PKey security when modifying QPs
On 4/7/2016 11:31 AM, Leon Romanovsky wrote:
> On Thu, Apr 07, 2016 at 02:33:54AM +0300, Dan Jurgens wrote:
>
>> +if (sec->qp == sec->qp->real_qp) {
>> +/* The caller of this function holds the QP security
>> + * mutex so this list traversal is safe
>> +*/
>
> Did the comment below pass checkpatch.pl?
It did.
>> +list_for_each_entry(shared_qp_sec,
>> +&sec->shared_qp_list,
>> +shared_qp_list) {
>
> Is this list always needed to be protected by lock?
> In general, I prefer to see lock/unlock operations near protected code.
> Otherwise, it is hard to follow all lock/unlock paths.
The mutex is required, and I'm not sure how to push it lower safely.
Consider the situation where a QP is being modified concurrently with an
open_shared_qp. Unless the lock is held the entire time for each
operation open_shared_qp could check permission using a ib_qp_security
struct in a partially modified state, i.e. there could be a mix of new
and old settings for the port/pkey index/alternate path. Shared QPs use
the port and pkey setting for the underlying real QP with the shared qp
q_security.
___
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 2/2 v2] checkpolicy: Warn if module name different than output filename
On 04/07/2016 03:34 PM, James Carter wrote:
On 04/07/2016 11:28 AM, Daniel J Walsh wrote:
On 04/07/2016 11:06 AM, James Carter wrote:
Since CIL treats files as modules and does not have a separate
module statement it can cause confusion when a Refpolicy module
has a name that is different than its base filename because older
SELinux userspaces will refer to the module by its module name while
a CIL-based userspace will refer to it by its filename.
Because of this, provide a warning message when compiling a module and
the output filename is different than the module name.
Signed-off-by: James Carter
---
checkpolicy/checkmodule.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/checkpolicy/checkmodule.c b/checkpolicy/checkmodule.c
index 5957d29..d807620 100644
--- a/checkpolicy/checkmodule.c
+++ b/checkpolicy/checkmodule.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
@@ -258,6 +259,20 @@ int main(int argc, char **argv)
}
}
+if (policy_type != POLICY_BASE && outfile) {
+char *mod_name = modpolicydb.name;
+char *out_path = strdup(outfile);
+char *out_name = basename(out_path);
+char *separator = strrchr(out_name, '.');
+if (separator) {
+*separator = '\0';
+}
+if (strcmp(mod_name, out_name) != 0) {
+fprintf(stderr,"Warning: SELinux userspace will
refer to the
module from %s as %s rather than as %s\n", file, out_name, mod_name);
+}
+free(out_path);
+}
+
if (modpolicydb.policy_type == POLICY_BASE && !cil) {
/* Verify that we can successfully expand the base module. */
policydb_t kernpolicydb;
Why not fail rather then warn. Don't let me do stupid things...
I am willing to do that for checkmodule if that is what everyone
wants. I wouldn't want to do it for pp since that could cause problems
for current systems. Just as a note, Fedora's passenger module has
"passanger" as the policy name in its policy_module statement.
Jim
And if it blew up someone would have fixed it. :^)
___
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 2/2 v2] checkpolicy: Warn if module name different than output filename
On 04/07/2016 11:28 AM, Daniel J Walsh wrote:
On 04/07/2016 11:06 AM, James Carter wrote:
Since CIL treats files as modules and does not have a separate
module statement it can cause confusion when a Refpolicy module
has a name that is different than its base filename because older
SELinux userspaces will refer to the module by its module name while
a CIL-based userspace will refer to it by its filename.
Because of this, provide a warning message when compiling a module and
the output filename is different than the module name.
Signed-off-by: James Carter
---
checkpolicy/checkmodule.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/checkpolicy/checkmodule.c b/checkpolicy/checkmodule.c
index 5957d29..d807620 100644
--- a/checkpolicy/checkmodule.c
+++ b/checkpolicy/checkmodule.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
@@ -258,6 +259,20 @@ int main(int argc, char **argv)
}
}
+if (policy_type != POLICY_BASE && outfile) {
+char *mod_name = modpolicydb.name;
+char *out_path = strdup(outfile);
+char *out_name = basename(out_path);
+char *separator = strrchr(out_name, '.');
+if (separator) {
+*separator = '\0';
+}
+if (strcmp(mod_name, out_name) != 0) {
+fprintf(stderr,"Warning: SELinux userspace will refer to the
module from %s as %s rather than as %s\n", file, out_name, mod_name);
+}
+free(out_path);
+}
+
if (modpolicydb.policy_type == POLICY_BASE && !cil) {
/* Verify that we can successfully expand the base module. */
policydb_t kernpolicydb;
Why not fail rather then warn. Don't let me do stupid things...
I am willing to do that for checkmodule if that is what everyone wants. I
wouldn't want to do it for pp since that could cause problems for current
systems. Just as a note, Fedora's passenger module has "passanger" as the policy
name in its policy_module statement.
Jim
--
James Carter
National Security Agency
___
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/2] selinux: avoid nf hooks overhead when not needed
On Thursday, April 07, 2016 01:45:32 AM Florian Westphal wrote: > Paul Moore wrote: > > On Wed, Apr 6, 2016 at 6:14 PM, Florian Westphal wrote: > > > netfilter hooks are per namespace -- so there is hook unregister when > > > netns is destroyed. > > > > Looking around, I see the global and per-namespace registration > > functions (nf_register_hook and nf_register_net_hook, respectively), > > but I'm looking to see if/how newly created namespace inherit > > netfilter hooks from the init network namespace ... if you can create > > a network namespace and dodge the SELinux hooks, that isn't a good > > thing from a SELinux point of view, although it might be a plus > > depending on where you view Paolo's original patches ;) > > Heh :-) > > If you use nf_register_net_hook, the hook is only registered in the > namespace. > > If you use nf_register_hook, the hook is put on a global list and > registed in all existing namespaces. > > New namespaces will have the hook added as well (see > netfilter_net_init -> nf_register_hook_list in netfilter/core.c ) > > Since nf_register_hook is used it should be impossible to get a netns > that doesn't call these hooks. Great, thanks. > > > Do you think it makes sense to rework the patch to delay registering > > > of the netfiler hooks until the system is in a state where they're > > > needed, without the 'unregister' aspect? > > > > I would need to see the patch to say for certain, but in principle > > that seems perfectly reasonable and I think would satisfy both the > > netdev and SELinux camps - good suggestion. My main goal is to drop > > the selinux_nf_ip_init() entirely so it can't be used as a ROP gadget. > > > > We might even be able to trim the secmark_active and peerlbl_active > > checks in the SELinux netfilter hooks (an earlier attempt at > > optimization; contrary to popular belief, I do care about SELinux > > performance), although that would mean that enabling the network > > access controls would be one way ... I guess you can disregard that > > last bit, I'm thinking aloud again. > > One way is fine I think. Yes, just disregard my second paragraph above. > > > Ideally this would even be per netns -- in perfect world we would > > > be able to make it so that a new netns are created with an empty > > > hook list. > > > > In general SELinux doesn't care about namespaces, for reasons that are > > sorta beyond the scope of this conversation, so I would like to stick > > to a all or nothing approach to enabling the SELinux netfilter hooks > > across namespaces. Perhaps we can revisit this at a later time, but > > let's keep it simple right now. > > Okay, I'd prefer to stick to your recommendation anyway wrt. to selinux > (Casey, I read your comment regarding smack. Noted, we don't want to > break smack either...) > > I think that in this case the entire question is: > > In your experience, how likely is a config where selinux is enabled BUT the > hooks are not needed (i.e., where we hit the > > if (!selinux_policycap_netpeer) > return NF_ACCEPT; > > if (!secmark_active && !peerlbl_active) >return NF_ACCEPT; > > tests inside the hooks)? If such setups are uncommon we should just > drop this idea or at least put it on the back burner until the more > expensive netfilter hooks (conntrack, cough) are out of the way. A few years ago I would have said that it is relatively uncommon for admins to enable the SELinux network access controls; it was typically just government/intelligence agencies who had very strict access control requirements and represented a small portion of SELinux users. However, over the past few years I've been fielding more and more questions from admins/devs in the virtualization space who are interested in some of these capabilities; it isn't clear to me how many of these people are switching it on, but there is definitely more interest than I have seen in the past and the interested is centered around some rather common use cases. So, to summarize, I don't know ;) If you've got bigger sources of overhead, my opinion would be to go tackle those first. Perhaps I can even find the time to work on the SELinux/netfilter stuff while you are off slaying the bigger dragons, no promises at the moment. -- paul moore www.paul-moore.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 1/2 v2] policycoreutils/hll/pp: Warn if module name different than output filename
On 04/07/2016 12:41 PM, Thomas Hurd wrote:
On Thu, Apr 7, 2016 at 11:06 AM, James Carter wrote:
Since CIL treats files as modules and does not have a separate
module statement it can cause confusion when a Refpolicy module
has a name that is not the same as its base filename because older
SELinux userspaces will refer to the module by its module name while
a CIL-based userspace will refer to it by its filename.
Because of this, provide a warning message when converting a policy
package to CIL and the output filename is different than the module
name.
Signed-off-by: James Carter
---
policycoreutils/hll/pp/pp.c | 28
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/policycoreutils/hll/pp/pp.c b/policycoreutils/hll/pp/pp.c
index 866734f..8621b50 100644
--- a/policycoreutils/hll/pp/pp.c
+++ b/policycoreutils/hll/pp/pp.c
@@ -28,6 +28,7 @@
#include
#include
+#include
char *progname;
@@ -68,6 +69,8 @@ int main(int argc, char **argv)
{ NULL, 0, NULL, 0 }
};
struct sepol_module_package *mod_pkg = NULL;
+ char *ifile = NULL;
+ char *ofile = NULL;
FILE *in = NULL;
FILE *out = NULL;
int outfd = -1;
@@ -89,20 +92,23 @@ int main(int argc, char **argv)
}
if (argc >= optind + 1 && strcmp(argv[1], "-") != 0) {
- in = fopen(argv[1], "rb");
+ ifile = argv[1];
+ in = fopen(ifile, "rb");
if (in == NULL) {
- log_err("Failed to open %s: %s", argv[1],
strerror(errno));
+ log_err("Failed to open %s: %s", ifile,
strerror(errno));
rc = -1;
goto exit;
}
} else {
+ ifile = "stdin";
in = stdin;
}
if (argc >= optind + 2 && strcmp(argv[2], "-") != 0) {
- out = fopen(argv[2], "w");
+ ofile = argv[2];
+ out = fopen(ofile, "w");
if (out == NULL) {
- log_err("Failed to open %s: %s", argv[2],
strerror(errno));
+ log_err("Failed to open %s: %s", ofile,
strerror(errno));
rc = -1;
goto exit;
}
@@ -122,6 +128,20 @@ int main(int argc, char **argv)
fclose(in);
in = NULL;
+ if (ofile) {
+ char *mod_name = mod_pkg->policy->p.name;
+ char *cil_path = strdup(ofile);
Check if strdup fails here and also in the checkmodule patch?
Yes, I do need to do that. Thanks.
Jim
+ char *cil_name = basename(cil_path);
+ char *separator = strrchr(cil_name, '.');
+ if (separator) {
+ *separator = '\0';
+ }
+ if (strcmp(mod_name, cil_name) != 0) {
+ fprintf(stderr, "Warning: SELinux userspace will refer to
the module from %s as %s rather than %s\n", ifile, cil_name, mod_name);
+ }
+ free(cil_path);
+ }
+
rc = sepol_module_package_to_cil(out, mod_pkg);
if (rc != 0) {
goto exit;
--
2.5.5
___
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.
--
James Carter
National Security Agency
___
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
On 4/7/2016 12:40 PM, l...@leon.nu wrote:
> On Thu, Apr 07, 2016 at 05:03:50PM +, 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
+ 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 v2 09/13] ib/core: Enforce PKey security when modifying QPs
On Thu, Apr 07, 2016 at 05:03:50PM +, 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
> >> + 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 ?
>
>
signature.asc
Description: Digital signature
___
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
On Thu, Apr 07, 2016 at 02:33:54AM +0300, Dan Jurgens wrote:
> From: Daniel Jurgens
>
> Allocate and free a security context when creating and destroying a QP.
> This context is used for controlling access to PKeys.
>
> When a request is made to modify a QP that changes the port, PKey index,
> alternate path port, or alternate path PKey index, check that the QP has
> permission for the PKey in the index on the subnet prefix of the port.
> If the QP is shared make sure all handles to the QP also have access.
>
> Store which port and pkey a QP is using. After the reset to init
> transition the user can modify the port and pkey independently. This
> adds a transactional aspect to modify QP when the port, pkey, or
> alternate path change. Permission must be checked for the new settings,
> then the modify can be attempted. If the modify fails the old setting
> should be restored.
>
> To keep the security changes isolated a new file is used to hold security
> related functionality.
>
> Signed-off-by: Daniel Jurgens
> Reviewed-by: Eli Cohen
> ---
> drivers/infiniband/core/Makefile|2 +-
> drivers/infiniband/core/core_priv.h | 41
> drivers/infiniband/core/core_security.c | 331
> +++
We are already in core, there is no need to call files core_XXX.
> drivers/infiniband/core/uverbs_cmd.c| 20 ++-
> drivers/infiniband/core/verbs.c | 24 +++-
> include/rdma/ib_verbs.h | 28 +++-
> 6 files changed, 439 insertions(+), 7 deletions(-)
> create mode 100644 drivers/infiniband/core/core_security.c
>
> diff --git a/drivers/infiniband/core/Makefile
> b/drivers/infiniband/core/Makefile
> index f818538..48a4013 100644
> --- a/drivers/infiniband/core/Makefile
> +++ b/drivers/infiniband/core/Makefile
> @@ -10,7 +10,7 @@ obj-$(CONFIG_INFINIBAND_USER_ACCESS) += ib_uverbs.o
> ib_ucm.o \
>
> ib_core-y := packer.o ud_header.o verbs.o cq.o sysfs.o \
> device.o fmr_pool.o cache.o netlink.o \
> - roce_gid_mgmt.o
> + roce_gid_mgmt.o core_security.o
> ib_core-$(CONFIG_INFINIBAND_USER_MEM) += umem.o
> ib_core-$(CONFIG_INFINIBAND_ON_DEMAND_PAGING) += umem_odp.o umem_rbtree.o
>
> diff --git a/drivers/infiniband/core/core_priv.h
> b/drivers/infiniband/core/core_priv.h
> index 722b866..27f2fa8 100644
> --- a/drivers/infiniband/core/core_priv.h
> +++ b/drivers/infiniband/core/core_priv.h
> @@ -140,4 +140,45 @@ static inline bool rdma_is_upper_dev_rcu(struct
> net_device *dev,
> int ib_get_cached_subnet_prefix(struct ib_device *device,
> u8port_num,
> u64 *sn_pfx);
> +
> +#ifdef CONFIG_SECURITY_INFINIBAND
> +int ib_security_modify_qp(struct ib_qp *qp,
> + struct ib_qp_attr *qp_attr,
> + int qp_attr_mask,
> + struct ib_udata *udata);
> +
> +int ib_security_create_qp_security(struct ib_qp *qp);
Why do we need xx_SECURITY_x_SECURITY in name?
> +void ib_security_destroy_qp(struct ib_qp_security *sec);
> +int ib_security_open_shared_qp(struct ib_qp *qp);
> +void ib_security_close_shared_qp(struct ib_qp_security *sec);
> +#else
> +static inline int ib_security_modify_qp(struct ib_qp *qp,
> + struct ib_qp_attr *qp_attr,
> + int qp_attr_mask,
> + struct ib_udata *udata)
> +{
> + return qp->device->modify_qp(qp->real_qp,
> + qp_attr,
> + qp_attr_mask,
> + udata);
> +}
> +
> +static inline int ib_security_create_qp_security(struct ib_qp *qp)
> +{
> + return 0;
> +}
> +
> +static inline void ib_security_destroy_qp(struct ib_qp_security *sec)
> +{
> +}
> +
> +static inline int ib_security_open_shared_qp(struct ib_qp *qp)
> +{
> + return 0;
> +}
> +
> +static inline void ib_security_close_shared_qp(struct ib_qp_security *sec)
> +{
> +}
> +#endif
> #endif /* _CORE_PRIV_H */
> diff --git a/drivers/infiniband/core/core_security.c
> b/drivers/infiniband/core/core_security.c
> new file mode 100644
> index 000..768edea
> --- /dev/null
> +++ b/drivers/infiniband/core/core_security.c
> @@ -0,0 +1,331 @@
> +/*
> + * Copyright (c) 2016 Mellanox Technologies Ltd. All rights reserved.
> + *
> + * This software is available to you under a choice of one of two
> + * licenses. You may choose to be licensed under the terms of the GNU
> + * General Public License (GPL) Version 2, available from the file
> + * COPYING in the main directory of this source tree, or the
> + * OpenIB.org BSD license below:
> + *
> + * Redistribution and use in source and binary forms, with or
> + * without modification, are permitted provided that the following
> + * conditions are met:
Re: [RFC PATCH v2 09/13] ib/core: Enforce PKey security when modifying QPs
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
>> drivers/infiniband/core/core_priv.h | 41
>> drivers/infiniband/core/core_security.c | 331
>> +++
>
> We are already in core, there is no need to call files core_XXX.
I can change core_security.c to security.c. core_priv.h already existed
so I won't change that in this patch.
>> +int ib_security_create_qp_security(struct ib_qp *qp);
>
> Why do we need xx_SECURITY_x_SECURITY in name?
I had called it ib_security_create_qp. Eli thought that implied more
work that it actually does. All the none static functions are named
ib_security_xxx, which I kind of liked as a convention, but I can
abandon that and go with ib_create_qp_security.
>> +return check_pkey && (qp->qp_num != IB_QPT_SMI &&
>> + qp->qp_num != IB_QPT_GSI);
>
> IB_QPT_SMI and IB_QPT_GSI are declared as struct ib_qp_type and setted
> in qp->qp_type and not in qp->qp_num.
>> +}
>> +
>> +static int check_alt_pkey(const struct ib_qp *qp,
>> + const struct ib_qp_attr *qp_attr,
>> + int qp_attr_mask)
>> +{
>> +bool check_alt_pkey = !!(qp_attr_mask & IB_QP_ALT_PATH);
>> +
>> +return check_alt_pkey && (qp->qp_num != IB_QPT_SMI &&
>> + qp->qp_num != IB_QPT_GSI);
>> +}
>
> The same as above.
I'll fix this.
>> +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.
___
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 1/2 v2] policycoreutils/hll/pp: Warn if module name different than output filename
On Thu, Apr 7, 2016 at 11:06 AM, James Carter wrote:
>
> Since CIL treats files as modules and does not have a separate
> module statement it can cause confusion when a Refpolicy module
> has a name that is not the same as its base filename because older
> SELinux userspaces will refer to the module by its module name while
> a CIL-based userspace will refer to it by its filename.
>
> Because of this, provide a warning message when converting a policy
> package to CIL and the output filename is different than the module
> name.
>
> Signed-off-by: James Carter
> ---
> policycoreutils/hll/pp/pp.c | 28
> 1 file changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/policycoreutils/hll/pp/pp.c b/policycoreutils/hll/pp/pp.c
> index 866734f..8621b50 100644
> --- a/policycoreutils/hll/pp/pp.c
> +++ b/policycoreutils/hll/pp/pp.c
> @@ -28,6 +28,7 @@
>
> #include
> #include
> +#include
>
> char *progname;
>
> @@ -68,6 +69,8 @@ int main(int argc, char **argv)
> { NULL, 0, NULL, 0 }
> };
> struct sepol_module_package *mod_pkg = NULL;
> + char *ifile = NULL;
> + char *ofile = NULL;
> FILE *in = NULL;
> FILE *out = NULL;
> int outfd = -1;
> @@ -89,20 +92,23 @@ int main(int argc, char **argv)
> }
>
> if (argc >= optind + 1 && strcmp(argv[1], "-") != 0) {
> - in = fopen(argv[1], "rb");
> + ifile = argv[1];
> + in = fopen(ifile, "rb");
> if (in == NULL) {
> - log_err("Failed to open %s: %s", argv[1],
> strerror(errno));
> + log_err("Failed to open %s: %s", ifile,
> strerror(errno));
> rc = -1;
> goto exit;
> }
> } else {
> + ifile = "stdin";
> in = stdin;
> }
>
> if (argc >= optind + 2 && strcmp(argv[2], "-") != 0) {
> - out = fopen(argv[2], "w");
> + ofile = argv[2];
> + out = fopen(ofile, "w");
> if (out == NULL) {
> - log_err("Failed to open %s: %s", argv[2],
> strerror(errno));
> + log_err("Failed to open %s: %s", ofile,
> strerror(errno));
> rc = -1;
> goto exit;
> }
> @@ -122,6 +128,20 @@ int main(int argc, char **argv)
> fclose(in);
> in = NULL;
>
> + if (ofile) {
> + char *mod_name = mod_pkg->policy->p.name;
> + char *cil_path = strdup(ofile);
Check if strdup fails here and also in the checkmodule patch?
> + char *cil_name = basename(cil_path);
> + char *separator = strrchr(cil_name, '.');
> + if (separator) {
> + *separator = '\0';
> + }
> + if (strcmp(mod_name, cil_name) != 0) {
> + fprintf(stderr, "Warning: SELinux userspace will
> refer to the module from %s as %s rather than %s\n", ifile, cil_name,
> mod_name);
> + }
> + free(cil_path);
> + }
> +
> rc = sepol_module_package_to_cil(out, mod_pkg);
> if (rc != 0) {
> goto exit;
> --
> 2.5.5
>
> ___
> 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.
___
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 08/13] ib/core: IB cache enhancements to support Infiniband security
On Thu, Apr 07, 2016 at 02:33:53AM +0300, Dan Jurgens wrote:
> From: Daniel Jurgens
>
> Cache the subnet prefix and add a function to access it. Enforcing
> security requires frequent queries of the subnet prefix and the pkeys in
> the pkey table.
>
> Signed-off-by: Daniel Jurgens
> Reviewed-by: Eli Cohen
> ---
> drivers/infiniband/core/cache.c | 36
> ++-
> drivers/infiniband/core/core_priv.h |3 ++
> include/rdma/ib_verbs.h |1 +
> 3 files changed, 39 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
> index cb00d59..83cf528 100644
> --- a/drivers/infiniband/core/cache.c
> +++ b/drivers/infiniband/core/cache.c
> @@ -925,6 +925,26 @@ int ib_get_cached_pkey(struct ib_device *device,
> }
> EXPORT_SYMBOL(ib_get_cached_pkey);
>
> +int ib_get_cached_subnet_prefix(struct ib_device *device,
> + u8port_num,
> + u64 *sn_pfx)
> +{
> + unsigned long flags;
> + int ret = 0;
> + int p = port_num - rdma_start_port(device);
> +
> + if (port_num < rdma_start_port(device) ||
> + port_num > rdma_end_port(device))
> + return -EINVAL;
> +
> + read_lock_irqsave(&device->cache.lock, flags);
> + *sn_pfx = device->cache.subnet_prefix_cache[p];
> + read_unlock_irqrestore(&device->cache.lock, flags);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(ib_get_cached_subnet_prefix);
> +
> int ib_find_cached_pkey(struct ib_device *device,
> u8port_num,
> u16 pkey,
> @@ -1101,6 +1121,8 @@ static void ib_cache_update(struct ib_device *device,
>
> device->cache.lmc_cache[port - rdma_start_port(device)] = tprops->lmc;
>
> + device->cache.subnet_prefix_cache[port - rdma_start_port(device)] =
> + tprops->subnet_prefix;
> write_unlock_irq(&device->cache.lock);
>
> kfree(gid_cache);
> @@ -1159,8 +1181,19 @@ int ib_cache_setup_one(struct ib_device *device)
> (rdma_end_port(device) -
> rdma_start_port(device) + 1),
> GFP_KERNEL);
> +
> + device->cache.subnet_prefix_cache = kcalloc((rdma_end_port(device) -
> + rdma_start_port(device) +
> 1),
> +
> sizeof(*device->cache.subnet_prefix_cache),
> + GFP_KERNEL);
> +
> if (!device->cache.pkey_cache ||
> - !device->cache.lmc_cache) {
> + !device->cache.lmc_cache ||
> + !device->cache.subnet_prefix_cache) {
> + kfree(device->cache.pkey_cache);
> + kfree(device->cache.lmc_cache);
> + kfree(device->cache.subnet_prefix_cache);
> +
> pr_warn("Couldn't allocate cache for %s\n", device->name);
Please remove this print. You won't miss failure in memory allocation.
> return -ENOMEM;
> }
> @@ -1204,6 +1237,7 @@ void ib_cache_release_one(struct ib_device *device)
> gid_table_release_one(device);
signature.asc
Description: Digital signature
___
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 08/13] ib/core: IB cache enhancements to support Infiniband security
On 4/6/2016 9:53 PM, Leon Romanovsky wrote:
> On Thu, Apr 07, 2016 at 02:33:53AM +0300, Dan Jurgens wrote:
>> From: Daniel Jurgens
>>
>> +int ret = 0;
>
> It is not needed, just return 0 directly.
Okay
On 4/7/2016 10:24 AM, Leon Romanovsky wrote:
> On Thu, Apr 07, 2016 at 02:33:53AM +0300, Dan Jurgens wrote:
>> pr_warn("Couldn't allocate cache for %s\n", device->name);
>
> Please remove this print. You won't miss failure in memory allocation.
Sure, I can do 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 2/2 v2] checkpolicy: Warn if module name different than output filename
On 04/07/2016 11:06 AM, James Carter wrote:
Since CIL treats files as modules and does not have a separate
module statement it can cause confusion when a Refpolicy module
has a name that is different than its base filename because older
SELinux userspaces will refer to the module by its module name while
a CIL-based userspace will refer to it by its filename.
Because of this, provide a warning message when compiling a module and
the output filename is different than the module name.
Signed-off-by: James Carter
---
checkpolicy/checkmodule.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/checkpolicy/checkmodule.c b/checkpolicy/checkmodule.c
index 5957d29..d807620 100644
--- a/checkpolicy/checkmodule.c
+++ b/checkpolicy/checkmodule.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
@@ -258,6 +259,20 @@ int main(int argc, char **argv)
}
}
+ if (policy_type != POLICY_BASE && outfile) {
+ char *mod_name = modpolicydb.name;
+ char *out_path = strdup(outfile);
+ char *out_name = basename(out_path);
+ char *separator = strrchr(out_name, '.');
+ if (separator) {
+ *separator = '\0';
+ }
+ if (strcmp(mod_name, out_name) != 0) {
+ fprintf(stderr, "Warning: SELinux userspace will refer to
the module from %s as %s rather than as %s\n", file, out_name, mod_name);
+ }
+ free(out_path);
+ }
+
if (modpolicydb.policy_type == POLICY_BASE && !cil) {
/* Verify that we can successfully expand the base module. */
policydb_t kernpolicydb;
Why not fail rather then warn. Don't let me do stupid things...
___
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.
[PATCH 0/2 v2] Warn if module name different than output filename
Since CIL treats files as modules and does not have a separate module statement it can cause confusion when a Refpolicy module has a name that is not the same as its base filename because older SELinux userspaces will refer to the module by its module name while a CIL-based userspace will refer to it by its filename. Because of this, provide a warning message when converting a policy package to CIL or compiling a module and the output filename is different than the module name. Changes from v1: - Added a "Warning:" prefix - Removed checks against the input filename - Since there are now only two checks and the base filename is used in the warning message, it no longer made sense to create common helper functions in libsepol. James Carter (2): policycoreutils/hll/pp: Warn if module name different than output filename checkpolicy: Warn if module name different than output filename checkpolicy/checkmodule.c | 15 +++ policycoreutils/hll/pp/pp.c | 28 2 files changed, 39 insertions(+), 4 deletions(-) -- 2.5.5 ___ 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.
[PATCH 1/2 v2] policycoreutils/hll/pp: Warn if module name different than output filename
Since CIL treats files as modules and does not have a separate
module statement it can cause confusion when a Refpolicy module
has a name that is not the same as its base filename because older
SELinux userspaces will refer to the module by its module name while
a CIL-based userspace will refer to it by its filename.
Because of this, provide a warning message when converting a policy
package to CIL and the output filename is different than the module
name.
Signed-off-by: James Carter
---
policycoreutils/hll/pp/pp.c | 28
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/policycoreutils/hll/pp/pp.c b/policycoreutils/hll/pp/pp.c
index 866734f..8621b50 100644
--- a/policycoreutils/hll/pp/pp.c
+++ b/policycoreutils/hll/pp/pp.c
@@ -28,6 +28,7 @@
#include
#include
+#include
char *progname;
@@ -68,6 +69,8 @@ int main(int argc, char **argv)
{ NULL, 0, NULL, 0 }
};
struct sepol_module_package *mod_pkg = NULL;
+ char *ifile = NULL;
+ char *ofile = NULL;
FILE *in = NULL;
FILE *out = NULL;
int outfd = -1;
@@ -89,20 +92,23 @@ int main(int argc, char **argv)
}
if (argc >= optind + 1 && strcmp(argv[1], "-") != 0) {
- in = fopen(argv[1], "rb");
+ ifile = argv[1];
+ in = fopen(ifile, "rb");
if (in == NULL) {
- log_err("Failed to open %s: %s", argv[1],
strerror(errno));
+ log_err("Failed to open %s: %s", ifile,
strerror(errno));
rc = -1;
goto exit;
}
} else {
+ ifile = "stdin";
in = stdin;
}
if (argc >= optind + 2 && strcmp(argv[2], "-") != 0) {
- out = fopen(argv[2], "w");
+ ofile = argv[2];
+ out = fopen(ofile, "w");
if (out == NULL) {
- log_err("Failed to open %s: %s", argv[2],
strerror(errno));
+ log_err("Failed to open %s: %s", ofile,
strerror(errno));
rc = -1;
goto exit;
}
@@ -122,6 +128,20 @@ int main(int argc, char **argv)
fclose(in);
in = NULL;
+ if (ofile) {
+ char *mod_name = mod_pkg->policy->p.name;
+ char *cil_path = strdup(ofile);
+ char *cil_name = basename(cil_path);
+ char *separator = strrchr(cil_name, '.');
+ if (separator) {
+ *separator = '\0';
+ }
+ if (strcmp(mod_name, cil_name) != 0) {
+ fprintf(stderr, "Warning: SELinux userspace will refer
to the module from %s as %s rather than %s\n", ifile, cil_name, mod_name);
+ }
+ free(cil_path);
+ }
+
rc = sepol_module_package_to_cil(out, mod_pkg);
if (rc != 0) {
goto exit;
--
2.5.5
___
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 2/3] policycoreutils/hll/pp: Warn if module name different from filenames
On 03/26/2016 04:07 PM, Daniel J Walsh wrote:
On 03/25/2016 02:57 PM, James Carter wrote:
On 03/25/2016 02:45 PM, Stephen Smalley wrote:
On 03/25/2016 02:04 PM, James Carter wrote:
Since CIL treats files as modules and does not have a separate
module statement it can cause confusion when a Refpolicy module
has a name that is not the same as its base filename because
older SELinux userspaces will refer to the module by its module
name, but CIL will refer to the module by its filename.
When converting a policy package to CIL warn if the module name is
different from the pp filename or the CIL filename.
Signed-off-by: James Carter
---
policycoreutils/hll/pp/pp.c | 29 +
1 file changed, 25 insertions(+), 4 deletions(-)
diff --git a/policycoreutils/hll/pp/pp.c b/policycoreutils/hll/pp/pp.c
index 866734f..22cef0d 100644
--- a/policycoreutils/hll/pp/pp.c
+++ b/policycoreutils/hll/pp/pp.c
@@ -28,6 +28,7 @@
#include
#include
+#include
char *progname;
@@ -68,6 +69,8 @@ int main(int argc, char **argv)
{ NULL, 0, NULL, 0 }
};
struct sepol_module_package *mod_pkg = NULL;
+char *ifile = NULL;
+char *ofile = NULL;
FILE *in = NULL;
FILE *out = NULL;
int outfd = -1;
@@ -89,9 +92,10 @@ int main(int argc, char **argv)
}
if (argc >= optind + 1 && strcmp(argv[1], "-") != 0) {
-in = fopen(argv[1], "rb");
+ifile = argv[1];
+in = fopen(ifile, "rb");
if (in == NULL) {
-log_err("Failed to open %s: %s", argv[1], strerror(errno));
+log_err("Failed to open %s: %s", ifile, strerror(errno));
rc = -1;
goto exit;
}
@@ -100,9 +104,10 @@ int main(int argc, char **argv)
}
if (argc >= optind + 2 && strcmp(argv[2], "-") != 0) {
-out = fopen(argv[2], "w");
+ofile = argv[2];
+out = fopen(ofile, "w");
if (out == NULL) {
-log_err("Failed to open %s: %s", argv[2], strerror(errno));
+log_err("Failed to open %s: %s", ofile, strerror(errno));
rc = -1;
goto exit;
}
@@ -122,6 +127,22 @@ int main(int argc, char **argv)
fclose(in);
in = NULL;
+if (ifile) {
+rc = sepol_module_check_name_matches_filename(mod_pkg->policy, ifile);
+if (rc != 0) {
+fprintf(stderr,"Module name %s does not match pp file %s\n",
+sepol_module_get_name(mod_pkg->policy), ifile);
+}
+}
+
+if (ofile) {
+rc = sepol_module_check_name_matches_filename(mod_pkg->policy, ofile);
+if (rc != 0) {
+fprintf(stderr,"Module name %s does not match cil file %s\n",
+sepol_module_get_name(mod_pkg->policy), ofile);
+}
+}
So what, if anything, should the user take away from such warnings? We
likely ought to prefix them with "Warning:" or similar to indicate that
it is non-fatal. And perhaps tell them which name will need to be used
for subsequent commands.
What they need to take away is that the module is going to be referred to by
its filename from now on, so telling them that would be helpful. I originally
had prefixed the message with "Warning:", but I thought that might be too
strong and scary.
Jim
Why not make it fatal. Is there any reason a user would ever want to do this, I
would think this is almost always a mistake.
I think that it is almost always a mistake, but there were, at least in the
past, modules that did this and I didn't want to cause problems if they are
still in use.
Jim
+
rc = sepol_module_package_to_cil(out, mod_pkg);
if (rc != 0) {
goto exit;
--
James Carter
National Security Agency
___
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.
[PATCH 2/2 v2] checkpolicy: Warn if module name different than output filename
Since CIL treats files as modules and does not have a separate
module statement it can cause confusion when a Refpolicy module
has a name that is different than its base filename because older
SELinux userspaces will refer to the module by its module name while
a CIL-based userspace will refer to it by its filename.
Because of this, provide a warning message when compiling a module and
the output filename is different than the module name.
Signed-off-by: James Carter
---
checkpolicy/checkmodule.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/checkpolicy/checkmodule.c b/checkpolicy/checkmodule.c
index 5957d29..d807620 100644
--- a/checkpolicy/checkmodule.c
+++ b/checkpolicy/checkmodule.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
@@ -258,6 +259,20 @@ int main(int argc, char **argv)
}
}
+ if (policy_type != POLICY_BASE && outfile) {
+ char *mod_name = modpolicydb.name;
+ char *out_path = strdup(outfile);
+ char *out_name = basename(out_path);
+ char *separator = strrchr(out_name, '.');
+ if (separator) {
+ *separator = '\0';
+ }
+ if (strcmp(mod_name, out_name) != 0) {
+ fprintf(stderr, "Warning: SELinux userspace will refer
to the module from %s as %s rather than as %s\n", file, out_name, mod_name);
+ }
+ free(out_path);
+ }
+
if (modpolicydb.policy_type == POLICY_BASE && !cil) {
/* Verify that we can successfully expand the base module. */
policydb_t kernpolicydb;
--
2.5.5
___
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/2] selinux: avoid nf hooks overhead when not needed
Paul Moore wrote: > On Wed, Apr 6, 2016 at 6:14 PM, Florian Westphal wrote: > > netfilter hooks are per namespace -- so there is hook unregister when > > netns is destroyed. > > Looking around, I see the global and per-namespace registration > functions (nf_register_hook and nf_register_net_hook, respectively), > but I'm looking to see if/how newly created namespace inherit > netfilter hooks from the init network namespace ... if you can create > a network namespace and dodge the SELinux hooks, that isn't a good > thing from a SELinux point of view, although it might be a plus > depending on where you view Paolo's original patches ;) Heh :-) If you use nf_register_net_hook, the hook is only registered in the namespace. If you use nf_register_hook, the hook is put on a global list and registed in all existing namespaces. New namespaces will have the hook added as well (see netfilter_net_init -> nf_register_hook_list in netfilter/core.c ) Since nf_register_hook is used it should be impossible to get a netns that doesn't call these hooks. > > Do you think it makes sense to rework the patch to delay registering > > of the netfiler hooks until the system is in a state where they're > > needed, without the 'unregister' aspect? > > I would need to see the patch to say for certain, but in principle > that seems perfectly reasonable and I think would satisfy both the > netdev and SELinux camps - good suggestion. My main goal is to drop > the selinux_nf_ip_init() entirely so it can't be used as a ROP gadget. > > We might even be able to trim the secmark_active and peerlbl_active > checks in the SELinux netfilter hooks (an earlier attempt at > optimization; contrary to popular belief, I do care about SELinux > performance), although that would mean that enabling the network > access controls would be one way ... I guess you can disregard that > last bit, I'm thinking aloud again. One way is fine I think. > > Ideally this would even be per netns -- in perfect world we would > > be able to make it so that a new netns are created with an empty > > hook list. > > In general SELinux doesn't care about namespaces, for reasons that are > sorta beyond the scope of this conversation, so I would like to stick > to a all or nothing approach to enabling the SELinux netfilter hooks > across namespaces. Perhaps we can revisit this at a later time, but > let's keep it simple right now. Okay, I'd prefer to stick to your recommendation anyway wrt. to selinux (Casey, I read your comment regarding smack. Noted, we don't want to break smack either...) I think that in this case the entire question is: In your experience, how likely is a config where selinux is enabled BUT the hooks are not needed (i.e., where we hit the if (!selinux_policycap_netpeer) return NF_ACCEPT; if (!secmark_active && !peerlbl_active) return NF_ACCEPT; tests inside the hooks)? If such setups are uncommon we should just drop this idea or at least put it on the back burner until the more expensive netfilter hooks (conntrack, cough) are out of the way. Thanks, Florian ___ 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 08/13] ib/core: IB cache enhancements to support Infiniband security
On Thu, Apr 07, 2016 at 02:33:53AM +0300, Dan Jurgens wrote:
> From: Daniel Jurgens
>
> Cache the subnet prefix and add a function to access it. Enforcing
> security requires frequent queries of the subnet prefix and the pkeys in
> the pkey table.
>
> Signed-off-by: Daniel Jurgens
> Reviewed-by: Eli Cohen
> ---
> drivers/infiniband/core/cache.c | 36
> ++-
> drivers/infiniband/core/core_priv.h |3 ++
> include/rdma/ib_verbs.h |1 +
> 3 files changed, 39 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
> index cb00d59..83cf528 100644
> --- a/drivers/infiniband/core/cache.c
> +++ b/drivers/infiniband/core/cache.c
> @@ -925,6 +925,26 @@ int ib_get_cached_pkey(struct ib_device *device,
> }
> EXPORT_SYMBOL(ib_get_cached_pkey);
>
> +int ib_get_cached_subnet_prefix(struct ib_device *device,
> + u8port_num,
> + u64 *sn_pfx)
> +{
> + unsigned long flags;
> + int ret = 0;
It is not needed, just return 0 directly.
> + int p = port_num - rdma_start_port(device);
> +
> + if (port_num < rdma_start_port(device) ||
> + port_num > rdma_end_port(device))
> + return -EINVAL;
> +
> + read_lock_irqsave(&device->cache.lock, flags);
> + *sn_pfx = device->cache.subnet_prefix_cache[p];
> + read_unlock_irqrestore(&device->cache.lock, flags);
> +
> + return ret;
> +}
signature.asc
Description: Digital signature
___
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/2] selinux: avoid nf hooks overhead when not needed
Hi Casey, On Wed, 2016-04-06 at 14:43 -0700, Casey Schaufler wrote: > On 4/6/2016 2:51 AM, Paolo Abeni wrote: > > Currently, selinux always registers iptables POSTROUTING hooks regarless of > > the running policy needs for any action to be performed by them. > > > > Even the socket_sock_rcv_skb() is always registered, but it can result in a > > no-op > > depending on the current policy configuration. > > > > The above invocations in the kernel datapath are cause of measurable > > overhead in networking performance test. > > > > This patch series adds explicit notification for netlabel status change > > (other relevant status change, like xfrm and secmark, are already notified > > to > > LSM) and use this information in selinux to register the above hooks only > > when > > the current status makes them relevant, deregistering them when no-op > > > > Avoiding the LSM hooks overhead, in netperf UDP_STREAM test with small > > packets, > > gives about 5% performance improvement on rx and about 8% on tx. > > > > Paolo Abeni (2): > > security: add hook for netlabel status change notification > > selinux: implement support for dynamic net hook [de-]registration Thank you for the feedback. The patch series is an RFC, so it's still rough and not yet well tested in all possible scenarios. > Did you consider the fact that netlabel and the LSM socket > hooks are used by Smack as well as SELinux? Actually yes. The patch series itself is explicitly targeted at reducing some overhead introduced by selinux in network loads (I'm sorry, now I see that the last sentence in the cover letter is misleading), and it tries to achieve that result without affecting others LSM users. The first patch in the series just introduces an optional LSM hook (netlbl_changed) that is invoked every time the 'netlabel_mgmt_protocount' values is changed. It do not modify the behavior nor meaning of any of the existing hooks and/or netlabel APIs. It's up to the security module to leverage (or not) the new one. > Did you measure the impact that your changes have on Smack? Actually I didn't. This is one of the reasons I posted the patch as RFC. As per design security modules not implementing 'netlbl_changed' should not be affected. Am I missing something ? Regards, Paolo ___ 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/2] selinux: avoid nf hooks overhead when not needed
Paul Moore wrote: > On Wed, Apr 6, 2016 at 5:51 AM, Paolo Abeni wrote: > > Currently, selinux always registers iptables POSTROUTING hooks regarless of > > the running policy needs for any action to be performed by them. > > > > Even the socket_sock_rcv_skb() is always registered, but it can result in a > > no-op > > depending on the current policy configuration. > > > > The above invocations in the kernel datapath are cause of measurable > > overhead in networking performance test. > > > > This patch series adds explicit notification for netlabel status change > > (other relevant status change, like xfrm and secmark, are already notified > > to > > LSM) and use this information in selinux to register the above hooks only > > when > > the current status makes them relevant, deregistering them when no-op > > > > Avoiding the LSM hooks overhead, in netperf UDP_STREAM test with small > > packets, > > gives about 5% performance improvement on rx and about 8% on tx. > > [NOTE: added the SELinux mailing list to the CC line, please include > when submitting SELinux patches] > > While I appreciate the patch and the work that went into development > and testing, I'm going to reject this patch on the grounds that it > conflicts with work we've just started thinking about which should > bring some tangible security benefit. > > The recent addition of post-init read only memory opens up some > interesting possibilities for SELinux and LSMs in general, the thing > which we've just started looking at is marking the LSM hook structure > read only after init. There are some complicating factors for > SELinux, but I'm confident those can be resolved, and from what I can > tell marking the hooks read only will have no effect on other LSMs. > While marking the LSM hook structure doesn't directly affect the > SELinux netfilter hooks, once we remove the ability to deregister the > LSM hooks we will have no need to support deregistering netfilter > hooks and I expect we will drop that functionality as well to help > decrease the risk of tampering. netfilter hooks are per namespace -- so there is hook unregister when netns is destroyed. Do you think it makes sense to rework the patch to delay registering of the netfiler hooks until the system is in a state where they're needed, without the 'unregister' aspect? Ideally this would even be per netns -- in perfect world we would be able to make it so that a new netns are created with an empty hook list. Thanks. ___ 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.
