Hi Kristofer
Please file a separate bug report for this:
* Memcpy memory block size isn't sizeof(struct addrinfo) but
sizeof(struct sockaddr_storage). This caused a crash.
* Don't call freeaddrinfo if we failed to get a res_lst. (caused crash)
Add davi or me as reviewer, and post a fix limited to these issues only,
so we can get this resolved and out of the way.
For the other issues affecting IPv6, I don't think this patch will work,
let's discuss the details and possible solutions on the phone, it will
be more efficient.
Best regards,
-- Marc
Kristofer Pettersson wrote:
> #At file:///home/thek/Development/cpp/mysqlbzr/mysql-6.0-bug38247/
>
> 2781 Kristofer Pettersson 2008-08-25
> Bug#38247 Server does not resolve connecting ip's
>
> Server does not resolve the ip's of incoming connections.
>
> This patch provides a quick solution for this bug by striping the "::ffff:"
> prefix from a mapped IPv4.
> modified:
> sql/hostname.cc
> vio/viosocket.c
>
> per-file messages:
> sql/hostname.cc
> * Memcpy memory block size isn't sizeof(struct addrinfo) but sizeof(struct
> sockaddr_storage). This caused a crash.
> * Request getnameinfo with host name representation and not numeric
> representation.
> * Don't call freeaddrinfo if we failed to get a res_lst. (caused crash)
> * Make sure we project the character representation of an IPv4 address to the
> address space of IPv4 and not as a mapped IPv4.
> vio/viosocket.c
> * Transform mapped IPv4 to old IPv4 format.
> === modified file 'sql/hostname.cc'
> --- a/sql/hostname.cc 2008-07-08 16:01:41 +0000
> +++ b/sql/hostname.cc 2008-08-25 12:52:43 +0000
> @@ -94,7 +94,7 @@ static void add_hostname(struct sockaddr
> if ((entry=(host_entry*) malloc(sizeof(host_entry)+length+1)))
> {
> char *new_name;
> - memcpy_fixed(&entry->ip, in, sizeof(struct addrinfo));
> + memcpy_fixed(&entry->ip, in, sizeof(struct sockaddr_storage));
> if (length)
> memcpy(new_name= (char *) (entry+1), name, length+1);
> else
> @@ -149,18 +149,21 @@ char *ip_to_hostname(struct sockaddr_sto
> DBUG_ENTER("ip_to_hostname");
> *errors=0;
>
> - /* Historical comparison for 127.0.0.1 */
> - gxi_error= getnameinfo((struct sockaddr *)in, addrLen,
> - hostname_buff, NI_MAXHOST,
> - NULL, 0, NI_NUMERICHOST);
> - if (gxi_error)
> + /*
> + Get host name in numeric form.
> + */
> + if ((gxi_error= getnameinfo((struct sockaddr *)in, addrLen,
> + hostname_buff, NI_MAXHOST,
> + NULL, 0, NI_NAMEREQD)))
> {
> - DBUG_PRINT("error",("getnameinfo returned %d", gxi_error));
> - DBUG_RETURN(0);
> - }
> - DBUG_PRINT("info",("resolved: %s", hostname_buff));
> + add_wrong_ip(in);
> + DBUG_RETURN(0);
> + };
>
> - /* The next three compares are to solve historical solutions with localhost */
> + /*
> + The next three compares are to solve historical solutions with
> + localhost.
> + */
> if (!memcmp(hostname_buff, "127.0.0.1", sizeof("127.0.0.1")))
> {
> DBUG_RETURN((char *)my_localhost);
> @@ -173,8 +176,17 @@ char *ip_to_hostname(struct sockaddr_sto
> {
> DBUG_RETURN((char *)my_localhost);
> }
> +
> + if (!(name= my_strdup(hostname_buff, MYF(0))))
> + {
> + DBUG_PRINT("error",("out of memory"));
> + DBUG_RETURN(0);
> + }
> +
> + DBUG_PRINT("info",("resolved: %s", hostname_buff));
>
> /* Check first if we have name in cache */
> +
> if (!(specialflag & SPECIAL_NO_HOST_CACHE))
> {
> pthread_mutex_lock(&hostname_cache->lock);
> @@ -193,14 +205,10 @@ char *ip_to_hostname(struct sockaddr_sto
> pthread_mutex_unlock(&hostname_cache->lock);
> }
>
> - if (!(name= my_strdup(hostname_buff, MYF(0))))
> - {
> - DBUG_PRINT("error",("out of memory"));
> - DBUG_RETURN(0);
> - }
> -
> /* Don't accept hostnames that starts with digits because they may be
> - false ip:s */
> + false ip:s. This can be used to exploit the current privilege system compare
> + function.
> + */
> if (my_isdigit(&my_charset_latin1, name[0]))
> {
> char *pos;
> @@ -236,23 +244,62 @@ char *ip_to_hostname(struct sockaddr_sto
> #else
> if (gxi_error == EAI_NONAME )
> #endif
> - add_wrong_ip(in);
> + add_wrong_ip(in);
>
> my_free(name,MYF(0));
> - freeaddrinfo(res_lst);
> DBUG_RETURN(0);
> }
>
> /* Check that 'getaddrinfo' returned the used ip */
> + char str_tmp_addr_representation[INET6_ADDRSTRLEN];
> + if (in->ss_family == AF_INET6)
> + {
> + sockaddr_in6 *addr2= (sockaddr_in6 *)in;
> + if (!inet_ntop(AF_INET6, &(addr2->sin6_addr),
> str_tmp_addr_representation, INET6_ADDRSTRLEN))
> + {
> + /*
> + Address conversion failed. This should normally never happen.
> + */
> + my_free(name,MYF(0));
> + add_wrong_ip(in);
> + DBUG_RETURN(0);
> + }
> +
> + /*
> + We expect the address to be a mapped IPv4 address with a prefix
> + of ::ffff: (7 characters long). Since the privilege system currently
> + expects regular IPv4 address we convert the representation IP here.
> + */
> + strmov(str_tmp_addr_representation, str_tmp_addr_representation+7);
> +
> + }
> + else
> + {
> + sockaddr_in *addr2= (sockaddr_in *)in;
> + inet_ntop(AF_INET, &(addr2->sin_addr), str_tmp_addr_representation,
> INET_ADDRSTRLEN);
> + }
> +
> for (t_res= res_lst; t_res; t_res=t_res->ai_next)
> {
> - if (!memcmp(&(t_res->ai_addr), in,
> - sizeof(struct sockaddr_storage) ) )
> + char str_addr[INET6_ADDRSTRLEN];
> + if (t_res->ai_family == AF_INET)
> + {
> + sockaddr_in *addr= (sockaddr_in *)t_res->ai_addr;
> + inet_ntop(AF_INET,&addr->sin_addr, str_addr, INET_ADDRSTRLEN);
> + }
> + else
> + {
> + sockaddr_in6 *addr= (sockaddr_in6 *)t_res->ai_addr;
> + inet_ntop(AF_INET6,&addr->sin6_addr, str_addr, INET6_ADDRSTRLEN);
> + }
> +
> + if (!memcmp(str_addr,str_tmp_addr_representation,
> + strlen(str_addr)))
> {
> add_hostname(in, name);
> freeaddrinfo(res_lst);
> DBUG_RETURN(name);
> - }
> + }
> }
> freeaddrinfo(res_lst);
> DBUG_PRINT("error",("Couldn't verify hostname with getaddrinfo"));
>
> === modified file 'vio/viosocket.c'
> --- a/vio/viosocket.c 2008-05-14 13:33:43 +0000
> +++ b/vio/viosocket.c 2008-08-25 12:52:43 +0000
> @@ -350,7 +350,17 @@ my_bool vio_peer_addr(Vio *vio, char *bu
> */
> if (!memcmp(buf, "::ffff:127.0.0.1", sizeof("::ffff:127.0.0.1")))
> strmov(buf, "127.0.0.1");
> + /*
> + We currently only support IPv4 addresses in the privilege system. If the
> + current address is a mapped IPv6 address, we strip the leading
> + characters by matching the prefix "::ffff"
> + */
> +
> + if (buf[0] == ':' && buf[1] == ':' && buf[2] == 'f' &&
> buf[3] == 'f' &&
> + buf[4] == 'f' && buf[5] == 'f')
> + strmov(buf,buf+7);
> }
> +
> DBUG_PRINT("exit", ("addr: %s", buf));
> DBUG_RETURN(0);
> }
>
>
>
