do hostname resolution only when it is absolutely necessary for ACL check

tinyproxy used to do a full hostname resolution whenever a new client
connection happened, which could cause very long delays (as reported in #198).

there's only a single place/scenario that actually requires a hostname, and
that is when an Allow/Deny rule exists for a hostname or domain, rather than
a raw IP address. since it is very likely this feature is not very widely used,
it makes absolute sense to only do the costly resolution when it is unavoidable.

10 files changed, 37 insertions, 51 deletions

diff --git a/src/acl.c b/src/acl.c
index b7a334c..7918a07 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -221,8 +221,8 @@ insert_acl (char *location, acl_access_t access_type, vector_t *access_list)
  *        -1 if no tests match, so skip
  */
 static int
-acl_string_processing (struct acl_s *acl,
-                       const char *ip_address, const char *string_address)
+acl_string_processing (struct acl_s *acl, const char *ip_address,
+                       union sockaddr_union *addr, char *string_addr)
 {
         int match;
         struct addrinfo hints, *res, *ressave;
@@ -231,7 +231,6 @@ acl_string_processing (struct acl_s *acl,
 
         assert (acl && acl->type == ACL_STRING);
         assert (ip_address && strlen (ip_address) > 0);
-        assert (string_address && strlen (string_address) > 0);
 
         /*
          * If the first character of the ACL string is a period, we need to
@@ -267,7 +266,15 @@ acl_string_processing (struct acl_s *acl,
         }
 
 STRING_TEST:
-        test_length = strlen (string_address);
+        if(string_addr[0] == 0) {
+                /* only do costly hostname resolution when it is absolutely needed,
+                   and only once */
+                if(getnameinfo ((void *) addr, sizeof (*addr),
+                                string_addr, HOSTNAME_LENGTH, NULL, 0, 0) != 0)
+                        return -1;
+        }
+
+        test_length = strlen (string_addr);
         match_length = strlen (acl->address.string);
 
         /*
@@ -278,7 +285,7 @@ STRING_TEST:
                 return -1;
 
         if (strcasecmp
-            (string_address + (test_length - match_length),
+            (string_addr + (test_length - match_length),
              acl->address.string) == 0) {
                 if (acl->access == ACL_DENY)
                         return 0;
@@ -329,15 +336,18 @@ static int check_numeric_acl (const struct acl_s *acl, const char *ip)
  *     1 if allowed
  *     0 if denied
  */
-int check_acl (const char *ip, const char *host, vector_t access_list)
+int check_acl (const char *ip, union sockaddr_union *addr, vector_t access_list)
 {
         struct acl_s *acl;
         int perm = 0;
         size_t i;
+        char string_addr[HOSTNAME_LENGTH];
 
         assert (ip != NULL);
         assert (host != NULL);
 
+        string_addr[0] = 0;
+
         /*
          * If there is no access list allow everything.
          */
@@ -348,7 +358,7 @@ int check_acl (const char *ip, const char *host, vector_t access_list)
                 acl = (struct acl_s *) vector_getentry (access_list, i, NULL);
                 switch (acl->type) {
                 case ACL_STRING:
-                        perm = acl_string_processing (acl, ip, host);
+                        perm = acl_string_processing (acl, ip, addr, string_addr);
                         break;
 
                 case ACL_NUMERIC:
@@ -371,8 +381,8 @@ int check_acl (const char *ip, const char *host, vector_t access_list)
         /*
          * Deny all connections by default.
          */
-        log_message (LOG_NOTICE, "Unauthorized connection from \"%s\" [%s].",
-                     host, ip);
+        log_message (LOG_NOTICE, "Unauthorized connection from \"%s\".",
+                     ip);
         return 0;
 }
 
diff --git a/src/acl.h b/src/acl.h
index 2d11cef..ba0aebe 100644
--- a/src/acl.h
+++ b/src/acl.h
@@ -22,12 +22,13 @@
 #define TINYPROXY_ACL_H
 
 #include "vector.h"
+#include "sock.h"
 
 typedef enum { ACL_ALLOW, ACL_DENY } acl_access_t;
 
 extern int insert_acl (char *location, acl_access_t access_type,
                        vector_t *access_list);
-extern int check_acl (const char *ip_address, const char *string_address,
+extern int check_acl (const char *ip_address, union sockaddr_union *addr,
                       vector_t access_list);
 extern void flush_access_list (vector_t access_list);
 
diff --git a/src/child.c b/src/child.c
index 3ae3d82..8bd713d 100644
--- a/src/child.c
+++ b/src/child.c
@@ -50,7 +50,7 @@ struct child {
 static void* child_thread(void* data)
 {
 	struct child *c = data;
-	handle_connection (c->client.fd);
+	handle_connection (c->client.fd, &c->client.addr);
 	c->done = 1;
 	return NULL;
 }
diff --git a/src/conns.c b/src/conns.c
index 94faeea..505b5c4 100644
--- a/src/conns.c
+++ b/src/conns.c
@@ -31,7 +31,6 @@
 #include "stats.h"
 
 struct conn_s *initialize_conn (int client_fd, const char *ipaddr,
-                                const char *string_addr,
                                 const char *sock_ipaddr)
 {
         struct conn_s *connptr;
@@ -79,7 +78,6 @@ struct conn_s *initialize_conn (int client_fd, const char *ipaddr,
         connptr->server_ip_addr = (sock_ipaddr ?
                                    safestrdup (sock_ipaddr) : NULL);
         connptr->client_ip_addr = safestrdup (ipaddr);
-        connptr->client_string_addr = safestrdup (string_addr);
 
         connptr->upstream_proxy = NULL;
 
@@ -134,8 +132,6 @@ void destroy_conn (struct conn_s *connptr)
                 safefree (connptr->server_ip_addr);
         if (connptr->client_ip_addr)
                 safefree (connptr->client_ip_addr);
-        if (connptr->client_string_addr)
-                safefree (connptr->client_string_addr);
 
 #ifdef REVERSE_SUPPORT
         if (connptr->reversepath)
diff --git a/src/conns.h b/src/conns.h
index b63d026..393e5d4 100644
--- a/src/conns.h
+++ b/src/conns.h
@@ -62,10 +62,9 @@ struct conn_s {
         char *server_ip_addr;
 
         /*
-         * Store the client's IP and hostname information
+         * Store the client's IP information
          */
         char *client_ip_addr;
-        char *client_string_addr;
 
         /*
          * Store the incoming request's HTTP protocol.
@@ -92,7 +91,6 @@ struct conn_s {
  * Functions for the creation and destruction of a connection structure.
  */
 extern struct conn_s *initialize_conn (int client_fd, const char *ipaddr,
-                                       const char *string_addr,
                                        const char *sock_ipaddr);
 extern void destroy_conn (struct conn_s *connptr);
 
diff --git a/src/html-error.c b/src/html-error.c
index ee3c987..2b15c08 100644
--- a/src/html-error.c
+++ b/src/html-error.c
@@ -262,7 +262,6 @@ int add_standard_vars (struct conn_s *connptr)
         ADD_VAR_RET ("cause", connptr->error_string);
         ADD_VAR_RET ("request", connptr->request_line);
         ADD_VAR_RET ("clientip", connptr->client_ip_addr);
-        ADD_VAR_RET ("clienthost", connptr->client_string_addr);
 
         /* The following value parts are all non-NULL and will
          * trigger warnings in ADD_VAR_RET(), so we use
diff --git a/src/reqs.c b/src/reqs.c
index 8450cff..c576412 100644
--- a/src/reqs.c
+++ b/src/reqs.c
@@ -1533,7 +1533,7 @@ get_request_entity(struct conn_s *connptr)
  * tinyproxy code, which was confusing, redundant. Hail progress.
  * 	- rjkaes
  */
-void handle_connection (int fd)
+void handle_connection (int fd, union sockaddr_union* addr)
 {
         ssize_t i;
         struct conn_s *connptr;
@@ -1542,26 +1542,25 @@ void handle_connection (int fd)
 
         char sock_ipaddr[IP_LENGTH];
         char peer_ipaddr[IP_LENGTH];
-        char peer_string[HOSTNAME_LENGTH];
 
-        getpeer_information (fd, peer_ipaddr, peer_string);
+        getpeer_information (addr, peer_ipaddr, sizeof(peer_ipaddr));
 
         if (config.bindsame)
                 getsock_ip (fd, sock_ipaddr);
 
         log_message (LOG_CONN, config.bindsame ?
-                     "Connect (file descriptor %d): %s [%s] at [%s]" :
-                     "Connect (file descriptor %d): %s [%s]",
-                     fd, peer_string, peer_ipaddr, sock_ipaddr);
+                     "Connect (file descriptor %d): %s at [%s]" :
+                     "Connect (file descriptor %d): %s",
+                     fd, peer_ipaddr, sock_ipaddr);
 
-        connptr = initialize_conn (fd, peer_ipaddr, peer_string,
+        connptr = initialize_conn (fd, peer_ipaddr,
                                    config.bindsame ? sock_ipaddr : NULL);
         if (!connptr) {
                 close (fd);
                 return;
         }
 
-        if (check_acl (peer_ipaddr, peer_string, config.access_list) <= 0) {
+        if (check_acl (peer_ipaddr, addr, config.access_list) <= 0) {
                 update_stats (STAT_DENIED);
                 indicate_http_error (connptr, 403, "Access denied",
                                      "detail",
diff --git a/src/reqs.h b/src/reqs.h
index 73dd030..c1c5100 100644
--- a/src/reqs.h
+++ b/src/reqs.h
@@ -23,6 +23,7 @@
 #define _TINYPROXY_REQS_H_
 
 #include "common.h"
+#include "sock.h"
 
 /*
  * Port constants for HTTP (80) and SSL (443)
@@ -43,6 +44,6 @@ struct request_s {
         char *path;
 };
 
-extern void handle_connection (int fd);
+extern void handle_connection (int fd, union sockaddr_union* addr);
 
 #endif
diff --git a/src/sock.c b/src/sock.c
index 59c2fa8..f74a588 100644
--- a/src/sock.c
+++ b/src/sock.c
@@ -354,27 +354,9 @@ int getsock_ip (int fd, char *ipaddr)
 /*
  * Return the peer's socket information.
  */
-int getpeer_information (int fd, char *ipaddr, char *string_addr)
+void getpeer_information (union sockaddr_union* addr, char *ipaddr, size_t ipaddr_len)
 {
-        struct sockaddr_storage sa;
-        socklen_t salen = sizeof sa;
-
-        assert (fd >= 0);
-        assert (ipaddr != NULL);
-        assert (string_addr != NULL);
-
-        /* Set the strings to default values */
-        ipaddr[0] = '\0';
-        strlcpy (string_addr, "[unknown]", HOSTNAME_LENGTH);
-
-        /* Look up the IP address */
-        if (getpeername (fd, (struct sockaddr *) &sa, &salen) != 0)
-                return -1;
-
-        if (get_ip_string ((struct sockaddr *) &sa, ipaddr, IP_LENGTH) == NULL)
-                return -1;
-
-        /* Get the full host name */
-        return getnameinfo ((struct sockaddr *) &sa, salen,
-                            string_addr, HOSTNAME_LENGTH, NULL, 0, 0);
+        int af = addr->v4.sin_family;
+        void *ipdata = af == AF_INET ? (void*)&addr->v4.sin_addr : (void*)&addr->v6.sin6_addr;
+        inet_ntop(af, ipdata, ipaddr, ipaddr_len);
 }
diff --git a/src/sock.h b/src/sock.h
index a516d4f..033e179 100644
--- a/src/sock.h
+++ b/src/sock.h
@@ -43,6 +43,6 @@ extern int socket_nonblocking (int sock);
 extern int socket_blocking (int sock);
 
 extern int getsock_ip (int fd, char *ipaddr);
-extern int getpeer_information (int fd, char *ipaddr, char *string_addr);
+extern void getpeer_information (union sockaddr_union *addr, char *ipaddr, size_t ipaddr_len);
 
 #endif