autofs v5: Problem with @network in access_lists

autofs v5: Problem with @network in access_lists

[Bill Maloy]

Solaris NFS servers can be configured to use a network
specifier in the access_list of an NFS exported file
system.

From <http://docs.sun.com/app/docs/doc/816-0211/6m6nc676n?a=view>
"The network or subnet component is preceded by an at-sign (@). "

This feature does not play nicely with autofs-5.0.1-rc3.

In the host_match() function within lib/rpc_subs.c, the
initial character of the formal parameter named "pattern"
is compared against '@' ... indicating a need to check
the NFS client's hostname (myname) for membership
in the given netgroup.

However, the existing "if/else-if" logic does not
consider the case where a network address (and mask)
follows the '@' character -- instead of an "expected"
netgroup name.

You can confirm this behavior by modifying the access
control list of a working v5 automounting export
(containing an @netgroup specification) to have
a Solaris-like @network/mask specifier, instead.

The fix?  Well, if the innetgr() function supported
"IPaddress/mask"-style strings in addition to netgroup
names, the code as written would probably support
access_lists containing network components.

Until that day, the character following the '@' sign
in an exports list item could be examined.  If numeric,
the masked_match() logic of the 'else' branch could
be used.  If non-numeric, the existing innetgr()
logic would be used.

This might beg for the inet_aton() logic to be
made into a function call, since it would be
potentially executed in the "if (*m_pattern == '@')"
branch as well as in the else branch.

I didn't feel comfortable submitting such a patch,
since I've only just recently started looking at
the automounter v5 source code (guess why), and
wasn't completely sure that my analysis was correct.

--
Bill.Maloy@gmail.com

Re: autofs v5: Problem with @network in access_lists

[Ian Kent]

On Wed, 2007-01-24 at 13:13 -0600, Bill Maloy wrote:
> Solaris NFS servers can be configured to use a network
> specifier in the access_list of an NFS exported file
> system.
> 
> >From <http://docs.sun.com/app/docs/doc/816-0211/6m6nc676n?a=view>
> "The network or subnet component is preceded by an at-sign (@). "

*Sigh*
I missed that as well when I updated to account for the Solaris export
semantics.

> 
> This feature does not play nicely with autofs-5.0.1-rc3.
> 
> In the host_match() function within lib/rpc_subs.c, the
> initial character of the formal parameter named "pattern"
> is compared against '@' ... indicating a need to check
> the NFS client's hostname (myname) for membership
> in the given netgroup.
> 
> However, the existing "if/else-if" logic does not
> consider the case where a network address (and mask)
> follows the '@' character -- instead of an "expected"
> netgroup name.

That's right.

> 
> You can confirm this behavior by modifying the access
> control list of a working v5 automounting export
> (containing an @netgroup specification) to have
> a Solaris-like @network/mask specifier, instead.

Don't need to confirm it, that is the way it works atm.

> 
> The fix?  Well, if the innetgr() function supported
> "IPaddress/mask"-style strings in addition to netgroup
> names, the code as written would probably support
> access_lists containing network components.

That's not going to happen.

> 
> Until that day, the character following the '@' sign
> in an exports list item could be examined.  If numeric,
> the masked_match() logic of the 'else' branch could
> be used.  If non-numeric, the existing innetgr()
> logic would be used.

Something like that, I'll have a look.

> 
> This might beg for the inet_aton() logic to be
> made into a function call, since it would be
> potentially executed in the "if (*m_pattern == '@')"
> branch as well as in the else branch.

Yes, I need to separate that anyway to make IPv6 conversion simpler when
it's needed.

> 
> I didn't feel comfortable submitting such a patch,
> since I've only just recently started looking at
> the automounter v5 source code (guess why), and
> wasn't completely sure that my analysis was correct.

Yep. You're right.

Ian

Re: autofs v5: Problem with @network in access_lists

[Ian Kent]

On Thu, 2007-01-25 at 16:37 +0900, Ian Kent wrote:
> On Wed, 2007-01-24 at 13:13 -0600, Bill Maloy wrote:
> > Solaris NFS servers can be configured to use a network
> > specifier in the access_list of an NFS exported file
> > system.
> > 
> > >From <http://docs.sun.com/app/docs/doc/816-0211/6m6nc676n?a=view>
> > "The network or subnet component is preceded by an at-sign (@). "
> 
> *Sigh*
> I missed that as well when I updated to account for the Solaris export
> semantics.

How about trying this patch out.
I haven't had a chance to test this at all yet so beware.

Ian

diff --git a/lib/rpc_subs.c b/lib/rpc_subs.c
index b4e9c91..a794d76 100644
--- a/lib/rpc_subs.c
+++ b/lib/rpc_subs.c
@@ -31,6 +31,7 @@
 #include <rpcsvc/ypclnt.h>
 #include <errno.h>
 #include <sys/ioctl.h>
+#include <pthread.h>
 
 #include "mount.h"
 #include "rpc_subs.h"
@@ -43,9 +44,14 @@
 #endif
 
 #define MAX_ERR_BUF	512
+/* Get numeric value of the n bits starting at position p */
+#define getbits(x, p, n)	((x >> (p + 1 - n)) & ~(~0 << n))
 
 static char *ypdomain = NULL;
 
+inline void dump_core(void);
+static pthread_mutex_t networks_mutex = PTHREAD_MUTEX_INITIALIZER;
+
 /*
  * Create a UDP RPC client
  */
@@ -965,12 +971,127 @@ static int string_match(const char *myname, const char *pattern)
 	return ret;
 }
 
+static unsigned int get_networks_mask(struct netent *net)
+{
+	if (IN_CLASSC(net->n_net))
+		return 24;
+	else if (IN_CLASSB(net->n_net))
+		return 16;
+	else if (IN_CLASSA(net->n_net))
+		return 8;
+
+	return 0;
+}
+
+static unsigned int inet_get_net_len(uint32_t net)
+{
+	int i;
+
+	for (i = 0; i < 32; i++) {
+		if (getbits(net, i, 1))
+			break;
+	}
+
+	return (unsigned int) i;
+}
+
+static char *inet_fill_net(const char *net_num, char *net)
+{
+	char *np;
+	unsigned int dots = 3;
+
+	*net = '\0';
+	strcpy(net, net_num);
+
+	np = net;
+	while (*np) {
+		if (*np++ == '.') {
+			dots--;
+			if (!*np && dots)
+				strcat(net, "0");
+		}
+	}
+
+	while (dots--)
+		strcat(net, ".0");
+
+	return net;
+}
+
+static int match_network(const char *network)
+{
+	struct netent *pnent, nent;
+	const char *pcnet;
+	char *net, cnet[17], mask[4], *pmask;
+	unsigned int size;
+	int status;
+
+	net = alloca(strlen(network) + 1);
+	if (!net)
+		return 0;
+	strcpy(net, network);
+
+	if ((pmask = strchr(net, '/')))
+		*pmask++ = '\0';
+
+	status = pthread_mutex_lock(&networks_mutex);
+	if (status)
+		fatal(status);
+
+	pnent = getnetbyname(net);
+	if (pnent)
+		memcpy(&nent, pnent, sizeof(struct netent));
+
+	status = pthread_mutex_unlock(&networks_mutex);
+	if (status)
+		fatal(status);
+
+	if (pnent) {
+		uint32_t n_net;
+
+		n_net = ntohl(nent.n_net);
+		pcnet = inet_ntop(nent.n_addrtype, &n_net, cnet, 16);
+		if (!pcnet)
+			return 0;
+
+		if (!pmask) {
+			size = get_networks_mask(&nent);
+			if (!size)
+				return 0;
+		}
+	} else {
+		struct in_addr addr;
+		int ret;
+
+		pcnet = inet_fill_net(net, cnet);
+		if (!pcnet)
+			return 0;
+
+		ret = inet_pton(AF_INET, pcnet, &addr);
+		if (ret <= 0)
+			return 0;
+
+		if (!pmask) {
+			size = inet_get_net_len(htonl(addr.s_addr));
+			if (!size)
+				return 0;
+		}
+	}
+
+	if (!pmask) {
+		if (sprintf(mask, "%u", size) <= 0)
+			return 0;
+		pmask = mask;
+	}
+
+	return masked_match(pcnet, mask);
+}
+
 static int host_match(char *pattern)
 {
 	unsigned int negate = (*pattern == '-');
 	const char *m_pattern = (negate ? pattern + 1 : pattern);
 	char myname[MAXHOSTNAMELEN + 1] = "\0";
-	struct in_addr tmp;
 	int ret = 0;
 
 	if (gethostname(myname, MAXHOSTNAMELEN))
@@ -982,26 +1103,13 @@ static int host_match(char *pattern)
 	if (*m_pattern == '@') {
 		if (ypdomain)
 			ret = innetgr(m_pattern + 1, myname, NULL, ypdomain);
-	} else if (inet_aton(m_pattern, &tmp) || strchr(m_pattern, '/')) {
-		size_t len = strlen(m_pattern) + 1;
-		char *addr, *mask;
-
-		addr = alloca(len);
-		if (!addr)
-			return 0;
-
-		memset(addr, 0, len);
-		memcpy(addr, m_pattern, len - 1);
-		mask = strchr(addr, '/');
-		if (mask) {
-			*mask++ = '\0';
-			ret = masked_match(addr, mask);
-		} else
-			ret = masked_match(addr, "32");
+		if (!ret)
+			ret = match_network(m_pattern + 1);
 	} else if (!strcmp(m_pattern, "gss/krb5")) {
 		/* Leave this to the GSS layer */
 		ret = 1;
-	} else
+	} else if ((ret = match_network(m_pattern))) ;
+	else
 		ret = string_match(myname, m_pattern);
 
 	if (negate)

Re: autofs v5: Problem with @network in access_lists

[Ian Kent]

On Wed, 2007-01-24 at 13:13 -0600, Bill Maloy wrote:
> >From <http://docs.sun.com/app/docs/doc/816-0211/6m6nc676n?a=view>
> "The network or subnet component is preceded by an at-sign (@). "
> 
> This feature does not play nicely with autofs-5.0.1-rc3.
> 
> In the host_match() function within lib/rpc_subs.c, the
> initial character of the formal parameter named "pattern"
> is compared against '@' ... indicating a need to check
> the NFS client's hostname (myname) for membership
> in the given netgroup.
> 
> However, the existing "if/else-if" logic does not
> consider the case where a network address (and mask)
> follows the '@' character -- instead of an "expected"
> netgroup name.
> 
> You can confirm this behavior by modifying the access
> control list of a working v5 automounting export
> (containing an @netgroup specification) to have
> a Solaris-like @network/mask specifier, instead.
> 
> The fix?  Well, if the innetgr() function supported
> "IPaddress/mask"-style strings in addition to netgroup
> names, the code as written would probably support
> access_lists containing network components.
> 
> Until that day, the character following the '@' sign
> in an exports list item could be examined.  If numeric,
> the masked_match() logic of the 'else' branch could
> be used.  If non-numeric, the existing innetgr()
> logic would be used.

No, that won't work.
According to the reference above a network name can contain any
printable character and so could itself be numeric.

Ian