From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760528AbZBXWi2 (ORCPT ); Tue, 24 Feb 2009 17:38:28 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753785AbZBXWiS (ORCPT ); Tue, 24 Feb 2009 17:38:18 -0500 Received: from g1t0027.austin.hp.com ([15.216.28.34]:1045 "EHLO g1t0027.austin.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756526AbZBXWiR (ORCPT ); Tue, 24 Feb 2009 17:38:17 -0500 From: Paul Moore Organization: Hewlett-Packard To: etienne Subject: Re: [PATCH][SMACK] add a socket_post_accept hook to fix netlabel issues with labeled TCP servers V1 Date: Tue, 24 Feb 2009 17:38:08 -0500 User-Agent: KMail/1.11.0 (Linux/2.6.27-gentoo-r8; KDE/4.2.0; i686; ; ) Cc: Casey Schaufler , Linux Kernel Mailing List , LSM References: <49A472BA.8090805@numericable.fr> In-Reply-To: <49A472BA.8090805@numericable.fr> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200902241738.08877.paul.moore@hp.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday 24 February 2009 05:20:42 pm etienne wrote: > Paul Moore wrote: > > On Tuesday 24 February 2009 04:28:24 pm etienne wrote: > >> /** > >> + * smack_socket_post_access - post access check > >> + * @sock: the socket > >> + * @newsock : the grafted sock > >> + * > >> + * we have to match client IP against smack_host_label() > >> + */ > >> +static void smack_socket_post_accept(struct socket *sock, struct > >> socket *newsock) +{ > >> + char *hostsp; > >> + struct sockaddr_storage address; > >> + struct sockaddr_in *sin; > >> + struct sockaddr_in6 *sin6; > >> + struct in6_addr *addr6; > >> + struct socket_smack *ssp = newsock->sk->sk_security; > >> + int len; > >> + > >> + if (sock->sk == NULL) > >> + return; > >> + > >> + /* sockets can listen on both IPv4 & IPv6, > >> + and fallback to V4 if client is V4 */ > >> + if (newsock->sk->sk_family != AF_INET && newsock->sk->sk_family != > >> AF_INET6) + return; > >> + > >> + /* get the client IP address **/ > >> + newsock->ops->getname(newsock, (struct sockaddr *)&address, &len, 2); > >> + > >> + switch (newsock->sk->sk_family) { > >> + case AF_INET: > >> + sin = (struct sockaddr_in *)&address; > >> + break; > >> + case AF_INET6: > >> + sin6 = (struct sockaddr_in6 *)&address; > >> + addr6 = &sin6->sin6_addr; > >> + /* if a V4 client connects to a V6 listening server, > >> + * we will get a IPV6_ADDR_MAPPED mapped address here > >> + * we have to handle this case too > >> + * the test below is ipv6_addr_type()== IPV6_ADDR_MAPPED > >> + * without the requirement to have IPv6 compiled in > >> + */ > >> + if ((addr6->s6_addr32[0] | addr6->s6_addr32[1]) == 0 && > >> + addr6->s6_addr32[2] == htonl(0x0000ffff)) { > >> + __be32 addr = sin6->sin6_addr.s6_addr32[3]; > >> + __be16 port = sin6->sin6_port; > >> + sin = (struct sockaddr_in *)&address; > >> + sin->sin_family = AF_INET; > >> + sin->sin_port = port; > >> + sin->sin_addr.s_addr = addr; > >> + } else { > >> + /* standard IPv6, we'll send unlabeled */ > >> + smack_netlabel(newsock->sk, SMACK_UNLABELED_SOCKET); > >> + return; > >> + } > >> + break; > >> + default: > >> + /** not possible to be there **/ > >> + return; > >> + } > >> + /* so, is there a label for the source IP **/ > >> + hostsp = smack_host_label(sin); > >> + > >> + if (hostsp == NULL) { > >> + if (ssp->smk_labeled != SMACK_CIPSO_SOCKET) > >> + smack_netlabel(newsock->sk, SMACK_CIPSO_SOCKET); > >> + return; > >> + } > >> + if (ssp->smk_labeled != SMACK_UNLABELED_SOCKET) > >> + smack_netlabel(newsock->sk, SMACK_UNLABELED_SOCKET); > >> + return; > >> +} > > > > NAK, you can't ignore return values like that. > > > > I'm sorry I didn't get a chance to respond to your email from this > > morning, but the problem with the post_accept() hook is that you can't > > fail in this hook. There has been a _lot_ of discussion about this over > > the past couple of years on the LSM list. You should check the archives > > for all the details but the main problem is that the post_accept() hook > > is too late to deny the incoming connection so you can't reject the > > connection at that point in any sane manner. > > well, i don't want to reject the connection here :) > > >I think I'm going to draft a patch to remove the post_accept() > > hook since no one in-tree is using it and it's existence seems to cause > > more problems than it solves. > > > > Now, I understand that your patch doesn't actually enforce any access > > controls but it does call smack_netlabel() in several places and that > > function can fail > > The smack_netlabel(newsock->sk, SMACK_CIPSO_SOCKET) can failed, but has no > interest in this function (because the socket has already be > SMACK_CIPSO_SOCKET labeled by the policy) I can remove it. > > but smack_netlabel(SMACK_UNLABELED_SOCKET) cannot fail, and that's what i'm > interested in could this make the patch acceptable? Please elaborate a bit more on how you would intend a user to configure and make use of this. Also, in what cases would you remove the NetLabel from a socket? What cases would you keep it? -- paul moore linux @ hp