-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 02/19/2010 08:23 PM, Yang Tse wrote: > More nits... >
Sorry for the late response, I've been quite busy lately.. > Not all systems have in6_addr, that's the reason for having > ares_in6_addr in ares.h, so it would be convenient to move ares_addr > definition further down in ares.h and use ares_in6_addr in the > definition of the ares_addr struct. ares_set_nameservers() would also > need to be moved further down in taht header file. > Hmm, fair enough. But I noticed that there are many places in c-ares where in6_addr is used unconditionally and also there is a definition of struct in6addr in ares_ipv6.h. Anyhow, attached is a second patch that uses ares_in6_addr at least in the code I touch with the first patch. > If you expose definitions of addrV4 and addrV6 in ares.h these should > be 'ares' or cares' prefixed. Better just keep them in the private > part of the library and avoid the need to rename them. > OK, good suggestion. > It seems that you are using getaddrinfo() to validate an address > string, and that it makes no sense to feed it with a host name. Not host name, I used the AI_NUMERICHOST flag. Per man getaddrinfo(3), that flag denotes that we are passing a numeric address to getaddrinfo and suppresses any network lookups. > Why > not use inet_addr() in a fisrt instance, this handles short hand IPv4 > dot notation, and if it fails then use ares_inet_pton() which should > be capable of also handling IPv6 afterwards, getting rid of the call > to getaddrinfo()? Or am I overlooking something here? > It is more or less just a matter of style. I generally dislike having if (IPv4) { do_stuff(); } else if (IPv6) { do_stuff6(); }. Using getaddrinfo there allowed for a protocol-agnostic code. The attached version of the patch uses inet_addr() and ares_inet_pton(), though. > There also seems to be some 'strange' memory management at the > beginning of set_nameserver() > Correct. Sloppy refactoring. Should be fixed. Thank you for the review! Hopefully, we're getting there :-) -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkuGtGwACgkQHsardTLnvCWQKwCcCExH9FGwxmKfazTOhz5gupW0 FesAn2HfTceuCNRechLpwNvEEZo2G7Ev =gFZq -----END PGP SIGNATURE-----
From 1f1893a428376810f2c2887cf51ca7891917b216 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Mon, 1 Feb 2010 21:23:59 +0100 Subject: [PATCH 1/2] Allow the use of IPv6 nameservers This patch allows the use of IPv6 addresses for nameserves in both /etc/resolv.conf and by using the ares_set_nameservers() API. --- Makefile.inc | 3 + ares.h | 13 +++ ares_destroy.c | 10 +-- ares_init.c | 208 ++++++++++++++++++++++++++++++++++++++--------- ares_private.h | 13 +-- ares_process.c | 95 +++++++++++++++++----- ares_set_nameservers.3 | 58 +++++++++++++ 7 files changed, 320 insertions(+), 80 deletions(-) create mode 100644 ares_set_nameservers.3 diff --git a/Makefile.inc b/Makefile.inc index 3227858..8e575fd 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -92,6 +92,7 @@ MANPAGES = ares_cancel.3 \ ares_search.3 \ ares_send.3 \ ares_set_socket_callback.3 \ + ares_set_nameservers.3 \ ares_strerror.3 \ ares_timeout.3 \ ares_version.3 @@ -128,6 +129,7 @@ HTMLPAGES = ares_cancel.html \ ares_search.html \ ares_send.html \ ares_set_socket_callback.html \ + ares_set_nameservers.html \ ares_strerror.html \ ares_timeout.html \ ares_version.html @@ -164,6 +166,7 @@ PDFPAGES = ares_cancel.pdf \ ares_search.pdf \ ares_send.pdf \ ares_set_socket_callback.pdf \ + ares_set_nameservers.pdf \ ares_strerror.pdf \ ares_timeout.pdf \ ares_version.pdf diff --git a/ares.h b/ares.h index b1c2c22..57bd445 100644 --- a/ares.h +++ b/ares.h @@ -426,6 +426,19 @@ struct ares_addr6ttl { int ttl; }; +struct ares_addr { + int family; + union { + struct in_addr addr4; + struct in6_addr addr6; + } addr; +}; + +/* Sets nameservers for the given ares channel handle */ +CARES_EXTERN int ares_set_nameservers(ares_channel channel, + struct ares_addr *servers, + int num_servers); + struct ares_srv_reply { struct ares_srv_reply *next; char *host; diff --git a/ares_destroy.c b/ares_destroy.c index 0044a71..4040971 100644 --- a/ares_destroy.c +++ b/ares_destroy.c @@ -67,15 +67,7 @@ void ares_destroy(ares_channel channel) } #endif - if (channel->servers) { - for (i = 0; i < channel->nservers; i++) - { - struct server_state *server = &channel->servers[i]; - ares__close_sockets(channel, server); - assert(ares__is_list_empty(&(server->queries_to_server))); - } - free(channel->servers); - } + ares__free_servers(channel); if (channel->domains) { for (i = 0; i < channel->ndomains; i++) diff --git a/ares_init.c b/ares_init.c index cb541af..98e9202 100644 --- a/ares_init.c +++ b/ares_init.c @@ -65,6 +65,7 @@ #include <ctype.h> #include <time.h> #include <errno.h> +#include <assert.h> #include "ares.h" #include "inet_net_pton.h" #include "ares_library_init.h" @@ -88,6 +89,7 @@ static int set_search(ares_channel channel, const char *str); static int set_options(ares_channel channel, const char *str); static const char *try_option(const char *p, const char *q, const char *opt); static int init_id_key(rc4_key* key,int key_data_len); +static void init_servers(ares_channel channel); #if !defined(WIN32) && !defined(WATT32) static int sortlist_alloc(struct apattern **sortlist, int *nsort, struct apattern *pat); @@ -118,7 +120,6 @@ int ares_init_options(ares_channel *channelptr, struct ares_options *options, ares_channel channel; int i; int status = ARES_SUCCESS; - struct server_state *server; struct timeval now; #ifdef CURLDEBUG @@ -248,20 +249,7 @@ int ares_init_options(ares_channel *channelptr, struct ares_options *options, channel->nservers = 1; /* Initialize server states. */ - for (i = 0; i < channel->nservers; i++) - { - server = &channel->servers[i]; - server->udp_socket = ARES_SOCKET_BAD; - server->tcp_socket = ARES_SOCKET_BAD; - server->tcp_connection_generation = ++channel->tcp_connection_generation; - server->tcp_lenbuf_pos = 0; - server->tcp_buffer = NULL; - server->qhead = NULL; - server->qtail = NULL; - ares__init_list_head(&(server->queries_to_server)); - server->channel = channel; - server->is_broken = 0; - } + init_servers(channel); *channelptr = channel; return ARES_SUCCESS; @@ -296,6 +284,29 @@ int ares_dup(ares_channel *dest, ares_channel src) (*dest)->sock_create_cb = src->sock_create_cb; (*dest)->sock_create_cb_data = src->sock_create_cb_data; + /* IPv4 nameservers were copied in ares_save_options, we + * can only copy v6 servers */ + if (src->nservers > (*dest)->nservers) + { + int i; + (*dest)->servers = + realloc((*dest)->servers, + src->nservers * sizeof(struct server_state)); + if (!(*dest)->servers) + return ARES_ENOMEM; + + for (i = 0; i < src->nservers; i++) + { + if (src->servers[i].addr.family == AF_INET6) + { + memcpy(&((*dest)->servers[(*dest)->nservers].addr), + &(src->servers[i].addr), + sizeof(struct ares_addr)); + (*dest)->nservers++; + } + } + init_servers(*dest); + } return ARES_SUCCESS; /* everything went fine */ @@ -334,17 +345,33 @@ int ares_save_options(ares_channel channel, struct ares_options *options, options->tcp_port = (unsigned short)channel->tcp_port; options->sock_state_cb = channel->sock_state_cb; options->sock_state_cb_data = channel->sock_state_cb_data; + options->nservers = 0; /* Copy servers */ if (channel->nservers) { + int numv4 = 0; options->servers = malloc(channel->nservers * sizeof(struct server_state)); - if (!options->servers && channel->nservers != 0) + if (!options->servers) return ARES_ENOMEM; for (i = 0; i < channel->nservers; i++) - options->servers[i] = channel->servers[i].addr; + { + if (channel->servers[i].addr.family == AF_INET) + { + /* Because struct ares_options should stay the same + * only IPv4 nameservers are saved in this patch. + * ares_dup() works for both v4 and v6, though + */ + options->servers[numv4++] = + channel->servers[i].addr.addrV4; + } + } + options->servers = + realloc(options->servers, numv4 * sizeof(struct server_state)); + if (numv4 && !options->servers) + return ARES_ENOMEM; + options->nservers = numv4; } - options->nservers = channel->nservers; /* copy domains */ if (channel->ndomains) { @@ -431,7 +458,15 @@ static int init_by_options(ares_channel channel, if (!channel->servers) return ARES_ENOMEM; for (i = 0; i < options->nservers; i++) - channel->servers[i].addr = options->servers[i]; + { + /* This is a rather crude way to allow usage of + * protocol-independent struct addrinfo while not + * breaking the public struct ares_options which is forbidden + * in favor of providing ares_set_XXX() functions + */ + channel->servers[i].addr.family = AF_INET; + channel->servers[i].addr.addrV4 = options->servers[i]; + } } channel->nservers = options->nservers; } @@ -1001,7 +1036,9 @@ static int init_by_defaults(ares_channel channel) rc = ARES_ENOMEM; goto error; } - channel->servers[0].addr.s_addr = htonl(INADDR_LOOPBACK); + + channel->servers[0].addr.family = AF_INET; + channel->servers[0].addr.addrV4.s_addr = htonl(INADDR_LOOPBACK); channel->nservers = 1; } @@ -1146,11 +1183,48 @@ static int config_lookup(ares_channel channel, const char *str, #endif /* !WIN32 & !WATT32 */ #ifndef WATT32 +static int set_nameserver(struct server_state **servers, int *nservers, + char *str) +{ + struct server_state *newserv; + struct in_addr addr; + struct in6_addr addr6; + + newserv = realloc(*servers, (*nservers + 1) * sizeof(struct server_state)); + if (!newserv) + { + return ARES_ENOMEM; + } + + addr.s_addr = inet_addr(str); + if (addr.s_addr == INADDR_NONE) + { + /* Not an IPv4 address, try IPv6 */ + if(ares_inet_pton(AF_INET6, str, (struct in6_addr *) &addr6) == 0) + { + /* Not IPv6 either, bail out */ + return ARES_EBADFAMILY; + } + + newserv[*nservers].addr.family = AF_INET6; + memcpy(&newserv[*nservers].addr.addrV6, &addr6, 16); + } + else + { + newserv[*nservers].addr.family = AF_INET; + newserv[*nservers].addr.addrV4 = addr; + } + + *servers = newserv; + (*nservers)++; + return ARES_SUCCESS; +} + static int config_nameserver(struct server_state **servers, int *nservers, char *str) { - struct in_addr addr; - struct server_state *newserv; + int ret; + /* On Windows, there may be more than one nameserver specified in the same * registry key, so we parse it as a space or comma seperated list. */ @@ -1178,15 +1252,9 @@ static int config_nameserver(struct server_state **servers, int *nservers, } /* This is the part that actually sets the nameserver */ - addr.s_addr = inet_addr(begin); - if (addr.s_addr == INADDR_NONE) - continue; - newserv = realloc(*servers, (*nservers + 1) * sizeof(struct server_state)); - if (!newserv) - return ARES_ENOMEM; - newserv[*nservers].addr = addr; - *servers = newserv; - (*nservers)++; + ret = set_nameserver(servers, nservers, begin); + if (ret) + return ret; if (!more) break; @@ -1194,15 +1262,9 @@ static int config_nameserver(struct server_state **servers, int *nservers, } #else /* Add a nameserver entry, if this is a valid address. */ - addr.s_addr = inet_addr(str); - if (addr.s_addr == INADDR_NONE) - return ARES_SUCCESS; - newserv = realloc(*servers, (*nservers + 1) * sizeof(struct server_state)); - if (!newserv) - return ARES_ENOMEM; - newserv[*nservers].addr = addr; - *servers = newserv; - (*nservers)++; + ret = set_nameserver(servers, nservers, str); + if (ret) + return ret; #endif return ARES_SUCCESS; } @@ -1580,3 +1642,69 @@ void ares_set_socket_callback(ares_channel channel, channel->sock_create_cb = cb; channel->sock_create_cb_data = data; } + +static void init_servers(ares_channel channel) +{ + struct server_state *server; + int i; + + for (i = 0; i < channel->nservers; i++) + { + server = &channel->servers[i]; + server->udp_socket = ARES_SOCKET_BAD; + server->tcp_socket = ARES_SOCKET_BAD; + server->tcp_connection_generation = ++channel->tcp_connection_generation; + server->tcp_lenbuf_pos = 0; + server->tcp_buffer = NULL; + server->qhead = NULL; + server->qtail = NULL; + ares__init_list_head(&(server->queries_to_server)); + server->channel = channel; + server->is_broken = 0; + } +} + +void ares__free_servers(ares_channel channel) +{ + int i; + + if (channel->servers) { + for (i = 0; i < channel->nservers; i++) + { + struct server_state *server = &channel->servers[i]; + ares__close_sockets(channel, server); + assert(ares__is_list_empty(&(server->queries_to_server))); + } + free(channel->servers); + } + + channel->servers = NULL; + channel->nservers = -1; +} + +int ares_set_nameservers(ares_channel channel, + struct ares_addr *servers, + int num_servers) +{ + int i; + + ares__free_servers(channel); + + channel->nservers = num_servers; + channel->servers = + malloc(channel->nservers * sizeof(struct server_state)); + if (!channel->servers) + return ARES_ENOMEM; + + for (i = 0; i < num_servers; i++) + { + struct server_state *ss = channel->servers + i; + memset(ss, 0, sizeof(struct server_state)); + + ss->channel = channel; + memcpy(&ss->addr, &servers[i], sizeof(struct ares_addr)); + } + + init_servers(channel); + return ARES_SUCCESS; +} diff --git a/ares_private.h b/ares_private.h index 4726d7a..286bf90 100644 --- a/ares_private.h +++ b/ares_private.h @@ -110,13 +110,7 @@ # define writev(s,ptr,cnt) ares_writev(s,ptr,cnt) #endif -struct ares_addr { - int family; - union { - struct in_addr addr4; - struct in6_addr addr6; - } addr; -}; +/* Shortcuts used for accessing the union addr inside struct ares_addr */ #define addrV4 addr.addr4 #define addrV6 addr.addr6 @@ -137,7 +131,8 @@ struct send_request { }; struct server_state { - struct in_addr addr; + struct ares_addr addr; + ares_socket_t udp_socket; ares_socket_t tcp_socket; @@ -319,6 +314,8 @@ struct timeval ares__tvnow(void); int ares__expand_name_for_response(const unsigned char *encoded, const unsigned char *abuf, int alen, char **s, long *enclen); +void ares__free_servers(ares_channel channel); + #if 0 /* Not used */ long ares__tvdiff(struct timeval t1, struct timeval t2); #endif diff --git a/ares_process.c b/ares_process.c index 71f9394..65b95f4 100644 --- a/ares_process.c +++ b/ares_process.c @@ -434,7 +434,7 @@ static void read_udp_packets(ares_channel channel, fd_set *read_fds, ssize_t count; unsigned char buf[PACKETSZ + 1]; #ifdef HAVE_RECVFROM - struct sockaddr_in from; + struct sockaddr_storage from; ares_socklen_t fromlen; #endif @@ -480,16 +480,29 @@ static void read_udp_packets(ares_channel channel, fd_set *read_fds, if (count == -1 && try_again(SOCKERRNO)) continue; else if (count <= 0) - handle_error(channel, i, now); + { + handle_error(channel, i, now); + break; + } #ifdef HAVE_RECVFROM - else if (from.sin_addr.s_addr != server->addr.s_addr) - /* Address response came from did not match the address - * we sent the request to. Someone may be attempting - * to perform a cache poisoning attack */ - break; + /* Check if address response came from did match the address + * we sent the request to. Someone may be attempting + * to perform a cache poisoning attack */ + else if (from.ss_family == AF_INET) + { + if (((struct sockaddr_in *) &from)->sin_addr.s_addr != + server->addr.addrV4.s_addr) + break; + } + else if (from.ss_family == AF_INET6) + { + if (memcmp(((struct sockaddr_in6 *) &from)->sin6_addr.s6_addr, + server->addr.addrV6.s6_addr, + 16)) + break; + } #endif - else - process_answer(channel, buf, (int)count, i, 0, now); + process_answer(channel, buf, (int)count, i, 0, now); } while (count > 0); } } @@ -889,14 +902,44 @@ static int configure_socket(ares_socket_t s, ares_channel channel) return 0; } +static int prepare_addrinfo(struct server_state *server, int port, + struct sockaddr_storage *addr) +{ + const unsigned short sh_port = (unsigned short)(port & 0xffff); + int rc = ARES_SUCCESS; + + switch (server->addr.family) + { + case AF_INET: + ((struct sockaddr_in *) addr)->sin_family = AF_INET; + ((struct sockaddr_in *) addr)->sin_port = sh_port; + ((struct sockaddr_in *) addr)->sin_addr.s_addr = server->addr.addrV4.s_addr; + break; + + case AF_INET6: + ((struct sockaddr_in6 *) addr)->sin6_family = AF_INET6; + ((struct sockaddr_in6 *) addr)->sin6_port = sh_port; + memcpy(((struct sockaddr_in6 *) addr)->sin6_addr.s6_addr, + server->addr.addrV6.s6_addr, + 16); + break; + + default: + rc = ARES_EBADFAMILY; + break; + } + + return rc; +} + static int open_tcp_socket(ares_channel channel, struct server_state *server) { ares_socket_t s; int opt; - struct sockaddr_in sockin; + struct sockaddr_storage addr; /* Acquire a socket. */ - s = socket(AF_INET, SOCK_STREAM, 0); + s = socket(server->addr.family, SOCK_STREAM, 0); if (s == ARES_SOCKET_BAD) return -1; @@ -923,12 +966,15 @@ static int open_tcp_socket(ares_channel channel, struct server_state *server) } #endif + if (prepare_addrinfo(server, channel->tcp_port, &addr) != ARES_SUCCESS) + { + sclose(s); + return -1; + } + /* Connect to the server. */ - memset(&sockin, 0, sizeof(sockin)); - sockin.sin_family = AF_INET; - sockin.sin_addr = server->addr; - sockin.sin_port = (unsigned short)(channel->tcp_port & 0xffff); - if (connect(s, (struct sockaddr *) &sockin, sizeof(sockin)) == -1) + if (connect(s, (struct sockaddr *) &addr, + sizeof(struct sockaddr_storage)) == -1) { int err = SOCKERRNO; @@ -960,10 +1006,10 @@ static int open_tcp_socket(ares_channel channel, struct server_state *server) static int open_udp_socket(ares_channel channel, struct server_state *server) { ares_socket_t s; - struct sockaddr_in sockin; + struct sockaddr_storage addr; /* Acquire a socket. */ - s = socket(AF_INET, SOCK_DGRAM, 0); + s = socket(server->addr.family, SOCK_DGRAM, 0); if (s == ARES_SOCKET_BAD) return -1; @@ -974,12 +1020,15 @@ static int open_udp_socket(ares_channel channel, struct server_state *server) return -1; } + if (prepare_addrinfo(server, channel->udp_port, &addr) != ARES_SUCCESS) + { + sclose(s); + return -1; + } + /* Connect to the server. */ - memset(&sockin, 0, sizeof(sockin)); - sockin.sin_family = AF_INET; - sockin.sin_addr = server->addr; - sockin.sin_port = (unsigned short)(channel->udp_port & 0xffff); - if (connect(s, (struct sockaddr *) &sockin, sizeof(sockin)) == -1) + if (connect(s, (struct sockaddr *) &addr, + sizeof(struct sockaddr_storage)) == -1) { int err = SOCKERRNO; diff --git a/ares_set_nameservers.3 b/ares_set_nameservers.3 new file mode 100644 index 0000000..f573790 --- /dev/null +++ b/ares_set_nameservers.3 @@ -0,0 +1,58 @@ +.TH ARES_SET_NAMESERVERS 3 "12 Feb 2010" +.SH NAME +ares_set_nameservers - Set nameservers +.SH SYNOPSIS +.nf +.B #include <ares.h> +.PP +.B int ares_set_nameservers(ares_channel \fIchannel\fP, + struct ares_addr *\fIservers\fP, + int \fInum_servers\fP) +.PP +.B cc file.c -lcares +.fi +.SH DESCRIPTION +.PP +This function sets nameservers for the given ares channel handle. +The array +.I servers +contains the addresses of nameservers, the length of the array +is stored in the +.I num_servers +parameter. +Contrary to initializing nameservers with +.B ares_init_options +this function can be used to set IPv6 nameservers. + +The structure +.I ares_addr +contains the following fields: +.sp +.in +4n +.nf +struct ares_addr { + int family; + union { + struct in_addr addr4; + struct in6_addr addr6; + } addr; +}; +.fi +.in + +.PP +.SH RETURN VALUES +.B ares_set_nameservers +can return any of the following values: +.TP 15 +.B ARES_SUCCESS +The response was successfully parsed. +.TP 15 +.B ARES_ENOMEM +Memory was exhausted. +.SH SEE ALSO +.BR ares_init_options (3) +.SH AUTHOR +Written by Jakub Hrozek <jhro...@redhat.com>, +on behalf of Red Hat, Inc http://www.redhat.com + -- 1.6.6
From c74380247cf05bd2ff19158856c95842f5bd3503 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Thu, 25 Feb 2010 18:17:34 +0100 Subject: [PATCH 2/2] Use ares_in6_addr --- ares.h | 3 ++- ares_gethostbyaddr.c | 2 +- ares_process.c | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/ares.h b/ares.h index 57bd445..06683eb 100644 --- a/ares.h +++ b/ares.h @@ -414,6 +414,7 @@ struct ares_in6_addr { union { unsigned char _S6_u8[16]; } _S6_un; + #define ares_s6_addr _S6_un._S6_u8 }; struct ares_addrttl { @@ -430,7 +431,7 @@ struct ares_addr { int family; union { struct in_addr addr4; - struct in6_addr addr6; + struct ares_in6_addr addr6; } addr; }; diff --git a/ares_gethostbyaddr.c b/ares_gethostbyaddr.c index 732a031..33a0785 100644 --- a/ares_gethostbyaddr.c +++ b/ares_gethostbyaddr.c @@ -272,7 +272,7 @@ static void ptr_rr_name(char *name, const struct ares_addr *addr) } else { - unsigned char *bytes = (unsigned char *)&addr->addrV6.s6_addr; + unsigned char *bytes = (unsigned char *)&addr->addrV6.ares_s6_addr; /* There are too many arguments to do this in one line using * minimally C89-compliant compilers */ sprintf(name, diff --git a/ares_process.c b/ares_process.c index 65b95f4..6677c9d 100644 --- a/ares_process.c +++ b/ares_process.c @@ -497,7 +497,7 @@ static void read_udp_packets(ares_channel channel, fd_set *read_fds, else if (from.ss_family == AF_INET6) { if (memcmp(((struct sockaddr_in6 *) &from)->sin6_addr.s6_addr, - server->addr.addrV6.s6_addr, + server->addr.addrV6.ares_s6_addr, 16)) break; } @@ -920,7 +920,7 @@ static int prepare_addrinfo(struct server_state *server, int port, ((struct sockaddr_in6 *) addr)->sin6_family = AF_INET6; ((struct sockaddr_in6 *) addr)->sin6_port = sh_port; memcpy(((struct sockaddr_in6 *) addr)->sin6_addr.s6_addr, - server->addr.addrV6.s6_addr, + server->addr.addrV6.ares_s6_addr, 16); break; -- 1.6.6
0001-Allow-the-use-of-IPv6-nameservers.patch.sig
Description: PGP signature
0002-Use-ares_in6_addr.patch.sig
Description: PGP signature