From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760701AbZBXVva (ORCPT ); Tue, 24 Feb 2009 16:51:30 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756718AbZBXVvT (ORCPT ); Tue, 24 Feb 2009 16:51:19 -0500 Received: from g4t0017.houston.hp.com ([15.201.24.20]:11084 "EHLO g4t0017.houston.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753290AbZBXVvS (ORCPT ); Tue, 24 Feb 2009 16:51:18 -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 16:50:42 -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: <49A46678.1030803@numericable.fr> In-Reply-To: <49A46678.1030803@numericable.fr> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200902241650.42554.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 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. 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 for a variety of reasons (actually it is netlbl_sock_setattr() which will fail) and if it does fail you could end up in a bad place. If you want to ensure _all_ the packets are labeled correctly based on destination address Smack will need to adopt the netfilter hooks as mentioned previously. -- paul moore linux @ hp