mac_portacl and automatic port allocation

[Michal Mertl]

Robert Watson wrote(a):
> On Sun, 21 Nov 2004, Michal Mertl wrote:
> 
> 
>>I really like the idea behind mac_portacl but I find it difficult to use
>>it because of one issue. When an unprivileged program binds to high
>>automatic port with a call to bind(2) and port number set to 0 the
>>system chooses the port to bind to itself. This mechanismus is used by
>>number of programs, most commonly by ftp clients in active mode.
>>Unfortunately this 0 is checked by the mac_portacl(4) module and the
>>call to bind is refused. Rather simple fix would be to check if the
>>local port is 0 and user hasn't asked for IP_PORTRANGE_LOW and then
>>allow the call to trivially succeed. It can be controlled by a sysctl if
>>needed. 
>>
>>What do you think of the patch below?
> 
> 
> Seems like a good change to me.  Technically, there's probably a slight
> atomicity problem relating to threads, since one thread could change the
> flag while another thread is making the call to bind the socket.  I'm not
> sure that's easily fixed without a specific MAC check in the inet code,
> and what you propose is certainly a big improvement over what is there.

I noticed this potential problem. I don't think it's too serious. But it may 
possibly allow attacker to bind to low ports, potentially block legal use of 
them. That's why I offered the behavior in this case may be controlled by a 
sysctl.

Extended patch attached. Binding to zero is disabled by default.

> 
> I'll get this, sans the printf, merged sometime today.
> 

Thank you.

> Thanks!
> 
> Robert N M Watson             FreeBSD Core Team, TrustedBSD Projects
> robert at fledge.watson.org      Principal Research Scientist, McAfee Research
> 
> 
> 
> 

-------------- next part --------------
--- mac_portacl.c.orig	Tue Nov 23 17:02:05 2004
+++ mac_portacl.c	Tue Nov 23 17:08:39 2004
@@ -79,6 +79,7 @@
 #include <sys/sysctl.h>
 
 #include <netinet/in.h>
+#include <netinet/in_pcb.h>
 
 #include <vm/vm.h>
 
@@ -100,6 +101,12 @@
 TUNABLE_INT("security.mac.portacl.suser_exempt",
     &mac_portacl_suser_exempt);
 
+static int	mac_portacl_allow_bind_zero = 0;
+SYSCTL_INT(_security_mac_portacl, OID_AUTO, allow_bind_zero, CTLFLAG_RW,
+    &mac_portacl_allow_bind_zero, 0, "Whether binding to port 0 (system chooses the port automaticaly) is permitted");
+TUNABLE_INT("security.mac.portacl.allow_bind_zero",
+    &mac_portacl_allow_bind_zero);
+
 static int	mac_portacl_port_high = 1023;
 SYSCTL_INT(_security_mac_portacl, OID_AUTO, port_high, CTLFLAG_RW,
     &mac_portacl_port_high, 0, "Highest port to enforce for");
@@ -441,6 +448,7 @@
     struct label *socketlabel, struct sockaddr *sockaddr)
 {
 	struct sockaddr_in *sin;
+	struct inpcb *inp = sotoinpcb(so); 
 	int family, type;
 	u_int16_t port;
 
@@ -467,6 +475,11 @@
 	type = so->so_type;
 	sin = (struct sockaddr_in *) sockaddr;
 	port = ntohs(sin->sin_port);
+	/* If port == 0 and user hasn't asked for IP_PORTRANGELOW return
+	success */
+	if (mac_portacl_allow_bind_zero && port == 0 &&
+	    (inp->inp_flags & INP_LOWPORT) == 0)
+		return (0);
 
 	return (rules_check(cred, family, type, port));
 }