Re: [PATCH V2] IB/sa: Restrict SA Netlink to admin users

[ira.weiny]

On Mon, Aug 10, 2015 at 10:46:55PM -0400, ira.we...@intel.com wrote:
 From: Ira Weiny ira.we...@intel.com
 
 The recently added SA Netlink service requires admin privileges to receive
 kernel requests.  This is only partially sufficient to protect the kernel from
 malicious users.  This patch fixes two issues.
 
   1) Path responses from user space could be spoofed if the sequence
  number was properly guessed.
   2) The set timeout request message could be issued by any user.
 
 Ignore these messages if not submitted by an admin user.
 
 Fixes: 6619209af36c (IB/sa: Route SA pathrecord query through netlink)
 Signed-off-by: Ira Weiny ira.we...@intel.com
 
 ---
 Changes from V1:
   Use netlink_net_capable rather than ns_capable

Doug,

As per the thread with the V1 patch we are looking to merge this into a v9 of
Kaikes series once we do some more testing with the netlink_bind and
namespaces.

So you can safely ignore both v1 and this patch.

Thanks,
Ira

 
 Doug let me know if you would prefer that I get Kaike to squash this into the
 original patch.
 
  drivers/infiniband/core/sa_query.c | 16 
  1 file changed, 16 insertions(+)
 
 diff --git a/drivers/infiniband/core/sa_query.c 
 b/drivers/infiniband/core/sa_query.c
 index 70ceec4df02a..6778644a6957 100644
 --- a/drivers/infiniband/core/sa_query.c
 +++ b/drivers/infiniband/core/sa_query.c
 @@ -699,6 +699,12 @@ static int ib_nl_handle_set_timeout(struct sk_buff *skb,
   struct nlattr *tb[LS_NLA_TYPE_MAX + 1];
   int ret;
  
 + if (!netlink_net_capable(skb, CAP_NET_ADMIN)) {
 + pr_warn_ratelimited(SA netlink: invalid perm for set timeout: 
 `%s'.\n,
 + current-comm);
 + return -EPERM;
 + }
 +
   ret = nla_parse(tb, LS_NLA_TYPE_MAX, nlmsg_data(nlh), nlmsg_len(nlh),
   NULL);
   attr = (const struct nlattr *)tb[LS_NLA_TYPE_TIMEOUT];
 @@ -706,6 +712,9 @@ static int ib_nl_handle_set_timeout(struct sk_buff *skb,
   goto settimeout_out;
  
   timeout = *(int *) nla_data(attr);
 +
 + pr_info(SA netlink: timeout: %d\n, timeout);
 +
   if (timeout  IB_SA_LOCAL_SVC_TIMEOUT_MIN)
   timeout = IB_SA_LOCAL_SVC_TIMEOUT_MIN;
   if (timeout  IB_SA_LOCAL_SVC_TIMEOUT_MAX)
 @@ -754,6 +763,12 @@ static int ib_nl_handle_resolve_resp(struct sk_buff *skb,
   int found = 0;
   int ret;
  
 + if (!netlink_net_capable(skb, CAP_NET_ADMIN)) {
 + pr_warn_ratelimited(SA netlink: invalid perm for response: 
 `%s'.\n,
 + current-comm);
 + return -EPERM;
 + }
 +
   spin_lock_irqsave(ib_nl_request_lock, flags);
   list_for_each_entry(query, ib_nl_request_list, list) {
   /*
 @@ -770,6 +785,7 @@ static int ib_nl_handle_resolve_resp(struct sk_buff *skb,
  
   if (!found) {
   spin_unlock_irqrestore(ib_nl_request_lock, flags);
 + pr_err_ratelimited(SA netlink: got unmatched response\n);
   goto resp_out;
   }
  
 -- 
 1.8.2
 
--
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

Re: [PATCH V2] IB/sa: Restrict SA Netlink to admin users

[Doug Ledford]

On 08/12/2015 03:43 PM, ira.weiny wrote:
 On Mon, Aug 10, 2015 at 10:46:55PM -0400, ira.we...@intel.com wrote:
 From: Ira Weiny ira.we...@intel.com

 The recently added SA Netlink service requires admin privileges to receive
 kernel requests.  This is only partially sufficient to protect the kernel 
 from
 malicious users.  This patch fixes two issues.

  1) Path responses from user space could be spoofed if the sequence
 number was properly guessed.
  2) The set timeout request message could be issued by any user.

 Ignore these messages if not submitted by an admin user.

 Fixes: 6619209af36c (IB/sa: Route SA pathrecord query through netlink)
 Signed-off-by: Ira Weiny ira.we...@intel.com

 ---
 Changes from V1:
  Use netlink_net_capable rather than ns_capable
 
 Doug,
 
 As per the thread with the V1 patch we are looking to merge this into a v9 of
 Kaikes series once we do some more testing with the netlink_bind and
 namespaces.
 
 So you can safely ignore both v1 and this patch.

Ok.


-- 
Doug Ledford dledf...@redhat.com
  GPG KeyID: 0E572FDD




signature.asc
Description: OpenPGP digital signature

[PATCH V2] IB/sa: Restrict SA Netlink to admin users

[ira . weiny]

From: Ira Weiny ira.we...@intel.com

The recently added SA Netlink service requires admin privileges to receive
kernel requests.  This is only partially sufficient to protect the kernel from
malicious users.  This patch fixes two issues.

1) Path responses from user space could be spoofed if the sequence
   number was properly guessed.
2) The set timeout request message could be issued by any user.

Ignore these messages if not submitted by an admin user.

Fixes: 6619209af36c (IB/sa: Route SA pathrecord query through netlink)
Signed-off-by: Ira Weiny ira.we...@intel.com

---
Changes from V1:
Use netlink_net_capable rather than ns_capable

Doug let me know if you would prefer that I get Kaike to squash this into the
original patch.

 drivers/infiniband/core/sa_query.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/drivers/infiniband/core/sa_query.c 
b/drivers/infiniband/core/sa_query.c
index 70ceec4df02a..6778644a6957 100644
--- a/drivers/infiniband/core/sa_query.c
+++ b/drivers/infiniband/core/sa_query.c
@@ -699,6 +699,12 @@ static int ib_nl_handle_set_timeout(struct sk_buff *skb,
struct nlattr *tb[LS_NLA_TYPE_MAX + 1];
int ret;
 
+   if (!netlink_net_capable(skb, CAP_NET_ADMIN)) {
+   pr_warn_ratelimited(SA netlink: invalid perm for set timeout: 
`%s'.\n,
+   current-comm);
+   return -EPERM;
+   }
+
ret = nla_parse(tb, LS_NLA_TYPE_MAX, nlmsg_data(nlh), nlmsg_len(nlh),
NULL);
attr = (const struct nlattr *)tb[LS_NLA_TYPE_TIMEOUT];
@@ -706,6 +712,9 @@ static int ib_nl_handle_set_timeout(struct sk_buff *skb,
goto settimeout_out;
 
timeout = *(int *) nla_data(attr);
+
+   pr_info(SA netlink: timeout: %d\n, timeout);
+
if (timeout  IB_SA_LOCAL_SVC_TIMEOUT_MIN)
timeout = IB_SA_LOCAL_SVC_TIMEOUT_MIN;
if (timeout  IB_SA_LOCAL_SVC_TIMEOUT_MAX)
@@ -754,6 +763,12 @@ static int ib_nl_handle_resolve_resp(struct sk_buff *skb,
int found = 0;
int ret;
 
+   if (!netlink_net_capable(skb, CAP_NET_ADMIN)) {
+   pr_warn_ratelimited(SA netlink: invalid perm for response: 
`%s'.\n,
+   current-comm);
+   return -EPERM;
+   }
+
spin_lock_irqsave(ib_nl_request_lock, flags);
list_for_each_entry(query, ib_nl_request_list, list) {
/*
@@ -770,6 +785,7 @@ static int ib_nl_handle_resolve_resp(struct sk_buff *skb,
 
if (!found) {
spin_unlock_irqrestore(ib_nl_request_lock, flags);
+   pr_err_ratelimited(SA netlink: got unmatched response\n);
goto resp_out;
}
 
-- 
1.8.2

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