MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Frazer Clement Date:July 28 2009 1:51pm
Subject:Re: bzr commit into mysql-5.4 branch (alik:2804) Bug#45584
View as plain text  
Alexander Nozdrin wrote:
> #At file:///mnt/raid/alik/MySQL/bzr/bug38247/azalea-bf-bug45584/ based on
> revid:alik@stripped
>
>  2804 Alexander Nozdrin	2009-07-28
>       Fix for Bug#45584 (Host name cache does not work as a cache).
>       
>       The problem is described in the bug report.
>       
>       The solution is the following:
>       
>         - Make hostname cache key type of (char *);
>       
>         - Use string representation of normalized IPv6 addresses
>           as hostname cache keys when IPv6 is supported.
>       
>           Use string representation of normalized IPv4 addresses
>           as hostname cache keys when IPv6 is not supported.
>       
>         - Use only the host part of client address for hostname
>           cache keys;
>       
>         - Actually resolve IP addresses to hostnames,
>           not to IP strings;
>       
>         - Minimal supported Windows version has been changed to Windows XP.
>           We don't support Windows 2000 for the newer versions any more.
>       
>           Windows XP has IPv6 support, so declaring it minimal supported
>           version removes much of "windows portability hassle".
>      @ configure.in
>         HAVE_STRUCT_IN6_ADDR will be defined when IPv6 is supported.
>      @ sql/mysql_priv.h
>         Change the ip_to_hostname() prototype:
>           - Make it more descriptive;
>           - Provide a way to return error status;
>      @ sql/sql_connect.cc
>         Handle errors from ip_to_hostname().
>
>     modified:
>       configure.in
>       include/config-win.h
>       sql/hostname.cc
>       sql/mysql_priv.h
>       sql/sql_connect.cc
> === modified file 'configure.in'
> --- a/configure.in	2009-06-17 07:30:19 +0000
> +++ b/configure.in	2009-07-28 13:09:58 +0000
> @@ -876,6 +876,24 @@ then
>  fi
>  
>  #--------------------------------------------------------------------
> +# Check for IPv6 support
> +#--------------------------------------------------------------------
> +
> +AC_CHECK_HEADERS(netinet/in6.h)
> +
> +AC_CHECK_TYPES([struct in6_addr], [], [], 
> +[[#ifdef WIN32
> +#include <winsock2.h>
> +#else
> +#include <sys/types.h>
> +#include <netinet/in.h>
> +#include <sys/socket.h>
> +#endif
> +#ifdef HAVE_NETINET_IN6_H
> +#include <netinet/in6.h>
> +#endif]])
> +
> +#--------------------------------------------------------------------
>  # Check for TCP wrapper support
>  #--------------------------------------------------------------------
>  
>
> === modified file 'include/config-win.h'
> --- a/include/config-win.h	2009-06-17 07:30:19 +0000
> +++ b/include/config-win.h	2009-07-28 13:09:58 +0000
> @@ -17,11 +17,11 @@
>  
>  #define BIG_TABLES
>  
> -/* 
> +/*
>    Minimal version of Windows we should be able to run on.
> -  Currently Windows 2000
> +  Currently Windows XP.
>  */
> -#define _WIN32_WINNT     0x0500
> +#define _WIN32_WINNT     0x0501
>  
>  
>  #if defined(_MSC_VER) && _MSC_VER >= 1400
>
> === modified file 'sql/hostname.cc'
> --- a/sql/hostname.cc	2009-01-27 02:08:48 +0000
> +++ b/sql/hostname.cc	2009-07-28 13:09:58 +0000
> @@ -18,10 +18,10 @@
>    @file
>  
>    @brief
> -  Get hostname for an IP.
> +  Get hostname for an IP address.
>  
> -    Hostnames are checked with reverse name lookup and
> -    checked that they doesn't resemble an ip.
> +  Hostnames are checked with reverse name lookup and checked that they
> +  doesn't resemble an IP address.
>  */
>  
>  #include "mysql_priv.h"
> @@ -40,12 +40,228 @@ extern "C" {					// Because of SCO 3.2V4
>  }
>  #endif
>  
> -class host_entry :public hash_filo_element
> +#ifdef __WIN__
> +#define HAVE_STRUCT_IN6_ADDR
> +#endif /* __WIN__ */
> +
> +/*
> +  HOST_ENTRY_KEY_SIZE -- size of IP address string in the hash cache. The
> +  system constant NI_MAXHOST could be used here. However, it means at least
> +  1024 bytes per IP, which seems to be quite big.
> +
> +  Since IP address string is created by our function get_ip_string(), we
> +  can reduce the space.  get_ip_string() returns a hexadecimal
> +  respresentation (1111:2222:...:8888) in case of IPv6 and the standard
> +  representation (111.222.333.444) in case of IPv4, which means 39 bytes at
> +  most. So, we need 40 bytes for storing IP address string including
> +  trailing zero.
> +*/
> +
> +#define HOST_ENTRY_KEY_SIZE 40
> +
> +/************************************************************************/
> +
> +/*
> +  When this code was written there were issues with winsock in pusbuild,
> +  this constant is in this place for this reason.
> +*/
> +
> +#ifdef EAI_NODATA
> +    const int MY_NONAME_ERR_CODE= EAI_NODATA;
> +#else
> +    const int MY_NONAME_ERR_CODE= EAI_NONAME;
> +#endif
> +
> +/************************************************************************/
> +
> +/**
> +  Get the string representation for IP address. IPv6 and IPv4 addresses are
> +  supported. The function is needed because getnameinfo() is known to be
> +  buggy in some circumstances. Actually, this is a basic replacement for
> +  getnameinfo() called with the NI_NUMERICHOST flag. Only the hostname part is
> +  dumped (the port part is omitted).
> +*/
> +
> +static void get_ip_string(const struct sockaddr *ip,
> +                          char *ip_str, int ip_str_size)
> +{
> +  switch (ip->sa_family) {
> +  case AF_INET:
> +  {
> +    struct in_addr *ip4= &((struct sockaddr_in *) ip)->sin_addr;
> +    uint32 ip4_int32= ntohl(ip4->s_addr); // XXX
>   
Is XXX going to become a comment, or?
> +    uint8 *ip4_int8= (uint8 *) &ip4_int32;
> +
> +    int n= my_snprintf(ip_str, ip_str_size, "%d.%d.%d.%d",
> +                       ip4_int8[0], ip4_int8[1], ip4_int8[2], ip4_int8[3]);
> +
> +    DBUG_ASSERT(n < ip_str_size);
> +    break;
> +  }
> +
> +#ifdef HAVE_STRUCT_IN6_ADDR
> +  case AF_INET6:
> +  {
> +    struct in6_addr *ip6= &((struct sockaddr_in6 *) ip)->sin6_addr;
> +    uint16 *ip6_int16= (uint16 *) ip6->s6_addr;
> +
> +    int n= my_snprintf(ip_str, ip_str_size,
> +                       "%04X:%04X:%04X:%04X:%04X:%04X:%04X:%04X",
> +                       ntohs(ip6_int16[0]), ntohs(ip6_int16[1]),
> +                       ntohs(ip6_int16[2]), ntohs(ip6_int16[3]),
> +                       ntohs(ip6_int16[4]), ntohs(ip6_int16[5]),
> +                       ntohs(ip6_int16[6]), ntohs(ip6_int16[7]));
> +
> +    DBUG_ASSERT(n < ip_str_size);
> +    break;
> +  }
> +#endif /* HAVE_STRUCT_IN6_ADDR */
> +
> +  default:
> +    DBUG_ASSERT(FALSE);
> +  }
> +}
> +
> +/**
> +  Get key value for IP address. Key value is a string representation of
> +  normalized IP address.
> +
> +  When IPv4 and IPv6 are both used in the network stack, or in the network
> +  path between a client and a server, a client can have different apparent
> +  IP addresses, based on the exact route taken.
> +
> +  This function normalize the client IP representation, so it's suitable to
> +  use as a key for searches.
> +
> +  Transformations are implemented as follows:
> +  - IPv4 a.b.c.d --> IPv6 mapped IPv4 ::ffff:a.b.c.d
> +  - IPv6 compat IPv4 ::a.b.c.d --> IPv6 mapped IPv4 ::ffff:a.b.c.d
> +  - IPv6 --> IPv6
> +
> +  If IPv6 is not available at compile-time, IPv4 form is used.
> +
> +  @param [in]   ip      IP address
> +  @param [out]  ip_key  Key for the given IP value
> +
> +  @note According to RFC3493 the only specified member of the in6_addr
> +  structure is s6_addr.
> +
> +  @note It is possible to call hostname_cache_get_key() with zeroed IP
> +  address (ip->sa_family == 0). In this case hostname_cache_get_key()
> +  returns TRUE (error status).
> +
> +  @return Error status.
> +  @retval FALSE Success
> +  @retval TRUE Error
>   
Personally not too keen on TRUE for error when the name of the function 
is not negative, but it's not important.
> +*/
> +
> +static bool hostname_cache_get_key(const struct sockaddr *ip, char *ip_key)
> +{
> +  const struct sockaddr *ip_to_generate_key= ip;
> +
> +  if (ip->sa_family == 0)
> +    return TRUE; /* IP address is not set. */
> +
> +#ifdef HAVE_STRUCT_IN6_ADDR
> +  /* Prepare normalized IP address. */
> +
> +  struct sockaddr_storage norm_ip;
> +  struct in6_addr *norm_ip6= &((sockaddr_in6 *) &norm_ip)->sin6_addr;
> +  uint32 *norm_ip6_int32= (uint32 *) norm_ip6->s6_addr;
> +
> +  memset(&norm_ip, 0, sizeof (sockaddr_storage));
> +
> +  switch (ip->sa_family) {
> +  case AF_INET:
> +  {
> +    struct in_addr *ip4= &((struct sockaddr_in *) ip)->sin_addr;
> +
> +    norm_ip6_int32[0]= 0;
> +    norm_ip6_int32[1]= 0;
> +    norm_ip6_int32[2]= htonl(0xffff);
> +    norm_ip6_int32[3]= ip4->s_addr; /* in net byte order */
> +
> +    DBUG_ASSERT(IN6_IS_ADDR_V4MAPPED(norm_ip6));
> +    break;
> +  }
> +
> +  case AF_INET6:
> +  {
> +    struct in6_addr *ip6= &((struct sockaddr_in6 *) ip)->sin6_addr;
> +    uint32 *ip6_int32= (uint32 *) ip6->s6_addr;
> +
> +    if (IN6_IS_ADDR_V4COMPAT(ip6))
> +    {
> +      norm_ip6_int32[0]= 0;
> +      norm_ip6_int32[1]= 0;
> +      norm_ip6_int32[2]= htonl(0xffff);
> +      norm_ip6_int32[3]= ip6_int32[3]; /* in net byte order */
> +      DBUG_ASSERT(IN6_IS_ADDR_V4MAPPED(norm_ip6));
> +    }
> +    else
> +    {
> +      /* All in net byte order: just copy 16 bytes. */
> +      memcpy(norm_ip6_int32, ip6_int32, 4 * sizeof (uint32));
> +    }
> +
> +    break;
> +  }
> +
> +  default:
> +    DBUG_ASSERT(FALSE);
> +    break;
> +  }
> +
> +  norm_ip.ss_family= AF_INET6;
> +  ip_to_generate_key= (sockaddr *) &norm_ip;
> +#endif /* HAVE_STRUCT_IN6_ADDR */
> +
> +  /*
> +    Zero all bytes of the key, because it's not just 0-terminated string.
> +    All bytes are taken into account during hash search.
> +  */
> +
> +  memset(ip_key, 0, HOST_ENTRY_KEY_SIZE);
> +
> +  /* Get numeric representation of the normalized IP address. */
> +  get_ip_string(ip_to_generate_key, ip_key, HOST_ENTRY_KEY_SIZE);
> +
> +  return FALSE;
> +}
> +
> +/**
> +  An entry in the hostname hash table cache.
> +
> +  Host name cache does two things:
> +    - caches host names to save DNS look ups;
> +    - counts connect errors from IP.
> +
> +  Host name can be NULL (that means DNS look up failed), but connect errors
> +  still are counted.
> +*/
> +
> +class Host_entry :public hash_filo_element
>  {
>  public:
> -  char	 ip[sizeof(struct sockaddr_storage)];
> -  uint	 errors;
> -  char	 *hostname;
> +  /**
> +    Client IP address. This is the key used with the hash table.
> +
> +    The client IP address is always expressed in IPv6, even when the
> +    network IPv6 stack is not present.
> +
> +    This IP address is never used to connect to a socket.
> +  */
> +  char ip[HOST_ENTRY_KEY_SIZE];
> +
> +  /**
> +    Number of errors during handshake phase from the IP address.
> +  */
> +  uint connect_errors;
> +
> +  /**
> +    One of host names for the IP address. May be NULL.
> +  */
> +  const char *hostname;
>  };
>  
>  static hash_filo *hostname_cache;
> @@ -57,13 +273,15 @@ void hostname_cache_refresh()
>  
>  bool hostname_cache_init()
>  {
> -  host_entry tmp;
> -  uint offset= (uint) ((char*) (&tmp.ip) - (char*) &tmp);
> -  if (!(hostname_cache=new hash_filo(HOST_CACHE_SIZE, offset,
> -				     sizeof(struct sockaddr_storage),NULL,
> -				     (my_hash_free_key) free,
> -				     &my_charset_bin)))
> +  Host_entry tmp;
> +  uint key_offset= (uint) ((char*) (&tmp.ip) - (char*) &tmp);
> +
> +  if (!(hostname_cache= new hash_filo(HOST_CACHE_SIZE,
> +                                      key_offset, HOST_ENTRY_KEY_SIZE,
> +                                      NULL, (my_hash_free_key) free,
> +                                      &my_charset_bin)))
>      return 1;
> +
>    hostname_cache->clear();
>  
>    return 0;
> @@ -71,190 +289,461 @@ bool hostname_cache_init()
>  
>  void hostname_cache_free()
>  {
> -  if (hostname_cache)
> -  {
> -    delete hostname_cache;
> -    hostname_cache= 0;
> -  }
> +  delete hostname_cache;
> +  hostname_cache= NULL;
>  }
>  
> -static void add_hostname(struct sockaddr_storage *in, const char *name)
> +static inline Host_entry *hostname_cache_search(const char *ip)
>  {
> -  if (!(specialflag & SPECIAL_NO_HOST_CACHE))
> +  return (Host_entry *) hostname_cache->search((uchar *) ip, 0);
> +}
> +
> +static bool add_hostname_impl(const char *ip, const char *hostname)
> +{
> +  if (hostname_cache_search(ip))
> +    return FALSE;
> +
> +  uint hostname_length= hostname ? (uint) strlen(hostname) : 0;
> +
> +  Host_entry *entry= (Host_entry *) malloc(sizeof (Host_entry) +
> +                                           hostname_length + 1);
>   
I don't work with MySQLD much, but it it ok to malloc like this?  Should 
some other allocator be used?
Could this be a DOS approach : Connect from lots of different hosts, 
causing them to be added to the host cache and consuming server memory?  
Is the Host Cache supposed to be infinitely sized?  If it's a 'Cache' 
then perhaps some eviction is required - although this would again 
increase the scope of this work.
>
> ------------------------------------------------------------------------
>
>
>   

-- 
Frazer Clement, Software Engineer, MySQL Cluster 
Sun Microsystems - www.mysql.com
Office: Edinburgh, UK

Are you MySQL certified?  www.mysql.com/certification

Thread
bzr commit into mysql-5.4 branch (alik:2804) Bug#45584Alexander Nozdrin28 Jul
  • Re: bzr commit into mysql-5.4 branch (alik:2804) Bug#45584Frazer Clement28 Jul
  • Re: bzr commit into mysql-5.4 branch (alik:2804) Bug#45584Frazer Clement29 Jul
  • Re: bzr commit into mysql-5.4 branch (alik:2804) Bug#45584Davi Arnaut1 Aug