[PATCH] Alternative patchset to avoid PTR lookups (take 2)

[PATCH] Alternative patchset to avoid PTR lookups (take 2)

[Steve Dickson]

Here is how I think we should handle the avoiding of
PTR lookups. Logically its the same as Simo's original 
patch but does change the following:

* Not doing the DNS lookups is now the default behavior
* the -D options can be used to restore the old behavior
* I changes some of the variable names to, hopefully, be
  more obvious as to what the code is doing.

Simo Sorce (1):
  Avoid DNS reverse resolution for server names

 utils/gssd/gss_util.h  |  2 ++
 utils/gssd/gssd.c      |  7 +++++--
 utils/gssd/gssd.man    |  8 +++++++-
 utils/gssd/gssd_proc.c | 29 +++++++++++++++++++++++++----
 4 files changed, 39 insertions(+), 7 deletions(-)

-- 
1.8.1.4

[PATCH] Avoid DNS reverse resolution for server names

[Steve Dickson]

From: Simo Sorce <simo@redhat.com>

A NFS client should be able to work properly even if the DNS Reverse
record for the server is not set. This means a DNS lookup should not be done on
server names at are passed to GSSAPI. This patch changes the default behavior
to no longer do those types of lookups

This change default behavior could negatively impact some current environments,
so the -D option is also being added that will re-enable the DNS reverse
looks on server names, which are passed to GSSAPI.

Signed-off-by: Steve Dickson <steved@redhat.com>
---
 utils/gssd/gss_util.h  |  2 ++
 utils/gssd/gssd.c      |  7 +++++--
 utils/gssd/gssd.man    |  8 +++++++-
 utils/gssd/gssd_proc.c | 29 +++++++++++++++++++++++++----
 4 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/utils/gssd/gss_util.h b/utils/gssd/gss_util.h
index aa9f778..c81fc5a 100644
--- a/utils/gssd/gss_util.h
+++ b/utils/gssd/gss_util.h
@@ -52,4 +52,6 @@ int gssd_check_mechs(void);
 		gss_krb5_set_allowable_enctypes(min, cred, num, types)
 #endif
 
+extern int avoid_dns;
+
 #endif /* _GSS_UTIL_H_ */
diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
index 07b1e52..8ee478b 100644
--- a/utils/gssd/gssd.c
+++ b/utils/gssd/gssd.c
@@ -85,7 +85,7 @@ sig_hup(int signal)
 static void
 usage(char *progname)
 {
-	fprintf(stderr, "usage: %s [-f] [-l] [-M] [-n] [-v] [-r] [-p pipefsdir] [-k keytab] [-d ccachedir] [-t timeout] [-R preferred realm]\n",
+	fprintf(stderr, "usage: %s [-f] [-l] [-M] [-n] [-v] [-r] [-p pipefsdir] [-k keytab] [-d ccachedir] [-t timeout] [-R preferred realm] [-D]\n",
 		progname);
 	exit(1);
 }
@@ -102,7 +102,7 @@ main(int argc, char *argv[])
 	char *progname;
 
 	memset(ccachesearch, 0, sizeof(ccachesearch));
-	while ((opt = getopt(argc, argv, "fvrlmnMp:k:d:t:R:")) != -1) {
+	while ((opt = getopt(argc, argv, "DfvrlmnMp:k:d:t:R:")) != -1) {
 		switch (opt) {
 			case 'f':
 				fg = 1;
@@ -150,6 +150,9 @@ main(int argc, char *argv[])
 				errx(1, "Encryption type limits not supported by Kerberos libraries.");
 #endif
 				break;
+			case 'D':
+				avoid_dns = 0;
+				break;
 			default:
 				usage(argv[0]);
 				break;
diff --git a/utils/gssd/gssd.man b/utils/gssd/gssd.man
index 79d9bf9..1df75c5 100644
--- a/utils/gssd/gssd.man
+++ b/utils/gssd/gssd.man
@@ -8,7 +8,7 @@
 rpc.gssd \- RPCSEC_GSS daemon
 .SH SYNOPSIS
 .B rpc.gssd
-.RB [ \-fMnlvr ]
+.RB [ \-DfMnlvr ]
 .RB [ \-k
 .IR keytab ]
 .RB [ \-p
@@ -195,6 +195,12 @@ option when starting
 .BR rpc.gssd .
 .SH OPTIONS
 .TP
+.B -D
+DNS Reverse lookups are not used for determining the
+server names pass to GSSAPI. This option will reverses that and forces 
+the use of DNS Reverse resolution of the server's IP address to 
+retrieve the server name to use in GSAPI authentication.
+.TP
 .B -f
 Runs
 .B rpc.gssd
diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index d6f07e6..aef5ca5 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -67,6 +67,7 @@
 #include <errno.h>
 #include <gssapi/gssapi.h>
 #include <netdb.h>
+#include <ctype.h>
 
 #include "gssd.h"
 #include "err_util.h"
@@ -107,6 +108,9 @@ struct pollfd * pollarray;
 
 unsigned long pollsize;  /* the size of pollaray (in pollfd's) */
 
+/* Avoid DNS reverse lookups on server names */
+int avoid_dns = 1;
+
 /*
  * convert a presentation address string to a sockaddr_storage struct. Returns
  * true on success or false on failure.
@@ -165,12 +169,29 @@ addrstr_to_sockaddr(struct sockaddr *sa, const char *node, const char *port)
  * convert a sockaddr to a hostname
  */
 static char *
-sockaddr_to_hostname(const struct sockaddr *sa, const char *addr)
+get_servername(const char *name, const struct sockaddr *sa, const char *addr)
 {
 	socklen_t		addrlen;
 	int			err;
 	char			*hostname;
 	char			hbuf[NI_MAXHOST];
+	unsigned char		buf[sizeof(struct in6_addr)];
+	int			do_dns_lookup = 0;
+
+	if (avoid_dns) {
+		/*
+		 * Determine if this is a server name, or an IP address.
+		 * If it is an IP address, do the DNS lookup
+		 */
+		if (strchr(name, '.') && inet_pton(AF_INET, name, buf) == 1)
+			do_dns_lookup = 1; /* IPv4 */
+		else if (strchr(name, ':') && inet_pton(AF_INET6, name, buf) == 1)
+			do_dns_lookup = 1; /* or IPv6 */
+
+		if (!do_dns_lookup) {
+			return strdup(name);
+		}
+	}
 
 	switch (sa->sa_family) {
 	case AF_INET:
@@ -208,7 +229,7 @@ read_service_info(char *info_file_name, char **servicename, char **servername,
 		  struct sockaddr *addr) {
 #define INFOBUFLEN 256
 	char		buf[INFOBUFLEN + 1];
-	static char	dummy[128];
+	static char	server[128];
 	int		nbytes;
 	static char	service[128];
 	static char	address[128];
@@ -236,7 +257,7 @@ read_service_info(char *info_file_name, char **servicename, char **servername,
 		   "service: %127s %15s version %15s\n"
 		   "address: %127s\n"
 		   "protocol: %15s\n",
-		   dummy,
+		   server,
 		   service, program, version,
 		   address,
 		   protoname);
@@ -258,7 +279,7 @@ read_service_info(char *info_file_name, char **servicename, char **servername,
 	if (!addrstr_to_sockaddr(addr, address, port))
 		goto fail;
 
-	*servername = sockaddr_to_hostname(addr, address);
+	*servername = get_servername(server, addr, address);
 	if (*servername == NULL)
 		goto fail;
 
-- 
1.8.1.4

Re: [PATCH] Avoid DNS reverse resolution for server names

[Jim Rees]

I find this very confusing. You've got a global named "avoid_dns", which you
turn off if you have the "-D" option. And if it's set, you do a dns lookup:

  +	if (avoid_dns) {
...
  +			do_dns_lookup = 1; /* IPv4 */

At the very least I would call it something like "do_ptr_lookup" and invert
its meaning.