[PATCH 0/6] nfs-utils: convert gssd to TI-RPC and add IPv6 support (RFC)

[PATCH 0/6] nfs-utils: convert gssd to TI-RPC and add IPv6 support (RFC)

[Jeff Layton]

This patchset is a first draft at adding support for IPv6 to rpc.gssd.
It's based on Chuck Lever's latest nfs-utils git tree. Some of the
needed infrastructure for this is not yet in mainline nfs-utils, so this
isn't really ready for inclusion yet. I'm just posting it to solicit
feedback.

In order to do this, we first have to convert rpc.gssd to use TI-RPC.
The first 5 patches do this and abstract out some of the code to make
this easier. Once that support is in place, the last patch in the series
actually adds the (fairly minimal) changes to add IPv6 support.

The series should be fully bisectable, but I've only really tested the
end result for anything other than to see if it compiles. With this
series on top of Chuck's tree, I've been able to mount an OpenSolaris
server using NFSv4+IPv6+krb5 auth

There's one other significant change in here as well. The current code
does what seems to be an unnecessary lookup on every upcall. It goes
something like this:

1) kernel calls gssd and hands it an address
2) we take that address and reverse-resolve it to a hostname
3) we store that hostname
4) we resolve that hostname back to an address for RPC client creation

Obviously, this could be more efficient. This set changes it so that we
store the address that we get from the upcall and skip the second
lookup.  I have no hard numbers yet, but it seems obvious that it should
improve upcall performance.

My main question here is -- is this OK? I don't see any need for us to
resolve the address forward and backward like we do. We do this sort of
thing in some places (statd, for instance) but the need is a little
clearer there.

Jeff Layton (6):
  nfs-utils: make getnameinfo() required for --enable-gss
  nfs-utils: store the address given in the upcall for later use
  nfs-utils: skip getaddrinfo in create_auth_rpc_client unless we need
    port
  nfs-utils: split out gssd rpc client creation into separate function
  nfs-utils: when TIRPC is enabled, use new API to create RPC client
  nfs-utils: add IPv6 code to gssd

 configure.ac           |    2 +
 utils/gssd/gssd.h      |    2 +-
 utils/gssd/gssd_proc.c |  417 +++++++++++++++++++++++++++++++++++++-----------
 3 files changed, 328 insertions(+), 93 deletions(-)

[PATCH 1/6] nfs-utils: make getnameinfo() required for --enable-gss

[Jeff Layton]

Systems that are so old that they don't have getnameinfo() in glibc are
probably also running kernels that are so old that they don't support
gssapi upcalls anyway.

Make --enable-gss dependent on the presence of the getnameinfo()
function.  This allows us to reduce some conditional compilation.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 configure.ac |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/configure.ac b/configure.ac
index 1f3c149..e7a5924 100644
--- a/configure.ac
+++ b/configure.ac
@@ -207,6 +207,8 @@ if test "$enable_nfsv4" = yes; then
   dnl but we need to make sure we get the right version
   if test "$enable_gss" = yes; then
     AC_RPCSEC_VERSION
+    AC_CHECK_FUNC([getnameinfo], ,
+		  [AC_MSG_ERROR([Function 'getnameinfo' not found.])])
   fi
 fi
 
-- 
1.6.0.6

[PATCH 2/6] nfs-utils: store the address given in the upcall for later use

[Jeff Layton]

The current upcall is rather inefficient. We first convert the address
to a hostname, and then later when we set up the RPC client, we do a
hostname lookup to convert it back to an address.

Begin to change this by keeping the address in the clnt_info that we
get out of the upcall. Since a sockaddr has a port field, we can also
eliminate the port from the clnt_info.

Finally, switch to getnameinfo() instead of gethostbyaddr(). We'll
need to use that call anyway when we switch to using IPv6.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/gssd/gssd.h      |    2 +-
 utils/gssd/gssd_proc.c |  105 ++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 90 insertions(+), 17 deletions(-)

diff --git a/utils/gssd/gssd.h b/utils/gssd/gssd.h
index 082039a..20f52ff 100644
--- a/utils/gssd/gssd.h
+++ b/utils/gssd/gssd.h
@@ -83,7 +83,7 @@ struct clnt_info {
 	int			krb5_poll_index;
 	int			spkm3_fd;
 	int			spkm3_poll_index;
-	int			port;
+	struct sockaddr_storage *addr;
 };
 
 void init_client_list(void);
diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 509946e..4f05040 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -102,10 +102,80 @@ struct pollfd * pollarray;
 
 int pollsize;  /* the size of pollaray (in pollfd's) */
 
+/*
+ * convert an address string to a sockaddr_storage struct
+ */
+static struct sockaddr_storage *
+addrstr_to_sockaddr(const char *addr, const int port)
+{
+	struct sockaddr_storage *ss;
+	struct sockaddr_in	*s4;
+
+	ss = calloc(1, sizeof(struct sockaddr_storage));
+	if (!ss) {
+		printerr(0, "ERROR: unable to allocate storage to convert "
+			    "address %s\n", addr);
+		goto out;
+	}
+
+	s4 = (struct sockaddr_in *) ss;
+
+	if (inet_pton(AF_INET, addr, &s4->sin_addr)) {
+		s4->sin_family = AF_INET;
+		s4->sin_port = htons(port);
+	} else {
+		printerr(0, "ERROR: unable to convert %s to address\n", addr);
+		free(ss);
+		ss = NULL;
+	}
+out:
+	return ss;
+}
+
+/*
+ * convert a sockaddr to a hostname
+ */
+static char *
+sockaddr_to_hostname(const struct sockaddr *sa, const char *addr)
+{
+#define MAX_HOSTNAME_LEN 127
+	char 			*hostname;
+	socklen_t		addrlen;
+	int			err;
+
+	switch (sa->sa_family) {
+	case AF_INET:
+		addrlen = sizeof(struct sockaddr_in);
+		break;
+	default:
+		printerr(0, "ERROR: unrecognized addr family %d\n",
+			 sa->sa_family);
+		return NULL;
+	}
+
+	hostname = calloc(1, MAX_HOSTNAME_LEN + 1);
+	if (!hostname) {
+		printerr(0, "ERROR: unable to allocate hostname string\n");
+		return NULL;
+	}
+
+	err = getnameinfo(sa, addrlen, hostname, MAX_HOSTNAME_LEN, NULL,
+			  0, NI_NAMEREQD);
+	if (err) {
+		free(hostname);
+		hostname = NULL;
+		printerr(0, "ERROR: unable to resolve %s to hostname: %s\n",
+			 addr, gai_strerror(err));
+	}
+
+	return hostname;
+}
+
 /* XXX buffer problems: */
 static int
 read_service_info(char *info_file_name, char **servicename, char **servername,
-		  int *prog, int *vers, char **protocol, int *port) {
+		  int *prog, int *vers, char **protocol,
+		  struct sockaddr_storage **addr) {
 #define INFOBUFLEN 256
 	char		buf[INFOBUFLEN + 1];
 	static char	dummy[128];
@@ -117,12 +187,12 @@ read_service_info(char *info_file_name, char **servicename, char **servername,
 	char		protoname[16];
 	char		cb_port[128];
 	char		*p;
-	in_addr_t	inaddr;
 	int		fd = -1;
-	struct hostent	*ent = NULL;
 	int		numfields;
+	int		port = 0;
 
 	*servicename = *servername = *protocol = NULL;
+	*addr = NULL;
 
 	if ((fd = open(info_file_name, O_RDONLY)) == -1) {
 		printerr(0, "ERROR: can't open %s: %s\n", info_file_name,
@@ -160,21 +230,20 @@ read_service_info(char *info_file_name, char **servicename, char **servername,
 	if((*prog != 100003) || ((*vers != 2) && (*vers != 3) && (*vers != 4)))
 		goto fail;
 
-	/* create service name */
-	inaddr = inet_addr(address);
-	if (!(ent = gethostbyaddr(&inaddr, sizeof(inaddr), AF_INET))) {
-		printerr(0, "ERROR: can't resolve server %s name\n", address);
+	if (cb_port[0] != '\0')
+		port = atoi(cb_port);
+	*addr = addrstr_to_sockaddr(address, port);
+	if (*addr == NULL)
 		goto fail;
-	}
-	if (!(*servername = calloc(strlen(ent->h_name) + 1, 1)))
+
+	*servername = sockaddr_to_hostname((struct sockaddr *) *addr, address);
+	if (*servername == NULL)
 		goto fail;
-	memcpy(*servername, ent->h_name, strlen(ent->h_name));
-	snprintf(buf, INFOBUFLEN, "%s@%s", service, ent->h_name);
+
+	snprintf(buf, INFOBUFLEN, "%s@%s", service, *servername);
 	if (!(*servicename = calloc(strlen(buf) + 1, 1)))
 		goto fail;
 	memcpy(*servicename, buf, strlen(buf));
-	if (cb_port[0] != '\0')
-		*port = atoi(cb_port);
 
 	if (!(*protocol = strdup(protoname)))
 		goto fail;
@@ -185,7 +254,9 @@ fail:
 	free(*servername);
 	free(*servicename);
 	free(*protocol);
+	free(*addr);
 	*servicename = *servername = *protocol = NULL;
+	*addr = NULL;
 	return -1;
 }
 
@@ -205,6 +276,7 @@ destroy_client(struct clnt_info *clp)
 	free(clp->servicename);
 	free(clp->servername);
 	free(clp->protocol);
+	free(clp->addr);
 	free(clp);
 }
 
@@ -251,7 +323,7 @@ process_clnt_dir_files(struct clnt_info * clp)
 	if ((clp->servicename == NULL) &&
 	     read_service_info(info_file_name, &clp->servicename,
 				&clp->servername, &clp->prog, &clp->vers,
-				&clp->protocol, &clp->port))
+				&clp->protocol, &clp->addr))
 		return -1;
 	return 0;
 }
@@ -599,8 +671,9 @@ int create_auth_rpc_client(struct clnt_info *clp,
 			 clp->servername, uid);
 		goto out_fail;
 	}
-	if (clp->port)
-		((struct sockaddr_in *)a->ai_addr)->sin_port = htons(clp->port);
+	if (((struct sockaddr_in *) clp->addr)->sin_port != 0)
+		((struct sockaddr_in *) a->ai_addr)->sin_port =
+				((struct sockaddr_in *) clp->addr)->sin_port;
 	if (a->ai_protocol == IPPROTO_TCP) {
 		if ((rpc_clnt = clnttcp_create(
 					(struct sockaddr_in *) a->ai_addr,
-- 
1.6.0.6

[PATCH 3/6] nfs-utils: skip getaddrinfo in create_auth_rpc_client unless we need port

[Jeff Layton]

We already have the server's address from the upcall, so we don't really
need to look it up again. Move the getaddrinfo call in
create_auth_rpc_client to a new function. Skip it if we already have the
port in the sockaddr that we saved from the info in the upcall. If
we need to get the port, don't bother looking up the hostname, just do
the getaddrinfo with AI_NUMERICHOST set.

This should reduce the amount of lookups that are needed.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/gssd/gssd_proc.c |  137 +++++++++++++++++++++++++++++------------------
 1 files changed, 84 insertions(+), 53 deletions(-)

diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 4f05040..349ed99 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -553,6 +553,80 @@ out_err:
 }
 
 /*
+ * Determine the port from the servicename and set the right field in the
+ * sockaddr. This is mostly a no-op with newer kernels that send the port
+ * in the upcall. Returns true on success and false on failure.
+ */
+static int
+populate_port(struct sockaddr_storage *ss, char *servicename, int socktype,
+	      int protocol)
+{
+	struct sockaddr_in	*s4 = (struct sockaddr_in *) ss;
+	struct addrinfo		ai_hints, *a = NULL;
+	char			node[INET_ADDRSTRLEN];
+	char			service[64];
+	char			*at_sign;
+	int			errcode;
+
+	memset(&ai_hints, '\0', sizeof(ai_hints));
+
+	switch (ss->ss_family) {
+	case AF_INET:
+		if (s4->sin_port != 0) {
+			printerr(2, "DEBUG: port already set to %d\n",
+				 ntohs(s4->sin_port));
+			return 1;
+		}
+		if (!inet_ntop(AF_INET, &s4->sin_addr, node,
+				sizeof(struct sockaddr_in)))
+			return 0;
+		break;
+	default:
+		printerr(0, "ERROR: unsupported address family %d\n",
+			    ss->ss_family);
+		return 0;
+	}
+
+	/* extract the service name from clp->servicename */
+	if ((at_sign = strchr(servicename, '@')) == NULL) {
+		printerr(0, "WARNING: servicename (%s) not formatted as "
+			"expected with service@host\n", servicename);
+		return 0;
+	}
+	if ((at_sign - servicename) >= sizeof(service)) {
+		printerr(0, "WARNING: service portion of servicename (%s) "
+			"is too long!\n", servicename);
+		return 0;
+	}
+	strncpy(service, servicename, at_sign - servicename);
+	service[at_sign - servicename] = '\0';
+
+	ai_hints.ai_family = ss->ss_family;
+	ai_hints.ai_socktype = socktype;
+	ai_hints.ai_protocol = protocol;
+	ai_hints.ai_flags |= AI_NUMERICHOST;
+
+	errcode = getaddrinfo(node, service, &ai_hints, &a);
+	if (errcode) {
+		printerr(0, "WARNING: Error from getaddrinfo for service "
+			 "'%s': %s\n", service, gai_strerror(errcode));
+		return 0;
+	}
+
+	/* XXX: walk all of the returned addrinfo list until we get port? */
+	if (a->ai_family == AF_INET) {
+		s4->sin_port = ((struct sockaddr_in *) a->ai_addr)->sin_port;
+	} else {
+		printerr(0, "ERROR: unrecognized address family returned by "
+			 "getaddrinfo %d\n", a->ai_family,
+			 gai_strerror(errcode));
+	}
+
+	freeaddrinfo(a);
+	return 1;
+}
+
+/*
  * Create an RPC connection and establish an authenticated
  * gss context with a server.
  */
@@ -567,14 +641,11 @@ int create_auth_rpc_client(struct clnt_info *clp,
 	AUTH			*auth = NULL;
 	uid_t			save_uid = -1;
 	int			retval = -1;
-	int			errcode;
 	OM_uint32		min_stat;
 	char			rpc_errmsg[1024];
 	int			sockp = RPC_ANYSOCK;
 	int			sendsz = 32768, recvsz = 32768;
-	struct addrinfo		ai_hints, *a = NULL;
-	char			service[64];
-	char			*at_sign;
+	int			socktype, protocol;
 
 	/* Create the context as the user (not as root) */
 	save_uid = geteuid();
@@ -628,15 +699,12 @@ int create_auth_rpc_client(struct clnt_info *clp,
 	printerr(2, "creating %s client for server %s\n", clp->protocol,
 			clp->servername);
 
-	memset(&ai_hints, '\0', sizeof(ai_hints));
-	ai_hints.ai_family = PF_INET;
-	ai_hints.ai_flags |= AI_CANONNAME;
 	if ((strcmp(clp->protocol, "tcp")) == 0) {
-		ai_hints.ai_socktype = SOCK_STREAM;
-		ai_hints.ai_protocol = IPPROTO_TCP;
+		socktype = SOCK_STREAM;
+		protocol = IPPROTO_TCP;
 	} else if ((strcmp(clp->protocol, "udp")) == 0) {
-		ai_hints.ai_socktype = SOCK_DGRAM;
-		ai_hints.ai_protocol = IPPROTO_UDP;
+		socktype = SOCK_DGRAM;
+		protocol = IPPROTO_UDP;
 	} else {
 		printerr(0, "WARNING: unrecognized protocol, '%s', requested "
 			 "for connection to server %s for user with uid %d\n",
@@ -644,39 +712,12 @@ int create_auth_rpc_client(struct clnt_info *clp,
 		goto out_fail;
 	}
 
-	/* extract the service name from clp->servicename */
-	if ((at_sign = strchr(clp->servicename, '@')) == NULL) {
-		printerr(0, "WARNING: servicename (%s) not formatted as "
-			"expected with service@host\n", clp->servicename);
+	if (!populate_port(clp->addr, clp->servicename, socktype, protocol))
 		goto out_fail;
-	}
-	if ((at_sign - clp->servicename) >= sizeof(service)) {
-		printerr(0, "WARNING: service portion of servicename (%s) "
-			"is too long!\n", clp->servicename);
-		goto out_fail;
-	}
-	strncpy(service, clp->servicename, at_sign - clp->servicename);
-	service[at_sign - clp->servicename] = '\0';
 
-	errcode = getaddrinfo(clp->servername, service, &ai_hints, &a);
-	if (errcode) {
-		printerr(0, "WARNING: Error from getaddrinfo for server "
-			 "'%s': %s\n", clp->servername, gai_strerror(errcode));
-		goto out_fail;
-	}
-
-	if (a == NULL) {
-		printerr(0, "WARNING: No address information found for "
-			 "connection to server %s for user with uid %d\n",
-			 clp->servername, uid);
-		goto out_fail;
-	}
-	if (((struct sockaddr_in *) clp->addr)->sin_port != 0)
-		((struct sockaddr_in *) a->ai_addr)->sin_port =
-				((struct sockaddr_in *) clp->addr)->sin_port;
-	if (a->ai_protocol == IPPROTO_TCP) {
+	if (protocol == IPPROTO_TCP) {
 		if ((rpc_clnt = clnttcp_create(
-					(struct sockaddr_in *) a->ai_addr,
+					(struct sockaddr_in *) clp->addr,
 					clp->prog, clp->vers, &sockp,
 					sendsz, recvsz)) == NULL) {
 			snprintf(rpc_errmsg, sizeof(rpc_errmsg),
@@ -687,10 +728,10 @@ int create_auth_rpc_client(struct clnt_info *clp,
 				 clnt_spcreateerror(rpc_errmsg));
 			goto out_fail;
 		}
-	} else if (a->ai_protocol == IPPROTO_UDP) {
+	} else if (protocol == IPPROTO_UDP) {
 		const struct timeval timeout = {5, 0};
 		if ((rpc_clnt = clntudp_bufcreate(
-					(struct sockaddr_in *) a->ai_addr,
+					(struct sockaddr_in *) clp->addr,
 					clp->prog, clp->vers, timeout,
 					&sockp, sendsz, recvsz)) == NULL) {
 			snprintf(rpc_errmsg, sizeof(rpc_errmsg),
@@ -701,16 +742,7 @@ int create_auth_rpc_client(struct clnt_info *clp,
 				 clnt_spcreateerror(rpc_errmsg));
 			goto out_fail;
 		}
-	} else {
-		/* Shouldn't happen! */
-		printerr(0, "ERROR: requested protocol '%s', but "
-			 "got addrinfo with protocol %d\n",
-			 clp->protocol, a->ai_protocol);
-		goto out_fail;
 	}
-	/* We're done with this */
-	freeaddrinfo(a);
-	a = NULL;
 
 	printerr(2, "creating context with server %s\n", clp->servicename);
 	auth = authgss_create_default(rpc_clnt, clp->servicename, &sec);
@@ -732,7 +764,6 @@ int create_auth_rpc_client(struct clnt_info *clp,
   out:
 	if (sec.cred != GSS_C_NO_CREDENTIAL)
 		gss_release_cred(&min_stat, &sec.cred);
-  	if (a != NULL) freeaddrinfo(a);
 	/* Restore euid to original value */
 	if ((save_uid != -1) && (setfsuid(save_uid) != uid)) {
 		printerr(0, "WARNING: Failed to restore fsuid"
-- 
1.6.0.6

[PATCH 4/6] nfs-utils: split out gssd rpc client creation into separate function

[Jeff Layton]

Abstract out the actual RPC client creation functions. This will allow
us to create a new TI-RPC enabled client creation function in a later
patch.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/gssd/gssd_proc.c |   80 +++++++++++++++++++++++++++++------------------
 1 files changed, 49 insertions(+), 31 deletions(-)

diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 349ed99..5cf2445 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -552,6 +552,52 @@ out_err:
 	return -1;
 }
 
+static CLIENT *
+create_rpc_client(struct clnt_info *clp, int protocol, uid_t uid)
+{
+	CLIENT			*rpc_clnt = NULL;
+	char			rpc_errmsg[1024];
+	int			sockp = RPC_ANYSOCK;
+	int			sendsz = 32768, recvsz = 32768;
+	const struct timeval 	timeout = {5, 0};
+
+	switch(protocol) {
+	case IPPROTO_TCP:
+		if ((rpc_clnt = clnttcp_create(
+					(struct sockaddr_in *) clp->addr,
+					clp->prog, clp->vers, &sockp,
+					sendsz, recvsz)) == NULL) {
+			snprintf(rpc_errmsg, sizeof(rpc_errmsg),
+				 "WARNING: can't create tcp rpc_clnt "
+				 "for server %s for user with uid %d",
+				 clp->servername, uid);
+			printerr(0, "%s\n",
+				 clnt_spcreateerror(rpc_errmsg));
+		}
+		break;
+	case IPPROTO_UDP:
+		if ((rpc_clnt = clntudp_bufcreate(
+					(struct sockaddr_in *) clp->addr,
+					clp->prog, clp->vers, timeout,
+					&sockp, sendsz, recvsz)) == NULL) {
+			snprintf(rpc_errmsg, sizeof(rpc_errmsg),
+				 "WARNING: can't create udp rpc_clnt "
+				 "for server %s for user with uid %d",
+				 clp->servername, uid);
+			printerr(0, "%s\n",
+				 clnt_spcreateerror(rpc_errmsg));
+		}
+		break;
+	default:
+		/* Shouldn't happen! */
+		printerr(0, "ERROR: requested protocol '%s', but "
+			 "got addrinfo with protocol %d\n",
+			 clp->protocol, protocol);
+	}
+
+	return rpc_clnt;
+}
+
 /*
  * Determine the port from the servicename and set the right field in the
  * sockaddr. This is mostly a no-op with newer kernels that send the port
@@ -642,9 +688,6 @@ int create_auth_rpc_client(struct clnt_info *clp,
 	uid_t			save_uid = -1;
 	int			retval = -1;
 	OM_uint32		min_stat;
-	char			rpc_errmsg[1024];
-	int			sockp = RPC_ANYSOCK;
-	int			sendsz = 32768, recvsz = 32768;
 	int			socktype, protocol;
 
 	/* Create the context as the user (not as root) */
@@ -715,34 +758,9 @@ int create_auth_rpc_client(struct clnt_info *clp,
 	if (!populate_port(clp->addr, clp->servicename, socktype, protocol))
 		goto out_fail;
 
-	if (protocol == IPPROTO_TCP) {
-		if ((rpc_clnt = clnttcp_create(
-					(struct sockaddr_in *) clp->addr,
-					clp->prog, clp->vers, &sockp,
-					sendsz, recvsz)) == NULL) {
-			snprintf(rpc_errmsg, sizeof(rpc_errmsg),
-				 "WARNING: can't create tcp rpc_clnt "
-				 "for server %s for user with uid %d",
-				 clp->servername, uid);
-			printerr(0, "%s\n",
-				 clnt_spcreateerror(rpc_errmsg));
-			goto out_fail;
-		}
-	} else if (protocol == IPPROTO_UDP) {
-		const struct timeval timeout = {5, 0};
-		if ((rpc_clnt = clntudp_bufcreate(
-					(struct sockaddr_in *) clp->addr,
-					clp->prog, clp->vers, timeout,
-					&sockp, sendsz, recvsz)) == NULL) {
-			snprintf(rpc_errmsg, sizeof(rpc_errmsg),
-				 "WARNING: can't create udp rpc_clnt "
-				 "for server %s for user with uid %d",
-				 clp->servername, uid);
-			printerr(0, "%s\n",
-				 clnt_spcreateerror(rpc_errmsg));
-			goto out_fail;
-		}
-	}
+	rpc_clnt = create_rpc_client(clp, protocol, uid);
+	if (!rpc_clnt)
+		goto out_fail;
 
 	printerr(2, "creating context with server %s\n", clp->servicename);
 	auth = authgss_create_default(rpc_clnt, clp->servicename, &sec);
-- 
1.6.0.6

[PATCH 5/6] nfs-utils: when TIRPC is enabled, use new API to create RPC client

[Jeff Layton]

When we add IPv6 code, we'll need to use the newer API to create
the rpc client. Add a new rpc client creation function that's
conditionally compiled in whenever TIRPC is enabled.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/gssd/gssd_proc.c |   76 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 76 insertions(+), 0 deletions(-)

diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 5cf2445..1e3dabc 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -552,6 +552,81 @@ out_err:
 	return -1;
 }
 
+#ifdef HAVE_LIBTIRPC
+static CLIENT *
+create_rpc_client(struct clnt_info *clp, int protocol, uid_t uid)
+{
+	CLIENT			*rpc_clnt = NULL;
+	char			rpc_errmsg[1024];
+	int			sendsz = 32768, recvsz = 32768;
+	struct netconfig	*nc = NULL;
+	struct netbuf		nb;
+	const struct timeval	timeout = {5, 0};
+	void 			*handle = NULL;
+	char			*nc_protofmly;
+	struct protoent		*proto;
+
+	proto = getprotobynumber(protocol);
+	if (proto == NULL) {
+		printerr(0, "ERROR: can't resolve %d to protocol\n",
+			 protocol);
+		goto out;
+	}
+
+	switch(clp->addr->ss_family) {
+	case AF_INET:
+		nc_protofmly = NC_INET;
+		nb.len = nb.maxlen = sizeof(struct sockaddr_in);
+		break;
+	default:
+		printerr(0, "ERROR: unsupported address family %d\n",
+			    clp->addr->ss_family);
+		goto out;
+	}
+
+	handle = setnetconfig();
+	while ((nc = getnetconfig(handle)) != NULL) {
+
+		if (nc->nc_protofmly != NULL &&
+		    strcmp(nc->nc_protofmly, nc_protofmly) != 0)
+			continue;
+		if (nc->nc_proto != NULL &&
+		    strcmp(nc->nc_proto, proto->p_name) != 0)
+			continue;
+
+		break;
+	}
+
+	if (!nc) {
+		printerr(0, "ERROR: unable to find netconfig entry for "
+			    "addr family %d and proto %d\n",
+			    clp->addr->ss_family, protocol);
+		goto out;
+	}
+
+	nb.buf = clp->addr;
+
+	rpc_clnt = clnt_tli_create(RPC_ANYFD, nc, &nb, clp->prog, clp->vers,
+				   sendsz, recvsz);
+	if (!rpc_clnt) {
+		snprintf(rpc_errmsg, sizeof(rpc_errmsg),
+			 "ERROR: unable to create %s RPC client for %s with "
+			 "uid %d", clp->protocol, clp->servername, uid);
+		printerr(0, "%s\n", clnt_spcreateerror(rpc_errmsg));
+		goto out;
+	}
+
+	/* set retry timeout for connectionless transports */
+	if (nc->nc_semantics == NC_TPI_CLTS)
+		clnt_control(rpc_clnt, CLSET_RETRY_TIMEOUT, (void *) &timeout);
+out:
+	if (handle)
+		endnetconfig(handle);
+	return rpc_clnt;
+}
+
+#else  /* HAVE_LIBTIRPC */
+
 static CLIENT *
 create_rpc_client(struct clnt_info *clp, int protocol, uid_t uid)
 {
@@ -597,6 +672,7 @@ create_rpc_client(struct clnt_info *clp, int protocol, uid_t uid)
 
 	return rpc_clnt;
 }
+#endif /* HAVE_LIBTIRPC */
 
 /*
  * Determine the port from the servicename and set the right field in the
-- 
1.6.0.6

[PATCH 6/6] nfs-utils: add IPv6 code to gssd

[Jeff Layton]

All of the pieces to handle IPv6 are now in place. Add IPv6-specific
code wrapped in the proper #ifdef's so that IPv6 support works when
it's enabled at build-time.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/gssd/gssd_proc.c |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 1e3dabc..ad66d33 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -110,6 +110,7 @@ addrstr_to_sockaddr(const char *addr, const int port)
 {
 	struct sockaddr_storage *ss;
 	struct sockaddr_in	*s4;
+	struct sockaddr_in6	*s6;
 
 	ss = calloc(1, sizeof(struct sockaddr_storage));
 	if (!ss) {
@@ -119,10 +120,16 @@ addrstr_to_sockaddr(const char *addr, const int port)
 	}
 
 	s4 = (struct sockaddr_in *) ss;
+	s6 = (struct sockaddr_in6 *) ss;
 
 	if (inet_pton(AF_INET, addr, &s4->sin_addr)) {
 		s4->sin_family = AF_INET;
 		s4->sin_port = htons(port);
+#ifdef IPV6_SUPPORTED
+	} else if (inet_pton(AF_INET6, addr, &s6->sin6_addr)) {
+		s6->sin6_family = AF_INET6;
+		s6->sin6_port = htons(port);
+#endif /* IPV6_SUPPORTED */
 	} else {
 		printerr(0, "ERROR: unable to convert %s to address\n", addr);
 		free(ss);
@@ -147,6 +154,11 @@ sockaddr_to_hostname(const struct sockaddr *sa, const char *addr)
 	case AF_INET:
 		addrlen = sizeof(struct sockaddr_in);
 		break;
+#ifdef IPV6_SUPPORTED
+	case AF_INET6:
+		addrlen = sizeof(struct sockaddr_in6);
+		break;
+#endif /* IPV6_SUPPORTED */
 	default:
 		printerr(0, "ERROR: unrecognized addr family %d\n",
 			 sa->sa_family);
@@ -578,6 +590,12 @@ create_rpc_client(struct clnt_info *clp, int protocol, uid_t uid)
 		nc_protofmly = NC_INET;
 		nb.len = nb.maxlen = sizeof(struct sockaddr_in);
 		break;
+#ifdef IPV6_SUPPORTED
+	case AF_INET6:
+		nc_protofmly = NC_INET6;
+		nb.len = nb.maxlen = sizeof(struct sockaddr_in6);
+		break;
+#endif /* IPV6_SUPPORTED */
 	default:
 		printerr(0, "ERROR: unsupported address family %d\n",
 			    clp->addr->ss_family);
@@ -684,6 +702,7 @@ populate_port(struct sockaddr_storage *ss, char *servicename, int socktype,
 	      int protocol)
 {
 	struct sockaddr_in	*s4 = (struct sockaddr_in *) ss;
+	struct sockaddr_in6	*s6 = (struct sockaddr_in6 *) ss;
 	struct addrinfo		ai_hints, *a = NULL;
 	char			node[INET_ADDRSTRLEN];
 	char			service[64];
@@ -703,6 +722,18 @@ populate_port(struct sockaddr_storage *ss, char *servicename, int socktype,
 				sizeof(struct sockaddr_in)))
 			return 0;
 		break;
+#ifdef IPV6_SUPPORTED
+	case AF_INET6:
+		if (s6->sin6_port != 0) {
+			printerr(2, "DEBUG: port already set to %d\n",
+				 ntohs(s6->sin6_port));
+			return 1;
+		}
+		if (!inet_ntop(AF_INET6, &s6->sin6_addr, node,
+				sizeof(struct sockaddr_in6)))
+			return 0;
+		break;
+#endif /* IPV6_SUPPORTED */
 	default:
 		printerr(0, "ERROR: unsupported address family %d\n",
 			    ss->ss_family);
@@ -738,6 +769,10 @@ populate_port(struct sockaddr_storage *ss, char *servicename, int socktype,
 	/* XXX: walk all of the returned addrinfo list until we get port? */
 	if (a->ai_family == AF_INET) {
 		s4->sin_port = ((struct sockaddr_in *) a->ai_addr)->sin_port;
+#ifdef IPV6_SUPPORTED
+	} else if (a->ai_family == AF_INET6) {
+		s6->sin6_port = ((struct sockaddr_in6 *) a->ai_addr)->sin6_port;
+#endif /* IPV6_SUPPORTED */
 	} else {
 		printerr(0, "ERROR: unrecognized address family returned by "
 			 "getaddrinfo %d\n", a->ai_family,
-- 
1.6.0.6

Re: [PATCH 2/6] nfs-utils: store the address given in the upcall for later use

[Chuck Lever]

On Mar 11, 2009, at 12:42 PM, Jeff Layton wrote:
> The current upcall is rather inefficient. We first convert the address
> to a hostname, and then later when we set up the RPC client, we do a
> hostname lookup to convert it back to an address.
>
> Begin to change this by keeping the address in the clnt_info that we
> get out of the upcall. Since a sockaddr has a port field, we can also
> eliminate the port from the clnt_info.
>
> Finally, switch to getnameinfo() instead of gethostbyaddr(). We'll
> need to use that call anyway when we switch to using IPv6.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> utils/gssd/gssd.h      |    2 +-
> utils/gssd/gssd_proc.c |  105 +++++++++++++++++++++++++++++++++++++++ 
> +-------
> 2 files changed, 90 insertions(+), 17 deletions(-)
>
> diff --git a/utils/gssd/gssd.h b/utils/gssd/gssd.h
> index 082039a..20f52ff 100644
> --- a/utils/gssd/gssd.h
> +++ b/utils/gssd/gssd.h
> @@ -83,7 +83,7 @@ struct clnt_info {
> 	int			krb5_poll_index;
> 	int			spkm3_fd;
> 	int			spkm3_poll_index;
> -	int			port;
> +	struct sockaddr_storage *addr;

By convention, if you want to store a pointer to an socket address,  
"struct sockaddr *" is appropriate.  If you want to allocate memory to  
store the socket address in this field, "struct sockaddr_storage" is  
appropriate.

I think you will be better off with:

	struct sockaddr_storage addr;

and skipping the extra memory allocation.

> };
>
> void init_client_list(void);
> diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
> index 509946e..4f05040 100644
> --- a/utils/gssd/gssd_proc.c
> +++ b/utils/gssd/gssd_proc.c
> @@ -102,10 +102,80 @@ struct pollfd * pollarray;
>
> int pollsize;  /* the size of pollaray (in pollfd's) */
>
> +/*
> + * convert an address string to a sockaddr_storage struct
> + */
> +static struct sockaddr_storage *
> +addrstr_to_sockaddr(const char *addr, const int port)
> +{
> +	struct sockaddr_storage *ss;
> +	struct sockaddr_in	*s4;
> +
> +	ss = calloc(1, sizeof(struct sockaddr_storage));
> +	if (!ss) {
> +		printerr(0, "ERROR: unable to allocate storage to convert "
> +			    "address %s\n", addr);
> +		goto out;
> +	}
> +
> +	s4 = (struct sockaddr_in *) ss;
> +
> +	if (inet_pton(AF_INET, addr, &s4->sin_addr)) {
> +		s4->sin_family = AF_INET;
> +		s4->sin_port = htons(port);
> +	} else {
> +		printerr(0, "ERROR: unable to convert %s to address\n", addr);
> +		free(ss);
> +		ss = NULL;
> +	}
> +out:
> +	return ss;
> +}
> +
> +/*
> + * convert a sockaddr to a hostname
> + */
> +static char *
> +sockaddr_to_hostname(const struct sockaddr *sa, const char *addr)
> +{
> +#define MAX_HOSTNAME_LEN 127
> +	char 			*hostname;
> +	socklen_t		addrlen;
> +	int			err;
> +
> +	switch (sa->sa_family) {
> +	case AF_INET:
> +		addrlen = sizeof(struct sockaddr_in);
> +		break;
> +	default:
> +		printerr(0, "ERROR: unrecognized addr family %d\n",
> +			 sa->sa_family);
> +		return NULL;
> +	}
> +
> +	hostname = calloc(1, MAX_HOSTNAME_LEN + 1);
> +	if (!hostname) {
> +		printerr(0, "ERROR: unable to allocate hostname string\n");
> +		return NULL;
> +	}
> +
> +	err = getnameinfo(sa, addrlen, hostname, MAX_HOSTNAME_LEN, NULL,
> +			  0, NI_NAMEREQD);
> +	if (err) {
> +		free(hostname);
> +		hostname = NULL;
> +		printerr(0, "ERROR: unable to resolve %s to hostname: %s\n",
> +			 addr, gai_strerror(err));
> +	}
> +
> +	return hostname;
> +}
> +
> /* XXX buffer problems: */
> static int
> read_service_info(char *info_file_name, char **servicename, char  
> **servername,
> -		  int *prog, int *vers, char **protocol, int *port) {
> +		  int *prog, int *vers, char **protocol,
> +		  struct sockaddr_storage **addr) {
> #define INFOBUFLEN 256
> 	char		buf[INFOBUFLEN + 1];
> 	static char	dummy[128];
> @@ -117,12 +187,12 @@ read_service_info(char *info_file_name, char  
> **servicename, char **servername,
> 	char		protoname[16];
> 	char		cb_port[128];
> 	char		*p;
> -	in_addr_t	inaddr;
> 	int		fd = -1;
> -	struct hostent	*ent = NULL;
> 	int		numfields;
> +	int		port = 0;
>
> 	*servicename = *servername = *protocol = NULL;
> +	*addr = NULL;
>
> 	if ((fd = open(info_file_name, O_RDONLY)) == -1) {
> 		printerr(0, "ERROR: can't open %s: %s\n", info_file_name,
> @@ -160,21 +230,20 @@ read_service_info(char *info_file_name, char  
> **servicename, char **servername,
> 	if((*prog != 100003) || ((*vers != 2) && (*vers != 3) && (*vers !=  
> 4)))
> 		goto fail;
>
> -	/* create service name */
> -	inaddr = inet_addr(address);
> -	if (!(ent = gethostbyaddr(&inaddr, sizeof(inaddr), AF_INET))) {
> -		printerr(0, "ERROR: can't resolve server %s name\n", address);
> +	if (cb_port[0] != '\0')
> +		port = atoi(cb_port);
> +	*addr = addrstr_to_sockaddr(address, port);
> +	if (*addr == NULL)
> 		goto fail;
> -	}
> -	if (!(*servername = calloc(strlen(ent->h_name) + 1, 1)))
> +
> +	*servername = sockaddr_to_hostname((struct sockaddr *) *addr,  
> address);
> +	if (*servername == NULL)
> 		goto fail;
> -	memcpy(*servername, ent->h_name, strlen(ent->h_name));
> -	snprintf(buf, INFOBUFLEN, "%s@%s", service, ent->h_name);
> +
> +	snprintf(buf, INFOBUFLEN, "%s@%s", service, *servername);
> 	if (!(*servicename = calloc(strlen(buf) + 1, 1)))
> 		goto fail;
> 	memcpy(*servicename, buf, strlen(buf));
> -	if (cb_port[0] != '\0')
> -		*port = atoi(cb_port);
>
> 	if (!(*protocol = strdup(protoname)))
> 		goto fail;
> @@ -185,7 +254,9 @@ fail:
> 	free(*servername);
> 	free(*servicename);
> 	free(*protocol);
> +	free(*addr);
> 	*servicename = *servername = *protocol = NULL;
> +	*addr = NULL;
> 	return -1;
> }
>
> @@ -205,6 +276,7 @@ destroy_client(struct clnt_info *clp)
> 	free(clp->servicename);
> 	free(clp->servername);
> 	free(clp->protocol);
> +	free(clp->addr);
> 	free(clp);
> }
>
> @@ -251,7 +323,7 @@ process_clnt_dir_files(struct clnt_info * clp)
> 	if ((clp->servicename == NULL) &&
> 	     read_service_info(info_file_name, &clp->servicename,
> 				&clp->servername, &clp->prog, &clp->vers,
> -				&clp->protocol, &clp->port))
> +				&clp->protocol, &clp->addr))
> 		return -1;
> 	return 0;
> }
> @@ -599,8 +671,9 @@ int create_auth_rpc_client(struct clnt_info *clp,
> 			 clp->servername, uid);
> 		goto out_fail;
> 	}
> -	if (clp->port)
> -		((struct sockaddr_in *)a->ai_addr)->sin_port = htons(clp->port);
> +	if (((struct sockaddr_in *) clp->addr)->sin_port != 0)
> +		((struct sockaddr_in *) a->ai_addr)->sin_port =
> +				((struct sockaddr_in *) clp->addr)->sin_port;
> 	if (a->ai_protocol == IPPROTO_TCP) {
> 		if ((rpc_clnt = clnttcp_create(
> 					(struct sockaddr_in *) a->ai_addr,
> -- 
> 1.6.0.6
>

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

Re: [PATCH 3/6] nfs-utils: skip getaddrinfo in create_auth_rpc_client unless we need port

[Chuck Lever]

On Mar 11, 2009, at 12:42 PM, Jeff Layton wrote:
> We already have the server's address from the upcall, so we don't  
> really
> need to look it up again. Move the getaddrinfo call in
> create_auth_rpc_client to a new function. Skip it if we already have  
> the
> port in the sockaddr that we saved from the info in the upcall. If
> we need to get the port, don't bother looking up the hostname, just do
> the getaddrinfo with AI_NUMERICHOST set.
>
> This should reduce the amount of lookups that are needed.
>
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> utils/gssd/gssd_proc.c |  137 ++++++++++++++++++++++++++++ 
> +------------------
> 1 files changed, 84 insertions(+), 53 deletions(-)
>
> diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
> index 4f05040..349ed99 100644
> --- a/utils/gssd/gssd_proc.c
> +++ b/utils/gssd/gssd_proc.c
> @@ -553,6 +553,80 @@ out_err:
> }
>
> /*
> + * Determine the port from the servicename and set the right field  
> in the
> + * sockaddr. This is mostly a no-op with newer kernels that send  
> the port
> + * in the upcall. Returns true on success and false on failure.
> + */
> +static int
> +populate_port(struct sockaddr_storage *ss, char *servicename, int  
> socktype,
> +	      int protocol)

Pointers to socket addresses are "struct sockaddr *" not "struct  
sockaddr_storage *".

> +{
> +	struct sockaddr_in	*s4 = (struct sockaddr_in *) ss;
> +	struct addrinfo		ai_hints, *a = NULL;
> +	char			node[INET_ADDRSTRLEN];
> +	char			service[64];
> +	char			*at_sign;
> +	int			errcode;
> +
> +	memset(&ai_hints, '\0', sizeof(ai_hints));

A C99 structure initializer would zero the structure, and you can  
initialize the fields at the same time.

> +
> +	switch (ss->ss_family) {
> +	case AF_INET:
> +		if (s4->sin_port != 0) {
> +			printerr(2, "DEBUG: port already set to %d\n",
> +				 ntohs(s4->sin_port));
> +			return 1;
> +		}
> +		if (!inet_ntop(AF_INET, &s4->sin_addr, node,
> +				sizeof(struct sockaddr_in)))
> +			return 0;

I know you are copying this mostly verbatim, but I'm not sure exactly  
what this is doing.  You can pass a C string containing the port  
number, or a C string containing the service name, to getaddrinfo(3)  
and have it fill in the port, I think.

> +		break;
> +	default:
> +		printerr(0, "ERROR: unsupported address family %d\n",
> +			    ss->ss_family);
> +		return 0;
> +	}
> +
> +	/* extract the service name from clp->servicename */
> +	if ((at_sign = strchr(servicename, '@')) == NULL) {
> +		printerr(0, "WARNING: servicename (%s) not formatted as "
> +			"expected with service@host\n", servicename);
> +		return 0;
> +	}
> +	if ((at_sign - servicename) >= sizeof(service)) {
> +		printerr(0, "WARNING: service portion of servicename (%s) "
> +			"is too long!\n", servicename);
> +		return 0;
> +	}
> +	strncpy(service, servicename, at_sign - servicename);
> +	service[at_sign - servicename] = '\0';
> +
> +	ai_hints.ai_family = ss->ss_family;
> +	ai_hints.ai_socktype = socktype;
> +	ai_hints.ai_protocol = protocol;
> +	ai_hints.ai_flags |= AI_NUMERICHOST;

I don't think we need to fill in ai_socktype.

AI_NUMERICHOST means we're not doing a DNS resolution here at all,  
we're just building a sockaddr.

> +
> +	errcode = getaddrinfo(node, service, &ai_hints, &a);
> +	if (errcode) {
> +		printerr(0, "WARNING: Error from getaddrinfo for service "
> +			 "'%s': %s\n", service, gai_strerror(errcode));

If the error code is EAI_SYSTEM then you should use strerror(errno).   
I usually use a switch statement to sort this out.

> +		return 0;
> +	}
> +
> +	/* XXX: walk all of the returned addrinfo list until we get port? */

Not sure we need to look at more than the first result.   
getaddrinfo(3) ought to fill in the port of all the returned addrinfo  
structures.

>
> +	if (a->ai_family == AF_INET) {
> +		s4->sin_port = ((struct sockaddr_in *) a->ai_addr)->sin_port;
> +	} else {
> +		printerr(0, "ERROR: unrecognized address family returned by "
> +			 "getaddrinfo %d\n", a->ai_family,
> +			 gai_strerror(errcode));
> +	}
> +
> +	freeaddrinfo(a);
> +	return 1;
> +}
> +
> +/*
>  * Create an RPC connection and establish an authenticated
>  * gss context with a server.
>  */
> @@ -567,14 +641,11 @@ int create_auth_rpc_client(struct clnt_info  
> *clp,
> 	AUTH			*auth = NULL;
> 	uid_t			save_uid = -1;
> 	int			retval = -1;
> -	int			errcode;
> 	OM_uint32		min_stat;
> 	char			rpc_errmsg[1024];
> 	int			sockp = RPC_ANYSOCK;
> 	int			sendsz = 32768, recvsz = 32768;
> -	struct addrinfo		ai_hints, *a = NULL;
> -	char			service[64];
> -	char			*at_sign;
> +	int			socktype, protocol;
>
> 	/* Create the context as the user (not as root) */
> 	save_uid = geteuid();
> @@ -628,15 +699,12 @@ int create_auth_rpc_client(struct clnt_info  
> *clp,
> 	printerr(2, "creating %s client for server %s\n", clp->protocol,
> 			clp->servername);
>
> -	memset(&ai_hints, '\0', sizeof(ai_hints));
> -	ai_hints.ai_family = PF_INET;
> -	ai_hints.ai_flags |= AI_CANONNAME;
> 	if ((strcmp(clp->protocol, "tcp")) == 0) {
> -		ai_hints.ai_socktype = SOCK_STREAM;
> -		ai_hints.ai_protocol = IPPROTO_TCP;
> +		socktype = SOCK_STREAM;
> +		protocol = IPPROTO_TCP;
> 	} else if ((strcmp(clp->protocol, "udp")) == 0) {
> -		ai_hints.ai_socktype = SOCK_DGRAM;
> -		ai_hints.ai_protocol = IPPROTO_UDP;
> +		socktype = SOCK_DGRAM;
> +		protocol = IPPROTO_UDP;
> 	} else {
> 		printerr(0, "WARNING: unrecognized protocol, '%s', requested "
> 			 "for connection to server %s for user with uid %d\n",
> @@ -644,39 +712,12 @@ int create_auth_rpc_client(struct clnt_info  
> *clp,
> 		goto out_fail;
> 	}
>
>
> -	/* extract the service name from clp->servicename */
> -	if ((at_sign = strchr(clp->servicename, '@')) == NULL) {
> -		printerr(0, "WARNING: servicename (%s) not formatted as "
> -			"expected with service@host\n", clp->servicename);
> +	if (!populate_port(clp->addr, clp->servicename, socktype, protocol))
> 		goto out_fail;
> -	}
> -	if ((at_sign - clp->servicename) >= sizeof(service)) {
> -		printerr(0, "WARNING: service portion of servicename (%s) "
> -			"is too long!\n", clp->servicename);
> -		goto out_fail;
> -	}
> -	strncpy(service, clp->servicename, at_sign - clp->servicename);
> -	service[at_sign - clp->servicename] = '\0';
>
> -	errcode = getaddrinfo(clp->servername, service, &ai_hints, &a);
> -	if (errcode) {
> -		printerr(0, "WARNING: Error from getaddrinfo for server "
> -			 "'%s': %s\n", clp->servername, gai_strerror(errcode));
> -		goto out_fail;
> -	}
> -
> -	if (a == NULL) {
> -		printerr(0, "WARNING: No address information found for "
> -			 "connection to server %s for user with uid %d\n",
> -			 clp->servername, uid);
> -		goto out_fail;
> -	}
> -	if (((struct sockaddr_in *) clp->addr)->sin_port != 0)
> -		((struct sockaddr_in *) a->ai_addr)->sin_port =
> -				((struct sockaddr_in *) clp->addr)->sin_port;
> -	if (a->ai_protocol == IPPROTO_TCP) {
> +	if (protocol == IPPROTO_TCP) {
> 		if ((rpc_clnt = clnttcp_create(
> -					(struct sockaddr_in *) a->ai_addr,
> +					(struct sockaddr_in *) clp->addr,
> 					clp->prog, clp->vers, &sockp,
> 					sendsz, recvsz)) == NULL) {
> 			snprintf(rpc_errmsg, sizeof(rpc_errmsg),
> @@ -687,10 +728,10 @@ int create_auth_rpc_client(struct clnt_info  
> *clp,
> 				 clnt_spcreateerror(rpc_errmsg));
> 			goto out_fail;
> 		}
> -	} else if (a->ai_protocol == IPPROTO_UDP) {
> +	} else if (protocol == IPPROTO_UDP) {
> 		const struct timeval timeout = {5, 0};
> 		if ((rpc_clnt = clntudp_bufcreate(
> -					(struct sockaddr_in *) a->ai_addr,
> +					(struct sockaddr_in *) clp->addr,
> 					clp->prog, clp->vers, timeout,
> 					&sockp, sendsz, recvsz)) == NULL) {
> 			snprintf(rpc_errmsg, sizeof(rpc_errmsg),
> @@ -701,16 +742,7 @@ int create_auth_rpc_client(struct clnt_info *clp,
> 				 clnt_spcreateerror(rpc_errmsg));
> 			goto out_fail;
> 		}
> -	} else {
> -		/* Shouldn't happen! */
> -		printerr(0, "ERROR: requested protocol '%s', but "
> -			 "got addrinfo with protocol %d\n",
> -			 clp->protocol, a->ai_protocol);
> -		goto out_fail;
> 	}
> -	/* We're done with this */
> -	freeaddrinfo(a);
> -	a = NULL;
>
> 	printerr(2, "creating context with server %s\n", clp->servicename);
> 	auth = authgss_create_default(rpc_clnt, clp->servicename, &sec);
> @@ -732,7 +764,6 @@ int create_auth_rpc_client(struct clnt_info *clp,
>   out:
> 	if (sec.cred != GSS_C_NO_CREDENTIAL)
> 		gss_release_cred(&min_stat, &sec.cred);
> -  	if (a != NULL) freeaddrinfo(a);
> 	/* Restore euid to original value */
> 	if ((save_uid != -1) && (setfsuid(save_uid) != uid)) {
> 		printerr(0, "WARNING: Failed to restore fsuid"
> -- 
> 1.6.0.6
>

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

Re: [PATCH 3/6] nfs-utils: skip getaddrinfo in create_auth_rpc_client unless we need port

[Jeff Layton]

On Wed, 11 Mar 2009 18:09:11 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:

> > +
> > +	switch (ss->ss_family) {
> > +	case AF_INET:
> > +		if (s4->sin_port != 0) {
> > +			printerr(2, "DEBUG: port already set to %d\n",
> > +				 ntohs(s4->sin_port));
> > +			return 1;
> > +		}
> > +		if (!inet_ntop(AF_INET, &s4->sin_addr, node,
> > +				sizeof(struct sockaddr_in)))
> > +			return 0;  
> 
> I know you are copying this mostly verbatim, but I'm not sure exactly  
> what this is doing.  You can pass a C string containing the port  
> number, or a C string containing the service name, to getaddrinfo(3)  
> and have it fill in the port, I think.

This part wasn't copied...

What I'm trying to do here is skip doing a getaddrinfo if the port
number was already sent in the upcall. There appear to be 2 versions of
the info sent by the kernel in the upcall. The older one did not have a
field for the port number. For older kernels that don't have this
field, we need to look it up with getaddrinfo.

But...there's really no need to do a getaddrinfo call if we already have
a sockaddr that's ready to go, and I think that'll be the common case.

Come to think of it, we could just do this for IPv4 since we know that
all IPv6-enabled kernels will send the port. I'll have to think about
that some more...

Anyway, thanks for the review so far. I'll work on incorporating your
suggestions (particularly the ones about sockaddr vs. sockaddr_storage)
and will respin the set and resend after some more testing.

Cheers,
-- 
Jeff Layton <jlayton@redhat.com>

[PATCH 1/6] nfs-utils: make getnameinfo() required for --enable-gss

[Jeff Layton]

Systems that are so old that they don't have getnameinfo() in glibc are
probably also running kernels that are so old that they don't support
gssapi upcalls anyway.

Make --enable-gss dependent on the presence of the getnameinfo()
function.  This allows us to reduce some conditional compilation.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 configure.ac |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/configure.ac b/configure.ac
index e34b7e2..00aee35 100644
--- a/configure.ac
+++ b/configure.ac
@@ -207,6 +207,8 @@ if test "$enable_nfsv4" = yes; then
   dnl but we need to make sure we get the right version
   if test "$enable_gss" = yes; then
     AC_RPCSEC_VERSION
+    AC_CHECK_FUNC([getnameinfo], ,
+		  [AC_MSG_ERROR([Function 'getnameinfo' not found.])])
   fi
 fi
 
-- 
1.6.0.6