[PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Tetsuo Handa]

Hello.

The security_socket_post_accept() hook was recently removed because this hook
was not used by any in-tree modules and its existence continued to cause
problems by confusing people about what can be safely accomplished using this
hook. Now, TOMOYO became in-tree and TOMOYO wants to add network access control
in 2.6.31.

TOMOYO is not a label based access control and won't cause packet labeling
problem. TOMOYO won't care as long as packets are not copied to userspace.

Regards.
--------------------
Subject: LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

This patch allows LSM modules to filter incoming connections/datagrams based on
the process's security context who is attempting to pick up.

There are already hooks for filtering incoming connections/datagrams based on
the socket's security context, but these hooks are not applicable when someone
wants to do TCP Wrapper-like filtering (e.g. App1 is permitted to accept TCP
connections from 192.168.0.0/16) because nobody can tell who picks them up
before the moment of accept()/recvmsg() request.

Precautions: These hooks have a side effect if improperly used.

If a socket is shared by multiple processes with different policy, the process
who should be able to accept this connection will not be able to accept this
connection because socket_post_accept() aborts this connection.
This is needed because the process who must not be able to accept this
connection will repeat accept() request (and consume CPU resource) forever
if socket_post_accept() doesn't abort this connection.

Similarly, if a socket is shared by multiple processes with different policy,
the process who should be able to pick up this datagram will not be able to
pick up this datagram because socket_post_recv_datagram() discards this
datagram.
This is needed because the process who must not be able to pick up this
datagram will repeat recvmsg() request (and consume CPU resource) forever
if socket_post_recv_datagram() doesn't discard this datagram.

Therefore, don't give different policy between processes who share one socket.
Otherwise, some connections/datagrams cannot be delivered to intended process.

Signed-off-by: Kentaro Takeda <takedakn@nttdata.co.jp>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Toshiharu Harada <haradats@nttdata.co.jp>

 include/linux/security.h |   39 +++++++++++++++++++++++++++++++++++++++
 net/core/datagram.c      |   29 ++++++++++++++++++++++++++++-
 net/socket.c             |    5 +++++
 security/capability.c    |   13 +++++++++++++
 security/security.c      |   11 +++++++++++
 5 files changed, 96 insertions(+), 1 deletion(-)

---

--- security-testing-2.6.git.orig/include/linux/security.h
+++ security-testing-2.6.git/include/linux/security.h
@@ -880,6 +880,17 @@ static inline void security_free_mnt_opt
  *	@sock contains the listening socket structure.
  *	@newsock contains the newly created server socket for connection.
  *	Return 0 if permission is granted.
+ * @socket_post_accept:
+ *	This hook allows a security module to filter connections from unwanted
+ *	peers based on the process accepting this connection.
+ *	The connection will be aborted if this hook returns nonzero.
+ *	This hook is not designed for updating security attributes of
+ *	an accept()ed socket, for the accept()ed socket has already sent
+ *	several packets (e.g. TCP's SYN/ACK packet and some ACK packets for
+ *	incoming data) before this hook is called.
+ *	@sock contains the listening socket structure.
+ *	@newsock contains the newly created server socket for connection.
+ *	Return 0 if permission is granted.
  * @socket_sendmsg:
  *	Check permission before transmitting a message to another socket.
  *	@sock contains the socket structure.
@@ -893,6 +904,15 @@ static inline void security_free_mnt_opt
  *	@size contains the size of message structure.
  *	@flags contains the operational flags.
  *	Return 0 if permission is granted.
+ * @socket_post_recv_datagram:
+ *	Check permission after receiving a datagram.
+ *	This hook allows a security module to filter packets
+ *	from unwanted peers based on the process receiving this datagram.
+ *	The packet will be discarded if this hook returns nonzero.
+ *	@sk contains the socket.
+ *	@skb contains the socket buffer.
+ *	@flags contains the operational flags.
+ *	Return 0 if permission is granted.
  * @socket_getsockname:
  *	Check permission before the local address (name) of the socket object
  *	@sock is retrieved.
@@ -1549,10 +1569,13 @@ struct security_operations {
 			       struct sockaddr *address, int addrlen);
 	int (*socket_listen) (struct socket *sock, int backlog);
 	int (*socket_accept) (struct socket *sock, struct socket *newsock);
+	int (*socket_post_accept) (struct socket *sock, struct socket *newsock);
 	int (*socket_sendmsg) (struct socket *sock,
 			       struct msghdr *msg, int size);
 	int (*socket_recvmsg) (struct socket *sock,
 			       struct msghdr *msg, int size, int flags);
+	int (*socket_post_recv_datagram) (struct sock *sk, struct sk_buff *skb,
+					  unsigned int flags);
 	int (*socket_getsockname) (struct socket *sock);
 	int (*socket_getpeername) (struct socket *sock);
 	int (*socket_getsockopt) (struct socket *sock, int level, int optname);
@@ -2530,9 +2553,12 @@ int security_socket_bind(struct socket *
 int security_socket_connect(struct socket *sock, struct sockaddr *address, int addrlen);
 int security_socket_listen(struct socket *sock, int backlog);
 int security_socket_accept(struct socket *sock, struct socket *newsock);
+int security_socket_post_accept(struct socket *sock, struct socket *newsock);
 int security_socket_sendmsg(struct socket *sock, struct msghdr *msg, int size);
 int security_socket_recvmsg(struct socket *sock, struct msghdr *msg,
 			    int size, int flags);
+int security_socket_post_recv_datagram(struct sock *sk, struct sk_buff *skb,
+				       unsigned int flags);
 int security_socket_getsockname(struct socket *sock);
 int security_socket_getpeername(struct socket *sock);
 int security_socket_getsockopt(struct socket *sock, int level, int optname);
@@ -2608,6 +2634,12 @@ static inline int security_socket_accept
 	return 0;
 }
 
+static inline int security_socket_post_accept(struct socket *sock,
+					      struct socket *newsock)
+{
+	return 0;
+}
+
 static inline int security_socket_sendmsg(struct socket *sock,
 					  struct msghdr *msg, int size)
 {
@@ -2621,6 +2653,13 @@ static inline int security_socket_recvms
 	return 0;
 }
 
+static inline int security_socket_post_recv_datagram(struct sock *sk,
+						     struct sk_buff *skb,
+						     unsigned int flags)
+{
+	return 0;
+}
+
 static inline int security_socket_getsockname(struct socket *sock)
 {
 	return 0;
--- security-testing-2.6.git.orig/net/core/datagram.c
+++ security-testing-2.6.git/net/core/datagram.c
@@ -55,6 +55,7 @@
 #include <net/checksum.h>
 #include <net/sock.h>
 #include <net/tcp_states.h>
+#include <linux/security.h>
 
 /*
  *	Is a socket 'connection oriented' ?
@@ -148,6 +149,7 @@ struct sk_buff *__skb_recv_datagram(stru
 {
 	struct sk_buff *skb;
 	long timeo;
+	unsigned long cpu_flags;
 	/*
 	 * Caller is allowed not to check sk->sk_err before skb_recv_datagram()
 	 */
@@ -165,7 +167,6 @@ struct sk_buff *__skb_recv_datagram(stru
 		 * Look at current nfs client by the way...
 		 * However, this function was corrent in any case. 8)
 		 */
-		unsigned long cpu_flags;
 
 		spin_lock_irqsave(&sk->sk_receive_queue.lock, cpu_flags);
 		skb = skb_peek(&sk->sk_receive_queue);
@@ -179,6 +180,14 @@ struct sk_buff *__skb_recv_datagram(stru
 		}
 		spin_unlock_irqrestore(&sk->sk_receive_queue.lock, cpu_flags);
 
+		/* Filter packets from unwanted peers. */
+		if (skb) {
+			error = security_socket_post_recv_datagram(sk, skb,
+								   flags);
+			if (error)
+				goto force_dequeue;
+		}
+
 		if (skb)
 			return skb;
 
@@ -191,6 +200,24 @@ struct sk_buff *__skb_recv_datagram(stru
 
 	return NULL;
 
+force_dequeue:
+	/* Drop this packet because LSM says "Don't pass it to the caller". */
+	if (!(flags & MSG_PEEK))
+		goto no_peek;
+	/*
+	 * If this packet is MSG_PEEK'ed, dequeue it forcibly
+	 * so that this packet won't prevent the caller from picking up
+	 * next packet.
+	 */
+	spin_lock_irqsave(&sk->sk_receive_queue.lock, cpu_flags);
+	if (skb == skb_peek(&sk->sk_receive_queue)) {
+		__skb_unlink(skb, &sk->sk_receive_queue);
+		atomic_dec(&skb->users);
+		/* Do I have something to do with skb->peeked ? */
+	}
+	spin_unlock_irqrestore(&sk->sk_receive_queue.lock, cpu_flags);
+no_peek:
+	kfree_skb(skb);
 no_packet:
 	*err = error;
 	return NULL;
--- security-testing-2.6.git.orig/net/socket.c
+++ security-testing-2.6.git/net/socket.c
@@ -1519,6 +1519,11 @@ SYSCALL_DEFINE4(accept4, int, fd, struct
 	if (err < 0)
 		goto out_fd;
 
+	/* Filter connections from unwanted peers. */
+	err = security_socket_post_accept(sock, newsock);
+	if (err)
+		goto out_fd;
+
 	if (upeer_sockaddr) {
 		if (newsock->ops->getname(newsock, (struct sockaddr *)&address,
 					  &len, 2) < 0) {
--- security-testing-2.6.git.orig/security/capability.c
+++ security-testing-2.6.git/security/capability.c
@@ -620,6 +620,11 @@ static int cap_socket_accept(struct sock
 	return 0;
 }
 
+static int cap_socket_post_accept(struct socket *sock, struct socket *newsock)
+{
+	return 0;
+}
+
 static int cap_socket_sendmsg(struct socket *sock, struct msghdr *msg, int size)
 {
 	return 0;
@@ -631,6 +636,12 @@ static int cap_socket_recvmsg(struct soc
 	return 0;
 }
 
+static int cap_socket_post_recv_datagram(struct sock *sk, struct sk_buff *skb,
+					 unsigned int flags)
+{
+	return 0;
+}
+
 static int cap_socket_getsockname(struct socket *sock)
 {
 	return 0;
@@ -1010,8 +1021,10 @@ void security_fixup_ops(struct security_
 	set_to_cap_if_null(ops, socket_connect);
 	set_to_cap_if_null(ops, socket_listen);
 	set_to_cap_if_null(ops, socket_accept);
+	set_to_cap_if_null(ops, socket_post_accept);
 	set_to_cap_if_null(ops, socket_sendmsg);
 	set_to_cap_if_null(ops, socket_recvmsg);
+	set_to_cap_if_null(ops, socket_post_recv_datagram);
 	set_to_cap_if_null(ops, socket_getsockname);
 	set_to_cap_if_null(ops, socket_getpeername);
 	set_to_cap_if_null(ops, socket_setsockopt);
--- security-testing-2.6.git.orig/security/security.c
+++ security-testing-2.6.git/security/security.c
@@ -1007,6 +1007,11 @@ int security_socket_accept(struct socket
 	return security_ops->socket_accept(sock, newsock);
 }
 
+int security_socket_post_accept(struct socket *sock, struct socket *newsock)
+{
+	return security_ops->socket_post_accept(sock, newsock);
+}
+
 int security_socket_sendmsg(struct socket *sock, struct msghdr *msg, int size)
 {
 	return security_ops->socket_sendmsg(sock, msg, size);
@@ -1018,6 +1023,12 @@ int security_socket_recvmsg(struct socke
 	return security_ops->socket_recvmsg(sock, msg, size, flags);
 }
 
+int security_socket_post_recv_datagram(struct sock *sk, struct sk_buff *skb,
+				       unsigned int flags)
+{
+	return security_ops->socket_post_recv_datagram(sk, skb, flags);
+}
+
 int security_socket_getsockname(struct socket *sock)
 {
 	return security_ops->socket_getsockname(sock);

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Paul Moore]

On Tuesday 14 April 2009 06:44:35 am Tetsuo Handa wrote:
> Hello.
>
> The security_socket_post_accept() hook was recently removed because this
> hook was not used by any in-tree modules and its existence continued to
> cause problems by confusing people about what can be safely accomplished
> using this hook. Now, TOMOYO became in-tree and TOMOYO wants to add network
> access control in 2.6.31.
>
> TOMOYO is not a label based access control and won't cause packet labeling
> problem. TOMOYO won't care as long as packets are not copied to userspace.

We've discussed this issue several times on the mailing list so I won't go 
over all of the details again, but I will say that my main concern with the 
post_accept() hook is that you are rejecting a connection after is has already 
gone through the connection handshake.  I personally don't feel this is a good 
approach but I don't want to stand in your way if I am alone on this point.

I'm less concerned about the recv_datagram() hook although the issues you 
point out about sharing sockets across multiple processes does highlight what 
I believe are some fundamental issues regarding the TOMOYO network security 
model.  Once again, I'm not going to object if I am the only one.

Lastly, I think in order to review these patches we need to see how they are 
used.  Please submit a patch set that includes both this patch as well as a 
patch to TOMOYO which makes use of these changes; this way we can properly 
review your patches in context.  Regardless, I took a quick look and noticed a 
few things (below).

Thanks.

> --- security-testing-2.6.git.orig/net/core/datagram.c
> +++ security-testing-2.6.git/net/core/datagram.c
> @@ -55,6 +55,7 @@
>  #include <net/checksum.h>
>  #include <net/sock.h>
>  #include <net/tcp_states.h>
> +#include <linux/security.h>
>
>  /*
>   *	Is a socket 'connection oriented' ?
> @@ -148,6 +149,7 @@ struct sk_buff *__skb_recv_datagram(stru
>  {
>  	struct sk_buff *skb;
>  	long timeo;
> +	unsigned long cpu_flags;
>  	/*
>  	 * Caller is allowed not to check sk->sk_err before skb_recv_datagram()
>  	 */
> @@ -165,7 +167,6 @@ struct sk_buff *__skb_recv_datagram(stru
>  		 * Look at current nfs client by the way...
>  		 * However, this function was corrent in any case. 8)
>  		 */
> -		unsigned long cpu_flags;
>
>  		spin_lock_irqsave(&sk->sk_receive_queue.lock, cpu_flags);
>  		skb = skb_peek(&sk->sk_receive_queue);
> @@ -179,6 +180,14 @@ struct sk_buff *__skb_recv_datagram(stru
>  		}
>  		spin_unlock_irqrestore(&sk->sk_receive_queue.lock, cpu_flags);
>
> +		/* Filter packets from unwanted peers. */

It might be best to drop this comment, we don't want to speculate how the LSM 
might be making access controls decisions here.

> +		if (skb) {
> +			error = security_socket_post_recv_datagram(sk, skb,
> +								   flags);
> +			if (error)
> +				goto force_dequeue;
> +		}
> +
>  		if (skb)
>  			return skb;

Why check to see if skb != NULL twice?  I think you should be able to move the 
skb return statement into the body of the first if block.

> @@ -191,6 +200,24 @@ struct sk_buff *__skb_recv_datagram(stru
>
>  	return NULL;
>
> +force_dequeue:
> +	/* Drop this packet because LSM says "Don't pass it to the caller". */

Once again, remove this comment since we don't know what a LSM might decide.

> +	if (!(flags & MSG_PEEK))
> +		goto no_peek;
> +	/*
> +	 * If this packet is MSG_PEEK'ed, dequeue it forcibly
> +	 * so that this packet won't prevent the caller from picking up
> +	 * next packet.
> +	 */
> +	spin_lock_irqsave(&sk->sk_receive_queue.lock, cpu_flags);
> +	if (skb == skb_peek(&sk->sk_receive_queue)) {
> +		__skb_unlink(skb, &sk->sk_receive_queue);
> +		atomic_dec(&skb->users);
> +		/* Do I have something to do with skb->peeked ? */

I don't know but you should find out before this code is merged :)

> +	}
> +	spin_unlock_irqrestore(&sk->sk_receive_queue.lock, cpu_flags);
> +no_peek:
> +	kfree_skb(skb);
>  no_packet:
>  	*err = error;
>  	return NULL;

-- 
paul moore
linux @ hp

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Tetsuo Handa]

Hello.

Paul Moore wrote:
> Please submit a patch set that includes both this patch as well as a
> patch to TOMOYO which makes use of these changes; this way we can properly
> review your patches in context.

I see.

I have some questions.

> > +	if (!(flags & MSG_PEEK))
> > +		goto no_peek;
> > +	/*
> > +	 * If this packet is MSG_PEEK'ed, dequeue it forcibly
> > +	 * so that this packet won't prevent the caller from picking up
> > +	 * next packet.
> > +	 */
> > +	spin_lock_irqsave(&sk->sk_receive_queue.lock, cpu_flags);
> > +	if (skb == skb_peek(&sk->sk_receive_queue)) {
> > +		__skb_unlink(skb, &sk->sk_receive_queue);
> > +		atomic_dec(&skb->users);
> > +		/* Do I have something to do with skb->peeked ? */
> 
> I don't know but you should find out before this code is merged :)

Q1: Can I use skb_kill_datagram() here?

    skb_kill_datagram() uses spin_lock_bh() while __skb_recv_datagram() uses
    spin_lock_irqsave(). Since this codepath is called inside
    __skb_recv_datagram(), I used spin_lock_irqsave() rather than calling
    skb_kill_datagram().
> 
> > +	}
> > +	spin_unlock_irqrestore(&sk->sk_receive_queue.lock, cpu_flags);
> > +no_peek:
> > +	kfree_skb(skb);

Q2: Do I need to use skb_free_datagram() here rather than kfree_skb()?

    In the past ( http://lkml.org/lkml/2007/11/16/406 ), there was no
    difference between skb_free_datagram() and kfree_skb().
    | void skb_free_datagram(struct sock *sk, struct sk_buff *skb)
    | {
    | 	kfree_skb(skb);
    | }
    But now (as of 2.6.30-rc2), there is a difference.
    | void skb_free_datagram(struct sock *sk, struct sk_buff *skb)
    | {
    | 	consume_skb(skb);
    | 	sk_mem_reclaim_partial(sk);
    | }

> >  no_packet:
> >  	*err = error;
> >  	return NULL;

Q3: Is __skb_recv_datagram() called from contexts that are not permitted to
    sleep?

    If so, TOMOYO has to check whether it is allowed to sleep, for TOMOYO will
    prompt the user "whether to allow App1 to read this datagram or not".

Q4: Is there a way to distinguish requests from userland programs and requests
    from kernel code?

    Some kernel code (e.g. NFS) sends/receives UDP packets to deal requests
    from userland program's requests. TOMOYO wants to distinguish "direct
    requests" (requests issued by userland programs, such as open()/read()/
    write() against files on NFS) and "indirect requests" (requests issued by
    reasons of kernel's own which are needed to handle "direct requests", such
    as fetching file data from NFS server). But currently, TOMOYO can't
    distinguish these requests. As a result, those who use NFS have to give
    permissions for sending/receiving UDP packets to/from NFS server to all
    userland programs.
    This means that TOMOYO allows userland programs to send/receive crafted
    packets to/from NFS server. I want to solve this problem.

Regards.

[PATCH 1/2] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Tetsuo Handa]

Subject: LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

This patch allows LSM modules to filter incoming connections/datagrams based on
the process's security context who is attempting to pick up.

There are already hooks for filtering incoming connections/datagrams based on
the socket's security context, but these hooks are not applicable when someone
wants to do TCP Wrapper-like filtering (e.g. App1 is permitted to accept TCP
connections from 192.168.0.0/16) because nobody can tell who picks them up
before the moment of sock->ops->accept()/sock->ops->recvmsg() call.

Precautions: These hooks have a side effect if improperly used.

If a socket is shared by multiple processes with different policy, the process
who should be able to accept this connection will not be able to accept this
connection because socket_post_accept() aborts this connection.
This is needed because the process who must not be able to accept this
connection will repeat accept() request (and consume CPU resource) forever
if socket_post_accept() doesn't abort this connection.

Similarly, if a socket is shared by multiple processes with different policy,
the process who should be able to pick up this datagram will not be able to
pick up this datagram because socket_post_recv_datagram() discards this
datagram.
This is needed because the process who must not be able to pick up this
datagram will repeat recvmsg() request (and consume CPU resource) forever
if socket_post_recv_datagram() doesn't discard this datagram.

Therefore, don't give different policy between processes who share one socket.
Otherwise, some connections/datagrams cannot be delivered to intended process.

Signed-off-by: Kentaro Takeda <takedakn@nttdata.co.jp>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Toshiharu Harada <haradats@nttdata.co.jp>

 include/linux/security.h |   39 +++++++++++++++++++++++++++++++++++++++
 net/core/datagram.c      |   21 +++++++++++++++++++--
 net/socket.c             |    5 +++++
 security/capability.c    |   13 +++++++++++++
 security/security.c      |   11 +++++++++++
 5 files changed, 87 insertions(+), 2 deletions(-)

---

--- security-testing-2.6.git.orig/include/linux/security.h
+++ security-testing-2.6.git/include/linux/security.h
@@ -880,6 +880,17 @@ static inline void security_free_mnt_opt
  *	@sock contains the listening socket structure.
  *	@newsock contains the newly created server socket for connection.
  *	Return 0 if permission is granted.
+ * @socket_post_accept:
+ *	This hook allows a security module to filter connections from unwanted
+ *	peers based on the process accepting this connection.
+ *	The connection will be aborted if this hook returns nonzero.
+ *	This hook is not designed for updating security attributes of
+ *	an accept()ed socket, for the accept()ed socket has already sent
+ *	several packets (e.g. TCP's SYN/ACK packet and some ACK packets for
+ *	incoming data) before this hook is called.
+ *	@sock contains the listening socket structure.
+ *	@newsock contains the newly created server socket for connection.
+ *	Return 0 if permission is granted.
  * @socket_sendmsg:
  *	Check permission before transmitting a message to another socket.
  *	@sock contains the socket structure.
@@ -893,6 +904,15 @@ static inline void security_free_mnt_opt
  *	@size contains the size of message structure.
  *	@flags contains the operational flags.
  *	Return 0 if permission is granted.
+ * @socket_post_recv_datagram:
+ *	Check permission after receiving a datagram.
+ *	This hook allows a security module to filter packets
+ *	from unwanted peers based on the process receiving this datagram.
+ *	The packet will be discarded if this hook returns nonzero.
+ *	@sk contains the socket.
+ *	@skb contains the socket buffer.
+ *	@flags contains the operational flags.
+ *	Return 0 if permission is granted.
  * @socket_getsockname:
  *	Check permission before the local address (name) of the socket object
  *	@sock is retrieved.
@@ -1549,10 +1569,13 @@ struct security_operations {
 			       struct sockaddr *address, int addrlen);
 	int (*socket_listen) (struct socket *sock, int backlog);
 	int (*socket_accept) (struct socket *sock, struct socket *newsock);
+	int (*socket_post_accept) (struct socket *sock, struct socket *newsock);
 	int (*socket_sendmsg) (struct socket *sock,
 			       struct msghdr *msg, int size);
 	int (*socket_recvmsg) (struct socket *sock,
 			       struct msghdr *msg, int size, int flags);
+	int (*socket_post_recv_datagram) (struct sock *sk, struct sk_buff *skb,
+					  unsigned int flags);
 	int (*socket_getsockname) (struct socket *sock);
 	int (*socket_getpeername) (struct socket *sock);
 	int (*socket_getsockopt) (struct socket *sock, int level, int optname);
@@ -2530,9 +2553,12 @@ int security_socket_bind(struct socket *
 int security_socket_connect(struct socket *sock, struct sockaddr *address, int addrlen);
 int security_socket_listen(struct socket *sock, int backlog);
 int security_socket_accept(struct socket *sock, struct socket *newsock);
+int security_socket_post_accept(struct socket *sock, struct socket *newsock);
 int security_socket_sendmsg(struct socket *sock, struct msghdr *msg, int size);
 int security_socket_recvmsg(struct socket *sock, struct msghdr *msg,
 			    int size, int flags);
+int security_socket_post_recv_datagram(struct sock *sk, struct sk_buff *skb,
+				       unsigned int flags);
 int security_socket_getsockname(struct socket *sock);
 int security_socket_getpeername(struct socket *sock);
 int security_socket_getsockopt(struct socket *sock, int level, int optname);
@@ -2608,6 +2634,12 @@ static inline int security_socket_accept
 	return 0;
 }
 
+static inline int security_socket_post_accept(struct socket *sock,
+					      struct socket *newsock)
+{
+	return 0;
+}
+
 static inline int security_socket_sendmsg(struct socket *sock,
 					  struct msghdr *msg, int size)
 {
@@ -2621,6 +2653,13 @@ static inline int security_socket_recvms
 	return 0;
 }
 
+static inline int security_socket_post_recv_datagram(struct sock *sk,
+						     struct sk_buff *skb,
+						     unsigned int flags)
+{
+	return 0;
+}
+
 static inline int security_socket_getsockname(struct socket *sock)
 {
 	return 0;
--- security-testing-2.6.git.orig/net/core/datagram.c
+++ security-testing-2.6.git/net/core/datagram.c
@@ -55,6 +55,7 @@
 #include <net/checksum.h>
 #include <net/sock.h>
 #include <net/tcp_states.h>
+#include <linux/security.h>
 
 /*
  *	Is a socket 'connection oriented' ?
@@ -148,6 +149,7 @@ struct sk_buff *__skb_recv_datagram(stru
 {
 	struct sk_buff *skb;
 	long timeo;
+	unsigned long cpu_flags;
 	/*
 	 * Caller is allowed not to check sk->sk_err before skb_recv_datagram()
 	 */
@@ -165,7 +167,6 @@ struct sk_buff *__skb_recv_datagram(stru
 		 * Look at current nfs client by the way...
 		 * However, this function was corrent in any case. 8)
 		 */
-		unsigned long cpu_flags;
 
 		spin_lock_irqsave(&sk->sk_receive_queue.lock, cpu_flags);
 		skb = skb_peek(&sk->sk_receive_queue);
@@ -179,8 +180,13 @@ struct sk_buff *__skb_recv_datagram(stru
 		}
 		spin_unlock_irqrestore(&sk->sk_receive_queue.lock, cpu_flags);
 
-		if (skb)
+		if (skb) {
+			error = security_socket_post_recv_datagram(sk, skb,
+								   flags);
+			if (error)
+				goto force_dequeue;
 			return skb;
+		}
 
 		/* User doesn't want to wait */
 		error = -EAGAIN;
@@ -191,6 +197,17 @@ struct sk_buff *__skb_recv_datagram(stru
 
 	return NULL;
 
+force_dequeue:
+	if (!(flags & MSG_PEEK))
+		goto no_peek;
+	spin_lock_irqsave(&sk->sk_receive_queue.lock, cpu_flags);
+	if (skb == skb_peek(&sk->sk_receive_queue)) {
+		__skb_unlink(skb, &sk->sk_receive_queue);
+		atomic_dec(&skb->users);
+	}
+	spin_unlock_irqrestore(&sk->sk_receive_queue.lock, cpu_flags);
+no_peek:
+	kfree_skb(skb);
 no_packet:
 	*err = error;
 	return NULL;
--- security-testing-2.6.git.orig/net/socket.c
+++ security-testing-2.6.git/net/socket.c
@@ -1519,6 +1519,11 @@ SYSCALL_DEFINE4(accept4, int, fd, struct
 	if (err < 0)
 		goto out_fd;
 
+	/* Filter connections from unwanted peers. */
+	err = security_socket_post_accept(sock, newsock);
+	if (err)
+		goto out_fd;
+
 	if (upeer_sockaddr) {
 		if (newsock->ops->getname(newsock, (struct sockaddr *)&address,
 					  &len, 2) < 0) {
--- security-testing-2.6.git.orig/security/capability.c
+++ security-testing-2.6.git/security/capability.c
@@ -620,6 +620,11 @@ static int cap_socket_accept(struct sock
 	return 0;
 }
 
+static int cap_socket_post_accept(struct socket *sock, struct socket *newsock)
+{
+	return 0;
+}
+
 static int cap_socket_sendmsg(struct socket *sock, struct msghdr *msg, int size)
 {
 	return 0;
@@ -631,6 +636,12 @@ static int cap_socket_recvmsg(struct soc
 	return 0;
 }
 
+static int cap_socket_post_recv_datagram(struct sock *sk, struct sk_buff *skb,
+					 unsigned int flags)
+{
+	return 0;
+}
+
 static int cap_socket_getsockname(struct socket *sock)
 {
 	return 0;
@@ -1010,8 +1021,10 @@ void security_fixup_ops(struct security_
 	set_to_cap_if_null(ops, socket_connect);
 	set_to_cap_if_null(ops, socket_listen);
 	set_to_cap_if_null(ops, socket_accept);
+	set_to_cap_if_null(ops, socket_post_accept);
 	set_to_cap_if_null(ops, socket_sendmsg);
 	set_to_cap_if_null(ops, socket_recvmsg);
+	set_to_cap_if_null(ops, socket_post_recv_datagram);
 	set_to_cap_if_null(ops, socket_getsockname);
 	set_to_cap_if_null(ops, socket_getpeername);
 	set_to_cap_if_null(ops, socket_setsockopt);
--- security-testing-2.6.git.orig/security/security.c
+++ security-testing-2.6.git/security/security.c
@@ -1007,6 +1007,11 @@ int security_socket_accept(struct socket
 	return security_ops->socket_accept(sock, newsock);
 }
 
+int security_socket_post_accept(struct socket *sock, struct socket *newsock)
+{
+	return security_ops->socket_post_accept(sock, newsock);
+}
+
 int security_socket_sendmsg(struct socket *sock, struct msghdr *msg, int size)
 {
 	return security_ops->socket_sendmsg(sock, msg, size);
@@ -1018,6 +1023,12 @@ int security_socket_recvmsg(struct socke
 	return security_ops->socket_recvmsg(sock, msg, size, flags);
 }
 
+int security_socket_post_recv_datagram(struct sock *sk, struct sk_buff *skb,
+				       unsigned int flags)
+{
+	return security_ops->socket_post_recv_datagram(sk, skb, flags);
+}
+
 int security_socket_getsockname(struct socket *sock)
 {
 	return security_ops->socket_getsockname(sock);

[PATCH 2/2] tomoyo: Add network access control support.

[Tetsuo Handa]

Subject: tomoyo: Add network access control support.

TOMOYO checks permissions for below IPv4/IPv6 network operations.

Binding TCP/UDP/RAW sockets.
Listening TCP sockets.
Accepting TCP sockets.
Connecting TCP/UDP/RAW sockets.
Sending UDP/RAW packets.
Receiving UDP/RAW packets.

TOMOYO uses security_socket_post_accept() and
security_socket_post_recv_datagram() in order to implement a packet filtering
with interactive enforcement. (Interactive enforcement is not implemented yet.)

Signed-off-by: Kentaro Takeda <takedakn@nttdata.co.jp>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Toshiharu Harada <haradats@nttdata.co.jp>
---
 security/tomoyo/Kconfig   |    1 
 security/tomoyo/Makefile  |    2 
 security/tomoyo/common.c  |  118 +++++
 security/tomoyo/common.h  |   68 +++
 security/tomoyo/network.c |  930 ++++++++++++++++++++++++++++++++++++++++++++++
 security/tomoyo/tomoyo.c  |   46 ++
 security/tomoyo/tomoyo.h  |   17 
 7 files changed, 1178 insertions(+), 4 deletions(-)

--- security-testing-2.6.git.orig/security/tomoyo/Kconfig
+++ security-testing-2.6.git/security/tomoyo/Kconfig
@@ -3,6 +3,7 @@ config SECURITY_TOMOYO
 	depends on SECURITY
 	select SECURITYFS
 	select SECURITY_PATH
+	select SECURITY_NETWORK
 	default n
 	help
 	  This selects TOMOYO Linux, pathname-based access control.
--- security-testing-2.6.git.orig/security/tomoyo/Makefile
+++ security-testing-2.6.git/security/tomoyo/Makefile
@@ -1 +1 @@
-obj-y = common.o realpath.o tomoyo.o domain.o file.o
+obj-y = common.o realpath.o tomoyo.o domain.o file.o network.o
--- security-testing-2.6.git.orig/security/tomoyo/common.c
+++ security-testing-2.6.git/security/tomoyo/common.c
@@ -12,6 +12,7 @@
 #include <linux/uaccess.h>
 #include <linux/security.h>
 #include <linux/hardirq.h>
+#include <linux/kernel.h>
 #include "realpath.h"
 #include "common.h"
 #include "tomoyo.h"
@@ -35,6 +36,7 @@ static struct {
 	const unsigned int max_value;
 } tomoyo_control_array[TOMOYO_MAX_CONTROL_INDEX] = {
 	[TOMOYO_MAC_FOR_FILE]     = { "MAC_FOR_FILE",        0,       3 },
+	[TOMOYO_MAC_FOR_NETWORK]  = { "MAC_FOR_NETWORK",     0,       3 },
 	[TOMOYO_MAX_ACCEPT_ENTRY] = { "MAX_ACCEPT_ENTRY", 2048, INT_MAX },
 	[TOMOYO_VERBOSE]          = { "TOMOYO_VERBOSE",      1,       1 },
 };
@@ -836,6 +838,9 @@ bool tomoyo_domain_quota_is_ok(struct to
 			if (perm & (1 << TOMOYO_TYPE_RENAME_ACL))
 				count++;
 			break;
+		case TOMOYO_TYPE_IP_NETWORK_ACL:
+			count++;
+			break;
 		}
 	}
 	up_read(&tomoyo_domain_acl_info_list_lock);
@@ -1290,6 +1295,8 @@ static int tomoyo_write_domain_policy(st
 			       TOMOYO_DOMAIN_FLAGS_IGNORE_GLOBAL_ALLOW_READ);
 		return 0;
 	}
+	if (tomoyo_str_starts(&data, TOMOYO_KEYWORD_ALLOW_NETWORK))
+		return tomoyo_write_network_policy(data, domain, is_delete);
 	return tomoyo_write_file_policy(data, domain, is_delete);
 }
 
@@ -1378,6 +1385,107 @@ static bool tomoyo_print_double_path_acl
 }
 
 /**
+ * tomoyo_print_ipv4_entry - Print IPv4 address of a network ACL entry.
+ *
+ * @head: Pointer to "struct tomoyo_io_buffer".
+ * @ptr:  Pointer to "struct tomoyo_ip_network_acl_record".
+ *
+ * Returns true on success, false otherwise.
+ */
+static bool tomoyo_print_ipv4_entry(struct tomoyo_io_buffer *head,
+				    struct tomoyo_ip_network_acl_record *ptr)
+{
+	const u32 min_address = htonl(ptr->u.ipv4.min);
+	const u32 max_address = htonl(ptr->u.ipv4.max);
+	if (!tomoyo_io_printf(head, "%pI4", &min_address))
+		return false;
+	if (min_address != max_address
+	    && !tomoyo_io_printf(head, "-%pI4", &max_address))
+		return false;
+	return true;
+}
+
+/**
+ * tomoyo_print_ipv6_entry - Print IPv6 address of a network ACL entry.
+ *
+ * @head: Pointer to "struct tomoyo_io_buffer".
+ * @ptr:  Pointer to "struct tomoyo_ip_network_acl_record".
+ *
+ * Returns true on success, false otherwise.
+ */
+static bool tomoyo_print_ipv6_entry(struct tomoyo_io_buffer *head,
+				    struct tomoyo_ip_network_acl_record *ptr)
+{
+	char buf[64];
+	const struct in6_addr *min_address = ptr->u.ipv6.min;
+	const struct in6_addr *max_address = ptr->u.ipv6.max;
+	tomoyo_print_ipv6(buf, sizeof(buf), min_address);
+	if (!tomoyo_io_printf(head, "%s", buf))
+		return false;
+	if (min_address != max_address) {
+		tomoyo_print_ipv6(buf, sizeof(buf), max_address);
+		if (!tomoyo_io_printf(head, "-%s", buf))
+			return false;
+	}
+	return true;
+}
+
+/**
+ * tomoyo_print_port_entry - Print port number of a network ACL entry.
+ *
+ * @head: Pointer to "struct tomoyo_io_buffer".
+ * @ptr:  Pointer to "struct tomoyo_ip_network_acl_record".
+ *
+ * Returns true on success, false otherwise.
+ */
+static bool tomoyo_print_port_entry(struct tomoyo_io_buffer *head,
+				    struct tomoyo_ip_network_acl_record *ptr)
+{
+	const u16 min_port = ptr->min_port;
+	const u16 max_port = ptr->max_port;
+	if (!tomoyo_io_printf(head, " %u", min_port))
+		return false;
+	if (min_port != max_port && !tomoyo_io_printf(head, "-%u", max_port))
+		return false;
+	if (!tomoyo_io_printf(head, "\n"))
+		return false;
+	return true;
+}
+
+/**
+ * tomoyo_print_network_acl - Print a network ACL entry.
+ *
+ * @head: Pointer to "struct tomoyo_io_buffer".
+ * @ptr:  Pointer to "struct tomoyo_ip_network_acl_record".
+ *
+ * Returns true on success, false otherwise.
+ */
+static bool tomoyo_print_network_acl(struct tomoyo_io_buffer *head,
+				     struct tomoyo_ip_network_acl_record *ptr)
+{
+	int pos = head->read_avail;
+	if (!tomoyo_io_printf(head, TOMOYO_KEYWORD_ALLOW_NETWORK "%s ",
+			      tomoyo_net2keyword(ptr->operation_type)))
+		goto out;
+	switch (ptr->record_type) {
+	case TOMOYO_IP_RECORD_TYPE_IPv4:
+		if (!tomoyo_print_ipv4_entry(head, ptr))
+			goto out;
+		break;
+	case TOMOYO_IP_RECORD_TYPE_IPv6:
+		if (!tomoyo_print_ipv6_entry(head, ptr))
+			goto out;
+		break;
+	}
+	if (!tomoyo_print_port_entry(head, ptr))
+		goto out;
+	return true;
+ out:
+	head->read_avail = pos;
+	return false;
+}
+
+/**
  * tomoyo_print_entry - Print an ACL entry.
  *
  * @head: Pointer to "struct tomoyo_io_buffer".
@@ -1406,6 +1514,13 @@ static bool tomoyo_print_entry(struct to
 				       head);
 		return tomoyo_print_double_path_acl(head, acl);
 	}
+	if (acl_type == TOMOYO_TYPE_IP_NETWORK_ACL) {
+		struct tomoyo_ip_network_acl_record *acl
+			= container_of(ptr,
+				       struct tomoyo_ip_network_acl_record,
+				       head);
+		return tomoyo_print_network_acl(head, acl);
+	}
 	BUG(); /* This must not happen. */
 	return false;
 }
@@ -2068,6 +2183,9 @@ void *tomoyo_alloc_acl_element(const u8 
 	case TOMOYO_TYPE_DOUBLE_PATH_ACL:
 		len = sizeof(struct tomoyo_double_path_acl_record);
 		break;
+	case TOMOYO_TYPE_IP_NETWORK_ACL:
+		len = sizeof(struct tomoyo_ip_network_acl_record);
+		break;
 	default:
 		return NULL;
 	}
--- security-testing-2.6.git.orig/security/tomoyo/common.h
+++ security-testing-2.6.git/security/tomoyo/common.h
@@ -130,8 +130,59 @@ struct tomoyo_double_path_acl_record {
 	const struct tomoyo_path_info *filename2;
 };
 
+/* Structure for "allow_network" directive. */
+struct tomoyo_ip_network_acl_record {
+	struct tomoyo_acl_info head; /* type = TOMOYO_TYPE_IP_NETWORK_ACL */
+	/*
+	 * operation_type takes one of the following constants.
+	 *   NETWORK_ACL_UDP_BIND for UDP's bind() operation.
+	 *   NETWORK_ACL_UDP_CONNECT for UDP's connect()/send()/recv()
+	 *                               operation.
+	 *   NETWORK_ACL_TCP_BIND for TCP's bind() operation.
+	 *   NETWORK_ACL_TCP_LISTEN for TCP's listen() operation.
+	 *   NETWORK_ACL_TCP_CONNECT for TCP's connect() operation.
+	 *   NETWORK_ACL_TCP_ACCEPT for TCP's accept() operation.
+	 *   NETWORK_ACL_RAW_BIND for IP's bind() operation.
+	 *   NETWORK_ACL_RAW_CONNECT for IP's connect()/send()/recv()
+	 *                               operation.
+	 */
+	u8 operation_type;
+	/*
+	 * record_type takes one of the following constants.
+	 *   TOMOYO_IP_RECORD_TYPE_IPv4
+	 *                if u points to an IPv4 address.
+	 *   TOMOYO_IP_RECORD_TYPE_IPv6
+	 *                if u points to an IPv6 address.
+	 */
+	u8 record_type;
+	/* Start of port number range. */
+	u16 min_port;
+	/* End of port number range.   */
+	u16 max_port;
+	union {
+		struct {
+			/* Start of IPv4 address range. Host endian. */
+			u32 min;
+			/* End of IPv4 address range. Host endian.   */
+			u32 max;
+		} ipv4;
+		struct {
+			/* Start of IPv6 address range. Big endian.  */
+			const struct in6_addr *min;
+			/* End of IPv6 address range. Big endian.    */
+			const struct in6_addr *max;
+		} ipv6;
+	} u;
+};
+
+enum tomoyo_ip_record_type {
+	TOMOYO_IP_RECORD_TYPE_IPv4,
+	TOMOYO_IP_RECORD_TYPE_IPv6
+};
+
 /* Keywords for ACLs. */
 #define TOMOYO_KEYWORD_ALIAS                     "alias "
+#define TOMOYO_KEYWORD_ALLOW_NETWORK             "allow_network "
 #define TOMOYO_KEYWORD_ALLOW_READ                "allow_read "
 #define TOMOYO_KEYWORD_DELETE                    "delete "
 #define TOMOYO_KEYWORD_DENY_REWRITE              "deny_rewrite "
@@ -149,9 +200,10 @@ struct tomoyo_double_path_acl_record {
 
 /* Index numbers for Access Controls. */
 #define TOMOYO_MAC_FOR_FILE                  0  /* domain_policy.conf */
-#define TOMOYO_MAX_ACCEPT_ENTRY              1
-#define TOMOYO_VERBOSE                       2
-#define TOMOYO_MAX_CONTROL_INDEX             3
+#define TOMOYO_MAC_FOR_NETWORK               1
+#define TOMOYO_MAX_ACCEPT_ENTRY              2
+#define TOMOYO_VERBOSE                       3
+#define TOMOYO_MAX_CONTROL_INDEX             4
 
 /* Structure for reading/writing policy via securityfs interfaces. */
 struct tomoyo_io_buffer {
@@ -204,6 +256,9 @@ bool tomoyo_is_domain_def(const unsigned
 /* Check whether the given filename matches the given pattern. */
 bool tomoyo_path_matches_pattern(const struct tomoyo_path_info *filename,
 				 const struct tomoyo_path_info *pattern);
+/* Print an IPv6 address. */
+void tomoyo_print_ipv6(char *buffer, const int buffer_len,
+		       const struct in6_addr *ip);
 /* Read "alias" entry in exception policy. */
 bool tomoyo_read_alias_policy(struct tomoyo_io_buffer *head);
 /*
@@ -275,6 +330,13 @@ void tomoyo_load_policy(const char *file
 /* Change "struct tomoyo_domain_info"->flags. */
 void tomoyo_set_domain_flag(struct tomoyo_domain_info *domain,
 			    const bool is_delete, const u8 flags);
+/* Read "allow_network" entry in domain policy. */
+bool tomoyo_read_network_policy(struct tomoyo_io_buffer *head);
+/* Create "allow_network" entry in domain policy. */
+int tomoyo_write_network_policy(char *data, struct tomoyo_domain_info *domain,
+				const bool is_delete);
+/* Convert network operation index to network operation name. */
+const char *tomoyo_net2keyword(const u8 operation);
 
 /* strcmp() for "struct tomoyo_path_info" structure. */
 static inline bool tomoyo_pathcmp(const struct tomoyo_path_info *a,
--- /dev/null
+++ security-testing-2.6.git/security/tomoyo/network.c
@@ -0,0 +1,930 @@
+/*
+ * security/tomoyo/network.c
+ *
+ * Implementation of the Domain-Based Mandatory Access Control.
+ *
+ * Copyright (C) 2005-2009  NTT DATA CORPORATION
+ *
+ * Version: 2.3.0-pre   2009/04/15
+ *
+ */
+
+#include "common.h"
+#include "tomoyo.h"
+#include "realpath.h"
+
+#include <linux/net.h>
+#include <linux/inet.h>
+#include <linux/in.h>
+#include <linux/in6.h>
+#include <net/ip.h>
+#include <net/ipv6.h>
+#include <net/udp.h>
+
+/* Index numbers for Network Controls. */
+enum tomoyo_network_acl_index {
+	NETWORK_ACL_UDP_BIND,
+	NETWORK_ACL_UDP_CONNECT,
+	NETWORK_ACL_TCP_BIND,
+	NETWORK_ACL_TCP_LISTEN,
+	NETWORK_ACL_TCP_CONNECT,
+	NETWORK_ACL_TCP_ACCEPT,
+	NETWORK_ACL_RAW_BIND,
+	NETWORK_ACL_RAW_CONNECT
+};
+
+/**
+ * tomoyo_save_ipv6_address - Keep the given IPv6 address on the RAM.
+ *
+ * @addr: Pointer to "struct in6_addr".
+ *
+ * Returns pointer to "struct in6_addr" on success, NULL otherwise.
+ *
+ * The RAM is shared, so NEVER try to modify or kfree() the returned address.
+ */
+static const struct in6_addr *tomoyo_save_ipv6_address(const struct in6_addr *
+						       addr)
+{
+	static const u8 tomoyo_block_size = 16;
+	struct tomoyo_addr_list {
+		/* Workaround for gcc 4.3's bug. */
+		struct in6_addr addr[16]; /* = tomoyo_block_size */
+		struct list_head list;
+		u32 in_use_count;
+	};
+	static LIST_HEAD(tomoyo_address_list);
+	struct tomoyo_addr_list *ptr;
+	static DEFINE_MUTEX(lock);
+	u8 i = tomoyo_block_size;
+	if (!addr)
+		return NULL;
+	/*
+	 * Since reader calls down_read(&tomoyo_domain_acl_info_list_lock),
+	 * this lock can remain as local lock.
+	 */
+	mutex_lock(&lock);
+	list_for_each_entry(ptr, &tomoyo_address_list, list) {
+		for (i = 0; i < ptr->in_use_count; i++) {
+			if (!memcmp(&ptr->addr[i], addr, sizeof(*addr)))
+				goto ok;
+		}
+		if (i < tomoyo_block_size)
+			break;
+	}
+	if (i == tomoyo_block_size) {
+		ptr = tomoyo_alloc_element(sizeof(*ptr));
+		if (!ptr)
+			goto ok;
+		list_add_tail(&ptr->list, &tomoyo_address_list);
+		i = 0;
+	}
+	ptr->addr[ptr->in_use_count++] = *addr;
+ ok:
+	mutex_unlock(&lock);
+	return ptr ? &ptr->addr[i] : NULL;
+}
+
+/**
+ * tomoyo_parse_ip_address - Parse an IP address.
+ *
+ * @address: String to parse.
+ * @min:     Pointer to store min address.
+ * @max:     Pointer to store max address.
+ *
+ * Returns 2 if @address is an IPv6, 1 if @address is an IPv4, 0 otherwise.
+ */
+static int tomoyo_parse_ip_address(char *address, u16 *min, u16 *max)
+{
+	/* "%pI6" is not supported for sscanf(). */
+	int count = sscanf(address, "%hx:%hx:%hx:%hx:%hx:%hx:%hx:%hx"
+			   "-%hx:%hx:%hx:%hx:%hx:%hx:%hx:%hx",
+			   &min[0], &min[1], &min[2], &min[3],
+			   &min[4], &min[5], &min[6], &min[7],
+			   &max[0], &max[1], &max[2], &max[3],
+			   &max[4], &max[5], &max[6], &max[7]);
+	if (count == 8 || count == 16) {
+		u8 i;
+		if (count == 8)
+			memmove(max, min, sizeof(u16) * 8);
+		for (i = 0; i < 8; i++) {
+			min[i] = htons(min[i]);
+			max[i] = htons(max[i]);
+		}
+		return 2;
+	}
+	/* "%pI4" is not supported for sscanf(). */
+	count = sscanf(address, "%hu.%hu.%hu.%hu-%hu.%hu.%hu.%hu",
+		       &min[0], &min[1], &min[2], &min[3],
+		       &max[0], &max[1], &max[2], &max[3]);
+	if (count == 4 || count == 8) {
+		u32 ip = htonl((((u8) min[0]) << 24) + (((u8) min[1]) << 16)
+			       + (((u8) min[2]) << 8) + (u8) min[3]);
+		memmove(min, &ip, sizeof(ip));
+		if (count == 8)
+			ip = htonl((((u8) max[0]) << 24) + (((u8) max[1]) << 16)
+				   + (((u8) max[2]) << 8) + (u8) max[3]);
+		memmove(max, &ip, sizeof(ip));
+		return 1;
+	}
+	return 0;
+}
+
+/**
+ * tomoyo_print_ipv6 - Print an IPv6 address.
+ *
+ * @buffer:     Buffer to write to.
+ * @buffer_len: Size of @buffer.
+ * @ip:         Pointer to "struct in6_addr".
+ *
+ * To make output shortest, TOMOYO doesn't use "%pI6".
+ *
+ * Returns nothing.
+ */
+void tomoyo_print_ipv6(char *buffer, const int buffer_len,
+		       const struct in6_addr *ip)
+{
+	memset(buffer, 0, buffer_len);
+	snprintf(buffer, buffer_len - 1, "%x:%x:%x:%x:%x:%x:%x:%x",
+		 ntohs(ip->s6_addr16[0]), ntohs(ip->s6_addr16[1]),
+		 ntohs(ip->s6_addr16[2]), ntohs(ip->s6_addr16[3]),
+		 ntohs(ip->s6_addr16[4]), ntohs(ip->s6_addr16[5]),
+		 ntohs(ip->s6_addr16[6]), ntohs(ip->s6_addr16[7]));
+}
+
+/**
+ * tomoyo_net2keyword - Convert network operation index to network operation name.
+ *
+ * @operation: Type of operation.
+ *
+ * Returns the name of operation.
+ */
+const char *tomoyo_net2keyword(const u8 operation)
+{
+	const char *keyword = "unknown";
+	switch (operation) {
+	case NETWORK_ACL_UDP_BIND:
+		keyword = "UDP bind";
+		break;
+	case NETWORK_ACL_UDP_CONNECT:
+		keyword = "UDP connect";
+		break;
+	case NETWORK_ACL_TCP_BIND:
+		keyword = "TCP bind";
+		break;
+	case NETWORK_ACL_TCP_LISTEN:
+		keyword = "TCP listen";
+		break;
+	case NETWORK_ACL_TCP_CONNECT:
+		keyword = "TCP connect";
+		break;
+	case NETWORK_ACL_TCP_ACCEPT:
+		keyword = "TCP accept";
+		break;
+	case NETWORK_ACL_RAW_BIND:
+		keyword = "RAW bind";
+		break;
+	case NETWORK_ACL_RAW_CONNECT:
+		keyword = "RAW connect";
+		break;
+	}
+	return keyword;
+}
+
+/**
+ * tomoyo_update_network_entry - Update "struct tomoyo_ip_network_acl_record" list.
+ *
+ * @operation:   Type of operation.
+ * @record_type: Type of address.
+ * @min_address: Start of IPv4 or IPv6 address range.
+ * @max_address: End of IPv4 or IPv6 address range.
+ * @min_port:    Start of port number range.
+ * @max_port:    End of port number range.
+ * @domain:      Pointer to "struct tomoyo_domain_info".
+ * @is_delete:   True if it is a delete request.
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+static int tomoyo_update_network_entry(const u8 operation, const u8 record_type,
+				       const u32 *min_address,
+				       const u32 *max_address,
+				       const u16 min_port, const u16 max_port,
+				       struct tomoyo_domain_info *domain,
+				       const bool is_delete)
+{
+	struct tomoyo_acl_info *ptr;
+	struct tomoyo_ip_network_acl_record *acl;
+	int error = -ENOMEM;
+	/* using host byte order to allow u32 comparison than memcmp().*/
+	const u32 min_ip = ntohl(*min_address);
+	const u32 max_ip = ntohl(*max_address);
+	const struct in6_addr *saved_min_address = NULL;
+	const struct in6_addr *saved_max_address = NULL;
+	if (!domain)
+		return -EINVAL;
+	if (record_type != TOMOYO_IP_RECORD_TYPE_IPv6)
+		goto not_ipv6;
+	saved_min_address = tomoyo_save_ipv6_address((struct in6_addr *)
+						     min_address);
+	saved_max_address = tomoyo_save_ipv6_address((struct in6_addr *)
+						     max_address);
+	if (!saved_min_address || !saved_max_address)
+		return -ENOMEM;
+ not_ipv6:
+	/***** EXCLUSIVE SECTION START *****/
+	down_write(&tomoyo_domain_acl_info_list_lock);
+	if (is_delete)
+		goto delete;
+	list_for_each_entry(ptr, &domain->acl_info_list, list) {
+		if (tomoyo_acl_type1(ptr) != TOMOYO_TYPE_IP_NETWORK_ACL)
+			continue;
+		acl = container_of(ptr, struct tomoyo_ip_network_acl_record,
+				   head);
+		if (acl->operation_type != operation ||
+		    acl->record_type != record_type ||
+		    acl->min_port != min_port || max_port != acl->max_port)
+			continue;
+		if (record_type == TOMOYO_IP_RECORD_TYPE_IPv4) {
+			if (acl->u.ipv4.min != min_ip ||
+			    max_ip != acl->u.ipv4.max)
+				continue;
+		} else if (record_type == TOMOYO_IP_RECORD_TYPE_IPv6) {
+			if (acl->u.ipv6.min != saved_min_address ||
+			    saved_max_address != acl->u.ipv6.max)
+				continue;
+		}
+		ptr->type &= ~TOMOYO_ACL_DELETED;
+		error = 0;
+		goto out;
+	}
+	/* Not found. Append it to the tail. */
+	acl = tomoyo_alloc_acl_element(TOMOYO_TYPE_IP_NETWORK_ACL);
+	if (!acl)
+		goto out;
+	acl->operation_type = operation;
+	acl->record_type = record_type;
+	if (record_type == TOMOYO_IP_RECORD_TYPE_IPv4) {
+		acl->u.ipv4.min = min_ip;
+		acl->u.ipv4.max = max_ip;
+	} else {
+		acl->u.ipv6.min = saved_min_address;
+		acl->u.ipv6.max = saved_max_address;
+	}
+	acl->min_port = min_port;
+	acl->max_port = max_port;
+	list_add_tail(&acl->head.list, &domain->acl_info_list);
+	error = 0;
+	goto out;
+ delete:
+	error = -ENOENT;
+	list_for_each_entry(ptr, &domain->acl_info_list, list) {
+		if (tomoyo_acl_type2(ptr) != TOMOYO_TYPE_IP_NETWORK_ACL)
+			continue;
+		acl = container_of(ptr, struct tomoyo_ip_network_acl_record,
+				   head);
+		if (acl->operation_type != operation ||
+		    acl->record_type != record_type ||
+		    acl->min_port != min_port || max_port != acl->max_port)
+			continue;
+		if (record_type == TOMOYO_IP_RECORD_TYPE_IPv4) {
+			if (acl->u.ipv4.min != min_ip ||
+			    max_ip != acl->u.ipv4.max)
+				continue;
+		} else if (record_type == TOMOYO_IP_RECORD_TYPE_IPv6) {
+			if (acl->u.ipv6.min != saved_min_address ||
+			    saved_max_address != acl->u.ipv6.max)
+				continue;
+		}
+		ptr->type |= TOMOYO_ACL_DELETED;
+		error = 0;
+		break;
+	}
+ out:
+	up_write(&tomoyo_domain_acl_info_list_lock);
+	/***** EXCLUSIVE SECTION START *****/
+	return error;
+}
+
+/**
+ * tomoyo_check_network_entry - Check permission for network operation.
+ *
+ * @is_ipv6:   True if @address is an IPv6 address.
+ * @operation: Type of operation.
+ * @address:   An IPv4 or IPv6 address.
+ * @port:      Port number.
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+static int tomoyo_check_network_entry(const bool is_ipv6, const u8 operation,
+				      const u32 *address, const u16 port)
+{
+	struct tomoyo_acl_info *ptr;
+	const char *keyword = tomoyo_net2keyword(operation);
+	struct tomoyo_domain_info *domain = tomoyo_domain();
+	const int mode = tomoyo_check_flags(domain, TOMOYO_MAC_FOR_NETWORK);
+	const bool is_enforce = (mode == 3);
+	/* using host byte order to allow u32 comparison than memcmp().*/
+	const u32 ip = ntohl(*address);
+	bool found = false;
+	char buf[64];
+	if (!mode)
+		return 0;
+	down_read(&tomoyo_domain_acl_info_list_lock);
+	list_for_each_entry(ptr, &domain->acl_info_list, list) {
+		struct tomoyo_ip_network_acl_record *acl;
+		if (tomoyo_acl_type2(ptr) != TOMOYO_TYPE_IP_NETWORK_ACL)
+			continue;
+		acl = container_of(ptr, struct tomoyo_ip_network_acl_record,
+				   head);
+		if (acl->operation_type != operation || port < acl->min_port ||
+		    acl->max_port < port)
+			continue;
+		if (acl->record_type == TOMOYO_IP_RECORD_TYPE_IPv4) {
+			if (is_ipv6 ||
+			    ip < acl->u.ipv4.min || acl->u.ipv4.max < ip)
+				continue;
+		} else {
+			if (!is_ipv6 ||
+			    memcmp(acl->u.ipv6.min, address, 16) > 0 ||
+			    memcmp(address, acl->u.ipv6.max, 16) > 0)
+				continue;
+		}
+		found = true;
+		break;
+	}
+	up_read(&tomoyo_domain_acl_info_list_lock);
+	if (found)
+		return 0;
+	memset(buf, 0, sizeof(buf));
+	if (is_ipv6)
+		tomoyo_print_ipv6(buf, sizeof(buf),
+				  (const struct in6_addr *) address);
+	else
+		snprintf(buf, sizeof(buf) - 1, "%pI4", address);
+	if (tomoyo_verbose_mode(domain))
+		printk(KERN_WARNING "TOMOYO-%s: %s to %s %u denied for %s\n",
+		       tomoyo_get_msg(is_enforce), keyword, buf, port,
+		       tomoyo_get_last_name(domain));
+	if (is_enforce)
+		return -EPERM;
+	if (mode == 1 && tomoyo_domain_quota_is_ok(domain))
+		tomoyo_update_network_entry(operation, is_ipv6 ?
+					    TOMOYO_IP_RECORD_TYPE_IPv6 :
+					    TOMOYO_IP_RECORD_TYPE_IPv4,
+					    address, address, port, port,
+					    domain, false);
+	return 0;
+}
+
+/**
+ * tomoyo_write_network_policy - Write "struct tomoyo_ip_network_acl_record" list.
+ *
+ * @data:      String to parse.
+ * @domain:    Pointer to "struct tomoyo_domain_info".
+ * @is_delete: True if it is a delete request.
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+int tomoyo_write_network_policy(char *data, struct tomoyo_domain_info *domain,
+				const bool is_delete)
+{
+	u8 sock_type;
+	u8 operation;
+	u8 record_type;
+	u16 min_address[8];
+	u16 max_address[8];
+	u16 min_port;
+	u16 max_port;
+	u8 count;
+	char *cp1 = strchr(data, ' ');
+	char *cp2;
+	if (!cp1)
+		goto out;
+	cp1++;
+	if (!strncmp(data, "TCP ", 4))
+		sock_type = SOCK_STREAM;
+	else if (!strncmp(data, "UDP ", 4))
+		sock_type = SOCK_DGRAM;
+	else if (!strncmp(data, "RAW ", 4))
+		sock_type = SOCK_RAW;
+	else
+		goto out;
+	cp2 = strchr(cp1, ' ');
+	if (!cp2)
+		goto out;
+	cp2++;
+	if (!strncmp(cp1, "bind ", 5))
+		switch (sock_type) {
+		case SOCK_STREAM:
+			operation = NETWORK_ACL_TCP_BIND;
+			break;
+		case SOCK_DGRAM:
+			operation = NETWORK_ACL_UDP_BIND;
+			break;
+		default:
+			operation = NETWORK_ACL_RAW_BIND;
+		}
+	else if (!strncmp(cp1, "connect ", 8))
+		switch (sock_type) {
+		case SOCK_STREAM:
+			operation = NETWORK_ACL_TCP_CONNECT;
+			break;
+		case SOCK_DGRAM:
+			operation = NETWORK_ACL_UDP_CONNECT;
+			break;
+		default:
+			operation = NETWORK_ACL_RAW_CONNECT;
+		}
+	else if (sock_type == SOCK_STREAM && !strncmp(cp1, "listen ", 7))
+		operation = NETWORK_ACL_TCP_LISTEN;
+	else if (sock_type == SOCK_STREAM && !strncmp(cp1, "accept ", 7))
+		operation = NETWORK_ACL_TCP_ACCEPT;
+	else
+		goto out;
+	cp1 = strchr(cp2, ' ');
+	if (!cp1)
+		goto out;
+	*cp1++ = '\0';
+	switch (tomoyo_parse_ip_address(cp2, min_address, max_address)) {
+	case 2:
+		record_type = TOMOYO_IP_RECORD_TYPE_IPv6;
+		break;
+	case 1:
+		record_type = TOMOYO_IP_RECORD_TYPE_IPv4;
+		break;
+	default:
+		goto out;
+	}
+	if (strchr(cp1, ' '))
+		goto out;
+	count = sscanf(cp1, "%hu-%hu", &min_port, &max_port);
+	if (count != 1 && count != 2)
+		goto out;
+	if (count == 1)
+		max_port = min_port;
+	return tomoyo_update_network_entry(operation, record_type,
+					   (u32 *) min_address,
+					   (u32 *) max_address,
+					   min_port, max_port, domain,
+					   is_delete);
+ out:
+	return -EINVAL;
+}
+
+/**
+ * tomoyo_check_network_listen_acl - Check permission for listen() operation.
+ *
+ * @is_ipv6: True if @address is an IPv6 address.
+ * @address: An IPv4 or IPv6 address.
+ * @port:    Port number.
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+static inline int tomoyo_check_network_listen_acl(const bool is_ipv6,
+						  const u8 *address,
+						  const u16 port)
+{
+	return tomoyo_check_network_entry(is_ipv6, NETWORK_ACL_TCP_LISTEN,
+					  (const u32 *) address, ntohs(port));
+}
+
+/**
+ * tomoyo_check_network_connect_acl - Check permission for connect() operation.
+ *
+ * @is_ipv6:   True if @address is an IPv6 address.
+ * @sock_type: Type of socket. (TCP or UDP or RAW)
+ * @address:   An IPv4 or IPv6 address.
+ * @port:      Port number.
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+static inline int tomoyo_check_network_connect_acl(const bool is_ipv6,
+						   const int sock_type,
+						   const u8 *address,
+						   const u16 port)
+{
+	u8 operation;
+	switch (sock_type) {
+	case SOCK_STREAM:
+		operation = NETWORK_ACL_TCP_CONNECT;
+		break;
+	case SOCK_DGRAM:
+		operation = NETWORK_ACL_UDP_CONNECT;
+		break;
+	default:
+		operation = NETWORK_ACL_RAW_CONNECT;
+	}
+	return tomoyo_check_network_entry(is_ipv6, operation,
+					  (const u32 *) address, ntohs(port));
+}
+
+/**
+ * tomoyo_check_network_bind_acl - Check permission for bind() operation.
+ *
+ * @is_ipv6:   True if @address is an IPv6 address.
+ * @sock_type: Type of socket. (TCP or UDP or RAW)
+ * @address:   An IPv4 or IPv6 address.
+ * @port:      Port number.
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+static int tomoyo_check_network_bind_acl(const bool is_ipv6,
+					 const int sock_type,
+					 const u8 *address, const u16 port)
+{
+	u8 operation;
+	switch (sock_type) {
+	case SOCK_STREAM:
+		operation = NETWORK_ACL_TCP_BIND;
+		break;
+	case SOCK_DGRAM:
+		operation = NETWORK_ACL_UDP_BIND;
+		break;
+	default:
+		operation = NETWORK_ACL_RAW_BIND;
+	}
+	return tomoyo_check_network_entry(is_ipv6, operation,
+					  (const u32 *) address, ntohs(port));
+}
+
+/**
+ * tomoyo_check_network_accept_acl - Check permission for accept() operation.
+ *
+ * @is_ipv6: True if @address is an IPv6 address.
+ * @address: An IPv4 or IPv6 address.
+ * @port:    Port number.
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+static inline int tomoyo_check_network_accept_acl(const bool is_ipv6,
+						  const u8 *address,
+						  const u16 port)
+{
+	return tomoyo_check_network_entry(is_ipv6, NETWORK_ACL_TCP_ACCEPT,
+					  (const u32 *) address, ntohs(port));
+}
+
+/**
+ * tomoyo_check_network_sendmsg_acl - Check permission for sendmsg() operation.
+ *
+ * @is_ipv6:   True if @address is an IPv6 address.
+ * @sock_type: Type of socket. (UDP or RAW)
+ * @address:   An IPv4 or IPv6 address.
+ * @port:      Port number.
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+static inline int tomoyo_check_network_sendmsg_acl(const bool is_ipv6,
+						   const int sock_type,
+						   const u8 *address,
+						   const u16 port)
+{
+	u8 operation;
+	if (sock_type == SOCK_DGRAM)
+		operation = NETWORK_ACL_UDP_CONNECT;
+	else
+		operation = NETWORK_ACL_RAW_CONNECT;
+	return tomoyo_check_network_entry(is_ipv6, operation,
+					  (const u32 *) address, ntohs(port));
+}
+
+/**
+ * tomoyo_check_network_recvmsg_acl - Check permission for recvmsg() operation.
+ *
+ * @is_ipv6:   True if @address is an IPv6 address.
+ * @sock_type: Type of socket. (UDP or RAW)
+ * @address:   An IPv4 or IPv6 address.
+ * @port:      Port number.
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+static inline int tomoyo_check_network_recvmsg_acl(const bool is_ipv6,
+						   const int sock_type,
+						   const u8 *address,
+						   const u16 port)
+{
+	const u8 operation
+		= (sock_type == SOCK_DGRAM) ?
+		NETWORK_ACL_UDP_CONNECT : NETWORK_ACL_RAW_CONNECT;
+	return tomoyo_check_network_entry(is_ipv6, operation,
+					  (const u32 *) address, ntohs(port));
+}
+
+#define MAX_SOCK_ADDR 128 /* net/socket.c */
+
+/**
+ * tomoyo_socket_listen_permission - Check permission for listening a TCP socket.
+ *
+ * @sock: Pointer to "struct socket".
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+int tomoyo_socket_listen_permission(struct socket *sock)
+{
+	int error = 0;
+	char addr[MAX_SOCK_ADDR];
+	int addr_len;
+	/* Nothing to do if I am a kernel service. */
+	if (segment_eq(get_fs(), KERNEL_DS))
+		return 0;
+	if (sock->type != SOCK_STREAM)
+		return 0;
+	switch (sock->sk->sk_family) {
+	case PF_INET:
+	case PF_INET6:
+		break;
+	default:
+		return 0;
+	}
+	if (sock->ops->getname(sock, (struct sockaddr *) addr, &addr_len, 0))
+		return -EPERM;
+	switch (((struct sockaddr *) addr)->sa_family) {
+		struct sockaddr_in6 *addr6;
+		struct sockaddr_in *addr4;
+	case AF_INET6:
+		addr6 = (struct sockaddr_in6 *) addr;
+		error = tomoyo_check_network_listen_acl(true, addr6->sin6_addr.
+							s6_addr,
+							addr6->sin6_port);
+		break;
+	case AF_INET:
+		addr4 = (struct sockaddr_in *) addr;
+		error = tomoyo_check_network_listen_acl(false,
+							(u8 *) &addr4->sin_addr,
+							addr4->sin_port);
+		break;
+	}
+	return error;
+}
+
+/**
+ * tomoyo_socket_connect_permission - Check permission for setting the remote IP address/port pair of a socket.
+ *
+ * @sock: Pointer to "struct socket".
+ * @addr: Pointer to "struct sockaddr".
+ * @len:  Size of @addr in bytes.
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+int tomoyo_socket_connect_permission(struct socket *sock, struct sockaddr *addr,
+				     int addr_len)
+{
+	int error = 0;
+	const unsigned int type = sock->type;
+	/* Nothing to do if I am a kernel service. */
+	if (segment_eq(get_fs(), KERNEL_DS))
+		return 0;
+	switch (type) {
+	case SOCK_STREAM:
+	case SOCK_DGRAM:
+	case SOCK_RAW:
+		break;
+	default:
+		return 0;
+	}
+	switch (addr->sa_family) {
+		struct sockaddr_in6 *addr6;
+		struct sockaddr_in *addr4;
+		u16 port;
+	case AF_INET6:
+		if (addr_len < SIN6_LEN_RFC2133)
+			break;
+		addr6 = (struct sockaddr_in6 *) addr;
+		if (type != SOCK_RAW)
+			port = addr6->sin6_port;
+		else
+			port = htons(sock->sk->sk_protocol);
+		error = tomoyo_check_network_connect_acl(true, type,
+							 addr6->sin6_addr.
+							 s6_addr, port);
+		break;
+	case AF_INET:
+		if (addr_len < sizeof(struct sockaddr_in))
+			break;
+		addr4 = (struct sockaddr_in *) addr;
+		if (type != SOCK_RAW)
+			port = addr4->sin_port;
+		else
+			port = htons(sock->sk->sk_protocol);
+		error = tomoyo_check_network_connect_acl(false, type, (u8 *)
+							 &addr4->sin_addr,
+							 port);
+		break;
+	}
+	return error;
+}
+
+/**
+ * tomoyo_socket_bind_permission - Check permission for setting the local IP address/port pair of a socket.
+ *
+ * @sock: Pointer to "struct socket".
+ * @addr: Pointer to "struct sockaddr".
+ * @len:  Size of @addr in bytes.
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+int tomoyo_socket_bind_permission(struct socket *sock, struct sockaddr *addr,
+				  int addr_len)
+{
+	int error = 0;
+	const unsigned int type = sock->type;
+	/* Nothing to do if I am a kernel service. */
+	if (segment_eq(get_fs(), KERNEL_DS))
+		return 0;
+	switch (type) {
+	case SOCK_STREAM:
+	case SOCK_DGRAM:
+	case SOCK_RAW:
+		break;
+	default:
+		return 0;
+	}
+	switch (addr->sa_family) {
+		struct sockaddr_in6 *addr6;
+		struct sockaddr_in *addr4;
+		u16 port;
+	case AF_INET6:
+		if (addr_len < SIN6_LEN_RFC2133)
+			break;
+		addr6 = (struct sockaddr_in6 *) addr;
+		if (type != SOCK_RAW)
+			port = addr6->sin6_port;
+		else
+			port = htons(sock->sk->sk_protocol);
+		error = tomoyo_check_network_bind_acl(true, type,
+						      addr6->sin6_addr.s6_addr,
+						      port);
+		break;
+	case AF_INET:
+		if (addr_len < sizeof(struct sockaddr_in))
+			break;
+		addr4 = (struct sockaddr_in *) addr;
+		if (type != SOCK_RAW)
+			port = addr4->sin_port;
+		else
+			port = htons(sock->sk->sk_protocol);
+		error = tomoyo_check_network_bind_acl(false, type,
+						      (u8 *) &addr4->sin_addr,
+						      port);
+		break;
+	}
+	return error;
+}
+
+/**
+ * tomoyo_socket_accept_permission - Check permission for accepting a TCP socket.
+ *
+ * @sock: Pointer to "struct socket".
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+int tomoyo_socket_accept_permission(struct socket *sock)
+{
+	int error = 0;
+	struct sockaddr_storage addr;
+	int addr_len;
+	/* Nothing to do if I am a kernel service. */
+	if (segment_eq(get_fs(), KERNEL_DS))
+		return 0;
+	switch (sock->sk->sk_family) {
+	case PF_INET:
+	case PF_INET6:
+		break;
+	default:
+		return 0;
+	}
+	error = sock->ops->getname(sock, (struct sockaddr *) &addr, &addr_len,
+				   2);
+	if (error)
+		return error;
+	switch (((struct sockaddr *) &addr)->sa_family) {
+		struct sockaddr_in6 *addr6;
+		struct sockaddr_in *addr4;
+	case AF_INET6:
+		addr6 = (struct sockaddr_in6 *) &addr;
+		error = tomoyo_check_network_accept_acl(true, addr6->sin6_addr.
+							s6_addr,
+							addr6->sin6_port);
+		break;
+	case AF_INET:
+		addr4 = (struct sockaddr_in *) &addr;
+		error = tomoyo_check_network_accept_acl(false, (u8 *) &addr4->
+							sin_addr,
+							addr4->sin_port);
+		break;
+	}
+	if (error)
+		error = -ECONNABORTED; /* Hope less harmful than -EPERM. */
+	return error;
+}
+
+/**
+ * tomoyo_socket_sendmsg_permission - Check permission for sending a datagram via a UDP or RAW socket.
+ *
+ * @sock:     Pointer to "struct socket".
+ * @addr:     Pointer to "struct sockaddr".
+ * @addr_len: Size of @addr in bytes.
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+int tomoyo_socket_sendmsg_permission(struct socket *sock, struct sockaddr *addr,
+				     int addr_len)
+{
+	int error = 0;
+	const int type = sock->type;
+	/* Nothing to do if I am a kernel service. */
+	if (segment_eq(get_fs(), KERNEL_DS))
+		return 0;
+	if (!addr || (type != SOCK_DGRAM && type != SOCK_RAW))
+		return 0;
+	switch (addr->sa_family) {
+		struct sockaddr_in6 *addr6;
+		struct sockaddr_in *addr4;
+		u16 port;
+	case AF_INET6:
+		if (addr_len < SIN6_LEN_RFC2133)
+			break;
+		addr6 = (struct sockaddr_in6 *) addr;
+		if (type == SOCK_DGRAM)
+			port = addr6->sin6_port;
+		else
+			port = htons(sock->sk->sk_protocol);
+		error = tomoyo_check_network_sendmsg_acl(true, type,
+							 addr6->sin6_addr.
+							 s6_addr, port);
+		break;
+	case AF_INET:
+		if (addr_len < sizeof(struct sockaddr_in))
+			break;
+		addr4 = (struct sockaddr_in *) addr;
+		if (type == SOCK_DGRAM)
+			port = addr4->sin_port;
+		else
+			port = htons(sock->sk->sk_protocol);
+		error = tomoyo_check_network_sendmsg_acl(false, type, (u8 *)
+							 &addr4->sin_addr,
+							 port);
+		break;
+	}
+	return error;
+}
+
+/**
+ * tomoyo_socket_recv_datagram_permission - Check permission for receiving a datagram via a UDP or RAW socket.
+ *
+ * @sk:    Pointer to "struct sock".
+ * @skb:   Pointer to "struct sk_buff".
+ * @flags: Flags for recvmsg().
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+int tomoyo_socket_recv_datagram_permission(struct sock *sk, struct sk_buff *skb,
+					   const unsigned int flags)
+{
+	int error = 0;
+	const unsigned int type = sk->sk_type;
+	/* Nothing to do if I can't sleep. */
+	if (in_atomic())
+		return 0;
+	/* Nothing to do if I am a kernel service. */
+	if (segment_eq(get_fs(), KERNEL_DS))
+		return 0;
+	if (type != SOCK_DGRAM && type != SOCK_RAW)
+		return 0;
+
+	switch (sk->sk_family) {
+		struct in6_addr sin6;
+		struct in_addr sin4;
+		u16 port;
+	case PF_INET6:
+		if (type == SOCK_DGRAM) { /* UDP IPv6 */
+			if (skb->protocol == htons(ETH_P_IP)) {
+				ipv6_addr_set(&sin6, 0, 0, htonl(0xffff),
+					      ip_hdr(skb)->saddr);
+			} else {
+				ipv6_addr_copy(&sin6, &ipv6_hdr(skb)->saddr);
+			}
+			port = udp_hdr(skb)->source;
+		} else { /* RAW IPv6 */
+			ipv6_addr_copy(&sin6, &ipv6_hdr(skb)->saddr);
+			port = htons(sk->sk_protocol);
+		}
+		error = tomoyo_check_network_recvmsg_acl(true, type,
+							 (u8 *) &sin6, port);
+		break;
+	case PF_INET:
+		if (type == SOCK_DGRAM) { /* UDP IPv4 */
+			sin4.s_addr = ip_hdr(skb)->saddr;
+			port = udp_hdr(skb)->source;
+		} else { /* RAW IPv4 */
+			sin4.s_addr = ip_hdr(skb)->saddr;
+			port = htons(sk->sk_protocol);
+		}
+		error = tomoyo_check_network_recvmsg_acl(false, type,
+							 (u8 *) &sin4, port);
+		break;
+	}
+	if (!error)
+		return 0;
+	/* Hope less harmful than -EPERM. */
+	return -EAGAIN;
+}
--- security-testing-2.6.git.orig/security/tomoyo/tomoyo.c
+++ security-testing-2.6.git/security/tomoyo/tomoyo.c
@@ -10,6 +10,7 @@
  */
 
 #include <linux/security.h>
+#include <linux/socket.h>
 #include "common.h"
 #include "tomoyo.h"
 #include "realpath.h"
@@ -256,6 +257,45 @@ static int tomoyo_dentry_open(struct fil
 	return tomoyo_check_open_permission(tomoyo_domain(), &f->f_path, flags);
 }
 
+static int tomoyo_socket_bind(struct socket *sock, struct sockaddr *address,
+			      int addrlen)
+{
+	return tomoyo_socket_bind_permission(sock, address, addrlen);
+}
+
+static int tomoyo_socket_connect(struct socket *sock, struct sockaddr *address,
+				 int addrlen)
+{
+	return tomoyo_socket_connect_permission(sock, address, addrlen);
+}
+
+static int tomoyo_socket_listen(struct socket *sock, int backlog)
+{
+	return tomoyo_socket_listen_permission(sock);
+}
+
+static int tomoyo_socket_post_accept(struct socket *sock,
+				     struct socket *newsock)
+{
+	return tomoyo_socket_accept_permission(newsock);
+}
+
+static int tomoyo_socket_sendmsg(struct socket *sock, struct msghdr *msg,
+				 int size)
+{
+	return tomoyo_socket_sendmsg_permission(sock, (struct sockaddr *)
+						msg->msg_name,
+						msg->msg_namelen);
+}
+
+
+static int tomoyo_socket_post_recv_datagram(struct sock *sk,
+					    struct sk_buff *skb,
+					    unsigned int flags)
+{
+	return tomoyo_socket_recv_datagram_permission(sk, skb, flags);
+}
+
 static struct security_operations tomoyo_security_ops = {
 	.name                = "tomoyo",
 	.cred_prepare        = tomoyo_cred_prepare,
@@ -274,6 +314,12 @@ static struct security_operations tomoyo
 	.path_mknod          = tomoyo_path_mknod,
 	.path_link           = tomoyo_path_link,
 	.path_rename         = tomoyo_path_rename,
+	.socket_bind               = tomoyo_socket_bind,
+	.socket_listen             = tomoyo_socket_listen,
+	.socket_connect            = tomoyo_socket_connect,
+	.socket_sendmsg            = tomoyo_socket_sendmsg,
+	.socket_post_recv_datagram = tomoyo_socket_post_recv_datagram,
+	.socket_post_accept        = tomoyo_socket_post_accept,
 };
 
 static int __init tomoyo_init(void)
--- security-testing-2.6.git.orig/security/tomoyo/tomoyo.h
+++ security-testing-2.6.git/security/tomoyo/tomoyo.h
@@ -36,10 +36,27 @@ int tomoyo_check_rewrite_permission(stru
 int tomoyo_find_next_domain(struct linux_binprm *bprm,
 			    struct tomoyo_domain_info **next_domain);
 
+struct sock;
+struct sk_buff;
+struct socket;
+struct sockaddr;
+int tomoyo_socket_bind_permission(struct socket *sock, struct sockaddr *addr,
+				  int addr_len);
+int tomoyo_socket_connect_permission(struct socket *sock, struct sockaddr *addr,
+				     int addr_len);
+int tomoyo_socket_listen_permission(struct socket *sock);
+int tomoyo_socket_accept_permission(struct socket *sock);
+int tomoyo_socket_sendmsg_permission(struct socket *sock, struct sockaddr *addr,
+				     int addr_len);
+int tomoyo_socket_recv_datagram_permission(struct sock *sk, struct sk_buff *skb,
+					   const unsigned int flags);
+
+
 /* Index numbers for Access Controls. */
 
 #define TOMOYO_TYPE_SINGLE_PATH_ACL                 0
 #define TOMOYO_TYPE_DOUBLE_PATH_ACL                 1
+#define TOMOYO_TYPE_IP_NETWORK_ACL                  2
 
 /* Index numbers for File Controls. */
 

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Paul Moore]

On Wednesday 15 April 2009 01:12:41 am Tetsuo Handa wrote:
> Hello.
>
> Paul Moore wrote:
> > Please submit a patch set that includes both this patch as well as a
> > patch to TOMOYO which makes use of these changes; this way we can
> > properly review your patches in context.

Thank you for sending a patchset with both TOMOYO and LSM/stack changes; this 
should give other developers who may not be familiar with TOMOYO a chance to 
review your code.  My comments/concerns about the LSM changes still stand, if 
it is decided to merge these changes I'll be happy to review the TOMOYO 
changes further.

> > > +	if (!(flags & MSG_PEEK))
> > > +		goto no_peek;
> > > +	/*
> > > +	 * If this packet is MSG_PEEK'ed, dequeue it forcibly
> > > +	 * so that this packet won't prevent the caller from picking up
> > > +	 * next packet.
> > > +	 */
> > > +	spin_lock_irqsave(&sk->sk_receive_queue.lock, cpu_flags);
> > > +	if (skb == skb_peek(&sk->sk_receive_queue)) {
> > > +		__skb_unlink(skb, &sk->sk_receive_queue);
> > > +		atomic_dec(&skb->users);
> > > +		/* Do I have something to do with skb->peeked ? */
> >
> > I don't know but you should find out before this code is merged :)
>
> Q1: Can I use skb_kill_datagram() here?
>
>     skb_kill_datagram() uses spin_lock_bh() while __skb_recv_datagram()
> uses spin_lock_irqsave(). Since this codepath is called inside
>     __skb_recv_datagram(), I used spin_lock_irqsave() rather than calling
>     skb_kill_datagram().

Since __skb_recv_datagram() is already using spin_lock_irqsave() it seems 
reasonable to do the same in your changes.  As far as skb->peeked is concerned 
I don't think it matters much since you are destroying the skb anyway.

> > > +no_peek:
> > > +	kfree_skb(skb);
>
> Q2: Do I need to use skb_free_datagram() here rather than kfree_skb()?
>
>     In the past ( http://lkml.org/lkml/2007/11/16/406 ), there was no
>     difference between skb_free_datagram() and kfree_skb().
>
>     | void skb_free_datagram(struct sock *sk, struct sk_buff *skb)
>     | {
>     | 	kfree_skb(skb);
>     | }
>
>     But now (as of 2.6.30-rc2), there is a difference.
>
>     | void skb_free_datagram(struct sock *sk, struct sk_buff *skb)
>     | {
>     | 	consume_skb(skb);
>     | 	sk_mem_reclaim_partial(sk);
>     | }

I don't know for certain, I would have to go look at other users of 
skb_free_datagram(), but it does look like using skb_free_datagram() instead 
of skb_free() might be preferable.

> Q3: Is __skb_recv_datagram() called from contexts that are not permitted to
>     sleep?
>
>     If so, TOMOYO has to check whether it is allowed to sleep, for TOMOYO
> will prompt the user "whether to allow App1 to read this datagram or not".

I believe __skb_recv_datagram() is only called via userspace so sleeping 
should not be an issue.

> Q4: Is there a way to distinguish requests from userland programs and
> requests from kernel code?

I'm not sure if this is the 100% correct way to do it, but in the past I have 
always checked current->mm, for kernel threads this will be NULL.

-- 
paul moore
linux @ hp

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Tetsuo Handa]

Hello.

Thank you for answering my questions.

Paul Moore wrote:
> > Q1: Can I use skb_kill_datagram() here?
> >
> >     skb_kill_datagram() uses spin_lock_bh() while __skb_recv_datagram()
> > uses spin_lock_irqsave(). Since this codepath is called inside
> >     __skb_recv_datagram(), I used spin_lock_irqsave() rather than calling
> >     skb_kill_datagram().
> 
> Since __skb_recv_datagram() is already using spin_lock_irqsave() it seems 
> reasonable to do the same in your changes.
I see. I'll use spin_lock_irqsave() here.

> > > > +no_peek:
> > > > +	kfree_skb(skb);
> >
> > Q2: Do I need to use skb_free_datagram() here rather than kfree_skb()?
> >
> >     In the past ( http://lkml.org/lkml/2007/11/16/406 ), there was no
> >     difference between skb_free_datagram() and kfree_skb().
> >
> >     | void skb_free_datagram(struct sock *sk, struct sk_buff *skb)
> >     | {
> >     | 	kfree_skb(skb);
> >     | }
> >
> >     But now (as of 2.6.30-rc2), there is a difference.
> >
> >     | void skb_free_datagram(struct sock *sk, struct sk_buff *skb)
> >     | {
> >     | 	consume_skb(skb);
> >     | 	sk_mem_reclaim_partial(sk);
> >     | }
> 
> I don't know for certain, I would have to go look at other users of 
> skb_free_datagram(), but it does look like using skb_free_datagram() instead
 
> of skb_free() might be preferable.
I see. I'll use skb_free_datagram() here.

Also, I'll need to protect skb_free_datagram() with lock_sock()/
release_sock().
(Thanks to Eric Dumazet.)

> > Q3: Is __skb_recv_datagram() called from contexts that are not permitted 
to
> >     sleep?
> >
> >     If so, TOMOYO has to check whether it is allowed to sleep, for TOMOYO
> > will prompt the user "whether to allow App1 to read this datagram or not".
> 
> I believe __skb_recv_datagram() is only called via userspace so sleeping 
> should not be an issue.
> 
NFS code needs to issue UDP send()/recv() requests from the 
kernel. Therefore,
I think __skb_recv_datagram() is called from kernel space.

I'm worrying that __skb_recv_datagram() is called with a 
spinlock held.

> > Q4: Is there a way to distinguish requests from userland programs and
> > requests from kernel code?
> 
> I'm not sure if this is the 100% correct way to do it, but in the past I 
have 
> always checked current->mm, for kernel threads this will be NULL.

Nowadays, it will be "current->mm && !(current->flags & PF_
KTHREAD)" because
get_task_mm() says a kernel workthread may temporarily have a 
user mm to do its
AIO.

Sorry for confusing question. What I wanted to know is not "how 
can I
distinguish kernel processes and userland processes". What I 
wanted to know is
"how can I distinguish requests issued for processing a request 
>from userland
process".

(a) If a file is on a simple filesystem like ext3, an open()/
read()/write()
    request from userspace application will not issue another
    open()/read()/write() request.

(b) If a file is on a stackable filesystem like unionfs, an open
()/create()/
    unlink() request from userspace application will issue 
another
    open()/create()/unlink() request from kernel space.

(c) If a file is on a networking filesystem like nfs, an open()/
create()/
    unlink() request from userspace application will issue send
()/recv()
    request from kernel space.

If (b) and (c) use a dedicated task_struct for handling requests
 from kernel
space, TOMOYO has nothing to do.
But unfortunately, (b) and (c) use a task_struct of the 
userspace application
for handling requests from kernel space. TOMOYO wants to 
distinguish
an open()/create()/unlink() request from userspace application 
and 
open()/create()/unlink()/send()/recv() requests needed for 
handling
the open()/create()/unlink() request from userspace application.
I wished that there is an indicator that tells TOMOYO that 
whether an
open()/create()/unlink()/send()/recv() request is issued for 
processing
a request from userland process.

I know little about NFS, but I think NFS does not use a kernel 
workthread for
sending/receiving UDP packets to handle an open()/read()/write()
 request from
a userspace application.

I'm afraid that checking "current->mm && !(current->flags & PF_
KTHREAD)" will
not help TOMOYO in distinguishing "direct request" and "indirect
 request".






By the way, I need to tell you one more thing about
security_socket_post_accept() hook's usage. Not now, but in 
future,
I want to introduce task's state variable which is used for 
dividing
permissions within a domain.

  # Example policy for /usr/sbin/sshd
  allow_network TCP accept 10.0.0.0-10.255.255.255.255 1024-
65535 ; set task.state[0]=1
  allow_network TCP accept 192.168.0.0-192.168.255.255 1024-
65535 ; set task.state[0]=2
  allow_execute /bin/bash if task.state[0]=1
  allow_execute /bin/rbash if task.state[0]=2

The above example policy allows an instance of /usr/sbin/sshd to
(a) execute /bin/bash if that instance accepted a TCP connection
 from
    10.0.0.0/8
(b) execute /bin/rbash if that instance accepted a TCP 
connection from
    192.168.0.0/16.
(c) abort TCP connections if that instance accepted a TCP 
connection from
    neither 10.0.0.0/8 nor 192.168.0.0/16.

The security_socket_post_accept() hook is used for not only 
aborting TCP
connections from unwanted peers but also associating client's 
information
with a process who handles that TCP connection. The task's state
 variable
definitely requires a LSM hook which is called after sock->ops->
accept() call.

[PATCH v2] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Tetsuo Handa]

David Miller wrote:
> >> We worked so hard to split out this common code, it is simply
> >> a non-starter for anyone to start putting protocol specific test
> >> into here, or even worse to move this code back to being locally
> >> copied into every protocol implementation.
> > You don't want LSM modules to perform protocol specific test inside
> > __skb_recv_datagram(). I see.
> > 
> >> You may want to think about how you can achieve your goals by putting
> >> these unpleasant hooks into some other location.
> > May I insert security_socket_post_recv_datagram() into the caller of
> > skb_recv_datagram() (as shown below)?
> 
> This definitely looks better, yes.
> 
I see. Thank you very much.

Now, I'd like to repost an updated patch.
Do you have questions/problems with this patch? > netdev and lsm folks
----------------------------------------
Subject: LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

This patch allows LSM modules to filter incoming connections/datagrams based on
the process's security context who is attempting to pick up.

There are already hooks for filtering incoming connections/datagrams based on
the socket's security context, but these hooks are not applicable when someone
wants to do TCP Wrapper-like filtering (e.g. App1 is permitted to accept TCP
connections from 192.168.0.0/16) because nobody can tell who picks them up
before the moment of sock->ops->accept()/sock->ops->recvmsg() call.

Precautions: These hooks have a side effect if improperly used.

If a socket is shared by multiple processes with different policy, the process
who should be able to accept this connection will not be able to accept this
connection because socket_post_accept() aborts this connection.
This is needed because the process who must not be able to accept this
connection will repeat accept() request (and consume CPU resource) forever
if socket_post_accept() doesn't abort this connection.

Similarly, if a socket is shared by multiple processes with different policy,
the process who should be able to pick up this datagram will not be able to
pick up this datagram because socket_post_recv_datagram() discards this
datagram.
This is needed because the process who must not be able to pick up this
datagram will repeat recvmsg() request (and consume CPU resource) forever
if socket_post_recv_datagram() doesn't discard this datagram.

Therefore, don't give different policy between processes who share one socket.
Otherwise, some connections/datagrams cannot be delivered to intended process.

Signed-off-by: Kentaro Takeda <takedakn@nttdata.co.jp>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Toshiharu Harada <haradats@nttdata.co.jp>

 include/linux/security.h |   39 +++++++++++++++++++++++++++++++++++++++
 net/ipv4/raw.c           |    5 +++++
 net/ipv4/udp.c           |    7 +++++++
 net/ipv6/raw.c           |    5 +++++
 net/ipv6/udp.c           |    7 +++++++
 net/socket.c             |    4 ++++
 security/capability.c    |   13 +++++++++++++
 security/security.c      |   11 +++++++++++
 8 files changed, 91 insertions(+)

---

--- security-testing-2.6.git.orig/include/linux/security.h
+++ security-testing-2.6.git/include/linux/security.h
@@ -880,6 +880,17 @@ static inline void security_free_mnt_opt
  *	@sock contains the listening socket structure.
  *	@newsock contains the newly created server socket for connection.
  *	Return 0 if permission is granted.
+ * @socket_post_accept:
+ *	This hook allows a security module to filter connections from unwanted
+ *	peers based on the process accepting this connection.
+ *	The connection will be aborted if this hook returns nonzero.
+ *	This hook is not designed for updating security attributes of
+ *	an accept()ed socket, for the accept()ed socket has already sent
+ *	several packets (e.g. TCP's SYN/ACK packet and some ACK packets for
+ *	incoming data) before this hook is called.
+ *	@sock contains the listening socket structure.
+ *	@newsock contains the newly created server socket for connection.
+ *	Return 0 if permission is granted.
  * @socket_sendmsg:
  *	Check permission before transmitting a message to another socket.
  *	@sock contains the socket structure.
@@ -893,6 +904,15 @@ static inline void security_free_mnt_opt
  *	@size contains the size of message structure.
  *	@flags contains the operational flags.
  *	Return 0 if permission is granted.
+ * @socket_post_recv_datagram:
+ *	Check permission after receiving a datagram.
+ *	This hook allows a security module to filter packets
+ *	from unwanted peers based on the process receiving this datagram.
+ *	The packet will be discarded if this hook returns nonzero.
+ *	@sk contains the socket.
+ *	@skb contains the socket buffer.
+ *	@flags contains the operational flags.
+ *	Return 0 if permission is granted.
  * @socket_getsockname:
  *	Check permission before the local address (name) of the socket object
  *	@sock is retrieved.
@@ -1549,10 +1569,13 @@ struct security_operations {
 			       struct sockaddr *address, int addrlen);
 	int (*socket_listen) (struct socket *sock, int backlog);
 	int (*socket_accept) (struct socket *sock, struct socket *newsock);
+	int (*socket_post_accept) (struct socket *sock, struct socket *newsock);
 	int (*socket_sendmsg) (struct socket *sock,
 			       struct msghdr *msg, int size);
 	int (*socket_recvmsg) (struct socket *sock,
 			       struct msghdr *msg, int size, int flags);
+	int (*socket_post_recv_datagram) (struct sock *sk, struct sk_buff *skb,
+					  unsigned int flags);
 	int (*socket_getsockname) (struct socket *sock);
 	int (*socket_getpeername) (struct socket *sock);
 	int (*socket_getsockopt) (struct socket *sock, int level, int optname);
@@ -2530,9 +2553,12 @@ int security_socket_bind(struct socket *
 int security_socket_connect(struct socket *sock, struct sockaddr *address, int addrlen);
 int security_socket_listen(struct socket *sock, int backlog);
 int security_socket_accept(struct socket *sock, struct socket *newsock);
+int security_socket_post_accept(struct socket *sock, struct socket *newsock);
 int security_socket_sendmsg(struct socket *sock, struct msghdr *msg, int size);
 int security_socket_recvmsg(struct socket *sock, struct msghdr *msg,
 			    int size, int flags);
+int security_socket_post_recv_datagram(struct sock *sk, struct sk_buff *skb,
+				       unsigned int flags);
 int security_socket_getsockname(struct socket *sock);
 int security_socket_getpeername(struct socket *sock);
 int security_socket_getsockopt(struct socket *sock, int level, int optname);
@@ -2608,6 +2634,12 @@ static inline int security_socket_accept
 	return 0;
 }
 
+static inline int security_socket_post_accept(struct socket *sock,
+					      struct socket *newsock)
+{
+	return 0;
+}
+
 static inline int security_socket_sendmsg(struct socket *sock,
 					  struct msghdr *msg, int size)
 {
@@ -2621,6 +2653,13 @@ static inline int security_socket_recvms
 	return 0;
 }
 
+static inline int security_socket_post_recv_datagram(struct sock *sk,
+						     struct sk_buff *skb,
+						     unsigned int flags)
+{
+	return 0;
+}
+
 static inline int security_socket_getsockname(struct socket *sock)
 {
 	return 0;
--- security-testing-2.6.git.orig/net/ipv4/raw.c
+++ security-testing-2.6.git/net/ipv4/raw.c
@@ -666,6 +666,11 @@ static int raw_recvmsg(struct kiocb *ioc
 	skb = skb_recv_datagram(sk, flags, noblock, &err);
 	if (!skb)
 		goto out;
+	err = security_socket_post_recv_datagram(sk, skb, flags);
+	if (err) {
+		skb_kill_datagram(sk, skb, flags);
+		goto out;
+	}
 
 	copied = skb->len;
 	if (len < copied) {
--- security-testing-2.6.git.orig/net/ipv4/udp.c
+++ security-testing-2.6.git/net/ipv4/udp.c
@@ -901,6 +901,13 @@ try_again:
 				  &peeked, &err);
 	if (!skb)
 		goto out;
+	err = security_socket_post_recv_datagram(sk, skb, flags);
+	if (err) {
+		lock_sock(sk);
+		skb_kill_datagram(sk, skb, flags);
+		release_sock(sk);
+		goto out;
+	}
 
 	ulen = skb->len - sizeof(struct udphdr);
 	copied = len;
--- security-testing-2.6.git.orig/net/ipv6/raw.c
+++ security-testing-2.6.git/net/ipv6/raw.c
@@ -465,6 +465,11 @@ static int rawv6_recvmsg(struct kiocb *i
 	skb = skb_recv_datagram(sk, flags, noblock, &err);
 	if (!skb)
 		goto out;
+	err = security_socket_post_recv_datagram(sk, skb, flags);
+	if (err) {
+		skb_kill_datagram(sk, skb, flags);
+		goto out;
+	}
 
 	copied = skb->len;
 	if (copied > len) {
--- security-testing-2.6.git.orig/net/ipv6/udp.c
+++ security-testing-2.6.git/net/ipv6/udp.c
@@ -208,6 +208,13 @@ try_again:
 				  &peeked, &err);
 	if (!skb)
 		goto out;
+	err = security_socket_post_recv_datagram(sk, skb, flags);
+	if (err) {
+		lock_sock(sk);
+		skb_kill_datagram(sk, skb, flags);
+		release_sock(sk);
+		goto out;
+	}
 
 	ulen = skb->len - sizeof(struct udphdr);
 	copied = len;
--- security-testing-2.6.git.orig/net/socket.c
+++ security-testing-2.6.git/net/socket.c
@@ -1519,6 +1519,10 @@ SYSCALL_DEFINE4(accept4, int, fd, struct
 	if (err < 0)
 		goto out_fd;
 
+	err = security_socket_post_accept(sock, newsock);
+	if (err)
+		goto out_fd;
+
 	if (upeer_sockaddr) {
 		if (newsock->ops->getname(newsock, (struct sockaddr *)&address,
 					  &len, 2) < 0) {
--- security-testing-2.6.git.orig/security/capability.c
+++ security-testing-2.6.git/security/capability.c
@@ -620,6 +620,11 @@ static int cap_socket_accept(struct sock
 	return 0;
 }
 
+static int cap_socket_post_accept(struct socket *sock, struct socket *newsock)
+{
+	return 0;
+}
+
 static int cap_socket_sendmsg(struct socket *sock, struct msghdr *msg, int size)
 {
 	return 0;
@@ -631,6 +636,12 @@ static int cap_socket_recvmsg(struct soc
 	return 0;
 }
 
+static int cap_socket_post_recv_datagram(struct sock *sk, struct sk_buff *skb,
+					 unsigned int flags)
+{
+	return 0;
+}
+
 static int cap_socket_getsockname(struct socket *sock)
 {
 	return 0;
@@ -1010,8 +1021,10 @@ void security_fixup_ops(struct security_
 	set_to_cap_if_null(ops, socket_connect);
 	set_to_cap_if_null(ops, socket_listen);
 	set_to_cap_if_null(ops, socket_accept);
+	set_to_cap_if_null(ops, socket_post_accept);
 	set_to_cap_if_null(ops, socket_sendmsg);
 	set_to_cap_if_null(ops, socket_recvmsg);
+	set_to_cap_if_null(ops, socket_post_recv_datagram);
 	set_to_cap_if_null(ops, socket_getsockname);
 	set_to_cap_if_null(ops, socket_getpeername);
 	set_to_cap_if_null(ops, socket_setsockopt);
--- security-testing-2.6.git.orig/security/security.c
+++ security-testing-2.6.git/security/security.c
@@ -1007,6 +1007,11 @@ int security_socket_accept(struct socket
 	return security_ops->socket_accept(sock, newsock);
 }
 
+int security_socket_post_accept(struct socket *sock, struct socket *newsock)
+{
+	return security_ops->socket_post_accept(sock, newsock);
+}
+
 int security_socket_sendmsg(struct socket *sock, struct msghdr *msg, int size)
 {
 	return security_ops->socket_sendmsg(sock, msg, size);
@@ -1018,6 +1023,12 @@ int security_socket_recvmsg(struct socke
 	return security_ops->socket_recvmsg(sock, msg, size, flags);
 }
 
+int security_socket_post_recv_datagram(struct sock *sk, struct sk_buff *skb,
+				       unsigned int flags)
+{
+	return security_ops->socket_post_recv_datagram(sk, skb, flags);
+}
+
 int security_socket_getsockname(struct socket *sock)
 {
 	return security_ops->socket_getsockname(sock);

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Paul Moore]

On Saturday 18 April 2009 04:34:02 am Tetsuo Handa wrote:
> Hello.
>
> Thank you for answering my questions.

No problem.

> > I believe __skb_recv_datagram() is only called via userspace so sleeping
> > should not be an issue.
>
> NFS code needs to issue UDP send()/recv() requests from the
> kernel. Therefore, I think __skb_recv_datagram() is called from kernel
> space.

My mistake, I trusted (or misread) the comment at the start of the function.

> I'm worrying that __skb_recv_datagram() is called with a
> spinlock held.

It looks like you've already solved that issue.

> > > Q4: Is there a way to distinguish requests from userland programs and
> > > requests from kernel code?
> >
> > I'm not sure if this is the 100% correct way to do it, but in the past I
> > have always checked current->mm, for kernel threads this will be NULL.
>
> Nowadays, it will be "current->mm && !(current->flags & PF_
> KTHREAD)" because get_task_mm() says a kernel workthread may temporarily
> have a user mm to do its AIO.

Thanks, that is good to know.

> Sorry for confusing question. What I wanted to know is not "how
> can I distinguish kernel processes and userland processes". What I
> wanted to know is "how can I distinguish requests issued for processing a
> request from userland process".

I do not know of a way but someone else reading this might.

> By the way, I need to tell you one more thing about
> security_socket_post_accept() hook's usage. Not now, but in future,
> I want to introduce task's state variable which is used for dividing
> permissions within a domain.
>
>   # Example policy for /usr/sbin/sshd
>   allow_network TCP accept 10.0.0.0-10.255.255.255.255 1024-
> 65535 ; set task.state[0]=1
>   allow_network TCP accept 192.168.0.0-192.168.255.255 1024-
> 65535 ; set task.state[0]=2
>   allow_execute /bin/bash if task.state[0]=1
>   allow_execute /bin/rbash if task.state[0]=2
>
> The above example policy allows an instance of /usr/sbin/sshd to
> (a) execute /bin/bash if that instance accepted a TCP connection
>  from
>     10.0.0.0/8
> (b) execute /bin/rbash if that instance accepted a TCP
> connection from
>     192.168.0.0/16.
> (c) abort TCP connections if that instance accepted a TCP
> connection from
>     neither 10.0.0.0/8 nor 192.168.0.0/16.
>
> The security_socket_post_accept() hook is used for not only
> aborting TCP connections from unwanted peers but also associating client's
> information with a process who handles that TCP connection. The task's state
> variable definitely requires a LSM hook which is called after sock->ops->
> accept() call.

I don't have a problem with using a socket_post_accept() hook to assign/modify 
state, however, I still not like the idea of using the socket_post_accept() 
hook to abort connections.

-- 
paul moore
linux @ hp

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Tetsuo Handa]

Paul Moore wrote:
> > The security_socket_post_accept() hook is used for not only
> > aborting TCP connections from unwanted peers but also associating client's
> > information with a process who handles that TCP connection. The task's state
> > variable definitely requires a LSM hook which is called after sock->ops->
> > accept() call.
> 
> I don't have a problem with using a socket_post_accept() hook to assign/modify 
> state, however, I still not like the idea of using the socket_post_accept()
> hook to abort connections.

What do you think that assigning/modifying state at socket_post_accept() could
fail?

TOMOYO 1.x never fails since task's state variable is directly attached to
"struct task_struct" but TOMOYO 2.x can fail since task's state variable will
be indirectly attached via current->cred->security and memory allocation
failure for new credential could occur. Though it unlikely fails, I have to
abort connections by returning an error at socket_post_accept() hook if failed.

I think the socket_post_accept() hook anyway need to be able to abort
connections. (Or prepare new credential at socket_accept() and add a hook for
discarding prepared credential when sock->ops->accept() returned an error?)

Regards.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[David Miller]

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Tue, 21 Apr 2009 19:54:06 +0900

> Paul Moore wrote:
>> > The security_socket_post_accept() hook is used for not only
>> > aborting TCP connections from unwanted peers but also associating client's
>> > information with a process who handles that TCP connection. The task's state
>> > variable definitely requires a LSM hook which is called after sock->ops->
>> > accept() call.
>> 
>> I don't have a problem with using a socket_post_accept() hook to assign/modify 
>> state, however, I still not like the idea of using the socket_post_accept()
>> hook to abort connections.
> 
> What do you think that assigning/modifying state at socket_post_accept() could
> fail?

I have to agree with Paul.

There is no sane way for the user to handle this connection
being aborted, and there is no way to insert the connection
back into the listening socket queue once we get to this
point so we can't replay this situation either.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Tetsuo Handa]

David Miller wrote:
> There is no sane way for the user to handle this connection
> being aborted, and there is no way to insert the connection
> back into the listening socket queue once we get to this
> point so we can't replay this situation either.
> 
Excuse me, I couldn't catch.

I don't have a problem that there is no way to insert the connection back into
the listening socket queue once we get to this point. I want to drop the
connection rather than pushing the connection back to the queue.

I meant that "if we allow doing something after sock->ops->accept(), we need to
drop the connection if something returned an error". An example of something is
to assign/modify state. To modify task's state, I need to allocate memory for
new credential which is an operation that could fail.
Therefore, the socket_post_accept() hook should allow aborting the connection.

Paul doesn't like using the socket_post_accept() hook to abort connections.
But the connection is aborted if either newsock->ops->getname() or
move_addr_to_user() returned an error. I wonder what's so problematic with
using socket_post_accept() for dropping the connection.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[David Miller]

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Tue, 21 Apr 2009 20:39:08 +0900

> David Miller wrote:
>> There is no sane way for the user to handle this connection
>> being aborted, and there is no way to insert the connection
>> back into the listening socket queue once we get to this
>> point so we can't replay this situation either.
>> 
> Excuse me, I couldn't catch.
> 
> I don't have a problem that there is no way to insert the connection back into
> the listening socket queue once we get to this point. I want to drop the
> connection rather than pushing the connection back to the queue.

I am saying that you shouldn't be dropping connections like
this.

You've already accepted the packet, you cannot reneg on this.
You must either allow the socket to have the connection with
current labels or give it a label appropriate for the socket.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Tetsuo Handa]

David Miller wrote:
> I am saying that you shouldn't be dropping connections like
> this.
Please don't assume that we are talking about labeled networking.
I want to implement TCPwrapper-like filtering (which is automatically linked to
all processes).

> You've already accepted the packet, you cannot reneg on this.
TCPwrapper accepts the packet before it decides to drop the connection.

> You must either allow the socket to have the connection with
> current labels or give it a label appropriate for the socket.
The socket_post_accept() hook is not designed for modifying labels of a socket.
It is too late to modify because we have already accepted the packet.

+ * @socket_post_accept:
+ *	This hook allows a security module to filter connections from unwanted
+ *	peers based on the process accepting this connection.
+ *	The connection will be aborted if this hook returns nonzero.
+ *	This hook is not designed for updating security attributes of
+ *	an accept()ed socket, for the accept()ed socket has already sent
+ *	several packets (e.g. TCP's SYN/ACK packet and some ACK packets for
+ *	incoming data) before this hook is called.
+ *	@sock contains the listening socket structure.
+ *	@newsock contains the newly created server socket for connection.
+ *	Return 0 if permission is granted.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[David Miller]

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Tue, 21 Apr 2009 21:26:36 +0900

> It is too late to modify because we have already accepted the
> packet.

Then queue it up somewhere, and deliver your verdict and continue
packet processing later.

In fact, I do not like killing accept()'s like this at all.

If we tell the application, via poll() or similar, that connections
are available and no other thread can get in there to steal the
connection, we must deliver a connection to the listening socket when
it calls accept() except in very unusual circumstances (out of file
descriptors, memory allocation failure, etc.)!

I believe this idea is conceptually very broken, sorry.  The semantics
are absolutely horrible.

Put this stuff in userspace, where you say it already is.  The kernel
isn't the place to dump every cool feature you want to bring in from
userspace.

Re: [PATCH] LSM: Add security_socket_post_accept() andsecurity_socket_post_recv_datagram().

[Tetsuo Handa]

David Miller wrote:
> Put this stuff in userspace, where you say it already is.  The kernel
> isn't the place to dump every cool feature you want to bring in from
> userspace.
If I can do this stuff in userspace, I already put this stuff in userspace.
Access control in userspace is easily bypassed, no good for policy based
mandatory access control. The reason I propose these hooks in kernel is that
nobody can bypass these hooks.

Re: [PATCH] LSM: Add security_socket_post_accept() andsecurity_socket_post_recv_datagram().

[David Miller]

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Tue, 21 Apr 2009 21:52:57 +0900

> David Miller wrote:
>> Put this stuff in userspace, where you say it already is.  The kernel
>> isn't the place to dump every cool feature you want to bring in from
>> userspace.
> If I can do this stuff in userspace, I already put this stuff in userspace.
> Access control in userspace is easily bypassed, no good for policy based
> mandatory access control. The reason I propose these hooks in kernel is that
> nobody can bypass these hooks.

They you have to make your decisions when the packet arrives
in order to not break application expected semantics.

If poll() says to a listening socket that connections are
available, we MUST return a connection from accept() unless
there is a hard error.

The current proposal is not acceptable.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Tetsuo Handa]

David Miller wrote:
> If poll() says to a listening socket that connections are
> available, we MUST return a connection from accept() unless
> there is a hard error.

No application is permitted to assume that accept() returns a connection if
poll() says that connections are available even if there is an assumption that
none of sock->ops->accept(), newsock->ops->getname(), move_addr_to_user()
fails, for there is security_socket_accept() hook which can interrupt between
"poll()" and "accept()".

There is a possibility that LSM module's policy changes from "allow picking
the connection up from the listening socket" to "deny picking the connection
up from the listening socket" after poll() said "connections are available".
We allow this policy change and we tolerate breakage of the application
expected semantics.

> If we tell the application, via poll() or similar, that connections
> are available and no other thread can get in there to steal the
> connection, we must deliver a connection to the listening socket when
> it calls accept() except in very unusual circumstances (out of file
> descriptors, memory allocation failure, etc.)!

If you think policy changes between poll() and security_socket_accept() is
one of unusual circumstances, I think policy changes between
security_socket_accept() and security_socket_post_accept()
(the old policy before poll() said "connections are available" was "allow
picking the connection up from any client" while the new policy after
poll() said "connections are available" is "allow picking the connection
up from only 127.0.0.1") is also one of unusual circumstances.
The only difference between security_socket_accept() and
security_socket_post_accept() is that whether the connection remains in the
listening socket's queue or not. We cannot avoid breakage of the application
expected semantics from the beginning.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[David Miller]

From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Date: Wed, 22 Apr 2009 09:55:26 +0900

> David Miller wrote:
>> If poll() says to a listening socket that connections are
>> available, we MUST return a connection from accept() unless
>> there is a hard error.
> 
> No application is permitted to assume that accept() returns a connection if
> poll() says that connections are available even if there is an assumption that
> none of sock->ops->accept(), newsock->ops->getname(), move_addr_to_user()
> fails, for there is security_socket_accept() hook which can interrupt between
> "poll()" and "accept()".
> 
> There is a possibility that LSM module's policy changes from "allow picking
> the connection up from the listening socket" to "deny picking the connection
> up from the listening socket" after poll() said "connections are available".
> We allow this policy change and we tolerate breakage of the application
> expected semantics.

We had a similar situation with read()'s on UDP sockets.

When poll() says something, it has to stick.

This is not discussable.  If these semantics are "necessary", then
what you're doing is definitely misdesigned in my opinion.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Tetsuo Handa]

David Miller wrote:
> We had a similar situation with read()'s on UDP sockets.
> 
> When poll() says something, it has to stick.
To adhere what poll() said (i.e. "connections are ready" or "datagrams are
ready"), security_socket_accept() and security_socket_recvmsg() hooks must be
removed. Otherwise, LSM users cannot adhere what poll() said.

However, security_socket_accept() and security_socket_recvmsg() hooks remain
there. LSM users are already using semantics which may not adhere what poll()
said.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Greg Lindahl]

On Tue, Apr 21, 2009 at 06:14:11PM -0700, David Miller wrote:

> We had a similar situation with read()'s on UDP sockets.
> 
> When poll() says something, it has to stick.

Isn't that completely different? Anyone who writes code that calls
accept() quickly finds out that in the real world it fails for all
kinds of reasons worth ignoring. As an example, a comment in ircd at
the only accept call (circa 1998):

   /*
   ** There may be many reasons for error return, but in otherwise
   ** correctly working environment the probable cause is running
   ** out of file descriptors (EMFILE, ENFILE or others?). The
   ** man pages for accept don't seem to list these as possible,
   ** although it's obvious that it may happen here.
   ** Thus no specific errors are tested at this point, just
   ** assume that connections cannot be accepted until some old
   ** is closed first.
   */

And it silently ignores EAGAIN, which of course is a can't happen when
used with select(). The recently-written only-runs-on-Linux system I'm
working on ignores EAGAIN, even though it's a can't happen with
epoll. I can ask the guy who wrote it, but he's probably ignoring it
because he was frequently seeing them.

I'd be surprised if you found much real-life code that didn't
gracefully tolerate accept failures. Can anyone come up with an
example?

-- greg

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[David Miller]

From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Date: Wed, 22 Apr 2009 10:49:42 +0900

> David Miller wrote:
>> We had a similar situation with read()'s on UDP sockets.
>> 
>> When poll() says something, it has to stick.
> To adhere what poll() said (i.e. "connections are ready" or "datagrams are
> ready"), security_socket_accept() and security_socket_recvmsg() hooks must be
> removed. Otherwise, LSM users cannot adhere what poll() said.
> 
> However, security_socket_accept() and security_socket_recvmsg() hooks remain
> there. LSM users are already using semantics which may not adhere what poll()
> said.

So what does your TOMOTO stuff do if the mapping changes again and
that incoming connection that became unacceptable is now acceptable?

We've lost the connection, and can never get it back.

These semantics don't make any sense at all, and the point at which
you make your choices here is totally arbitrary.

Read that carefully: the point at which you are making this
drop decision is arbitrary.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[David Miller]

From: Greg Lindahl <greg@blekko.com>
Date: Tue, 21 Apr 2009 18:52:28 -0700

> On Tue, Apr 21, 2009 at 06:14:11PM -0700, David Miller wrote:
> 
>> We had a similar situation with read()'s on UDP sockets.
>> 
>> When poll() says something, it has to stick.
> 
> Isn't that completely different? Anyone who writes code that calls
> accept() quickly finds out that in the real world it fails for all
> kinds of reasons worth ignoring. As an example, a comment in ircd at
> the only accept call (circa 1998):

I said explicitly that hard errors are allows (out of file
descriptors, memory allocation failure)

Feel free to ignore what I'm saying, and I'll feel free to
ignore you too.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Tetsuo Handa]

David Miller wrote:
> So what does your TOMOTO stuff do if the mapping changes again and
> that incoming connection that became unacceptable is now acceptable?
Nothing.

> We've lost the connection, and can never get it back.
TOMOYO won't keep the connection. That's fine.
TOMOYO thinks that dropping the connection/datagram is better than choking
the queue/buffer by keeping the connection/datagram for future policy changes.

I browsed the poll() logic for TCP/IPv4's listening socket.

SYSCALL_DEFINE3(poll, struct pollfd __user *, ufds, unsigned int, nfds,
                long, timeout_msecs)
{
    int do_sys_poll(struct pollfd __user *ufds, unsigned int nfds,
                    struct timespec *end_time)
    {
        static int do_poll(unsigned int nfds,  struct poll_list *list,
                           struct poll_wqueues *wait,
                           struct timespec *end_time)
        {
            static inline unsigned int do_pollfd(struct pollfd *pollfd,
                                                 poll_table *pwait)
            {
                file->f_op->poll(file, pwait);
            }
        }
    }
}

(file->f_op->poll == sock_poll in net/socket.c)

static unsigned int sock_poll(struct file *file, poll_table *wait)
{
    sock->ops->poll(file, sock, wait);
}

(sock->ops->poll == tcp_poll in net/ipv4/af_inet.c)

unsigned int tcp_poll(struct file *file, struct socket *sock, poll_table *wait)
{
    if (sk->sk_state == TCP_LISTEN) {
        static inline unsigned int inet_csk_listen_poll(const struct sock *sk)
        {
            return !reqsk_queue_empty(&inet_csk(sk)->icsk_accept_queue) ?
                                      (POLLIN | POLLRDNORM) : 0;
        }
    }
}

(and reqsk_queue_empty() is defined as)

static inline int reqsk_queue_empty(struct request_sock_queue *queue)
{
    return queue->rskq_accept_head == NULL;
}

If I read the code correctly, there is no LSM hook called for controlling
whether poll() is allowed to say "connections are ready" or not.
If security_socket_accept() rejects the accept() request, a connection remains
in the listening socket's queue. This will cause the subsequent poll() requests
to say "connections are ready" but the subsequent accept() requests to say
"you are not permitted to pick a connection up", and will continue consuming
100% of CPU resource until security_socket_accept() says "you are permitted to
pick a connection up".

Did I miss something?

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[David Miller]

From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Date: Wed, 22 Apr 2009 14:02:16 +0900

> Did I miss something?

Do me a favor.  We have mechanisms by which you can queue up packets
(even into userspace) to make policy decisions.

You can make your decision there, and if appropriate, reinject the
packet back into the kernel input processing.

This is how netfilter ip_queue allows people to implement policies
in userspace, and you could use it in the kernel and then need
none of these ugly hooks.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Tetsuo Handa]

David Miller wrote:
> Do me a favor.  We have mechanisms by which you can queue up packets
> (even into userspace) to make policy decisions.
I know.
> 
> You can make your decision there, and if appropriate, reinject the
> packet back into the kernel input processing.
No I can't.
> 
> This is how netfilter ip_queue allows people to implement policies
> in userspace, and you could use it in the kernel and then need
> none of these ugly hooks.
I was suggested to use that approach in the past discussion.

I want to make policy decisions based on the "task_struct" who picks up
the connection/datagram.

The netfilter can't predict which "task_struct" will pick up the
connection/datagram. Nobody can predict it. Even security_socket_accept()
and security_socket_recvmsg() can't predict it.

If TOMOYO is allowed to make policy decisions based on the "task_struct" who
picks up the connection/datagram, the only possible approach is to introduce
security_socket_post_accept() and security_socket_post_recv_datagram().

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[David Miller]

From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Date: Wed, 22 Apr 2009 14:38:04 +0900

> If TOMOYO is allowed to make policy decisions based on the "task_struct" who
> picks up the connection/datagram, the only possible approach is to introduce
> security_socket_post_accept() and security_socket_post_recv_datagram().

Fine.

FWIW I do not agree with TOMOYO conceptually.  But if people are
generally going to let such a scheme into the kernel, I guess
I have to accept these socket layer hacks :-/

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Greg Lindahl]

On Tue, Apr 21, 2009 at 09:23:42PM -0700, David Miller wrote:

> I said explicitly that hard errors are allows (out of file
> descriptors, memory allocation failure)

I have no idea what you meant by a "hard" error. Note that I also
discussed EAGAIN, which appears to happen commonly historically and
today, and appears to be what the security module folks would want to
have happen and you're rejecting. Do you consider that to be a hard
error? I'm betting not.

-- greg

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[David Miller]

From: Greg Lindahl <greg@blekko.com>
Date: Tue, 21 Apr 2009 23:10:06 -0700

> On Tue, Apr 21, 2009 at 09:23:42PM -0700, David Miller wrote:
> 
>> I said explicitly that hard errors are allows (out of file
>> descriptors, memory allocation failure)
> 
> I have no idea what you meant by a "hard" error. Note that I also
> discussed EAGAIN, which appears to happen commonly historically and
> today, and appears to be what the security module folks would want to
> have happen and you're rejecting. Do you consider that to be a hard
> error? I'm betting not.

People use poll() to avoid -EAGAIN and blocking, they expect the bits
to tell them what fd's they can work on to do real work.

But this whole task_struct based security is bogus from the start.

If I dup a file descriptor for a listening socket, and accept() in the
"wrong" task, the other task has no way to accept() that connection
even if it's security settings allow it.  The connection is lost
forever.

The realm of file descriptors overlaps that of tasks.  Trying to
pretend this isn't the case results in all kinds of crazy things like
we see being attempted here.

I consider TOMOYO conceptually broken in many regards.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Greg Lindahl]

On Tue, Apr 21, 2009 at 11:34:03PM -0700, David Miller wrote:

> People use poll() to avoid -EAGAIN and blocking, they expect the bits
> to tell them what fd's they can work on to do real work.

My point is that EAGAIN happens already. So you can't claim that
returning it in accept() breaks the interface, when it's common enough
that today's user-level network code already handles it.

I have no opinion about TOMOYO. There are many reasons other than
EAGAIN from accept() to complain about.

-- greg

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[David Miller]

From: Greg Lindahl <greg@blekko.com>
Date: Tue, 21 Apr 2009 23:41:59 -0700

> On Tue, Apr 21, 2009 at 11:34:03PM -0700, David Miller wrote:
> 
>> People use poll() to avoid -EAGAIN and blocking, they expect the bits
>> to tell them what fd's they can work on to do real work.
> 
> My point is that EAGAIN happens already. So you can't claim that
> returning it in accept() breaks the interface, when it's common enough
> that today's user-level network code already handles it.

EAGAIN does not happen if the application calls poll(), gets
an indication that connections are available, and then
immediately calls accept() on the indicated FD.

It can only happen if it calls accept() multiple times after such a
poll() call _OR_ it does not control multi-threaded access to the
listen file descriptor.

This new behavior from TOMOYO would make accept() return -EAGAIN in
cases which are of no fault of the application.  It is definitely
unexpected behavior.

If overly anal apps "code for it" that is entirely besides the point.
What we have to be concerned for, from a kernel behavioral standpoint,
is that some apps "might not code for it".  This is why we don't
change behavior.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Greg Lindahl]

On Tue, Apr 21, 2009 at 11:46:18PM -0700, David Miller wrote:

> EAGAIN does not happen if the application calls poll(), gets
> an indication that connections are available, and then
> immediately calls accept() on the indicated FD.

I have observed it recently and historically, and not by calling
accept() repeatedly. I don't know what you mean by "immediately" since I
don't think you're advocating race conditions; the other end can
always exit/reset/whatever between the poll() and the accept().

> If overly anal apps "code for it" that is entirely besides the point.
> What we have to be concerned for, from a kernel behavioral standpoint,
> is that some apps "might not code for it".  This is why we don't
> change behavior.

I am suggesting that you survey actual apps. If you find that they're
all overly anal, then maybe you've learned something about EAGAIN
already happening today. I assure you that the co-worker who stuck in
the "ignore EAGAIN without logging it" only did so because he saw it
fairly frequently. He's that way.

-- greg

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[David Miller]

From: Greg Lindahl <greg@blekko.com>
Date: Tue, 21 Apr 2009 23:54:43 -0700

> On Tue, Apr 21, 2009 at 11:46:18PM -0700, David Miller wrote:
> 
>> If overly anal apps "code for it" that is entirely besides the point.
>> What we have to be concerned for, from a kernel behavioral standpoint,
>> is that some apps "might not code for it".  This is why we don't
>> change behavior.
> 
> I am suggesting that you survey actual apps. If you find that they're
> all overly anal, then maybe you've learned something about EAGAIN
> already happening today. I assure you that the co-worker who stuck in
> the "ignore EAGAIN without logging it" only did so because he saw it
> fairly frequently. He's that way.

It's likely a bug and should have been reported.

Unexplainable behavior isn't something to ignore.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Tetsuo Handa]

David Miller wrote:
> If I dup a file descriptor for a listening socket, and accept() in the
> "wrong" task, the other task has no way to accept() that connection
> even if it's security settings allow it.  The connection is lost
> forever.
Why the connection gets lost? If two tasks' security settings are the same,
the process whichever reached sock->ops->accept() first will get the connetion.
If two tasks' security settings are not the same, I warned it on the patch
descripption.

> This new behavior from TOMOYO would make accept() return -EAGAIN in
> cases which are of no fault of the application.  It is definitely
> unexpected behavior.
TOMOYO will return -ECONNABORTED, which is also returned by failure of
newsock->ops->getname().

If there were some application which can't handle accept() returning
-ECONNABORTED error, we can simply disable this filtering (by giving such
application permission to accept connection from all addresses).
Applications should be able to handle accept() error other than -EAGAIN.
It is legal to return (for example) -ENOMEM, -EPERM. "man 2 accept" says:

ERRORS
       accept() shall fail if:

       EAGAIN or EWOULDBLOCK
              The socket is marked non-blocking and no connections are present to be accepted.

       EBADF  The descriptor is invalid.

       ECONNABORTED
              A connection has been aborted.

       EINTR  The system call was interrupted by a signal that was caught before a valid connection arrived.

       EINVAL Socket is not listening for connections, or addrlen is invalid (e.g., is negative).

       EMFILE The per-process limit of open file descriptors has been reached.

       ENFILE The system limit on the total number of open files has been reached.

       ENOTSOCK
              The descriptor references a file, not a socket.

       EOPNOTSUPP
              The referenced socket is not of type SOCK_STREAM.

       accept() may fail if:

       EFAULT The addr argument is not in a writable part of the user address space.

       ENOBUFS, ENOMEM
              Not  enough free memory.  This often means that the memory allocation is limited by the socket buffer limits, not by the system memory.

       EPROTO Protocol error.

       Linux accept() may fail if:

       EPERM  Firewall rules forbid connection.

       In addition, network errors for the new socket and as defined for the protocol may be returned. Various  Linux  kernels  can  return
       other errors such as ENOSR, ESOCKTNOSUPPORT, EPROTONOSUPPORT, ETIMEDOUT.  The value ERESTARTSYS may be seen during a trace.

Linux 2.6.7                       2004-06-17                         ACCEPT(2)

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Tetsuo Handa]

David Miller wrote:
> FWIW I do not agree with TOMOYO conceptually.
That will be my fault. I haven't explained you the background of this proposal.
Would you please be patient and read below explanation?

Thanks.
----------
TOMOYO is a security module which focuses on behavior of a system. A process is
created to achieve something. TOMOYO lets each process declare behaviors and
resources needed to achieve its purpose (like an immigration officer) and
permits only declared behaviors and resources (like an operation watchdog).

TOMOYO has an unprecedented concept called "process invocation history" (in
short, PIH). TOMOYO utilizes the PIH for categorizing the purpose of a process.
The PIH is stored into current->cred->security and is defined as concatenation
of program's pathnames ever executed. For example, /sbin/init invoked from the
kernel is defined as "<kernel> /sbin/init", /etc/rc.d/rc.sysinit invoked from
/sbin/init invoked from the kernel is defined as
"<kernel> /sbin/init /etc/rc.d/rc.sysinit". (There are some exceptions, but I
omit explanation because exceptions have no linkage with this proposal.)

**TOMOYO's policy is PIH-driven.** For example,

  <kernel> /sbin/init
  allow_read /etc/inittab

means that any process with PIH "<kernel> /sbin/init" is allowed to open a file
named /etc/inittab for reading.

  <kernel> /usr/sbin/sshd
  allow_create /var/run/sshd.pid

means that any process with PIH "<kernel> /usr/sbin/sshd" is allowed to create
a file named /var/run/sshd.pid .

  <kernel> /usr/sbin/sshd /bin/bash /usr/bin/curl
  allow_network TCP connect 192.168.1.1 80
  allow_network UDP connect 192.168.1.2 53

means that any process with PIH
"<kernel> /usr/sbin/sshd /bin/bash /usr/bin/curl" is allowed to send TCP
connect() requests to 192.168.1.1 port 80 and is allowed to send UDP datagrams
to 192.168.1.2 port 53.

TOMOYO wants to allow writing policy for incoming connections/datagrams in the
same manner. For example,

  <kernel> /usr/sbin/sshd
  allow_network TCP accept 10.0.0.0-10.255.255.255 1024-65535

means that any process with PIH "<kernel> /usr/sbin/sshd" is allowed to pick up
TCP connections from 10.0.0.0/8 port 1024-65535.

To be able to write in the same manner, TOMOYO needs to know the PIH of a
process who is about to pick up the incoming connection/datagram.
The PIH (i.e. current->cred->security) is different from the security context
of a socket which is going to enqueue the incoming connection/datagram
(i.e. "struct sock"->sk_security). And LSM has no hooks which allow TOMOYO to
use current->cred->security for incoming connections/datagrams.

There could be some programs which get confused by accept()/recvmsg() returning
an error when poll() said "connections are ready" or "datagrams are ready".
If we find such programs, we can tell TOMOYO to disable filtering for such
programs.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[David Miller]

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Thu, 23 Apr 2009 23:00:09 +0900

> David Miller wrote:
>> FWIW I do not agree with TOMOYO conceptually.
> That will be my fault. I haven't explained you the background of this proposal.
> Would you please be patient and read below explanation?

Save your typing, I've read all of these descriptions, I still
do not like it.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Samir Bellabes]

Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> writes:

> There could be some programs which get confused by accept()/recvmsg() returning
> an error when poll() said "connections are ready" or "datagrams are ready".
> If we find such programs, we can tell TOMOYO to disable filtering for such
> programs.

Hello Tetsuo,

this will introduce a way to bypass the security system (?)

If TOMOYO won't filter such programs, people may add this "poll()"
feature to their code, in order to escape the security system.

I think it's strange for a security system to allow some programs
because of specific code issue, and not because of security reasons.

sam

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Tetsuo Handa]

David Miller wrote:
> People use poll() to avoid -EAGAIN and blocking, they expect the bits
> to tell them what fd's they can work on to do real work.

I found that "man 2 select" says

  Under Linux, select() may report a socket file descriptor as "ready for
  reading", while nevertheless a subsequent read blocks. This could for example
  happen when data has arrived but upon examination has wrong checksum and is
  discarded. There may be other circumstances in which a file descriptor is
  spuriously reported as ready. Thus it may be safer to use O_NONBLOCK on
  sockets that should not block.

  Linux 2.6.16                   2006-03-11                      SELECT(2)

People cannot use "poll()" to avoid blocking.
Applications had better not to completely depend on what poll() says.

> The current proposal is not acceptable.
You don't like TOMOYO's concept. I see.
But I don't see the reason you can't accept this proposal.
What does this proposal break? Please explain me.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[David Miller]

From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Date: Fri, 24 Apr 2009 11:07:28 +0900

> David Miller wrote:
>> People use poll() to avoid -EAGAIN and blocking, they expect the bits
>> to tell them what fd's they can work on to do real work.
> 
> I found that "man 2 select" says
> 
>   Under Linux, select() may report a socket file descriptor as "ready for
>   reading", while nevertheless a subsequent read blocks. This could for example
>   happen when data has arrived but upon examination has wrong checksum and is
>   discarded.

You won't give up will you?  If you're trying to irritate me, it's
working.

This behavior mentioned in that manpage snippet is a BUG which we
FIXED!  You're just proving my point even more!

The poll() paths now cause a bypass of the delayed checksum
verification, which used to cause that above mentioned incorrect
behavior.

Don't push man page crap at me.  Instead, actually understand what the
code does and how it behaves.

This man page snipped above is completely wrong and buggy.  Never
trust documentation over code.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Thu, 23 Apr 2009 21:35:42 -0700 (PDT)

> This behavior mentioned in that manpage snippet is a BUG which we
> FIXED!  You're just proving my point even more!

And here is that bug fix, for your reference.  The behavior was
wrong, it BROKE real applications, we removed it.

commit edea8fef1415a7499c09149b383a171e83480375
Author: Stephen Hemminger <shemminger@osdl.org>
Date:   Tue Nov 30 05:26:12 2004 -0800

    [UDP]: Select handling of bad checksums.
    
    Alternate workaround for blocking usage of select() by UDP applications.
    The problem is Linux optimizes the UDP receive checksum path so that checksum
    validation is not performed until the application read. This is a performance win
    but can cause applications that do select with blocking file descriptors to get false
    positives if the received message has a checksum error.
    There is a long running thread about this on LKML.
    
    This patch makes these applications work, but keeps the one-pass performance gain
    for those applications smart enough to use non-blocking file descriptors with
    select/poll. There is still a possibility to get a false positive if application does
    select on non-blocking fd then makes it blocking before doing the receive, but that
    is unlikely.
    
    Tested by injecting bad packets with SOCK_RAW.
    
    Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
    Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/include/net/udp.h b/include/net/udp.h
index 2ef99a7..c496d10 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -71,6 +71,8 @@ extern int	udp_sendmsg(struct kiocb *iocb, struct sock *sk,
 extern int	udp_rcv(struct sk_buff *skb);
 extern int	udp_ioctl(struct sock *sk, int cmd, unsigned long arg);
 extern int	udp_disconnect(struct sock *sk, int flags);
+extern unsigned int udp_poll(struct file *file, struct socket *sock,
+			     poll_table *wait);
 
 DECLARE_SNMP_STAT(struct udp_mib, udp_statistics);
 #define UDP_INC_STATS(field)		SNMP_INC_STATS(udp_statistics, field)
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index f9cacaf..d127c16 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -809,7 +809,7 @@ struct proto_ops inet_dgram_ops = {
 	.socketpair =	sock_no_socketpair,
 	.accept =	sock_no_accept,
 	.getname =	inet_getname,
-	.poll =		datagram_poll,
+	.poll =		udp_poll,
 	.ioctl =	inet_ioctl,
 	.listen =	sock_no_listen,
 	.shutdown =	inet_shutdown,
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index d38f2b2..4a1bfb3 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1303,6 +1303,52 @@ static int udp_getsockopt(struct sock *sk, int level, int optname,
   	return 0;
 }
 
+/**
+ * 	udp_poll - wait for a UDP event.
+ *	@file - file struct
+ *	@sock - socket
+ *	@wait - poll table
+ *
+ *	This is same as datagram poll, except for the special case of 
+ *	blocking sockets. If application is using a blocking fd
+ *	and a packet with checksum error is in the queue;
+ *	then it could get return from select indicating data available
+ *	but then block when reading it. Add special case code
+ *	to work around these arguably broken applications.
+ */
+unsigned int udp_poll(struct file *file, struct socket *sock, poll_table *wait)
+{
+	unsigned int mask = datagram_poll(file, sock, wait);
+	struct sock *sk = sock->sk;
+	
+	/* Check for false positives due to checksum errors */
+	if ( (mask & POLLRDNORM) &&
+	     !(file->f_flags & O_NONBLOCK) &&
+	     !(sk->sk_shutdown & RCV_SHUTDOWN)){
+		struct sk_buff_head *rcvq = &sk->sk_receive_queue;
+		struct sk_buff *skb;
+
+		spin_lock_irq(&rcvq->lock);
+		while ((skb = skb_peek(rcvq)) != NULL) {
+			if (udp_checksum_complete(skb)) {
+				UDP_INC_STATS_BH(UDP_MIB_INERRORS);
+				__skb_unlink(skb, rcvq);
+				kfree_skb(skb);
+			} else {
+				skb->ip_summed = CHECKSUM_UNNECESSARY;
+				break;
+			}
+		}
+		spin_unlock_irq(&rcvq->lock);
+
+		/* nothing to see, move along */
+		if (skb == NULL)
+			mask &= ~(POLLIN | POLLRDNORM);
+	}
+
+	return mask;
+	
+}
 
 struct proto udp_prot = {
  	.name =		"UDP",
@@ -1517,6 +1563,7 @@ EXPORT_SYMBOL(udp_ioctl);
 EXPORT_SYMBOL(udp_port_rover);
 EXPORT_SYMBOL(udp_prot);
 EXPORT_SYMBOL(udp_sendmsg);
+EXPORT_SYMBOL(udp_poll);
 
 #ifdef CONFIG_PROC_FS
 EXPORT_SYMBOL(udp_proc_register);
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 0190359..1d10fb1 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -501,7 +501,7 @@ struct proto_ops inet6_dgram_ops = {
 	.socketpair =	sock_no_socketpair,		/* a do nothing	*/
 	.accept =	sock_no_accept,			/* a do nothing	*/
 	.getname =	inet6_getname, 
-	.poll =		datagram_poll,			/* ok		*/
+	.poll =		udp_poll,			/* ok		*/
 	.ioctl =	inet6_ioctl,			/* must change  */
 	.listen =	sock_no_listen,			/* ok		*/
 	.shutdown =	inet_shutdown,			/* ok		*/

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Tetsuo Handa]

David Miller wrote:
> This behavior mentioned in that manpage snippet is a BUG which we
> FIXED!  You're just proving my point even more!
So, people can use poll() to avoid -EAGAIN and blocking. I see.

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[Tetsuo Handa]

OK. I understood that security_socket_post_recv_datagram() must not return
-EAGAIN if a process calls recvmsg() after poll() said "ready".
That will be also true for security_socket_recvmsg().


Is it OK for security_socket_recvmsg()/security_socket_accept() to return
an error other than -EAGAIN?
(In other words, security_socket_recvmsg()/security_socket_accept() errors are
one of "hard" errors?)

Re: [PATCH] LSM: Add security_socket_post_accept() and security_socket_post_recv_datagram().

[David Miller]

From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Date: Fri, 24 Apr 2009 14:26:24 +0900

> OK. I understood that security_socket_post_recv_datagram() must not return
> -EAGAIN if a process calls recvmsg() after poll() said "ready".
> That will be also true for security_socket_recvmsg().
> 
> 
> Is it OK for security_socket_recvmsg()/security_socket_accept() to return
> an error other than -EAGAIN?
> (In other words, security_socket_recvmsg()/security_socket_accept() errors are
> one of "hard" errors?)

I think you really need to wrap your head around the fact that you
can't decide after you've accepted a packet, that it's no longer
acceptable.  Once it's in the socket's queue, and you tell the
application it's there at poll() time, you simply cannot reneg.

You just can't.

Otherwise you're breaking the whole premise upon which these UNIX
system calls are based.  This is how people use these things.

Are you beginning to understand the fundamental problems I have with
your work?

Re: [PATCH] LSM: Add security_socket_post_accept() andsecurity_socket_post_recv_datagram().

[Tetsuo Handa]

David Miller wrote:
> Are you beginning to understand the fundamental problems I have with
> your work?
You are saying that we must not discard what we have in the queue after we told
somebody that we have something in the queue unless we get "hard" errors.

> If poll() says to a listening socket that connections are
> available, we MUST return a connection from accept() unless
> there is a hard error.
I haven't understood what you meant by a "hard" error. What I want to know is,
are errors returned by security_socket_accept() and security_socket_recvmsg()
"hard" errors?

If these errors are not "hard" errors, security_socket_accept() and
security_socket_recvmsg() must not return an error because it will result in
"not returning a connection/datagram", for you are saying that only "hard"
errors can justify "not returning a connection/datagram".

If these errors are "hard" errors, security_socket_accept() and
security_socket_recvmsg() can justify "not returning a connection/datagram".
But errors by newsock->ops->getname() or move_addr_to_user() (these are also
"hard" errors, aren't they?) justified "discarding a connection/datagram".
Then, if a "hard" error was returned by LSM hooks after a connection/datagram
was dequeued, I think we can justify "discarding that connection/datagram".