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

["ira.weiny" <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]

On Mon, Aug 10, 2015 at 10:46:55PM -0400, ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org wrote:
> From: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> 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.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> ---
> 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html