public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] IPv6 support for nfs-utils tcpwrapper shim
@ 2010-01-15 17:49 Chuck Lever
       [not found] ` <20100115174426.30104.3492.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Chuck Lever @ 2010-01-15 17:49 UTC (permalink / raw)
  To: steved; +Cc: chris.mason, linux-nfs

These patches provide IPv6 support for the tcpwrapper shim inside
nfs-utils.  It assumes that the generic tcpwrapper library can
support IPv6 addresses.  It has not been extensively tested, but
I think the framework is reasonable, and only minor bug fixes might
be needed as we go along.

---

Chuck Lever (6):
      tcpwrapper: Add support for IPv6
      tcpwrapper: Eliminated shadowed declaration warnings
      tcpwrapper: Fix signage problems in the tcp_wrappers hash function
      tcp_wrapper: Clean up logit()
      tcp_wrappers: Use getifaddrs(3) if it is available
      tcpwrappers: Use xlog() instead of perror(3) and syslog(2)


 aclocal/ipv6.m4               |    4 -
 configure.ac                  |    2 
 support/include/tcpwrapper.h  |   12 +-
 support/misc/from_local.c     |  112 +++++++++++++++++++----
 support/misc/tcpwrapper.c     |  203 ++++++++++++++++++++++++-----------------
 utils/mountd/mount_dispatch.c |    6 -
 utils/statd/statd.c           |    5 -
 utils/statd/statd.man         |    3 -
 8 files changed, 223 insertions(+), 124 deletions(-)

-- 
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/6] tcpwrappers: Use xlog() instead of perror(3) and syslog(2)
       [not found] ` <20100115174426.30104.3492.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
@ 2010-01-15 17:49   ` Chuck Lever
  2010-01-15 17:49   ` [PATCH 2/6] tcp_wrappers: Use getifaddrs(3) if it is available Chuck Lever
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Chuck Lever @ 2010-01-15 17:49 UTC (permalink / raw)
  To: steved; +Cc: chris.mason, linux-nfs

Clean up: Replace calls to syslog(2) and perror(3) in from_local.c
with calls to xlog().  The problems displayed by the perror(3) calls
especially should be reported.  Currently they are never seen in the
system log.

As part of a build test, I defined TEST, and found a couple of
problems with main(), which are also addressed in this patch.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 support/misc/from_local.c |   25 +++++++++++++------------
 1 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/support/misc/from_local.c b/support/misc/from_local.c
index 89ccc4a..3f46b99 100644
--- a/support/misc/from_local.c
+++ b/support/misc/from_local.c
@@ -37,8 +37,8 @@
 static char sccsid[] = "@(#) from_local.c 1.3 96/05/31 15:52:57";
 #endif
 
-#ifdef TEST
-#undef perror
+#ifdef HAVE_CONFIG_H
+#include <config.h>
 #endif
 
 #include <sys/types.h>
@@ -49,10 +49,12 @@ static char sccsid[] = "@(#) from_local.c 1.3 96/05/31 15:52:57";
 #include <netinet/in.h>
 #include <net/if.h>
 #include <sys/ioctl.h>
-#include <syslog.h>
 #include <stdlib.h>
 #include <string.h>
 
+#include "tcpwrapper.h"
+#include "xlog.h"
+
 #ifndef TRUE
 #define	TRUE	1
 #define FALSE	0
@@ -81,7 +83,7 @@ static int grow_addrs(void)
     new_num = (addrs == 0) ? 1 : num_addrs + num_addrs;
     new_addrs = (struct in_addr *) malloc(sizeof(*addrs) * new_num);
     if (new_addrs == 0) {
-	perror("portmap: out of memory");
+	xlog_warn("%s: out of memory", __func__);
 	return (0);
     } else {
 	if (addrs != 0) {
@@ -112,13 +114,13 @@ find_local(void)
      */
 
     if ((sock = socket(PF_INET, SOCK_DGRAM, 0)) < 0) {
-	perror("socket");
+	xlog_warn("%s: socket(2): %m", __func__);
 	return (0);
     }
     ifc.ifc_len = sizeof(buf);
     ifc.ifc_buf = buf;
     if (ioctl(sock, SIOCGIFCONF, (char *) &ifc) < 0) {
-	perror("SIOCGIFCONF");
+	xlog_warn("%s: ioctl(SIOCGIFCONF): %m", __func__);
 	(void) close(sock);
 	return (0);
     }
@@ -130,10 +132,10 @@ find_local(void)
 	if (ifr->ifr_addr.sa_family == AF_INET) {	/* IP net interface */
 	    ifreq = *ifr;
 	    if (ioctl(sock, SIOCGIFFLAGS, (char *) &ifreq) < 0) {
-		perror("SIOCGIFFLAGS");
+		xlog_warn("%s: ioctl(SIOCGIFFLAGS): %m", __func__);
 	    } else if (ifreq.ifr_flags & IFF_UP) {	/* active interface */
 		if (ioctl(sock, SIOCGIFADDR, (char *) &ifreq) < 0) {
-		    perror("SIOCGIFADDR");
+		    xlog_warn("%s: ioctl(SIOCGIFADDR): %m", __func__);
 		} else {
 		    if (num_local >= num_addrs)
 			if (grow_addrs() == 0)
@@ -160,7 +162,7 @@ from_local(struct sockaddr_in *addr)
     int     i;
 
     if (addrs == 0 && find_local() == 0)
-	syslog(LOG_ERR, "cannot find any active local network interfaces");
+	xlog(L_ERROR, "Cannot find any active local network interfaces");
 
     for (i = 0; i < num_local; i++) {
 	if (memcmp((char *) &(addr->sin_addr), (char *) &(addrs[i]),
@@ -172,9 +174,8 @@ from_local(struct sockaddr_in *addr)
 
 #ifdef TEST
 
-main()
+int main(void)
 {
-    char   *inet_ntoa();
     int     i;
 
     find_local();
@@ -182,4 +183,4 @@ main()
 	printf("%s\n", inet_ntoa(addrs[i]));
 }
 
-#endif
+#endif	/* TEST */


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/6] tcp_wrappers: Use getifaddrs(3) if it is available
       [not found] ` <20100115174426.30104.3492.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
  2010-01-15 17:49   ` [PATCH 1/6] tcpwrappers: Use xlog() instead of perror(3) and syslog(2) Chuck Lever
@ 2010-01-15 17:49   ` Chuck Lever
  2010-01-15 17:49   ` [PATCH 3/6] tcp_wrapper: Clean up logit() Chuck Lever
                     ` (4 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Chuck Lever @ 2010-01-15 17:49 UTC (permalink / raw)
  To: steved; +Cc: chris.mason, linux-nfs

After glibc 2.3.3, getifaddrs(3) can return AF_INET6 addresses for
local network interfaces.  Using the library call is easier than
trying to update the open code in from_local(), and means we have
less to maintain in nfs-utils.

Since from_local() can now support IPv6, change its synopsis to take a
"struct sockaddr *" .

Note that the original code discovers local addresses once.  These
days, with wifi, DHCP, and NetworkManager, the local network
configuration can change dynamically over time.  So, call getifaddrs()
more often to ensure from_local() has up-to-date network configuration
information.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 aclocal/ipv6.m4              |    4 +-
 configure.ac                 |    2 -
 support/include/tcpwrapper.h |    2 -
 support/misc/from_local.c    |   87 +++++++++++++++++++++++++++++++++++++++---
 support/misc/tcpwrapper.c    |    2 -
 5 files changed, 85 insertions(+), 12 deletions(-)

diff --git a/aclocal/ipv6.m4 b/aclocal/ipv6.m4
index 2490f3d..5ee8fb6 100644
--- a/aclocal/ipv6.m4
+++ b/aclocal/ipv6.m4
@@ -15,8 +15,8 @@ AC_DEFUN([AC_IPV6], [
     fi
 
     dnl IPv6-enabled networking functions required for IPv6
-    AC_CHECK_FUNCS([getnameinfo bindresvport_sa], ,
-                   [AC_MSG_ERROR([Missing functions needed for IPv6.])])
+    AC_CHECK_FUNCS([getifaddrs getnameinfo bindresvport_sa], ,
+                   [AC_MSG_ERROR([Missing library functions needed for IPv6.])])
 
     dnl Need to detect presence of IPv6 networking at run time via
     dnl getaddrinfo(3); old versions of glibc do not support ADDRCONFIG
diff --git a/configure.ac b/configure.ac
index c77c5ba..1dc4249 100644
--- a/configure.ac
+++ b/configure.ac
@@ -330,7 +330,7 @@ AC_FUNC_STAT
 AC_FUNC_VPRINTF
 AC_CHECK_FUNCS([alarm atexit dup2 fdatasync ftruncate getcwd \
                gethostbyaddr gethostbyname gethostname getmntent \
-               getnameinfo getrpcbyname \
+               getnameinfo getrpcbyname getifaddrs \
                gettimeofday hasmntopt inet_ntoa innetgr memset mkdir pathconf \
                realpath rmdir select socket strcasecmp strchr strdup \
                strerror strrchr strtol strtoul sigprocmask])
diff --git a/support/include/tcpwrapper.h b/support/include/tcpwrapper.h
index 98cf806..f1145bd 100644
--- a/support/include/tcpwrapper.h
+++ b/support/include/tcpwrapper.h
@@ -11,7 +11,7 @@ extern int allow_severity;
 extern int deny_severity;
 
 extern int good_client(char *daemon, struct sockaddr_in *addr);
-extern int from_local (struct sockaddr_in *addr);
+extern int from_local(const struct sockaddr *sap);
 extern int check_default(char *daemon, struct sockaddr_in *addr,
 			 u_long proc, u_long prog);
 
diff --git a/support/misc/from_local.c b/support/misc/from_local.c
index 3f46b99..57529ad 100644
--- a/support/misc/from_local.c
+++ b/support/misc/from_local.c
@@ -43,6 +43,7 @@ static char sccsid[] = "@(#) from_local.c 1.3 96/05/31 15:52:57";
 
 #include <sys/types.h>
 #include <sys/socket.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <unistd.h>
 #include <netdb.h>
@@ -52,6 +53,7 @@ static char sccsid[] = "@(#) from_local.c 1.3 96/05/31 15:52:57";
 #include <stdlib.h>
 #include <string.h>
 
+#include "sockaddr.h"
 #include "tcpwrapper.h"
 #include "xlog.h"
 
@@ -60,11 +62,66 @@ static char sccsid[] = "@(#) from_local.c 1.3 96/05/31 15:52:57";
 #define FALSE	0
 #endif
 
- /*
-  * With virtual hosting, each hardware network interface can have multiple
-  * network addresses. On such machines the number of machine addresses can
-  * be surprisingly large.
-  */
+#ifdef HAVE_GETIFADDRS
+
+#include <ifaddrs.h>
+#include <time.h>
+
+/**
+ * from_local - determine whether request comes from the local system
+ * @sap: pointer to socket address to check
+ *
+ * With virtual hosting, each hardware network interface can have
+ * multiple network addresses. On such machines the number of machine
+ * addresses can be surprisingly large.
+ *
+ * We also expect the local network configuration to change over time,
+ * so call getifaddrs(3) more than once, but not too often.
+ *
+ * Returns TRUE if the sockaddr contains an address of one of the local
+ * network interfaces.  Otherwise FALSE is returned.
+ */
+int
+from_local(const struct sockaddr *sap)
+{
+	static struct ifaddrs *ifaddr = NULL;
+	static time_t last_update = 0;
+	struct ifaddrs *ifa;
+	time_t now;
+
+	if (time(&now) == ((time_t)-1)) {
+		xlog(L_ERROR, "%s: time(2): %m", __func__);
+
+		/* If we don't know what time it is, use the
+		 * existing ifaddr list, if one exists  */
+		now = last_update;
+		if (ifaddr == NULL)
+			now++;
+	}
+	if (now != last_update) {
+		xlog(D_GENERAL, "%s: updating if addr list", __func__);
+
+		if (ifaddr)
+			freeifaddrs(ifaddr);
+
+		if (getifaddrs(&ifaddr) == -1) {
+			xlog(L_ERROR, "%s: getifaddrs(3): %m", __func__);
+			return FALSE;
+		}
+
+		last_update = now;
+	}
+
+	for (ifa = ifaddr; ifa; ifa = ifa->ifa_next)
+		if ((ifa->ifa_flags & IFF_UP) &&
+		    nfs_compare_sockaddr(sap, ifa->ifa_addr))
+			return TRUE;
+
+	return FALSE;
+}
+
+#else	/* !HAVE_GETIFADDRS */
+
 static int num_local;
 static int num_addrs;
 static struct in_addr *addrs;
@@ -155,12 +212,26 @@ find_local(void)
     return (num_local);
 }
 
-/* from_local - determine whether request comes from the local system */
+/**
+ * from_local - determine whether request comes from the local system
+ * @sap: pointer to socket address to check
+ *
+ * With virtual hosting, each hardware network interface can have
+ * multiple network addresses. On such machines the number of machine
+ * addresses can be surprisingly large.
+ *
+ * Returns TRUE if the sockaddr contains an address of one of the local
+ * network interfaces.  Otherwise FALSE is returned.
+ */
 int
-from_local(struct sockaddr_in *addr)
+from_local(const struct sockaddr *sap)
 {
+    const struct sockaddr_in *addr = (const struct sockaddr_in *)sap;
     int     i;
 
+    if (sap->sa_family != AF_INET)
+	return (FALSE);
+
     if (addrs == 0 && find_local() == 0)
 	xlog(L_ERROR, "Cannot find any active local network interfaces");
 
@@ -184,3 +255,5 @@ int main(void)
 }
 
 #endif	/* TEST */
+
+#endif	/* !HAVE_GETIFADDRS */
diff --git a/support/misc/tcpwrapper.c b/support/misc/tcpwrapper.c
index 1da6020..af626ad 100644
--- a/support/misc/tcpwrapper.c
+++ b/support/misc/tcpwrapper.c
@@ -202,7 +202,7 @@ u_long  prog;
 	if (acc && changed == 0)
 		return (acc->access);
 
-	if (!(from_local(addr) || good_client(daemon, addr))) {
+	if (!(from_local((struct sockaddr *)addr) || good_client(daemon, addr))) {
 		log_bad_host(addr, proc, prog);
 		if (acc)
 			acc->access = FALSE;


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/6] tcp_wrapper: Clean up logit()
       [not found] ` <20100115174426.30104.3492.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
  2010-01-15 17:49   ` [PATCH 1/6] tcpwrappers: Use xlog() instead of perror(3) and syslog(2) Chuck Lever
  2010-01-15 17:49   ` [PATCH 2/6] tcp_wrappers: Use getifaddrs(3) if it is available Chuck Lever
@ 2010-01-15 17:49   ` Chuck Lever
  2010-01-15 17:50   ` [PATCH 4/6] tcpwrapper: Fix signage problems in the tcp_wrappers hash function Chuck Lever
                     ` (3 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Chuck Lever @ 2010-01-15 17:49 UTC (permalink / raw)
  To: steved; +Cc: chris.mason, linux-nfs

Eliminate these compiler warnings:

tcpwrapper.c: In function =E2=80=98logit=E2=80=99:
tcpwrapper.c:225: warning: unused parameter =E2=80=98procnum=E2=80=99
tcpwrapper.c:225: warning: unused parameter =E2=80=98prognum=E2=80=99

Actually, @procnum is not used anywhere in our tcpwrapper.c, so let's
just get rid of it.

Since there is only one logit() call site in tcpwrapper.c, the macro
wrapper just adds needless clutter.  Let's get rid of that too.

=46inally, both mountd and statd now use xlog(), which adds an
appropriate program name prefix to every message.  Replace the
open-coded syslog(2) call with an xlog() call in order to consistently
identify the RPC service reporting the intrusion.

Since logit() no longer references "deny_severity" and no nfs-utils
caller sets either allow_severity or deny_severity, we remove them.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 support/include/tcpwrapper.h  |    8 +-----
 support/misc/tcpwrapper.c     |   56 ++++++++++++++++++---------------=
--------
 utils/mountd/mount_dispatch.c |    2 +
 utils/statd/statd.c           |    2 +
 4 files changed, 27 insertions(+), 41 deletions(-)

diff --git a/support/include/tcpwrapper.h b/support/include/tcpwrapper.=
h
index f1145bd..941394e 100644
--- a/support/include/tcpwrapper.h
+++ b/support/include/tcpwrapper.h
@@ -5,14 +5,8 @@
 #include <netinet/in.h>
 #include <arpa/inet.h>
=20
-extern int verboselog;
-
-extern int allow_severity;
-extern int deny_severity;
-
 extern int good_client(char *daemon, struct sockaddr_in *addr);
 extern int from_local(const struct sockaddr *sap);
-extern int check_default(char *daemon, struct sockaddr_in *addr,
-			 u_long proc, u_long prog);
+extern int check_default(char *daemon, struct sockaddr_in *addr, u_lon=
g prog);
=20
 #endif /* TCP_WRAPPER_H */
diff --git a/support/misc/tcpwrapper.c b/support/misc/tcpwrapper.c
index af626ad..b981d58 100644
--- a/support/misc/tcpwrapper.c
+++ b/support/misc/tcpwrapper.c
@@ -34,13 +34,12 @@
 #ifdef HAVE_CONFIG_H
 #include <config.h>
 #endif
+
 #ifdef HAVE_LIBWRAP
-#include <tcpwrapper.h>
 #include <unistd.h>
 #include <string.h>
 #include <rpc/rpc.h>
 #include <rpc/pmap_prot.h>
-#include <syslog.h>
 #include <netdb.h>
 #include <pwd.h>
 #include <sys/types.h>
@@ -49,6 +48,7 @@
 #include <sys/stat.h>
 #include <tcpd.h>
=20
+#include "tcpwrapper.h"
 #include "xlog.h"
=20
 #ifdef SYSV40
@@ -56,21 +56,8 @@
 #include <rpc/rpcent.h>
 #endif
=20
-static void logit(int severity, struct sockaddr_in *addr,
-		  u_long procnum, u_long prognum, char *text);
 static int check_files(void);
=20
-/*
- * These need to exist since they are externed=20
- * public header files.
- */
-int     verboselog =3D 0;
-int     allow_severity =3D LOG_INFO;
-int     deny_severity =3D LOG_WARNING;
-
-#define log_bad_host(addr, proc, prog) \
-  logit(deny_severity, addr, proc, prog, "request from unauthorized ho=
st")
-
 #define ALLOW 1
 #define DENY 0
=20
@@ -143,6 +130,16 @@ haccess_t *haccess_lookup(struct sockaddr_in *addr=
, u_long prog)
 	return NULL;
 }
=20
+static void
+logit(const struct sockaddr_in *sin)
+{
+	char buf[INET_ADDRSTRLEN];
+
+	xlog_warn("connect from %s denied: request from unauthorized host",
+			inet_ntop(AF_INET, &sin->sin_addr, buf, sizeof(buf)));
+	=09
+}
+
 int
 good_client(daemon, addr)
 char *daemon;
@@ -186,14 +183,17 @@ static int check_files()
 	return changed;
 }
=20
-/* check_default - additional checks for NULL, DUMP, GETPORT and unkno=
wn */
-
+/**
+ * check_default - additional checks for NULL, DUMP, GETPORT and unkno=
wn
+ * @daemon: pointer to '\0'-terminated ASCII string containing name of=
 the
+ *		daemon requesting the access check
+ * @addr: pointer to socket address containing address of caller
+ * @prog: RPC program number caller is attempting to access
+ *
+ * Returns TRUE if the caller is allowed access; otherwise FALSE is re=
turned.
+ */
 int
-check_default(daemon, addr, proc, prog)
-char *daemon;
-struct sockaddr_in *addr;
-u_long  proc;
-u_long  prog;
+check_default(char *daemon, struct sockaddr_in *addr, u_long prog)
 {
 	haccess_t *acc =3D NULL;
 	int changed =3D check_files();
@@ -203,7 +203,7 @@ u_long  prog;
 		return (acc->access);
=20
 	if (!(from_local((struct sockaddr *)addr) || good_client(daemon, addr=
))) {
-		log_bad_host(addr, proc, prog);
+		logit(addr);
 		if (acc)
 			acc->access =3D FALSE;
 		else=20
@@ -219,12 +219,4 @@ u_long  prog;
     return (TRUE);
 }
=20
-/* logit - report events of interest via the syslog daemon */
-
-static void logit(int severity, struct sockaddr_in *addr,
-		  u_long procnum, u_long prognum, char *text)
-{
-	syslog(severity, "connect from %s denied: %s",
-	       inet_ntoa(addr->sin_addr), text);
-}
-#endif
+#endif	/* HAVE_LIBWRAP */
diff --git a/utils/mountd/mount_dispatch.c b/utils/mountd/mount_dispatc=
h.c
index 199fcec..d2802ef 100644
--- a/utils/mountd/mount_dispatch.c
+++ b/utils/mountd/mount_dispatch.c
@@ -75,7 +75,7 @@ mount_dispatch(struct svc_req *rqstp, SVCXPRT *transp=
)
=20
 	/* remote host authorization check */
 	if (sin->sin_family =3D=3D AF_INET &&
-	    !check_default("mountd", sin, rqstp->rq_proc, MOUNTPROG)) {
+	    !check_default("mountd", sin, MOUNTPROG)) {
 		svcerr_auth (transp, AUTH_FAILED);
 		return;
 	}
diff --git a/utils/statd/statd.c b/utils/statd/statd.c
index 7be6454..fa3c6d5 100644
--- a/utils/statd/statd.c
+++ b/utils/statd/statd.c
@@ -79,7 +79,7 @@ sm_prog_1_wrapper (struct svc_req *rqstp, register SV=
CXPRT *transp)
=20
 	/* remote host authorization check */
 	if (sin->sin_family =3D=3D AF_INET &&
-	    !check_default("statd", sin, rqstp->rq_proc, SM_PROG)) {
+	    !check_default("statd", sin, SM_PROG)) {
 		svcerr_auth (transp, AUTH_FAILED);
 		return;
 	}


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/6] tcpwrapper: Fix signage problems in the tcp_wrappers hash function
       [not found] ` <20100115174426.30104.3492.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
                     ` (2 preceding siblings ...)
  2010-01-15 17:49   ` [PATCH 3/6] tcp_wrapper: Clean up logit() Chuck Lever
@ 2010-01-15 17:50   ` Chuck Lever
  2010-01-15 17:50   ` [PATCH 5/6] tcpwrapper: Eliminated shadowed declaration warnings Chuck Lever
                     ` (2 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Chuck Lever @ 2010-01-15 17:50 UTC (permalink / raw)
  To: steved; +Cc: chris.mason, linux-nfs

Eliminate the following compiler warnings:

tcpwrapper.c:78: warning: no previous prototype for =E2=80=98strtoint=E2=
=80=99
tcpwrapper.c: In function =E2=80=98strtoint=E2=80=99:
tcpwrapper.c:81: warning: conversion to =E2=80=98int=E2=80=99 from =E2=80=
=98size_t=E2=80=99 may change the sign of the result
tcpwrapper.c:85: warning: conversion to =E2=80=98unsigned int=E2=80=99 =
from =E2=80=98int=E2=80=99 may change the sign of the result
tcpwrapper.c: In function =E2=80=98hashint=E2=80=99:
tcpwrapper.c:91: warning: conversion to =E2=80=98int=E2=80=99 from =E2=80=
=98unsigned int=E2=80=99 may change the sign of the result

The hash value is probably computed consistently even with unexpected
sign inversions.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 support/misc/tcpwrapper.c |   34 ++++++++++++++++++++--------------
 1 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/support/misc/tcpwrapper.c b/support/misc/tcpwrapper.c
index b981d58..6f65c13 100644
--- a/support/misc/tcpwrapper.c
+++ b/support/misc/tcpwrapper.c
@@ -75,29 +75,35 @@ hash_head haccess_tbl[HASH_TABLE_SIZE];
 static haccess_t *haccess_lookup(struct sockaddr_in *addr, u_long);
 static void haccess_add(struct sockaddr_in *addr, u_long, int);
=20
-inline unsigned int strtoint(char *str)
+static unsigned long
+strtoint(const char *str)
 {
-	unsigned int n =3D 0;
-	int len =3D strlen(str);
-	int i;
+	unsigned long i, n =3D 0;
+	size_t len =3D strlen(str);
=20
-	for (i=3D0; i < len; i++)
-		n+=3D((int)str[i])*i;
+	for (i =3D 0; i < len; i++)
+		n +=3D (unsigned char)str[i] * i;
=20
 	return n;
 }
-static inline int hashint(unsigned int num)
+
+static unsigned int
+hashint(const unsigned long num)
+{
+	return (unsigned int)(num % HASH_TABLE_SIZE);
+}
+
+static unsigned int
+HASH(const char *addr, const unsigned long program)
 {
-	return num % HASH_TABLE_SIZE;
+	return hashint(strtoint(addr) + program);
 }
-#define HASH(_addr, _prog) \
-	hashint((strtoint((_addr))+(_prog)))
=20
 void haccess_add(struct sockaddr_in *addr, u_long prog, int access)
 {
 	hash_head *head;
- 	haccess_t *hptr;
-	int hash;
+	haccess_t *hptr;
+	unsigned int hash;
=20
 	hptr =3D (haccess_t *)malloc(sizeof(haccess_t));
 	if (hptr =3D=3D NULL)
@@ -117,8 +123,8 @@ void haccess_add(struct sockaddr_in *addr, u_long p=
rog, int access)
 haccess_t *haccess_lookup(struct sockaddr_in *addr, u_long prog)
 {
 	hash_head *head;
- 	haccess_t *hptr;
-	int hash;
+	haccess_t *hptr;
+	unsigned int hash;
=20
 	hash =3D HASH(inet_ntoa(addr->sin_addr), prog);
 	head =3D &(haccess_tbl[hash]);


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/6] tcpwrapper: Eliminated shadowed declaration warnings
       [not found] ` <20100115174426.30104.3492.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
                     ` (3 preceding siblings ...)
  2010-01-15 17:50   ` [PATCH 4/6] tcpwrapper: Fix signage problems in the tcp_wrappers hash function Chuck Lever
@ 2010-01-15 17:50   ` Chuck Lever
  2010-01-15 17:50   ` [PATCH 6/6] tcpwrapper: Add support for IPv6 Chuck Lever
  2010-01-15 18:04   ` [PATCH 0/6] IPv6 support for nfs-utils tcpwrapper shim Steve Dickson
  6 siblings, 0 replies; 13+ messages in thread
From: Chuck Lever @ 2010-01-15 17:50 UTC (permalink / raw)
  To: steved; +Cc: chris.mason, linux-nfs

Clean up: the use of identifiers called "access" and "daemon" shadow
function declarations in unistd.h.  Seen with "-Wextra -pedantic".

tcpwrapper.c: In function =E2=80=98haccess_add=E2=80=99:
tcpwrapper.c:112: warning: declaration of =E2=80=98access=E2=80=99 shad=
ows a global
declaration
/usr/include/unistd.h:288: warning: shadowed declaration is here
tcpwrapper.c: In function =E2=80=98good_client=E2=80=99:
tcpwrapper.c:161: warning: declaration of =E2=80=98daemon=E2=80=99 shad=
ows a global
declaration
/usr/include/unistd.h:953: warning: shadowed declaration is here
tcpwrapper.c: In function =E2=80=98check_default=E2=80=99:
tcpwrapper.c:212: warning: declaration of =E2=80=98daemon=E2=80=99 shad=
ows a global
declaration
/usr/include/unistd.h:953: warning: shadowed declaration is here

Note that good_client() is used only in support/misc/tcpwrapper.c, so
make it static (and update its prototype to c99 standard form).

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 support/include/tcpwrapper.h |    3 +--
 support/misc/tcpwrapper.c    |   32 +++++++++++++++-----------------
 2 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/support/include/tcpwrapper.h b/support/include/tcpwrapper.=
h
index 941394e..930ec6a 100644
--- a/support/include/tcpwrapper.h
+++ b/support/include/tcpwrapper.h
@@ -5,8 +5,7 @@
 #include <netinet/in.h>
 #include <arpa/inet.h>
=20
-extern int good_client(char *daemon, struct sockaddr_in *addr);
 extern int from_local(const struct sockaddr *sap);
-extern int check_default(char *daemon, struct sockaddr_in *addr, u_lon=
g prog);
+extern int check_default(char *name, struct sockaddr_in *addr, u_long =
prog);
=20
 #endif /* TCP_WRAPPER_H */
diff --git a/support/misc/tcpwrapper.c b/support/misc/tcpwrapper.c
index 6f65c13..03f5dc4 100644
--- a/support/misc/tcpwrapper.c
+++ b/support/misc/tcpwrapper.c
@@ -62,9 +62,9 @@ static int check_files(void);
 #define DENY 0
=20
 typedef struct _haccess_t {
-	TAILQ_ENTRY(_haccess_t) list;
-	int access;
-    struct in_addr addr;
+	TAILQ_ENTRY(_haccess_t)	list;
+	int			allowed;
+	struct in_addr		addr;
 } haccess_t;
=20
 #define HASH_TABLE_SIZE 1021
@@ -73,7 +73,6 @@ typedef struct _hash_head {
 } hash_head;
 hash_head haccess_tbl[HASH_TABLE_SIZE];
 static haccess_t *haccess_lookup(struct sockaddr_in *addr, u_long);
-static void haccess_add(struct sockaddr_in *addr, u_long, int);
=20
 static unsigned long
 strtoint(const char *str)
@@ -99,7 +98,8 @@ HASH(const char *addr, const unsigned long program)
 	return hashint(strtoint(addr) + program);
 }
=20
-void haccess_add(struct sockaddr_in *addr, u_long prog, int access)
+static void
+haccess_add(struct sockaddr_in *addr, u_long prog, int allowed)
 {
 	hash_head *head;
 	haccess_t *hptr;
@@ -112,7 +112,7 @@ void haccess_add(struct sockaddr_in *addr, u_long p=
rog, int access)
 	hash =3D HASH(inet_ntoa(addr->sin_addr), prog);
 	head =3D &(haccess_tbl[hash]);
=20
-	hptr->access =3D access;
+	hptr->allowed =3D allowed;
 	hptr->addr.s_addr =3D addr->sin_addr.s_addr;
=20
 	if (TAILQ_EMPTY(&head->h_head))
@@ -146,14 +146,12 @@ logit(const struct sockaddr_in *sin)
 	=09
 }
=20
-int
-good_client(daemon, addr)
-char *daemon;
-struct sockaddr_in *addr;
+static int
+good_client(char *name, struct sockaddr_in *addr)
 {
 	struct request_info req;
=20
-	request_init(&req, RQ_DAEMON, daemon, RQ_CLIENT_SIN, addr, 0);
+	request_init(&req, RQ_DAEMON, name, RQ_CLIENT_SIN, addr, 0);
 	sock_methods(&req);
=20
 	if (hosts_access(&req))=20
@@ -191,7 +189,7 @@ static int check_files()
=20
 /**
  * check_default - additional checks for NULL, DUMP, GETPORT and unkno=
wn
- * @daemon: pointer to '\0'-terminated ASCII string containing name of=
 the
+ * @name: pointer to '\0'-terminated ASCII string containing name of t=
he
  *		daemon requesting the access check
  * @addr: pointer to socket address containing address of caller
  * @prog: RPC program number caller is attempting to access
@@ -199,26 +197,26 @@ static int check_files()
  * Returns TRUE if the caller is allowed access; otherwise FALSE is re=
turned.
  */
 int
-check_default(char *daemon, struct sockaddr_in *addr, u_long prog)
+check_default(char *name, struct sockaddr_in *addr, u_long prog)
 {
 	haccess_t *acc =3D NULL;
 	int changed =3D check_files();
=20
 	acc =3D haccess_lookup(addr, prog);
 	if (acc && changed =3D=3D 0)
-		return (acc->access);
+		return acc->allowed;
=20
-	if (!(from_local((struct sockaddr *)addr) || good_client(daemon, addr=
))) {
+	if (!(from_local((struct sockaddr *)addr) || good_client(name, addr))=
) {
 		logit(addr);
 		if (acc)
-			acc->access =3D FALSE;
+			acc->allowed =3D FALSE;
 		else=20
 			haccess_add(addr, prog, FALSE);
 		return (FALSE);
 	}
=20
 	if (acc)
-		acc->access =3D TRUE;
+		acc->allowed =3D TRUE;
 	else=20
 		haccess_add(addr, prog, TRUE);
=20


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 6/6] tcpwrapper: Add support for IPv6
       [not found] ` <20100115174426.30104.3492.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
                     ` (4 preceding siblings ...)
  2010-01-15 17:50   ` [PATCH 5/6] tcpwrapper: Eliminated shadowed declaration warnings Chuck Lever
@ 2010-01-15 17:50   ` Chuck Lever
  2010-01-15 18:04   ` [PATCH 0/6] IPv6 support for nfs-utils tcpwrapper shim Steve Dickson
  6 siblings, 0 replies; 13+ messages in thread
From: Chuck Lever @ 2010-01-15 17:50 UTC (permalink / raw)
  To: steved; +Cc: chris.mason, linux-nfs

Assuming the tcp_wrappers library can actually support IPv6 addresses,
here's a crack at IPv6 support in nfs-utils' TCP wrapper shim.

Some reorganization is done to limit the number of times that @sap
is converted to a presentation address string.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 support/include/tcpwrapper.h  |    3 +
 support/misc/tcpwrapper.c     |  115 ++++++++++++++++++++++++++++-------------
 utils/mountd/mount_dispatch.c |    6 +-
 utils/statd/statd.c           |    5 --
 utils/statd/statd.man         |    3 -
 5 files changed, 84 insertions(+), 48 deletions(-)

diff --git a/support/include/tcpwrapper.h b/support/include/tcpwrapper.h
index 930ec6a..f735106 100644
--- a/support/include/tcpwrapper.h
+++ b/support/include/tcpwrapper.h
@@ -6,6 +6,7 @@
 #include <arpa/inet.h>
 
 extern int from_local(const struct sockaddr *sap);
-extern int check_default(char *name, struct sockaddr_in *addr, u_long prog);
+extern int check_default(char *name, struct sockaddr *sap,
+			const unsigned long program);
 
 #endif /* TCP_WRAPPER_H */
diff --git a/support/misc/tcpwrapper.c b/support/misc/tcpwrapper.c
index 03f5dc4..06b0a46 100644
--- a/support/misc/tcpwrapper.c
+++ b/support/misc/tcpwrapper.c
@@ -48,31 +48,66 @@
 #include <sys/stat.h>
 #include <tcpd.h>
 
+#include "sockaddr.h"
 #include "tcpwrapper.h"
 #include "xlog.h"
 
 #ifdef SYSV40
 #include <netinet/in.h>
 #include <rpc/rpcent.h>
-#endif
-
-static int check_files(void);
+#endif	/* SYSV40 */
 
 #define ALLOW 1
 #define DENY 0
 
+#ifdef IPV6_SUPPORTED
+static void
+present_address(const struct sockaddr *sap, char *buf, const size_t buflen)
+{
+	const struct sockaddr_in *sin = (const struct sockaddr_in *)sap;
+	const struct sockaddr_in6 *sin6 = (const struct sockaddr_in6 *)sap;
+	socklen_t len = (socklen_t)buflen;
+
+	switch (sap->sa_family) {
+	case AF_INET:
+		if (inet_ntop(AF_INET, &sin->sin_addr, buf, len) != 0)
+			return;
+	case AF_INET6:
+		if (inet_ntop(AF_INET6, &sin6->sin6_addr, buf, len) != 0)
+			return;
+	}
+
+	memset(buf, 0, buflen);
+	strncpy(buf, "unrecognized caller", buflen);
+}
+#else	/* !IPV6_SUPPORTED */
+static void
+present_address(const struct sockaddr *sap, char *buf, const size_t buflen)
+{
+	const struct sockaddr_in *sin = (const struct sockaddr_in *)sap;
+	socklen_t len = (socklen_t)buflen;
+
+	if (sap->sa_family == AF_INET)
+		if (inet_ntop(AF_INET, &sin->sin_addr, buf, len) != 0)
+			return;
+
+	memset(buf, 0, buflen);
+	strncpy(buf, "unrecognized caller", (size_t)buflen);
+}
+#endif	/* !IPV6_SUPPORTED */
+
 typedef struct _haccess_t {
 	TAILQ_ENTRY(_haccess_t)	list;
 	int			allowed;
-	struct in_addr		addr;
+	union nfs_sockaddr	address;
 } haccess_t;
 
 #define HASH_TABLE_SIZE 1021
 typedef struct _hash_head {
 	TAILQ_HEAD(host_list, _haccess_t) h_head;
 } hash_head;
-hash_head haccess_tbl[HASH_TABLE_SIZE];
-static haccess_t *haccess_lookup(struct sockaddr_in *addr, u_long);
+
+static hash_head haccess_tbl[HASH_TABLE_SIZE];
 
 static unsigned long
 strtoint(const char *str)
@@ -99,7 +134,8 @@ HASH(const char *addr, const unsigned long program)
 }
 
 static void
-haccess_add(struct sockaddr_in *addr, u_long prog, int allowed)
+haccess_add(const struct sockaddr *sap, const char *address,
+		const unsigned long program, const int allowed)
 {
 	hash_head *head;
 	haccess_t *hptr;
@@ -109,49 +145,49 @@ haccess_add(struct sockaddr_in *addr, u_long prog, int allowed)
 	if (hptr == NULL)
 		return;
 
-	hash = HASH(inet_ntoa(addr->sin_addr), prog);
+	hash = HASH(address, program);
 	head = &(haccess_tbl[hash]);
 
 	hptr->allowed = allowed;
-	hptr->addr.s_addr = addr->sin_addr.s_addr;
+	memcpy(&hptr->address, sap, (size_t)nfs_sockaddr_length(sap));
 
 	if (TAILQ_EMPTY(&head->h_head))
 		TAILQ_INSERT_HEAD(&head->h_head, hptr, list);
 	else
 		TAILQ_INSERT_TAIL(&head->h_head, hptr, list);
 }
-haccess_t *haccess_lookup(struct sockaddr_in *addr, u_long prog)
+
+static haccess_t *
+haccess_lookup(const struct sockaddr *sap, const char *address,
+		const unsigned long program)
 {
 	hash_head *head;
 	haccess_t *hptr;
 	unsigned int hash;
 
-	hash = HASH(inet_ntoa(addr->sin_addr), prog);
+	hash = HASH(address, program);
 	head = &(haccess_tbl[hash]);
 
 	TAILQ_FOREACH(hptr, &head->h_head, list) {
-		if (hptr->addr.s_addr == addr->sin_addr.s_addr)
+		if (nfs_compare_sockaddr(&hptr->address.sa, sap))
 			return hptr;
 	}
 	return NULL;
 }
 
 static void
-logit(const struct sockaddr_in *sin)
+logit(const char *address)
 {
-	char buf[INET_ADDRSTRLEN];
-
 	xlog_warn("connect from %s denied: request from unauthorized host",
-			inet_ntop(AF_INET, &sin->sin_addr, buf, sizeof(buf)));
-		
+			address);
 }
 
 static int
-good_client(char *name, struct sockaddr_in *addr)
+good_client(char *name, struct sockaddr *sap)
 {
 	struct request_info req;
 
-	request_init(&req, RQ_DAEMON, name, RQ_CLIENT_SIN, addr, 0);
+	request_init(&req, RQ_DAEMON, name, RQ_CLIENT_SIN, sap, 0);
 	sock_methods(&req);
 
 	if (hosts_access(&req)) 
@@ -160,9 +196,8 @@ good_client(char *name, struct sockaddr_in *addr)
 	return DENY;
 }
 
-/* check_files - check to see if either access files have changed */
-
-static int check_files()
+static int
+check_files(void)
 {
 	static time_t allow_mtime, deny_mtime;
 	struct stat astat, dstat;
@@ -191,36 +226,44 @@ static int check_files()
  * check_default - additional checks for NULL, DUMP, GETPORT and unknown
  * @name: pointer to '\0'-terminated ASCII string containing name of the
  *		daemon requesting the access check
- * @addr: pointer to socket address containing address of caller
- * @prog: RPC program number caller is attempting to access
+ * @sap: pointer to sockaddr containing network address of caller
+ * @program: RPC program number caller is attempting to access
  *
  * Returns TRUE if the caller is allowed access; otherwise FALSE is returned.
  */
 int
-check_default(char *name, struct sockaddr_in *addr, u_long prog)
+check_default(char *name, struct sockaddr *sap, const unsigned long program)
 {
 	haccess_t *acc = NULL;
 	int changed = check_files();
+	char buf[INET6_ADDRSTRLEN];
+
+	present_address(sap, buf, sizeof(buf));
 
-	acc = haccess_lookup(addr, prog);
-	if (acc && changed == 0)
+	acc = haccess_lookup(sap, buf, program);
+	if (acc != NULL && changed == 0) {
+		xlog(D_GENERAL, "%s: access by %s %s (cached)", __func__,
+			buf, acc->allowed ? "ALLOWED" : "DENIED");
 		return acc->allowed;
+	}
 
-	if (!(from_local((struct sockaddr *)addr) || good_client(name, addr))) {
-		logit(addr);
-		if (acc)
+	if (!(from_local(sap) || good_client(name, sap))) {
+		logit(buf);
+		if (acc != NULL)
 			acc->allowed = FALSE;
-		else 
-			haccess_add(addr, prog, FALSE);
+		else
+			haccess_add(sap, buf, program, FALSE);
+		xlog(D_GENERAL, "%s: access by %s DENIED", __func__, buf);
 		return (FALSE);
 	}
 
-	if (acc)
+	if (acc != NULL)
 		acc->allowed = TRUE;
-	else 
-		haccess_add(addr, prog, TRUE);
+	else
+		haccess_add(sap, buf, program, TRUE);
+	xlog(D_GENERAL, "%s: access by %s ALLOWED", __func__, buf);
 
-    return (TRUE);
+	return (TRUE);
 }
 
 #endif	/* HAVE_LIBWRAP */
diff --git a/utils/mountd/mount_dispatch.c b/utils/mountd/mount_dispatch.c
index d2802ef..ba6981d 100644
--- a/utils/mountd/mount_dispatch.c
+++ b/utils/mountd/mount_dispatch.c
@@ -70,12 +70,10 @@ mount_dispatch(struct svc_req *rqstp, SVCXPRT *transp)
 {
 	union mountd_arguments 	argument;
 	union mountd_results	result;
-#ifdef HAVE_TCP_WRAPPER
-	struct sockaddr_in *sin = nfs_getrpccaller_in(transp);
 
+#ifdef HAVE_TCP_WRAPPER
 	/* remote host authorization check */
-	if (sin->sin_family == AF_INET &&
-	    !check_default("mountd", sin, MOUNTPROG)) {
+	if (!check_default("mountd", nfs_getrpccaller(transp), MOUNTPROG)) {
 		svcerr_auth (transp, AUTH_FAILED);
 		return;
 	}
diff --git a/utils/statd/statd.c b/utils/statd/statd.c
index fa3c6d5..01fdb41 100644
--- a/utils/statd/statd.c
+++ b/utils/statd/statd.c
@@ -75,11 +75,8 @@ extern void simulator (int, char **);
 static void 
 sm_prog_1_wrapper (struct svc_req *rqstp, register SVCXPRT *transp)
 {
-	struct sockaddr_in *sin = nfs_getrpccaller_in(transp);
-
 	/* remote host authorization check */
-	if (sin->sin_family == AF_INET &&
-	    !check_default("statd", sin, SM_PROG)) {
+	if (!check_default("statd", nfs_getrpccaller(transp), SM_PROG)) {
 		svcerr_auth (transp, AUTH_FAILED);
 		return;
 	}
diff --git a/utils/statd/statd.man b/utils/statd/statd.man
index 4ddb634..ffc5e95 100644
--- a/utils/statd/statd.man
+++ b/utils/statd/statd.man
@@ -274,9 +274,6 @@ listeners using the
 .B tcp_wrapper
 library or
 .BR iptables (8).
-Note that the
-.B tcp_wrapper
-library supports only IPv4 networking.
 To use the
 .B tcp_wrapper
 library, add the hostnames of peers that should be allowed access to


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/6] IPv6 support for nfs-utils tcpwrapper shim
       [not found] ` <20100115174426.30104.3492.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
                     ` (5 preceding siblings ...)
  2010-01-15 17:50   ` [PATCH 6/6] tcpwrapper: Add support for IPv6 Chuck Lever
@ 2010-01-15 18:04   ` Steve Dickson
       [not found]     ` <4B50AE34.3020009-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
  6 siblings, 1 reply; 13+ messages in thread
From: Steve Dickson @ 2010-01-15 18:04 UTC (permalink / raw)
  To: Chuck Lever; +Cc: chris.mason, linux-nfs



On 01/15/2010 12:49 PM, Chuck Lever wrote:
> These patches provide IPv6 support for the tcpwrapper shim inside
> nfs-utils.  It assumes that the generic tcpwrapper library can
> support IPv6 addresses.  It has not been extensively tested, but
> I think the framework is reasonable, and only minor bug fixes might
> be needed as we go along.
Did you do any simple "hello world" testing?

steved.

> 
> ---
> 
> Chuck Lever (6):
>       tcpwrapper: Add support for IPv6
>       tcpwrapper: Eliminated shadowed declaration warnings
>       tcpwrapper: Fix signage problems in the tcp_wrappers hash function
>       tcp_wrapper: Clean up logit()
>       tcp_wrappers: Use getifaddrs(3) if it is available
>       tcpwrappers: Use xlog() instead of perror(3) and syslog(2)
> 
> 
>  aclocal/ipv6.m4               |    4 -
>  configure.ac                  |    2 
>  support/include/tcpwrapper.h  |   12 +-
>  support/misc/from_local.c     |  112 +++++++++++++++++++----
>  support/misc/tcpwrapper.c     |  203 ++++++++++++++++++++++++-----------------
>  utils/mountd/mount_dispatch.c |    6 -
>  utils/statd/statd.c           |    5 -
>  utils/statd/statd.man         |    3 -
>  8 files changed, 223 insertions(+), 124 deletions(-)
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/6] IPv6 support for nfs-utils tcpwrapper shim
       [not found]     ` <4B50AE34.3020009-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
@ 2010-01-15 18:27       ` Chuck Lever
  2010-01-15 18:37         ` Steve Dickson
  0 siblings, 1 reply; 13+ messages in thread
From: Chuck Lever @ 2010-01-15 18:27 UTC (permalink / raw)
  To: Steve Dickson; +Cc: chris.mason, linux-nfs

On Jan 15, 2010, at 1:04 PM, Steve Dickson wrote:
> On 01/15/2010 12:49 PM, Chuck Lever wrote:
>> These patches provide IPv6 support for the tcpwrapper shim inside
>> nfs-utils.  It assumes that the generic tcpwrapper library can
>> support IPv6 addresses.  It has not been extensively tested, but
>> I think the framework is reasonable, and only minor bug fixes might
>> be needed as we go along.
> Did you do any simple "hello world" testing?

I've build-tested them.  Jeff and I had them applied while doing the  
statd testing.  They don't appear to cause problems when no allow/deny  
sets exist.

I thought we would have more time to test and review these, so I  
haven't done more extensive testing so far.  In any case, I don't  
think they will be harmful, and can serve as a place marker for that  
feature as the beta moves forward.

> steved.
>
>>
>> ---
>>
>> Chuck Lever (6):
>>      tcpwrapper: Add support for IPv6
>>      tcpwrapper: Eliminated shadowed declaration warnings
>>      tcpwrapper: Fix signage problems in the tcp_wrappers hash  
>> function
>>      tcp_wrapper: Clean up logit()
>>      tcp_wrappers: Use getifaddrs(3) if it is available
>>      tcpwrappers: Use xlog() instead of perror(3) and syslog(2)
>>
>>
>> aclocal/ipv6.m4               |    4 -
>> configure.ac                  |    2
>> support/include/tcpwrapper.h  |   12 +-
>> support/misc/from_local.c     |  112 +++++++++++++++++++----
>> support/misc/tcpwrapper.c     |  203 +++++++++++++++++++++++ 
>> +-----------------
>> utils/mountd/mount_dispatch.c |    6 -
>> utils/statd/statd.c           |    5 -
>> utils/statd/statd.man         |    3 -
>> 8 files changed, 223 insertions(+), 124 deletions(-)
>>

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/6] IPv6 support for nfs-utils tcpwrapper shim
  2010-01-15 18:27       ` Chuck Lever
@ 2010-01-15 18:37         ` Steve Dickson
       [not found]           ` <4B50B5F0.6020202-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Steve Dickson @ 2010-01-15 18:37 UTC (permalink / raw)
  To: Chuck Lever; +Cc: chris.mason, linux-nfs



On 01/15/2010 01:27 PM, Chuck Lever wrote:
> On Jan 15, 2010, at 1:04 PM, Steve Dickson wrote:
>> On 01/15/2010 12:49 PM, Chuck Lever wrote:
>>> These patches provide IPv6 support for the tcpwrapper shim inside
>>> nfs-utils.  It assumes that the generic tcpwrapper library can
>>> support IPv6 addresses.  It has not been extensively tested, but
>>> I think the framework is reasonable, and only minor bug fixes might
>>> be needed as we go along.
>> Did you do any simple "hello world" testing?
> 
> I've build-tested them.  Jeff and I had them applied while doing the
> statd testing.  They don't appear to cause problems when no allow/deny
> sets exist.
> 
> I thought we would have more time to test and review these, so I haven't
> done more extensive testing so far.  In any case, I don't think they
> will be harmful, and can serve as a place marker for that feature as the
> beta moves forward.
Unfortunately I have broken these before and it was awful painful... :-(
Right or wrong... there is still a large number of people that depend
on this archaic routines... we need to be very careful... 

steved.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/6] IPv6 support for nfs-utils tcpwrapper shim
       [not found]           ` <4B50B5F0.6020202-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
@ 2010-01-15 18:59             ` Chuck Lever
  2010-01-15 19:32               ` Steve Dickson
  0 siblings, 1 reply; 13+ messages in thread
From: Chuck Lever @ 2010-01-15 18:59 UTC (permalink / raw)
  To: Steve Dickson; +Cc: chris.mason, linux-nfs

On Jan 15, 2010, at 1:37 PM, Steve Dickson wrote:
> On 01/15/2010 01:27 PM, Chuck Lever wrote:
>> On Jan 15, 2010, at 1:04 PM, Steve Dickson wrote:
>>> On 01/15/2010 12:49 PM, Chuck Lever wrote:
>>>> These patches provide IPv6 support for the tcpwrapper shim inside
>>>> nfs-utils.  It assumes that the generic tcpwrapper library can
>>>> support IPv6 addresses.  It has not been extensively tested, but
>>>> I think the framework is reasonable, and only minor bug fixes might
>>>> be needed as we go along.
>>> Did you do any simple "hello world" testing?
>>
>> I've build-tested them.  Jeff and I had them applied while doing the
>> statd testing.  They don't appear to cause problems when no allow/ 
>> deny
>> sets exist.
>>
>> I thought we would have more time to test and review these, so I  
>> haven't
>> done more extensive testing so far.  In any case, I don't think they
>> will be harmful, and can serve as a place marker for that feature  
>> as the
>> beta moves forward.
> Unfortunately I have broken these before and it was awful  
> painful... :-(
> Right or wrong... there is still a large number of people that depend
> on this archaic routines... we need to be very careful...

Do you have tests I can run to validate these patches to your  
satisfaction?

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/6] IPv6 support for nfs-utils tcpwrapper shim
  2010-01-15 18:59             ` Chuck Lever
@ 2010-01-15 19:32               ` Steve Dickson
  0 siblings, 0 replies; 13+ messages in thread
From: Steve Dickson @ 2010-01-15 19:32 UTC (permalink / raw)
  To: Chuck Lever; +Cc: chris.mason, linux-nfs



On 01/15/2010 01:59 PM, Chuck Lever wrote:
> On Jan 15, 2010, at 1:37 PM, Steve Dickson wrote:
>> On 01/15/2010 01:27 PM, Chuck Lever wrote:
>>> On Jan 15, 2010, at 1:04 PM, Steve Dickson wrote:
>>>> On 01/15/2010 12:49 PM, Chuck Lever wrote:
>>>>> These patches provide IPv6 support for the tcpwrapper shim inside
>>>>> nfs-utils.  It assumes that the generic tcpwrapper library can
>>>>> support IPv6 addresses.  It has not been extensively tested, but
>>>>> I think the framework is reasonable, and only minor bug fixes might
>>>>> be needed as we go along.
>>>> Did you do any simple "hello world" testing?
>>>
>>> I've build-tested them.  Jeff and I had them applied while doing the
>>> statd testing.  They don't appear to cause problems when no allow/deny
>>> sets exist.
>>>
>>> I thought we would have more time to test and review these, so I haven't
>>> done more extensive testing so far.  In any case, I don't think they
>>> will be harmful, and can serve as a place marker for that feature as the
>>> beta moves forward.
>> Unfortunately I have broken these before and it was awful painful... :-(
>> Right or wrong... there is still a large number of people that depend
>> on this archaic routines... we need to be very careful...
> 
> Do you have tests I can run to validate these patches to your satisfaction?
No... I wish I did... I just do the testing by hand... 

steved.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/6] tcp_wrappers: Use getifaddrs(3) if it is available
       [not found] ` <20100115212102.18214.19398.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
@ 2010-01-15 21:31   ` Chuck Lever
  0 siblings, 0 replies; 13+ messages in thread
From: Chuck Lever @ 2010-01-15 21:31 UTC (permalink / raw)
  To: steved; +Cc: chris.mason, linux-nfs

After glibc 2.3.3, getifaddrs(3) can return AF_INET6 addresses for
local network interfaces.  Using the library call is easier than
trying to update the open code in from_local(), and means we have
less to maintain in nfs-utils going forward.

And, since from_local() can now support IPv6, change its synopsis to
take a "struct sockaddr *" .

Note that the original code discovers local addresses once.  These
days, with wifi, DHCP, and NetworkManager, the local network
configuration can change dynamically over time.  So, call getifaddrs()
more often to ensure from_local() has up-to-date network configuration
information.

This implementation refreshes the list if from_local() has not been
called in the last second.  This is actually not terribly honerous.
check_default() invokes from_local() only when the remote host is not
in its access cache, or the access/deny files have changed.

So new hosts will cause a refresh, but previously seen hosts
(including localhost) should not.

On the other hand, it still may not be often enough.  After the first
call, if only previously seen hosts attempt to access our daemons,
from_local() would never be called, and the local list would never be
updated.  This might be possible during steady-state operation with
a small number of servers and clients.

It would also be nice if we could free the local interface address
list at shutdown time, but that would be a lot of trouble for little
gain.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 aclocal/ipv6.m4              |    4 +-
 configure.ac                 |    2 -
 support/include/tcpwrapper.h |    2 -
 support/misc/from_local.c    |   96 +++++++++++++++++++++++++++++++++++++++---
 support/misc/tcpwrapper.c    |    2 -
 5 files changed, 94 insertions(+), 12 deletions(-)

diff --git a/aclocal/ipv6.m4 b/aclocal/ipv6.m4
index 2490f3d..5ee8fb6 100644
--- a/aclocal/ipv6.m4
+++ b/aclocal/ipv6.m4
@@ -15,8 +15,8 @@ AC_DEFUN([AC_IPV6], [
     fi
 
     dnl IPv6-enabled networking functions required for IPv6
-    AC_CHECK_FUNCS([getnameinfo bindresvport_sa], ,
-                   [AC_MSG_ERROR([Missing functions needed for IPv6.])])
+    AC_CHECK_FUNCS([getifaddrs getnameinfo bindresvport_sa], ,
+                   [AC_MSG_ERROR([Missing library functions needed for IPv6.])])
 
     dnl Need to detect presence of IPv6 networking at run time via
     dnl getaddrinfo(3); old versions of glibc do not support ADDRCONFIG
diff --git a/configure.ac b/configure.ac
index c77c5ba..1dc4249 100644
--- a/configure.ac
+++ b/configure.ac
@@ -330,7 +330,7 @@ AC_FUNC_STAT
 AC_FUNC_VPRINTF
 AC_CHECK_FUNCS([alarm atexit dup2 fdatasync ftruncate getcwd \
                gethostbyaddr gethostbyname gethostname getmntent \
-               getnameinfo getrpcbyname \
+               getnameinfo getrpcbyname getifaddrs \
                gettimeofday hasmntopt inet_ntoa innetgr memset mkdir pathconf \
                realpath rmdir select socket strcasecmp strchr strdup \
                strerror strrchr strtol strtoul sigprocmask])
diff --git a/support/include/tcpwrapper.h b/support/include/tcpwrapper.h
index 98cf806..f1145bd 100644
--- a/support/include/tcpwrapper.h
+++ b/support/include/tcpwrapper.h
@@ -11,7 +11,7 @@ extern int allow_severity;
 extern int deny_severity;
 
 extern int good_client(char *daemon, struct sockaddr_in *addr);
-extern int from_local (struct sockaddr_in *addr);
+extern int from_local(const struct sockaddr *sap);
 extern int check_default(char *daemon, struct sockaddr_in *addr,
 			 u_long proc, u_long prog);
 
diff --git a/support/misc/from_local.c b/support/misc/from_local.c
index 3f46b99..e2de969 100644
--- a/support/misc/from_local.c
+++ b/support/misc/from_local.c
@@ -43,6 +43,7 @@ static char sccsid[] = "@(#) from_local.c 1.3 96/05/31 15:52:57";
 
 #include <sys/types.h>
 #include <sys/socket.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <unistd.h>
 #include <netdb.h>
@@ -52,6 +53,7 @@ static char sccsid[] = "@(#) from_local.c 1.3 96/05/31 15:52:57";
 #include <stdlib.h>
 #include <string.h>
 
+#include "sockaddr.h"
 #include "tcpwrapper.h"
 #include "xlog.h"
 
@@ -60,11 +62,75 @@ static char sccsid[] = "@(#) from_local.c 1.3 96/05/31 15:52:57";
 #define FALSE	0
 #endif
 
- /*
-  * With virtual hosting, each hardware network interface can have multiple
-  * network addresses. On such machines the number of machine addresses can
-  * be surprisingly large.
-  */
+#ifdef HAVE_GETIFADDRS
+
+#include <ifaddrs.h>
+#include <time.h>
+
+/**
+ * from_local - determine whether request comes from the local system
+ * @sap: pointer to socket address to check
+ *
+ * With virtual hosting, each hardware network interface can have
+ * multiple network addresses. On such machines the number of machine
+ * addresses can be surprisingly large.
+ *
+ * We also expect the local network configuration to change over time,
+ * so call getifaddrs(3) more than once, but not too often.
+ *
+ * Returns TRUE if the sockaddr contains an address of one of the local
+ * network interfaces.  Otherwise FALSE is returned.
+ */
+int
+from_local(const struct sockaddr *sap)
+{
+	static struct ifaddrs *ifaddr = NULL;
+	static time_t last_update = 0;
+	struct ifaddrs *ifa;
+	unsigned int count;
+	time_t now;
+
+	if (time(&now) == ((time_t)-1)) {
+		xlog(L_ERROR, "%s: time(2): %m", __func__);
+
+		/* If we don't know what time it is, use the
+		 * existing ifaddr list, if one exists  */
+		now = last_update;
+		if (ifaddr == NULL)
+			now++;
+	}
+	if (now != last_update) {
+		xlog(D_GENERAL, "%s: updating local if addr list", __func__);
+
+		if (ifaddr)
+			freeifaddrs(ifaddr);
+
+		if (getifaddrs(&ifaddr) == -1) {
+			xlog(L_ERROR, "%s: getifaddrs(3): %m", __func__);
+			return FALSE;
+		}
+
+		last_update = now;
+	}
+
+	count = 0;
+	for (ifa = ifaddr; ifa; ifa = ifa->ifa_next) {
+		if ((ifa->ifa_flags & IFF_UP) &&
+		    nfs_compare_sockaddr(sap, ifa->ifa_addr)) {
+			xlog(D_GENERAL, "%s: incoming address matches "
+					"local interface address", __func__);
+			return TRUE;
+		} else
+			count++;
+	}
+
+	xlog(D_GENERAL, "%s: checked %u local if addrs; "
+			"incoming address not found", __func__, count);
+	return FALSE;
+}
+
+#else	/* !HAVE_GETIFADDRS */
+
 static int num_local;
 static int num_addrs;
 static struct in_addr *addrs;
@@ -155,12 +221,26 @@ find_local(void)
     return (num_local);
 }
 
-/* from_local - determine whether request comes from the local system */
+/**
+ * from_local - determine whether request comes from the local system
+ * @sap: pointer to socket address to check
+ *
+ * With virtual hosting, each hardware network interface can have
+ * multiple network addresses. On such machines the number of machine
+ * addresses can be surprisingly large.
+ *
+ * Returns TRUE if the sockaddr contains an address of one of the local
+ * network interfaces.  Otherwise FALSE is returned.
+ */
 int
-from_local(struct sockaddr_in *addr)
+from_local(const struct sockaddr *sap)
 {
+    const struct sockaddr_in *addr = (const struct sockaddr_in *)sap;
     int     i;
 
+    if (sap->sa_family != AF_INET)
+	return (FALSE);
+
     if (addrs == 0 && find_local() == 0)
 	xlog(L_ERROR, "Cannot find any active local network interfaces");
 
@@ -184,3 +264,5 @@ int main(void)
 }
 
 #endif	/* TEST */
+
+#endif	/* !HAVE_GETIFADDRS */
diff --git a/support/misc/tcpwrapper.c b/support/misc/tcpwrapper.c
index 1da6020..af626ad 100644
--- a/support/misc/tcpwrapper.c
+++ b/support/misc/tcpwrapper.c
@@ -202,7 +202,7 @@ u_long  prog;
 	if (acc && changed == 0)
 		return (acc->access);
 
-	if (!(from_local(addr) || good_client(daemon, addr))) {
+	if (!(from_local((struct sockaddr *)addr) || good_client(daemon, addr))) {
 		log_bad_host(addr, proc, prog);
 		if (acc)
 			acc->access = FALSE;


^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2010-01-15 21:36 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-15 17:49 [PATCH 0/6] IPv6 support for nfs-utils tcpwrapper shim Chuck Lever
     [not found] ` <20100115174426.30104.3492.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2010-01-15 17:49   ` [PATCH 1/6] tcpwrappers: Use xlog() instead of perror(3) and syslog(2) Chuck Lever
2010-01-15 17:49   ` [PATCH 2/6] tcp_wrappers: Use getifaddrs(3) if it is available Chuck Lever
2010-01-15 17:49   ` [PATCH 3/6] tcp_wrapper: Clean up logit() Chuck Lever
2010-01-15 17:50   ` [PATCH 4/6] tcpwrapper: Fix signage problems in the tcp_wrappers hash function Chuck Lever
2010-01-15 17:50   ` [PATCH 5/6] tcpwrapper: Eliminated shadowed declaration warnings Chuck Lever
2010-01-15 17:50   ` [PATCH 6/6] tcpwrapper: Add support for IPv6 Chuck Lever
2010-01-15 18:04   ` [PATCH 0/6] IPv6 support for nfs-utils tcpwrapper shim Steve Dickson
     [not found]     ` <4B50AE34.3020009-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
2010-01-15 18:27       ` Chuck Lever
2010-01-15 18:37         ` Steve Dickson
     [not found]           ` <4B50B5F0.6020202-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
2010-01-15 18:59             ` Chuck Lever
2010-01-15 19:32               ` Steve Dickson
  -- strict thread matches above, loose matches on Subject: below --
2010-01-15 21:31 [PATCH 0/6] IPv6 support for nfs-utils tcpwrapper shim (take 2) Chuck Lever
     [not found] ` <20100115212102.18214.19398.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2010-01-15 21:31   ` [PATCH 2/6] tcp_wrappers: Use getifaddrs(3) if it is available Chuck Lever

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox