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.
This commit is contained in:
rofl0r 2018-12-31 15:47:40 +00:00 committed by rofl0r
parent 82e10935d2
commit f6d4da5d81
11 changed files with 40 additions and 51 deletions

View File

@ -196,6 +196,9 @@ The possible keywords and their descriptions are as follows:
end of the client host name, i.e, this can be a full host name
like `host.example.com` or a domain name like `.example.com` or
even a top level domain name like `.com`.
Note that by adding a rule using a host or domain name, a costly name
lookup has to be done for every new connection, which could slow down
the service considerably.
*AddHeader*::

View File

@ -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;
}

View File

@ -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);

View File

@ -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;
}

View File

@ -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)

View File

@ -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);

View File

@ -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

View File

@ -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",

View File

@ -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

View File

@ -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);
}

View File

@ -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